Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Jeff Garzik wrote: Jeff Garzik wrote: Tejun Heo wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-sff.c |5 +++-- drivers/ata/pata_acpi.c|2 +- drivers/ata/pata_ali.c |2 +- drivers/ata/pata_hpt366.c |2 +- drivers/ata/pata_hpt37x.c |4 ++-- drivers/ata/pata_pdc2027x.c|4 ++-- drivers/ata/pata_scc.c |2 +- drivers/ata/pata_serverworks.c |4 ++-- include/linux/libata.h |5 +++-- 9 files changed, 16 insertions(+), 14 deletions(-) as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms. er, s/permit/prefer/ Alright. I don't agree but don't have much problem either. Will update them the other way. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Jeff Garzik wrote: Tejun Heo wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-sff.c |5 +++-- drivers/ata/pata_acpi.c|2 +- drivers/ata/pata_ali.c |2 +- drivers/ata/pata_hpt366.c |2 +- drivers/ata/pata_hpt37x.c |4 ++-- drivers/ata/pata_pdc2027x.c|4 ++-- drivers/ata/pata_scc.c |2 +- drivers/ata/pata_serverworks.c |4 ++-- include/linux/libata.h |5 +++-- 9 files changed, 16 insertions(+), 14 deletions(-) as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms. er, s/permit/prefer/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Tejun Heo wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-sff.c |5 +++-- drivers/ata/pata_acpi.c|2 +- drivers/ata/pata_ali.c |2 +- drivers/ata/pata_hpt366.c |2 +- drivers/ata/pata_hpt37x.c |4 ++-- drivers/ata/pata_pdc2027x.c|4 ++-- drivers/ata/pata_scc.c |2 +- drivers/ata/pata_serverworks.c |4 ++-- include/linux/libata.h |5 +++-- 9 files changed, 16 insertions(+), 14 deletions(-) as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms. Consistency patches in the other direction (moving ATA_MASK_* to a separate enum and marking with UL suffix) are welcome. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo [EMAIL PROTECTED] wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say unsigned int was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion. Alan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Alan Cox wrote: On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo [EMAIL PROTECTED] wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say unsigned int was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion. I agree it should be one of u* types and am happy to convert. This patch is primarily for consistency as in libata-core, xfer_mask is unsigned int. My first proposal was u32 too but Jeff vetoed it. IIRC, Jeff has affection for machine-independent integral types. :-) Anyways, I think ulong is worse than uint because ulong differs in size between 32 and 64 archs. What's the good of extra 32bits in flags space if it's not there on 32bit archs? Also, we currently only use 20bits of xfermask, 12 extra bits seem enough for the time, especially as everything is moving over to SATA where xfermode doesn't really matter, no? Jeff, are you okay with u32 or 64? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Tejun Heo wrote: Alan Cox wrote: On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo [EMAIL PROTECTED] wrote: xfer_mask is unsigned int not unsigned long. Change -mode_filter to take and return unsigned int. While at it, rename @adev of ata_pci_default_filter() to @dev for consistency. Signed-off-by: Tejun Heo [EMAIL PROTECTED] The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say unsigned int was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion. I agree it should be one of u* types and am happy to convert. This patch is primarily for consistency as in libata-core, xfer_mask is unsigned int. My first proposal was u32 too but Jeff vetoed it. IIRC, Jeff has affection for machine-independent integral types. :-) Anyways, I think ulong is worse than uint because ulong differs in size between 32 and 64 archs. What's the good of extra 32bits in flags space if it's not there on 32bit archs? Also, we currently only use 20bits of xfermask, 12 extra bits seem enough for the time, especially as everything is moving over to SATA where xfermode doesn't really matter, no? Jeff, are you okay with u32 or 64? unsigned long is IMO the best choice for bitmaps. * it is a machine int on all(?) architectures * it is 32-bit on 32-bit architectures * it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces * conversion to test_bit() and lib/bitmap.c interfaces as a future step is trivial * your structs inflate on 64-bit due to pointers anyway, so I see little real consequence to using the lower 32 bits of an unsigned long as a portable meme. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Jeff Garzik wrote: Jeff, are you okay with u32 or 64? unsigned long is IMO the best choice for bitmaps. * it is a machine int on all(?) architectures I don't really see much point in this. What's the advantage of a machine int for xfer mask? * it is 32-bit on 32-bit architectures The problem I see for using native integral types for flags or mask is that it's not fixed in size, so you can only use the smallest of all the supported architectures. We know for all archs we support, both unsigned int and unsigned long are at least 32bits long but to me making the assumption about expected number of bits using u32 or u64 is much more important than other considerations. * it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces * conversion to test_bit() and lib/bitmap.c interfaces as a future step is trivial I don't think it's likely that we'll need those heavy machineries for xfermask and the correct approach is to convert when the need actually arises. * your structs inflate on 64-bit due to pointers anyway, so I see little real consequence to using the lower 32 bits of an unsigned long as a portable meme. I'm not trying to optimize performance or size. I'm trying to make programming assumptions clear. I think it's silly to optimize anything for xfermask. It just has to work and be clear to people working with it. An optimal but overkill solution would be using fast_u32 or fast_u64 types such that the compiler can choose as necessary but as we don't have that yet xfermask handling is a very very cold path, I think we should be aiming for clarity. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
xfer_mask is unsigned int not unsigned long. Change -mode_filter to
take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for
consistency.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
drivers/ata/libata-sff.c |5 +++--
drivers/ata/pata_acpi.c|2 +-
drivers/ata/pata_ali.c |2 +-
drivers/ata/pata_hpt366.c |2 +-
drivers/ata/pata_hpt37x.c |4 ++--
drivers/ata/pata_pdc2027x.c|4 ++--
drivers/ata/pata_scc.c |2 +-
drivers/ata/pata_serverworks.c |4 ++--
include/linux/libata.h |5 +++--
9 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 48acc09..2f2ca3d 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -877,12 +877,13 @@ int ata_pci_clear_simplex(struct pci_dev *pdev)
return 0;
}
-unsigned long ata_pci_default_filter(struct ata_device *adev, unsigned long
xfer_mask)
+unsigned int ata_pci_default_filter(struct ata_device *dev,
+ unsigned int xfer_mask)
{
/* Filter out DMA modes if the device has been configured by
the BIOS as PIO only */
- if (adev-link-ap-ioaddr.bmdma_addr == NULL)
+ if (dev-link-ap-ioaddr.bmdma_addr == NULL)
xfer_mask = ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
return xfer_mask;
}
diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index e4542ab..2323646 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -164,7 +164,7 @@ static unsigned long pacpi_discover_modes(struct ata_port
*ap, struct ata_device
* this case the list of discovered valid modes obtained by ACPI probing
*/
-static unsigned long pacpi_mode_filter(struct ata_device *adev, unsigned long
mask)
+static unsigned int pacpi_mode_filter(struct ata_device *adev, unsigned int
mask)
{
struct pata_acpi *acpi = adev-link-ap-private_data;
return ata_pci_default_filter(adev, mask acpi-mask[adev-devno]);
diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
index 364534e..aae8a44 100644
--- a/drivers/ata/pata_ali.c
+++ b/drivers/ata/pata_ali.c
@@ -105,7 +105,7 @@ static int ali_c2_cable_detect(struct ata_port *ap)
* fix that later on. Also ensure we do not do UDMA on WDC drives
*/
-static unsigned long ali_20_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int ali_20_filter(struct ata_device *adev, unsigned int mask)
{
char model_num[ATA_ID_PROD_LEN + 1];
/* No DMA on anything but a disk for now */
diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index 0713872..1f277d5 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -174,7 +174,7 @@ static int hpt_dma_blacklisted(const struct ata_device
*dev, char *modestr, cons
* Block UDMA on devices that cause trouble with this controller.
*/
-static unsigned long hpt366_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int hpt366_filter(struct ata_device *adev, unsigned int mask)
{
if (adev-class == ATA_DEV_ATA) {
if (hpt_dma_blacklisted(adev, UDMA, bad_ata33))
diff --git a/drivers/ata/pata_hpt37x.c b/drivers/ata/pata_hpt37x.c
index 3816b86..37f327e 100644
--- a/drivers/ata/pata_hpt37x.c
+++ b/drivers/ata/pata_hpt37x.c
@@ -275,7 +275,7 @@ static const char *bad_ata100_5[] = {
* Block UDMA on devices that cause trouble with this controller.
*/
-static unsigned long hpt370_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int hpt370_filter(struct ata_device *adev, unsigned int mask)
{
if (adev-class == ATA_DEV_ATA) {
if (hpt_dma_blacklisted(adev, UDMA, bad_ata33))
@@ -293,7 +293,7 @@ static unsigned long hpt370_filter(struct ata_device *adev,
unsigned long mask)
* Block UDMA on devices that cause trouble with this controller.
*/
-static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long
mask)
+static unsigned int hpt370a_filter(struct ata_device *adev, unsigned int mask)
{
if (adev-class == ATA_DEV_ATA) {
if (hpt_dma_blacklisted(adev, UDMA100, bad_ata100_5))
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 2622577..dd20239 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -67,7 +67,7 @@ static void pdc2027x_error_handler(struct ata_port *ap);
static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev);
static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev);
static int pdc2027x_check_atapi_dma(struct ata_queued_cmd *qc);
-static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned
long mask);
+static unsigned int pdc2027x_mode_filter(struct ata_device *adev, unsigned int
mask);
static int pdc2027x_cable_detect(struct ata_port *ap);
static int
