* [PATCH 0/3] Make PATA transfer mode masks always being 32-bit @ 2022-05-08 20:41 Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* Sergey Shtylyov ` (4 more replies) 0 siblings, 5 replies; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-08 20:41 UTC (permalink / raw) To: Damien Le Moal, linux-ide The PATA transfer mode masks (direct and packed) in libata are sometimes declared as *unsigned int* and sometimes as *unsigned long* (which is a 64-bit type on 64-bit architectures), while the packed mask really only uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to the uniform 32-bit masks saves siginificant amount of the object code... Sergey Shtylyov (3): ata: make packed transfer mode masks *unsigned int* ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* drivers/ata/libata-acpi.c | 8 +++--- drivers/ata/libata-core.c | 38 +++++++++++++------------- drivers/ata/pata_acpi.c | 2 +- drivers/ata/pata_ali.c | 2 +- drivers/ata/pata_amd.c | 14 +++++----- drivers/ata/pata_hpt366.c | 2 +- drivers/ata/pata_hpt37x.c | 6 ++--- drivers/ata/pata_hpt3x2n.c | 2 +- drivers/ata/pata_pdc2027x.c | 4 +-- drivers/ata/pata_serverworks.c | 4 +-- drivers/ata/pata_sis.c | 2 +- drivers/ata/pata_via.c | 2 +- include/linux/libata.h | 49 +++++++++++++++++----------------- 13 files changed, 67 insertions(+), 68 deletions(-) -- 2.26.3 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov @ 2022-05-08 20:41 ` Sergey Shtylyov 2022-05-16 11:17 ` Damien Le Moal 2022-05-08 20:41 ` [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask " Sergey Shtylyov ` (3 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-08 20:41 UTC (permalink / raw) To: Damien Le Moal, linux-ide The packed transfer mode masks are declared as *unsigned long* (which is a 64-bit type on 64-bit architectures), however the actual mask occupies only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can safely use 32-bit *unsigned int* variables instead. Convert all libata functions taking as a parameter or returning a packed transfer mode mask. This saves 470 bytes of object code in libata-core.o alone... Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- drivers/ata/libata-acpi.c | 9 +++++---- drivers/ata/libata-core.c | 28 ++++++++++++------------- drivers/ata/pata_acpi.c | 2 +- drivers/ata/pata_ali.c | 2 +- drivers/ata/pata_amd.c | 14 ++++++------- drivers/ata/pata_hpt366.c | 2 +- drivers/ata/pata_hpt37x.c | 6 +++--- drivers/ata/pata_hpt3x2n.c | 2 +- drivers/ata/pata_pdc2027x.c | 4 ++-- drivers/ata/pata_serverworks.c | 4 ++-- drivers/ata/pata_sis.c | 2 +- drivers/ata/pata_via.c | 2 +- include/linux/libata.h | 37 +++++++++++++++++----------------- 13 files changed, 57 insertions(+), 57 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 3d345d173556..cdb7c0238a43 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -480,10 +480,10 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) * RETURNS: * Determined xfermask. */ -unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev, - const struct ata_acpi_gtm *gtm) +unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev, + const struct ata_acpi_gtm *gtm) { - unsigned long xfer_mask = 0; + unsigned int xfer_mask = 0; unsigned int type; int unit; u8 mode; @@ -525,7 +525,8 @@ int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm) struct ata_device *dev; ata_for_each_dev(dev, &ap->link, ENABLED) { - unsigned long xfer_mask, udma_mask; + unsigned long udma_mask; + unsigned int xfer_mask; xfer_mask = ata_acpi_gtm_xfermask(dev, gtm); ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask); diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 95bd028aea4f..2a6f301c3359 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -93,7 +93,7 @@ struct ata_force_param { const char *name; u8 cbl; u8 spd_limit; - unsigned long xfer_mask; + unsigned int xfer_mask; unsigned int horkage_on; unsigned int horkage_off; u16 lflags; @@ -796,9 +796,9 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, * RETURNS: * Packed xfer_mask. */ -unsigned long ata_pack_xfermask(unsigned long pio_mask, - unsigned long mwdma_mask, - unsigned long udma_mask) +unsigned int ata_pack_xfermask(unsigned long pio_mask, + unsigned long mwdma_mask, + unsigned long udma_mask) { return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | ((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) | @@ -816,7 +816,7 @@ EXPORT_SYMBOL_GPL(ata_pack_xfermask); * Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask. * Any NULL destination masks will be ignored. */ -void ata_unpack_xfermask(unsigned long xfer_mask, unsigned long *pio_mask, +void ata_unpack_xfermask(unsigned int xfer_mask, unsigned long *pio_mask, unsigned long *mwdma_mask, unsigned long *udma_mask) { if (pio_mask) @@ -850,7 +850,7 @@ static const struct ata_xfer_ent { * RETURNS: * Matching XFER_* value, 0xff if no match found. */ -u8 ata_xfer_mask2mode(unsigned long xfer_mask) +u8 ata_xfer_mask2mode(unsigned int xfer_mask) { int highbit = fls(xfer_mask) - 1; const struct ata_xfer_ent *ent; @@ -874,7 +874,7 @@ EXPORT_SYMBOL_GPL(ata_xfer_mask2mode); * RETURNS: * Matching xfer_mask, 0 if no match found. */ -unsigned long ata_xfer_mode2mask(u8 xfer_mode) +unsigned int ata_xfer_mode2mask(u8 xfer_mode) { const struct ata_xfer_ent *ent; @@ -923,7 +923,7 @@ EXPORT_SYMBOL_GPL(ata_xfer_mode2shift); * Constant C string representing highest speed listed in * @mode_mask, or the constant C string "<n/a>". */ -const char *ata_mode_string(unsigned long xfer_mask) +const char *ata_mode_string(unsigned int xfer_mask) { static const char * const xfer_mode_str[] = { "PIO0", @@ -1376,7 +1376,7 @@ static inline void ata_dump_id(struct ata_device *dev, const u16 *id) * RETURNS: * Computed xfermask */ -unsigned long ata_id_xfermask(const u16 *id) +unsigned int ata_id_xfermask(const u16 *id) { unsigned long pio_mask, mwdma_mask, udma_mask; @@ -2522,7 +2522,7 @@ int ata_dev_configure(struct ata_device *dev) struct ata_port *ap = dev->link->ap; bool print_info = ata_dev_print_info(dev); const u16 *id = dev->id; - unsigned long xfer_mask; + unsigned int xfer_mask; unsigned int err_mask; char revbuf[7]; /* XYZ-99\0 */ char fwrevbuf[ATA_ID_FW_REV_LEN+1]; @@ -3190,7 +3190,7 @@ u8 ata_timing_cycle2mode(unsigned int xfer_shift, int cycle) int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel) { char buf[32]; - unsigned long orig_mask, xfer_mask; + unsigned int orig_mask, xfer_mask; unsigned long pio_mask, mwdma_mask, udma_mask; int quiet, highbit; @@ -3369,7 +3369,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev) /* step 1: calculate xfer_mask */ ata_for_each_dev(dev, link, ENABLED) { - unsigned long pio_mask, dma_mask; + unsigned int pio_mask, dma_mask; unsigned int mode_mask; mode_mask = ATA_DMA_MASK_ATA; @@ -4205,7 +4205,7 @@ static void ata_dev_xfermask(struct ata_device *dev) struct ata_link *link = dev->link; struct ata_port *ap = link->ap; struct ata_host *host = ap->host; - unsigned long xfer_mask; + unsigned int xfer_mask; /* controller modes available */ xfer_mask = ata_pack_xfermask(ap->pio_mask, @@ -5764,7 +5764,7 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) /* set cable, sata_spd_limit and report */ for (i = 0; i < host->n_ports; i++) { struct ata_port *ap = host->ports[i]; - unsigned long xfer_mask; + unsigned int xfer_mask; /* set SATA cable type if still unset */ if (ap->cbl == ATA_CBL_NONE && (ap->flags & ATA_FLAG_SATA)) diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c index ade4c3eee230..f8706ee427d2 100644 --- a/drivers/ata/pata_acpi.c +++ b/drivers/ata/pata_acpi.c @@ -97,7 +97,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 mask & acpi->mask[adev->devno]; diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c index 1b90cda27246..76ad0e73fe2a 100644 --- a/drivers/ata/pata_ali.c +++ b/drivers/ata/pata_ali.c @@ -115,7 +115,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_amd.c b/drivers/ata/pata_amd.c index 154748cfcc79..f216f9d7b9ec 100644 --- a/drivers/ata/pata_amd.c +++ b/drivers/ata/pata_amd.c @@ -264,8 +264,8 @@ static void amd133_set_dmamode(struct ata_port *ap, struct ata_device *adev) * cached during driver attach and are consulted to select transfer * mode. */ -static unsigned long nv_mode_filter(struct ata_device *dev, - unsigned long xfer_mask) +static unsigned int nv_mode_filter(struct ata_device *dev, + unsigned int xfer_mask) { static const unsigned int udma_mask_map[] = { ATA_UDMA2, ATA_UDMA1, ATA_UDMA0, 0, @@ -274,7 +274,7 @@ static unsigned long nv_mode_filter(struct ata_device *dev, char acpi_str[32] = ""; u32 saved_udma, udma; const struct ata_acpi_gtm *gtm; - unsigned long bios_limit = 0, acpi_limit = 0, limit; + unsigned int bios_limit = 0, acpi_limit = 0, limit; /* find out what BIOS configured */ udma = saved_udma = (unsigned long)ap->host->private_data; @@ -310,10 +310,10 @@ static unsigned long nv_mode_filter(struct ata_device *dev, cable detection result */ limit |= ata_pack_xfermask(ATA_PIO4, ATA_MWDMA2, ATA_UDMA2); - ata_port_dbg(ap, "nv_mode_filter: 0x%lx&0x%lx->0x%lx, " - "BIOS=0x%lx (0x%x) ACPI=0x%lx%s\n", - xfer_mask, limit, xfer_mask & limit, bios_limit, - saved_udma, acpi_limit, acpi_str); + ata_port_dbg(ap, + "nv_mode_filter: 0x%x&0x%x->0x%x, BIOS=0x%x (0x%x) ACPI=0x%x%s\n", + xfer_mask, limit, xfer_mask & limit, bios_limit, + saved_udma, acpi_limit, acpi_str); return xfer_mask & limit; } diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c index c99e8f0708b3..7e441fb304d3 100644 --- a/drivers/ata/pata_hpt366.c +++ b/drivers/ata/pata_hpt366.c @@ -194,7 +194,7 @@ static int hpt_dma_blacklisted(const struct ata_device *dev, char *modestr, * 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 156f304ef051..0e9f490214de 100644 --- a/drivers/ata/pata_hpt37x.c +++ b/drivers/ata/pata_hpt37x.c @@ -278,7 +278,7 @@ static const char * const 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)) @@ -297,7 +297,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)) @@ -314,7 +314,7 @@ static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask) * The Marvell bridge chips used on the HighPoint SATA cards do not seem * to support the UltraDMA modes 1, 2, and 3 as well as any MWDMA modes... */ -static unsigned long hpt372_filter(struct ata_device *adev, unsigned long mask) +static unsigned int hpt372_filter(struct ata_device *adev, unsigned int mask) { if (ata_id_is_sata(adev->id)) mask &= ~((0xE << ATA_SHIFT_UDMA) | ATA_MASK_MWDMA); diff --git a/drivers/ata/pata_hpt3x2n.c b/drivers/ata/pata_hpt3x2n.c index 1f6afd8ee29b..071b2e4abf1d 100644 --- a/drivers/ata/pata_hpt3x2n.c +++ b/drivers/ata/pata_hpt3x2n.c @@ -113,7 +113,7 @@ static u32 hpt3x2n_find_mode(struct ata_port *ap, int speed) * The Marvell bridge chips used on the HighPoint SATA cards do not seem * to support the UltraDMA modes 1, 2, and 3 as well as any MWDMA modes... */ -static unsigned long hpt372n_filter(struct ata_device *adev, unsigned long mask) +static unsigned int hpt372n_filter(struct ata_device *adev, unsigned int mask) { if (ata_id_is_sata(adev->id)) mask &= ~((0xE << ATA_SHIFT_UDMA) | ATA_MASK_MWDMA); diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c index 4fbb3eed8b0b..4191aa61c8e4 100644 --- a/drivers/ata/pata_pdc2027x.c +++ b/drivers/ata/pata_pdc2027x.c @@ -57,7 +57,7 @@ static int pdc2027x_prereset(struct ata_link *link, unsigned long deadline); 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 pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed); @@ -251,7 +251,7 @@ static int pdc2027x_prereset(struct ata_link *link, unsigned long deadline) * Block UDMA on devices that cause trouble with this controller. */ -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) { unsigned char model_num[ATA_ID_PROD_LEN + 1]; struct ata_device *pair = ata_dev_pair(adev); diff --git a/drivers/ata/pata_serverworks.c b/drivers/ata/pata_serverworks.c index e410fe44177f..c0bc4af0d196 100644 --- a/drivers/ata/pata_serverworks.c +++ b/drivers/ata/pata_serverworks.c @@ -150,7 +150,7 @@ static u8 serverworks_is_csb(struct pci_dev *pdev) * bug we hit. */ -static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned long mask) +static unsigned int serverworks_osb4_filter(struct ata_device *adev, unsigned int mask) { if (adev->class == ATA_DEV_ATA) mask &= ~ATA_MASK_UDMA; @@ -166,7 +166,7 @@ static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned l * Check the blacklist and disable UDMA5 if matched */ -static unsigned long serverworks_csb_filter(struct ata_device *adev, unsigned long mask) +static unsigned int serverworks_csb_filter(struct ata_device *adev, unsigned int mask) { const char *p; char model_num[ATA_ID_PROD_LEN + 1]; diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c index b5b764e18adf..92e4cf05de2c 100644 --- a/drivers/ata/pata_sis.c +++ b/drivers/ata/pata_sis.c @@ -525,7 +525,7 @@ static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev) * Block UDMA6 on devices that do not support it. */ -static unsigned long sis_133_mode_filter(struct ata_device *adev, unsigned long mask) +static unsigned int sis_133_mode_filter(struct ata_device *adev, unsigned int mask) { struct ata_port *ap = adev->link->ap; struct pci_dev *pdev = to_pci_dev(ap->host->dev); diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c index 215c02d4056a..34f00f389932 100644 --- a/drivers/ata/pata_via.c +++ b/drivers/ata/pata_via.c @@ -352,7 +352,7 @@ static void via_set_dmamode(struct ata_port *ap, struct ata_device *adev) * one breed of Transcend SSD. Return the updated mask. */ -static unsigned long via_mode_filter(struct ata_device *dev, unsigned long mask) +static unsigned int via_mode_filter(struct ata_device *dev, unsigned int mask) { struct ata_host *host = dev->link->ap->host; const struct via_isa_bridge *config = host->private_data; diff --git a/include/linux/libata.h b/include/linux/libata.h index 732de9014626..1429b7012ae8 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -275,7 +275,7 @@ enum { PORT_DISABLED = 2, /* encoding various smaller bitmaps into a single - * unsigned long bitmap + * unsigned int bitmap */ ATA_NR_PIO_MODES = 7, ATA_NR_MWDMA_MODES = 5, @@ -426,12 +426,9 @@ enum { }; enum ata_xfer_mask { - ATA_MASK_PIO = ((1LU << ATA_NR_PIO_MODES) - 1) - << ATA_SHIFT_PIO, - ATA_MASK_MWDMA = ((1LU << ATA_NR_MWDMA_MODES) - 1) - << ATA_SHIFT_MWDMA, - ATA_MASK_UDMA = ((1LU << ATA_NR_UDMA_MODES) - 1) - << ATA_SHIFT_UDMA, + ATA_MASK_PIO = ((1U << ATA_NR_PIO_MODES) - 1) << ATA_SHIFT_PIO, + ATA_MASK_MWDMA = ((1U << ATA_NR_MWDMA_MODES) - 1) << ATA_SHIFT_MWDMA, + ATA_MASK_UDMA = ((1U << ATA_NR_UDMA_MODES) - 1) << ATA_SHIFT_UDMA, }; enum hsm_task_states { @@ -886,7 +883,7 @@ struct ata_port_operations { * Configuration and exception handling */ int (*cable_detect)(struct ata_port *ap); - unsigned long (*mode_filter)(struct ata_device *dev, unsigned long xfer_mask); + unsigned int (*mode_filter)(struct ata_device *dev, unsigned int xfer_mask); void (*set_piomode)(struct ata_port *ap, struct ata_device *dev); void (*set_dmamode)(struct ata_port *ap, struct ata_device *dev); int (*set_mode)(struct ata_link *link, struct ata_device **r_failed_dev); @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, u32 val, unsigned long interval, unsigned long timeout); extern int atapi_cmd_type(u8 opcode); -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, - unsigned long mwdma_mask, unsigned long udma_mask); -extern void ata_unpack_xfermask(unsigned long xfer_mask, - unsigned long *pio_mask, unsigned long *mwdma_mask, - unsigned long *udma_mask); -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, + unsigned long mwdma_mask, + unsigned long udma_mask); +extern void ata_unpack_xfermask(unsigned int xfer_mask, + unsigned long *pio_mask, + unsigned long *mwdma_mask, + unsigned long *udma_mask); +extern u8 ata_xfer_mask2mode(unsigned int xfer_mask); +extern unsigned int ata_xfer_mode2mask(u8 xfer_mode); extern int ata_xfer_mode2shift(u8 xfer_mode); -extern const char *ata_mode_string(unsigned long xfer_mask); -extern unsigned long ata_id_xfermask(const u16 *id); +extern const char *ata_mode_string(unsigned int xfer_mask); +extern unsigned int ata_id_xfermask(const u16 *id); extern int ata_std_qc_defer(struct ata_queued_cmd *qc); extern enum ata_completion_errors ata_noop_qc_prep(struct ata_queued_cmd *qc); extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg, @@ -1284,8 +1283,8 @@ static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) } int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm); int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm); -unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev, - const struct ata_acpi_gtm *gtm); +unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev, + const struct ata_acpi_gtm *gtm); int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm); #else static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) -- 2.26.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-05-08 20:41 ` [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* Sergey Shtylyov @ 2022-05-16 11:17 ` Damien Le Moal 2022-05-19 20:19 ` Sergey Shtylyov 0 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-05-16 11:17 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 2022/05/08 22:41, Sergey Shtylyov wrote: > The packed transfer mode masks are declared as *unsigned long* (which is > a 64-bit type on 64-bit architectures), however the actual mask occupies > only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can > safely use 32-bit *unsigned int* variables instead. Convert all libata > functions taking as a parameter or returning a packed transfer mode mask. > This saves 470 bytes of object code in libata-core.o alone... > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > --- > drivers/ata/libata-acpi.c | 9 +++++---- > drivers/ata/libata-core.c | 28 ++++++++++++------------- > drivers/ata/pata_acpi.c | 2 +- > drivers/ata/pata_ali.c | 2 +- > drivers/ata/pata_amd.c | 14 ++++++------- > drivers/ata/pata_hpt366.c | 2 +- > drivers/ata/pata_hpt37x.c | 6 +++--- > drivers/ata/pata_hpt3x2n.c | 2 +- > drivers/ata/pata_pdc2027x.c | 4 ++-- > drivers/ata/pata_serverworks.c | 4 ++-- > drivers/ata/pata_sis.c | 2 +- > drivers/ata/pata_via.c | 2 +- > include/linux/libata.h | 37 +++++++++++++++++----------------- > 13 files changed, 57 insertions(+), 57 deletions(-) > > diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c > index 3d345d173556..cdb7c0238a43 100644 > --- a/drivers/ata/libata-acpi.c > +++ b/drivers/ata/libata-acpi.c > @@ -480,10 +480,10 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) > * RETURNS: > * Determined xfermask. > */ > -unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev, > - const struct ata_acpi_gtm *gtm) > +unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev, > + const struct ata_acpi_gtm *gtm) > { > - unsigned long xfer_mask = 0; > + unsigned int xfer_mask = 0; > unsigned int type; > int unit; > u8 mode; > @@ -525,7 +525,8 @@ int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm) > struct ata_device *dev; > > ata_for_each_dev(dev, &ap->link, ENABLED) { > - unsigned long xfer_mask, udma_mask; > + unsigned long udma_mask; Why not change this one to unsigned int too ? See below. > + unsigned int xfer_mask; > > xfer_mask = ata_acpi_gtm_xfermask(dev, gtm); > ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask); > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 95bd028aea4f..2a6f301c3359 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -93,7 +93,7 @@ struct ata_force_param { > const char *name; > u8 cbl; > u8 spd_limit; > - unsigned long xfer_mask; > + unsigned int xfer_mask; > unsigned int horkage_on; > unsigned int horkage_off; > u16 lflags; > @@ -796,9 +796,9 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, > * RETURNS: > * Packed xfer_mask. > */ > -unsigned long ata_pack_xfermask(unsigned long pio_mask, > - unsigned long mwdma_mask, > - unsigned long udma_mask) > +unsigned int ata_pack_xfermask(unsigned long pio_mask, > + unsigned long mwdma_mask, > + unsigned long udma_mask) > { > return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | > ((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) | > @@ -816,7 +816,7 @@ EXPORT_SYMBOL_GPL(ata_pack_xfermask); > * Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask. > * Any NULL destination masks will be ignored. > */ > -void ata_unpack_xfermask(unsigned long xfer_mask, unsigned long *pio_mask, > +void ata_unpack_xfermask(unsigned int xfer_mask, unsigned long *pio_mask, > unsigned long *mwdma_mask, unsigned long *udma_mask) > { > if (pio_mask) > @@ -850,7 +850,7 @@ static const struct ata_xfer_ent { > * RETURNS: > * Matching XFER_* value, 0xff if no match found. > */ > -u8 ata_xfer_mask2mode(unsigned long xfer_mask) > +u8 ata_xfer_mask2mode(unsigned int xfer_mask) > { > int highbit = fls(xfer_mask) - 1; > const struct ata_xfer_ent *ent; > @@ -874,7 +874,7 @@ EXPORT_SYMBOL_GPL(ata_xfer_mask2mode); > * RETURNS: > * Matching xfer_mask, 0 if no match found. > */ > -unsigned long ata_xfer_mode2mask(u8 xfer_mode) > +unsigned int ata_xfer_mode2mask(u8 xfer_mode) > { > const struct ata_xfer_ent *ent; > > @@ -923,7 +923,7 @@ EXPORT_SYMBOL_GPL(ata_xfer_mode2shift); > * Constant C string representing highest speed listed in > * @mode_mask, or the constant C string "<n/a>". > */ > -const char *ata_mode_string(unsigned long xfer_mask) > +const char *ata_mode_string(unsigned int xfer_mask) > { > static const char * const xfer_mode_str[] = { > "PIO0", > @@ -1376,7 +1376,7 @@ static inline void ata_dump_id(struct ata_device *dev, const u16 *id) > * RETURNS: > * Computed xfermask > */ > -unsigned long ata_id_xfermask(const u16 *id) > +unsigned int ata_id_xfermask(const u16 *id) > { > unsigned long pio_mask, mwdma_mask, udma_mask; > > @@ -2522,7 +2522,7 @@ int ata_dev_configure(struct ata_device *dev) > struct ata_port *ap = dev->link->ap; > bool print_info = ata_dev_print_info(dev); > const u16 *id = dev->id; > - unsigned long xfer_mask; > + unsigned int xfer_mask; > unsigned int err_mask; > char revbuf[7]; /* XYZ-99\0 */ > char fwrevbuf[ATA_ID_FW_REV_LEN+1]; > @@ -3190,7 +3190,7 @@ u8 ata_timing_cycle2mode(unsigned int xfer_shift, int cycle) > int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel) > { > char buf[32]; > - unsigned long orig_mask, xfer_mask; > + unsigned int orig_mask, xfer_mask; > unsigned long pio_mask, mwdma_mask, udma_mask; > int quiet, highbit; > > @@ -3369,7 +3369,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev) > > /* step 1: calculate xfer_mask */ > ata_for_each_dev(dev, link, ENABLED) { > - unsigned long pio_mask, dma_mask; > + unsigned int pio_mask, dma_mask; > unsigned int mode_mask; > > mode_mask = ATA_DMA_MASK_ATA; > @@ -4205,7 +4205,7 @@ static void ata_dev_xfermask(struct ata_device *dev) > struct ata_link *link = dev->link; > struct ata_port *ap = link->ap; > struct ata_host *host = ap->host; > - unsigned long xfer_mask; > + unsigned int xfer_mask; > > /* controller modes available */ > xfer_mask = ata_pack_xfermask(ap->pio_mask, > @@ -5764,7 +5764,7 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) > /* set cable, sata_spd_limit and report */ > for (i = 0; i < host->n_ports; i++) { > struct ata_port *ap = host->ports[i]; > - unsigned long xfer_mask; > + unsigned int xfer_mask; > > /* set SATA cable type if still unset */ > if (ap->cbl == ATA_CBL_NONE && (ap->flags & ATA_FLAG_SATA)) > diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c > index ade4c3eee230..f8706ee427d2 100644 > --- a/drivers/ata/pata_acpi.c > +++ b/drivers/ata/pata_acpi.c > @@ -97,7 +97,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 mask & acpi->mask[adev->devno]; > diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c > index 1b90cda27246..76ad0e73fe2a 100644 > --- a/drivers/ata/pata_ali.c > +++ b/drivers/ata/pata_ali.c > @@ -115,7 +115,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_amd.c b/drivers/ata/pata_amd.c > index 154748cfcc79..f216f9d7b9ec 100644 > --- a/drivers/ata/pata_amd.c > +++ b/drivers/ata/pata_amd.c > @@ -264,8 +264,8 @@ static void amd133_set_dmamode(struct ata_port *ap, struct ata_device *adev) > * cached during driver attach and are consulted to select transfer > * mode. > */ > -static unsigned long nv_mode_filter(struct ata_device *dev, > - unsigned long xfer_mask) > +static unsigned int nv_mode_filter(struct ata_device *dev, > + unsigned int xfer_mask) > { > static const unsigned int udma_mask_map[] = > { ATA_UDMA2, ATA_UDMA1, ATA_UDMA0, 0, > @@ -274,7 +274,7 @@ static unsigned long nv_mode_filter(struct ata_device *dev, > char acpi_str[32] = ""; > u32 saved_udma, udma; > const struct ata_acpi_gtm *gtm; > - unsigned long bios_limit = 0, acpi_limit = 0, limit; > + unsigned int bios_limit = 0, acpi_limit = 0, limit; > > /* find out what BIOS configured */ > udma = saved_udma = (unsigned long)ap->host->private_data; > @@ -310,10 +310,10 @@ static unsigned long nv_mode_filter(struct ata_device *dev, > cable detection result */ > limit |= ata_pack_xfermask(ATA_PIO4, ATA_MWDMA2, ATA_UDMA2); > > - ata_port_dbg(ap, "nv_mode_filter: 0x%lx&0x%lx->0x%lx, " > - "BIOS=0x%lx (0x%x) ACPI=0x%lx%s\n", > - xfer_mask, limit, xfer_mask & limit, bios_limit, > - saved_udma, acpi_limit, acpi_str); > + ata_port_dbg(ap, > + "nv_mode_filter: 0x%x&0x%x->0x%x, BIOS=0x%x (0x%x) ACPI=0x%x%s\n", > + xfer_mask, limit, xfer_mask & limit, bios_limit, > + saved_udma, acpi_limit, acpi_str); > > return xfer_mask & limit; > } > diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c > index c99e8f0708b3..7e441fb304d3 100644 > --- a/drivers/ata/pata_hpt366.c > +++ b/drivers/ata/pata_hpt366.c > @@ -194,7 +194,7 @@ static int hpt_dma_blacklisted(const struct ata_device *dev, char *modestr, > * 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 156f304ef051..0e9f490214de 100644 > --- a/drivers/ata/pata_hpt37x.c > +++ b/drivers/ata/pata_hpt37x.c > @@ -278,7 +278,7 @@ static const char * const 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)) > @@ -297,7 +297,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)) > @@ -314,7 +314,7 @@ static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask) > * The Marvell bridge chips used on the HighPoint SATA cards do not seem > * to support the UltraDMA modes 1, 2, and 3 as well as any MWDMA modes... > */ > -static unsigned long hpt372_filter(struct ata_device *adev, unsigned long mask) > +static unsigned int hpt372_filter(struct ata_device *adev, unsigned int mask) > { > if (ata_id_is_sata(adev->id)) > mask &= ~((0xE << ATA_SHIFT_UDMA) | ATA_MASK_MWDMA); > diff --git a/drivers/ata/pata_hpt3x2n.c b/drivers/ata/pata_hpt3x2n.c > index 1f6afd8ee29b..071b2e4abf1d 100644 > --- a/drivers/ata/pata_hpt3x2n.c > +++ b/drivers/ata/pata_hpt3x2n.c > @@ -113,7 +113,7 @@ static u32 hpt3x2n_find_mode(struct ata_port *ap, int speed) > * The Marvell bridge chips used on the HighPoint SATA cards do not seem > * to support the UltraDMA modes 1, 2, and 3 as well as any MWDMA modes... > */ > -static unsigned long hpt372n_filter(struct ata_device *adev, unsigned long mask) > +static unsigned int hpt372n_filter(struct ata_device *adev, unsigned int mask) > { > if (ata_id_is_sata(adev->id)) > mask &= ~((0xE << ATA_SHIFT_UDMA) | ATA_MASK_MWDMA); > diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c > index 4fbb3eed8b0b..4191aa61c8e4 100644 > --- a/drivers/ata/pata_pdc2027x.c > +++ b/drivers/ata/pata_pdc2027x.c > @@ -57,7 +57,7 @@ static int pdc2027x_prereset(struct ata_link *link, unsigned long deadline); > 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 pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed); > > @@ -251,7 +251,7 @@ static int pdc2027x_prereset(struct ata_link *link, unsigned long deadline) > * Block UDMA on devices that cause trouble with this controller. > */ > > -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) > { > unsigned char model_num[ATA_ID_PROD_LEN + 1]; > struct ata_device *pair = ata_dev_pair(adev); > diff --git a/drivers/ata/pata_serverworks.c b/drivers/ata/pata_serverworks.c > index e410fe44177f..c0bc4af0d196 100644 > --- a/drivers/ata/pata_serverworks.c > +++ b/drivers/ata/pata_serverworks.c > @@ -150,7 +150,7 @@ static u8 serverworks_is_csb(struct pci_dev *pdev) > * bug we hit. > */ > > -static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned long mask) > +static unsigned int serverworks_osb4_filter(struct ata_device *adev, unsigned int mask) > { > if (adev->class == ATA_DEV_ATA) > mask &= ~ATA_MASK_UDMA; > @@ -166,7 +166,7 @@ static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned l > * Check the blacklist and disable UDMA5 if matched > */ > > -static unsigned long serverworks_csb_filter(struct ata_device *adev, unsigned long mask) > +static unsigned int serverworks_csb_filter(struct ata_device *adev, unsigned int mask) > { > const char *p; > char model_num[ATA_ID_PROD_LEN + 1]; > diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c > index b5b764e18adf..92e4cf05de2c 100644 > --- a/drivers/ata/pata_sis.c > +++ b/drivers/ata/pata_sis.c > @@ -525,7 +525,7 @@ static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev) > * Block UDMA6 on devices that do not support it. > */ > > -static unsigned long sis_133_mode_filter(struct ata_device *adev, unsigned long mask) > +static unsigned int sis_133_mode_filter(struct ata_device *adev, unsigned int mask) > { > struct ata_port *ap = adev->link->ap; > struct pci_dev *pdev = to_pci_dev(ap->host->dev); > diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c > index 215c02d4056a..34f00f389932 100644 > --- a/drivers/ata/pata_via.c > +++ b/drivers/ata/pata_via.c > @@ -352,7 +352,7 @@ static void via_set_dmamode(struct ata_port *ap, struct ata_device *adev) > * one breed of Transcend SSD. Return the updated mask. > */ > > -static unsigned long via_mode_filter(struct ata_device *dev, unsigned long mask) > +static unsigned int via_mode_filter(struct ata_device *dev, unsigned int mask) > { > struct ata_host *host = dev->link->ap->host; > const struct via_isa_bridge *config = host->private_data; > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 732de9014626..1429b7012ae8 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -275,7 +275,7 @@ enum { > PORT_DISABLED = 2, > > /* encoding various smaller bitmaps into a single > - * unsigned long bitmap > + * unsigned int bitmap > */ > ATA_NR_PIO_MODES = 7, > ATA_NR_MWDMA_MODES = 5, > @@ -426,12 +426,9 @@ enum { > }; > > enum ata_xfer_mask { > - ATA_MASK_PIO = ((1LU << ATA_NR_PIO_MODES) - 1) > - << ATA_SHIFT_PIO, > - ATA_MASK_MWDMA = ((1LU << ATA_NR_MWDMA_MODES) - 1) > - << ATA_SHIFT_MWDMA, > - ATA_MASK_UDMA = ((1LU << ATA_NR_UDMA_MODES) - 1) > - << ATA_SHIFT_UDMA, > + ATA_MASK_PIO = ((1U << ATA_NR_PIO_MODES) - 1) << ATA_SHIFT_PIO, > + ATA_MASK_MWDMA = ((1U << ATA_NR_MWDMA_MODES) - 1) << ATA_SHIFT_MWDMA, > + ATA_MASK_UDMA = ((1U << ATA_NR_UDMA_MODES) - 1) << ATA_SHIFT_UDMA, > }; > > enum hsm_task_states { > @@ -886,7 +883,7 @@ struct ata_port_operations { > * Configuration and exception handling > */ > int (*cable_detect)(struct ata_port *ap); > - unsigned long (*mode_filter)(struct ata_device *dev, unsigned long xfer_mask); > + unsigned int (*mode_filter)(struct ata_device *dev, unsigned int xfer_mask); > void (*set_piomode)(struct ata_port *ap, struct ata_device *dev); > void (*set_dmamode)(struct ata_port *ap, struct ata_device *dev); > int (*set_mode)(struct ata_link *link, struct ata_device **r_failed_dev); > @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); > extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, > u32 val, unsigned long interval, unsigned long timeout); > extern int atapi_cmd_type(u8 opcode); > -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, > - unsigned long mwdma_mask, unsigned long udma_mask); > -extern void ata_unpack_xfermask(unsigned long xfer_mask, > - unsigned long *pio_mask, unsigned long *mwdma_mask, > - unsigned long *udma_mask); > -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); > -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); > +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, > + unsigned long mwdma_mask, > + unsigned long udma_mask); > +extern void ata_unpack_xfermask(unsigned int xfer_mask, > + unsigned long *pio_mask, > + unsigned long *mwdma_mask, > + unsigned long *udma_mask); Why not change all of these to unsigned int too ? They are defined as "1LU << shift" but everything actually fits within 32 bits > +extern u8 ata_xfer_mask2mode(unsigned int xfer_mask); > +extern unsigned int ata_xfer_mode2mask(u8 xfer_mode); > extern int ata_xfer_mode2shift(u8 xfer_mode); > -extern const char *ata_mode_string(unsigned long xfer_mask); > -extern unsigned long ata_id_xfermask(const u16 *id); > +extern const char *ata_mode_string(unsigned int xfer_mask); > +extern unsigned int ata_id_xfermask(const u16 *id); > extern int ata_std_qc_defer(struct ata_queued_cmd *qc); > extern enum ata_completion_errors ata_noop_qc_prep(struct ata_queued_cmd *qc); > extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg, > @@ -1284,8 +1283,8 @@ static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) > } > int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm); > int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm); > -unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev, > - const struct ata_acpi_gtm *gtm); > +unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev, > + const struct ata_acpi_gtm *gtm); > int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm); > #else > static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-05-16 11:17 ` Damien Le Moal @ 2022-05-19 20:19 ` Sergey Shtylyov 2022-05-20 3:37 ` Damien Le Moal 0 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-19 20:19 UTC (permalink / raw) To: Damien Le Moal, linux-ide Hello! On 5/16/22 2:17 PM, Damien Le Moal wrote: >> The packed transfer mode masks are declared as *unsigned long* (which is >> a 64-bit type on 64-bit architectures), however the actual mask occupies >> only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can >> safely use 32-bit *unsigned int* variables instead. Convert all libata >> functions taking as a parameter or returning a packed transfer mode mask. >> This saves 470 bytes of object code in libata-core.o alone... >> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] >> diff --git a/include/linux/libata.h b/include/linux/libata.h >> index 732de9014626..1429b7012ae8 100644 >> --- a/include/linux/libata.h >> +++ b/include/linux/libata.h [...] >> @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); >> extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, >> u32 val, unsigned long interval, unsigned long timeout); >> extern int atapi_cmd_type(u8 opcode); >> -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, >> - unsigned long mwdma_mask, unsigned long udma_mask); >> -extern void ata_unpack_xfermask(unsigned long xfer_mask, >> - unsigned long *pio_mask, unsigned long *mwdma_mask, >> - unsigned long *udma_mask); >> -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); >> -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); >> +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, >> + unsigned long mwdma_mask, >> + unsigned long udma_mask); >> +extern void ata_unpack_xfermask(unsigned int xfer_mask, >> + unsigned long *pio_mask, >> + unsigned long *mwdma_mask, >> + unsigned long *udma_mask); > > Why not change all of these to unsigned int too ? Done in the 2nd patch. > They are defined as "1LU << shift" but everything actually fits within 32 bits No, they are #define'd as *int* masks in ata.h, not as *unsigned long* in libata.h... [...] MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-05-19 20:19 ` Sergey Shtylyov @ 2022-05-20 3:37 ` Damien Le Moal 2022-06-10 20:19 ` Sergey Shtylyov 0 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-05-20 3:37 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 5/20/22 05:19, Sergey Shtylyov wrote: > Hello! > > On 5/16/22 2:17 PM, Damien Le Moal wrote: > >>> The packed transfer mode masks are declared as *unsigned long* (which is >>> a 64-bit type on 64-bit architectures), however the actual mask occupies >>> only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can >>> safely use 32-bit *unsigned int* variables instead. Convert all libata >>> functions taking as a parameter or returning a packed transfer mode mask. >>> This saves 470 bytes of object code in libata-core.o alone... >>> >>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > [...] >>> diff --git a/include/linux/libata.h b/include/linux/libata.h >>> index 732de9014626..1429b7012ae8 100644 >>> --- a/include/linux/libata.h >>> +++ b/include/linux/libata.h > [...] >>> @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); >>> extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, >>> u32 val, unsigned long interval, unsigned long timeout); >>> extern int atapi_cmd_type(u8 opcode); >>> -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, >>> - unsigned long mwdma_mask, unsigned long udma_mask); >>> -extern void ata_unpack_xfermask(unsigned long xfer_mask, >>> - unsigned long *pio_mask, unsigned long *mwdma_mask, >>> - unsigned long *udma_mask); >>> -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); >>> -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); >>> +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, >>> + unsigned long mwdma_mask, >>> + unsigned long udma_mask); >>> +extern void ata_unpack_xfermask(unsigned int xfer_mask, >>> + unsigned long *pio_mask, >>> + unsigned long *mwdma_mask, >>> + unsigned long *udma_mask); >> >> Why not change all of these to unsigned int too ? > > Done in the 2nd patch. > >> They are defined as "1LU << shift" but everything actually fits within 32 bits > > No, they are #define'd as *int* masks in ata.h, not as *unsigned long* in libata.h... I meant the mask macro values used to set these fields are all defined as unsigned long with "1LU << shift" defines. See enum ata_xfer_mask in libata.h. > > [...] > > MBR, Sergey -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-05-20 3:37 ` Damien Le Moal @ 2022-06-10 20:19 ` Sergey Shtylyov 2022-06-12 23:18 ` Damien Le Moal 0 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-06-10 20:19 UTC (permalink / raw) To: Damien Le Moal, linux-ide On 5/20/22 6:37 AM, Damien Le Moal wrote: >>>> The packed transfer mode masks are declared as *unsigned long* (which is >>>> a 64-bit type on 64-bit architectures), however the actual mask occupies >>>> only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can >>>> safely use 32-bit *unsigned int* variables instead. Convert all libata >>>> functions taking as a parameter or returning a packed transfer mode mask. >>>> This saves 470 bytes of object code in libata-core.o alone... >>>> >>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> >> [...] >>>> diff --git a/include/linux/libata.h b/include/linux/libata.h >>>> index 732de9014626..1429b7012ae8 100644 >>>> --- a/include/linux/libata.h >>>> +++ b/include/linux/libata.h >> [...] >>>> @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); >>>> extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, >>>> u32 val, unsigned long interval, unsigned long timeout); >>>> extern int atapi_cmd_type(u8 opcode); >>>> -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, >>>> - unsigned long mwdma_mask, unsigned long udma_mask); >>>> -extern void ata_unpack_xfermask(unsigned long xfer_mask, >>>> - unsigned long *pio_mask, unsigned long *mwdma_mask, >>>> - unsigned long *udma_mask); >>>> -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); >>>> -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); >>>> +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, >>>> + unsigned long mwdma_mask, >>>> + unsigned long udma_mask); >>>> +extern void ata_unpack_xfermask(unsigned int xfer_mask, >>>> + unsigned long *pio_mask, >>>> + unsigned long *mwdma_mask, >>>> + unsigned long *udma_mask); >>> >>> Why not change all of these to unsigned int too ? >> >> Done in the 2nd patch. >> >>> They are defined as "1LU << shift" but everything actually fits within 32 bits >> >> No, they are #define'd as *int* masks in ata.h, not as *unsigned long* in libata.h... > > I meant the mask macro values used to set these fields are all defined as > unsigned long with "1LU << shift" defines. See enum ata_xfer_mask in libata.h. What does it have to do with {pio|mwdma|udma}_mask? These *enum*s values are used solely for the packed mask... MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* 2022-06-10 20:19 ` Sergey Shtylyov @ 2022-06-12 23:18 ` Damien Le Moal 0 siblings, 0 replies; 19+ messages in thread From: Damien Le Moal @ 2022-06-12 23:18 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 6/11/22 05:19, Sergey Shtylyov wrote: > On 5/20/22 6:37 AM, Damien Le Moal wrote: > >>>>> The packed transfer mode masks are declared as *unsigned long* (which is >>>>> a 64-bit type on 64-bit architectures), however the actual mask occupies >>>>> only 20 bits (7 PIO modes, 5 MWDMA modes, and 8 UDMA modes), so we can >>>>> safely use 32-bit *unsigned int* variables instead. Convert all libata >>>>> functions taking as a parameter or returning a packed transfer mode mask. >>>>> This saves 470 bytes of object code in libata-core.o alone... >>>>> >>>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> >>> [...] >>>>> diff --git a/include/linux/libata.h b/include/linux/libata.h >>>>> index 732de9014626..1429b7012ae8 100644 >>>>> --- a/include/linux/libata.h >>>>> +++ b/include/linux/libata.h >>> [...] >>>>> @@ -1103,16 +1100,18 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); >>>>> extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, >>>>> u32 val, unsigned long interval, unsigned long timeout); >>>>> extern int atapi_cmd_type(u8 opcode); >>>>> -extern unsigned long ata_pack_xfermask(unsigned long pio_mask, >>>>> - unsigned long mwdma_mask, unsigned long udma_mask); >>>>> -extern void ata_unpack_xfermask(unsigned long xfer_mask, >>>>> - unsigned long *pio_mask, unsigned long *mwdma_mask, >>>>> - unsigned long *udma_mask); >>>>> -extern u8 ata_xfer_mask2mode(unsigned long xfer_mask); >>>>> -extern unsigned long ata_xfer_mode2mask(u8 xfer_mode); >>>>> +extern unsigned int ata_pack_xfermask(unsigned long pio_mask, >>>>> + unsigned long mwdma_mask, >>>>> + unsigned long udma_mask); >>>>> +extern void ata_unpack_xfermask(unsigned int xfer_mask, >>>>> + unsigned long *pio_mask, >>>>> + unsigned long *mwdma_mask, >>>>> + unsigned long *udma_mask); >>>> >>>> Why not change all of these to unsigned int too ? >>> >>> Done in the 2nd patch. >>> >>>> They are defined as "1LU << shift" but everything actually fits within 32 bits >>> >>> No, they are #define'd as *int* masks in ata.h, not as *unsigned long* in libata.h... >> >> I meant the mask macro values used to set these fields are all defined as >> unsigned long with "1LU << shift" defines. See enum ata_xfer_mask in libata.h. > > What does it have to do with {pio|mwdma|udma}_mask? These *enum*s values are > used solely for the packed mask... These values are updated in patch 1, which I had missed. So OK. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* Sergey Shtylyov @ 2022-05-08 20:41 ` Sergey Shtylyov 2022-05-16 11:19 ` Damien Le Moal 2022-05-08 20:41 ` [PATCH 3/3] ata: make ata_port_info::{pio|mwdma|udma}_mask " Sergey Shtylyov ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-08 20:41 UTC (permalink / raw) To: Damien Le Moal, linux-ide The {pio|mwdma|udma}_mask fields of the *struct* ata_device are declared as *unsigned long* (which is a 64-bit type on 64-bit architectures) while the actual masks should easily fit into just 8 bits. The alike fields in the *struct* ata_port are already declared as *unsigned int*, so follow the suit, converting ata_[un]pack_xfermask() as necessary... Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- drivers/ata/libata-acpi.c | 3 +-- drivers/ata/libata-core.c | 18 +++++++++--------- include/linux/libata.h | 18 +++++++++--------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index cdb7c0238a43..61b4ccf88bf1 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -525,8 +525,7 @@ int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm) struct ata_device *dev; ata_for_each_dev(dev, &ap->link, ENABLED) { - unsigned long udma_mask; - unsigned int xfer_mask; + unsigned int xfer_mask, udma_mask; xfer_mask = ata_acpi_gtm_xfermask(dev, gtm); ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask); diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 2a6f301c3359..44af61dfba77 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -418,7 +418,7 @@ static void ata_force_xfermask(struct ata_device *dev) for (i = ata_force_tbl_size - 1; i >= 0; i--) { const struct ata_force_ent *fe = &ata_force_tbl[i]; - unsigned long pio_mask, mwdma_mask, udma_mask; + unsigned int pio_mask, mwdma_mask, udma_mask; if (fe->port != -1 && fe->port != dev->link->ap->print_id) continue; @@ -796,11 +796,11 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, * RETURNS: * Packed xfer_mask. */ -unsigned int ata_pack_xfermask(unsigned long pio_mask, - unsigned long mwdma_mask, - unsigned long udma_mask) +unsigned int ata_pack_xfermask(unsigned int pio_mask, + unsigned int mwdma_mask, + unsigned int udma_mask) { - return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | + return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | ((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) | ((udma_mask << ATA_SHIFT_UDMA) & ATA_MASK_UDMA); } @@ -816,8 +816,8 @@ EXPORT_SYMBOL_GPL(ata_pack_xfermask); * Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask. * Any NULL destination masks will be ignored. */ -void ata_unpack_xfermask(unsigned int xfer_mask, unsigned long *pio_mask, - unsigned long *mwdma_mask, unsigned long *udma_mask) +void ata_unpack_xfermask(unsigned int xfer_mask, unsigned int *pio_mask, + unsigned int *mwdma_mask, unsigned int *udma_mask) { if (pio_mask) *pio_mask = (xfer_mask & ATA_MASK_PIO) >> ATA_SHIFT_PIO; @@ -1378,7 +1378,7 @@ static inline void ata_dump_id(struct ata_device *dev, const u16 *id) */ unsigned int ata_id_xfermask(const u16 *id) { - unsigned long pio_mask, mwdma_mask, udma_mask; + unsigned int pio_mask, mwdma_mask, udma_mask; /* Usual case. Word 53 indicates word 64 is valid */ if (id[ATA_ID_FIELD_VALID] & (1 << 1)) { @@ -3191,7 +3191,7 @@ int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel) { char buf[32]; unsigned int orig_mask, xfer_mask; - unsigned long pio_mask, mwdma_mask, udma_mask; + unsigned int pio_mask, mwdma_mask, udma_mask; int quiet, highbit; quiet = !!(sel & ATA_DNXFER_QUIET); diff --git a/include/linux/libata.h b/include/linux/libata.h index 1429b7012ae8..f6fc482d767a 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -677,9 +677,9 @@ struct ata_device { unsigned int cdb_len; /* per-dev xfer mask */ - unsigned long pio_mask; - unsigned long mwdma_mask; - unsigned long udma_mask; + unsigned int pio_mask; + unsigned int mwdma_mask; + unsigned int udma_mask; /* for CHS addressing */ u16 cylinders; /* Number of cylinders */ @@ -1100,13 +1100,13 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, u32 val, unsigned long interval, unsigned long timeout); extern int atapi_cmd_type(u8 opcode); -extern unsigned int ata_pack_xfermask(unsigned long pio_mask, - unsigned long mwdma_mask, - unsigned long udma_mask); +extern unsigned int ata_pack_xfermask(unsigned int pio_mask, + unsigned int mwdma_mask, + unsigned int udma_mask); extern void ata_unpack_xfermask(unsigned int xfer_mask, - unsigned long *pio_mask, - unsigned long *mwdma_mask, - unsigned long *udma_mask); + unsigned int *pio_mask, + unsigned int *mwdma_mask, + unsigned int *udma_mask); extern u8 ata_xfer_mask2mode(unsigned int xfer_mask); extern unsigned int ata_xfer_mode2mask(u8 xfer_mode); extern int ata_xfer_mode2shift(u8 xfer_mode); -- 2.26.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* 2022-05-08 20:41 ` [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask " Sergey Shtylyov @ 2022-05-16 11:19 ` Damien Le Moal 2022-05-19 20:58 ` Sergey Shtylyov 0 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-05-16 11:19 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 2022/05/08 22:41, Sergey Shtylyov wrote: > The {pio|mwdma|udma}_mask fields of the *struct* ata_device are declared > as *unsigned long* (which is a 64-bit type on 64-bit architectures) while > the actual masks should easily fit into just 8 bits. The alike fields in > the *struct* ata_port are already declared as *unsigned int*, so follow > the suit, converting ata_[un]pack_xfermask() as necessary... > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > --- > drivers/ata/libata-acpi.c | 3 +-- > drivers/ata/libata-core.c | 18 +++++++++--------- > include/linux/libata.h | 18 +++++++++--------- > 3 files changed, 19 insertions(+), 20 deletions(-) > > diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c > index cdb7c0238a43..61b4ccf88bf1 100644 > --- a/drivers/ata/libata-acpi.c > +++ b/drivers/ata/libata-acpi.c > @@ -525,8 +525,7 @@ int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm) > struct ata_device *dev; > > ata_for_each_dev(dev, &ap->link, ENABLED) { > - unsigned long udma_mask; > - unsigned int xfer_mask; > + unsigned int xfer_mask, udma_mask; > > xfer_mask = ata_acpi_gtm_xfermask(dev, gtm); > ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask); > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 2a6f301c3359..44af61dfba77 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -418,7 +418,7 @@ static void ata_force_xfermask(struct ata_device *dev) > > for (i = ata_force_tbl_size - 1; i >= 0; i--) { > const struct ata_force_ent *fe = &ata_force_tbl[i]; > - unsigned long pio_mask, mwdma_mask, udma_mask; > + unsigned int pio_mask, mwdma_mask, udma_mask; > > if (fe->port != -1 && fe->port != dev->link->ap->print_id) > continue; > @@ -796,11 +796,11 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev, > * RETURNS: > * Packed xfer_mask. > */ > -unsigned int ata_pack_xfermask(unsigned long pio_mask, > - unsigned long mwdma_mask, > - unsigned long udma_mask) > +unsigned int ata_pack_xfermask(unsigned int pio_mask, > + unsigned int mwdma_mask, > + unsigned int udma_mask) > { > - return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | > + return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) | > ((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) | > ((udma_mask << ATA_SHIFT_UDMA) & ATA_MASK_UDMA); > } > @@ -816,8 +816,8 @@ EXPORT_SYMBOL_GPL(ata_pack_xfermask); > * Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask. > * Any NULL destination masks will be ignored. > */ > -void ata_unpack_xfermask(unsigned int xfer_mask, unsigned long *pio_mask, > - unsigned long *mwdma_mask, unsigned long *udma_mask) > +void ata_unpack_xfermask(unsigned int xfer_mask, unsigned int *pio_mask, > + unsigned int *mwdma_mask, unsigned int *udma_mask) > { > if (pio_mask) > *pio_mask = (xfer_mask & ATA_MASK_PIO) >> ATA_SHIFT_PIO; > @@ -1378,7 +1378,7 @@ static inline void ata_dump_id(struct ata_device *dev, const u16 *id) > */ > unsigned int ata_id_xfermask(const u16 *id) > { > - unsigned long pio_mask, mwdma_mask, udma_mask; > + unsigned int pio_mask, mwdma_mask, udma_mask; > > /* Usual case. Word 53 indicates word 64 is valid */ > if (id[ATA_ID_FIELD_VALID] & (1 << 1)) { > @@ -3191,7 +3191,7 @@ int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel) > { > char buf[32]; > unsigned int orig_mask, xfer_mask; > - unsigned long pio_mask, mwdma_mask, udma_mask; > + unsigned int pio_mask, mwdma_mask, udma_mask; > int quiet, highbit; > > quiet = !!(sel & ATA_DNXFER_QUIET); > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 1429b7012ae8..f6fc482d767a 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -677,9 +677,9 @@ struct ata_device { > unsigned int cdb_len; > > /* per-dev xfer mask */ > - unsigned long pio_mask; > - unsigned long mwdma_mask; > - unsigned long udma_mask; > + unsigned int pio_mask; > + unsigned int mwdma_mask; > + unsigned int udma_mask; Ah. OK. So you did this here... Hmmm. I would squash these 3 patches into a single one. Otherwise, we have sort-of a mess without all patches applied (making revert a pain if needed). > > /* for CHS addressing */ > u16 cylinders; /* Number of cylinders */ > @@ -1100,13 +1100,13 @@ extern void ata_msleep(struct ata_port *ap, unsigned int msecs); > extern u32 ata_wait_register(struct ata_port *ap, void __iomem *reg, u32 mask, > u32 val, unsigned long interval, unsigned long timeout); > extern int atapi_cmd_type(u8 opcode); > -extern unsigned int ata_pack_xfermask(unsigned long pio_mask, > - unsigned long mwdma_mask, > - unsigned long udma_mask); > +extern unsigned int ata_pack_xfermask(unsigned int pio_mask, > + unsigned int mwdma_mask, > + unsigned int udma_mask); > extern void ata_unpack_xfermask(unsigned int xfer_mask, > - unsigned long *pio_mask, > - unsigned long *mwdma_mask, > - unsigned long *udma_mask); > + unsigned int *pio_mask, > + unsigned int *mwdma_mask, > + unsigned int *udma_mask); > extern u8 ata_xfer_mask2mode(unsigned int xfer_mask); > extern unsigned int ata_xfer_mode2mask(u8 xfer_mode); > extern int ata_xfer_mode2shift(u8 xfer_mode); -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* 2022-05-16 11:19 ` Damien Le Moal @ 2022-05-19 20:58 ` Sergey Shtylyov 2022-05-20 3:31 ` Damien Le Moal 0 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-19 20:58 UTC (permalink / raw) To: Damien Le Moal, linux-ide On 5/16/22 2:19 PM, Damien Le Moal wrote: >> The {pio|mwdma|udma}_mask fields of the *struct* ata_device are declared >> as *unsigned long* (which is a 64-bit type on 64-bit architectures) while >> the actual masks should easily fit into just 8 bits. The alike fields in >> the *struct* ata_port are already declared as *unsigned int*, so follow >> the suit, converting ata_[un]pack_xfermask() as necessary... >> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] >> diff --git a/include/linux/libata.h b/include/linux/libata.h >> index 1429b7012ae8..f6fc482d767a 100644 >> --- a/include/linux/libata.h >> +++ b/include/linux/libata.h >> @@ -677,9 +677,9 @@ struct ata_device { >> unsigned int cdb_len; >> >> /* per-dev xfer mask */ >> - unsigned long pio_mask; >> - unsigned long mwdma_mask; >> - unsigned long udma_mask; >> + unsigned int pio_mask; >> + unsigned int mwdma_mask; >> + unsigned int udma_mask; > > Ah. OK. So you did this here... > Hmmm. I would squash these 3 patches into a single one. Otherwise, we have > sort-of a mess without all patches applied (making revert a pain if needed). Hm... please explain what kind of a mess... BTW do you really expect a revert? [...] MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* 2022-05-19 20:58 ` Sergey Shtylyov @ 2022-05-20 3:31 ` Damien Le Moal 2022-06-10 21:15 ` Sergey Shtylyov 0 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-05-20 3:31 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 5/20/22 05:58, Sergey Shtylyov wrote: > On 5/16/22 2:19 PM, Damien Le Moal wrote: > >>> The {pio|mwdma|udma}_mask fields of the *struct* ata_device are declared >>> as *unsigned long* (which is a 64-bit type on 64-bit architectures) while >>> the actual masks should easily fit into just 8 bits. The alike fields in >>> the *struct* ata_port are already declared as *unsigned int*, so follow >>> the suit, converting ata_[un]pack_xfermask() as necessary... >>> >>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > [...] >>> diff --git a/include/linux/libata.h b/include/linux/libata.h >>> index 1429b7012ae8..f6fc482d767a 100644 >>> --- a/include/linux/libata.h >>> +++ b/include/linux/libata.h >>> @@ -677,9 +677,9 @@ struct ata_device { >>> unsigned int cdb_len; >>> >>> /* per-dev xfer mask */ >>> - unsigned long pio_mask; >>> - unsigned long mwdma_mask; >>> - unsigned long udma_mask; >>> + unsigned int pio_mask; >>> + unsigned int mwdma_mask; >>> + unsigned int udma_mask; >> >> Ah. OK. So you did this here... >> Hmmm. I would squash these 3 patches into a single one. Otherwise, we have >> sort-of a mess without all patches applied (making revert a pain if needed). > > Hm... please explain what kind of a mess... BTW do you really expect a revert? The mess would be a partial conversion of the type in case of a revert being needed. And no, I do not expect a revert would be ever needed, but hey, never know :) > > [...] > > MBR, Sergey -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* 2022-05-20 3:31 ` Damien Le Moal @ 2022-06-10 21:15 ` Sergey Shtylyov 0 siblings, 0 replies; 19+ messages in thread From: Sergey Shtylyov @ 2022-06-10 21:15 UTC (permalink / raw) To: Damien Le Moal, linux-ide On 5/20/22 6:31 AM, Damien Le Moal wrote: [...] >>>> The {pio|mwdma|udma}_mask fields of the *struct* ata_device are declared >>>> as *unsigned long* (which is a 64-bit type on 64-bit architectures) while >>>> the actual masks should easily fit into just 8 bits. The alike fields in >>>> the *struct* ata_port are already declared as *unsigned int*, so follow >>>> the suit, converting ata_[un]pack_xfermask() as necessary... >>>> >>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> >> [...] >>>> diff --git a/include/linux/libata.h b/include/linux/libata.h >>>> index 1429b7012ae8..f6fc482d767a 100644 >>>> --- a/include/linux/libata.h >>>> +++ b/include/linux/libata.h >>>> @@ -677,9 +677,9 @@ struct ata_device { >>>> unsigned int cdb_len; >>>> >>>> /* per-dev xfer mask */ >>>> - unsigned long pio_mask; >>>> - unsigned long mwdma_mask; >>>> - unsigned long udma_mask; >>>> + unsigned int pio_mask; >>>> + unsigned int mwdma_mask; >>>> + unsigned int udma_mask; >>> >>> Ah. OK. So you did this here... >>> Hmmm. I would squash these 3 patches into a single one. Otherwise, we have >>> sort-of a mess without all patches applied (making revert a pain if needed). >> >> Hm... please explain what kind of a mess... BTW do you really expect a revert? > > The mess would be a partial conversion of the type in case of a revert > being needed. And no, I do not expect a revert would be ever needed, but > hey, never know :) There shouldn't be a "partial conversion" -- all the patches are actually independent from each other (except for the context)... MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask " Sergey Shtylyov @ 2022-05-08 20:41 ` Sergey Shtylyov 2022-05-09 17:57 ` [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov 2022-06-08 6:44 ` Damien Le Moal 4 siblings, 0 replies; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-08 20:41 UTC (permalink / raw) To: Damien Le Moal, linux-ide The {pio|mwdma|udma}_mask fields of the *struct* ata_port_info are declared as *unsigned long* (which is a 64-bit type on 64-bit architectures) while the actual masks should easily fit into just 8 bits. The alike fields in the *struct* ata_port are declared as *unsigned int*, so there are silent truncations going on in ata_host_alloc_pinfo() anyway... Using *unsigned* *int* fields instead saves some object code in many LLDDs too... Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- include/linux/libata.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/libata.h b/include/linux/libata.h index f6fc482d767a..6417f1fd4852 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -979,9 +979,9 @@ struct ata_port_operations { struct ata_port_info { unsigned long flags; unsigned long link_flags; - unsigned long pio_mask; - unsigned long mwdma_mask; - unsigned long udma_mask; + unsigned int pio_mask; + unsigned int mwdma_mask; + unsigned int udma_mask; struct ata_port_operations *port_ops; void *private_data; }; -- 2.26.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov ` (2 preceding siblings ...) 2022-05-08 20:41 ` [PATCH 3/3] ata: make ata_port_info::{pio|mwdma|udma}_mask " Sergey Shtylyov @ 2022-05-09 17:57 ` Sergey Shtylyov 2022-06-08 6:44 ` Damien Le Moal 4 siblings, 0 replies; 19+ messages in thread From: Sergey Shtylyov @ 2022-05-09 17:57 UTC (permalink / raw) To: Damien Le Moal, linux-ide On 5/8/22 11:41 PM, Sergey Shtylyov wrote: > The PATA transfer mode masks (direct and packed) in libata are sometimes > declared as *unsigned int* and sometimes as *unsigned long* (which is a > 64-bit type on 64-bit architectures), while the packed mask really only > uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to > the uniform 32-bit masks saves siginificant amount of the object code... Forgot to mention the patchset was against the 'for-next' branch of Damien's 'libata.git' repo. In fact, I did even forgot to use a working branch while preparing the patches to posting -- did that atop of the real local copy of the 'for-next' branch. :-/ [...] MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov ` (3 preceding siblings ...) 2022-05-09 17:57 ` [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov @ 2022-06-08 6:44 ` Damien Le Moal 2022-06-10 21:20 ` Sergey Shtylyov 4 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-06-08 6:44 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 5/9/22 05:41, Sergey Shtylyov wrote: > The PATA transfer mode masks (direct and packed) in libata are sometimes > declared as *unsigned int* and sometimes as *unsigned long* (which is a > 64-bit type on 64-bit architectures), while the packed mask really only > uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to > the uniform 32-bit masks saves siginificant amount of the object code... > > Sergey Shtylyov (3): > ata: make packed transfer mode masks *unsigned int* > ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* > ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* > > drivers/ata/libata-acpi.c | 8 +++--- > drivers/ata/libata-core.c | 38 +++++++++++++------------- > drivers/ata/pata_acpi.c | 2 +- > drivers/ata/pata_ali.c | 2 +- > drivers/ata/pata_amd.c | 14 +++++----- > drivers/ata/pata_hpt366.c | 2 +- > drivers/ata/pata_hpt37x.c | 6 ++--- > drivers/ata/pata_hpt3x2n.c | 2 +- > drivers/ata/pata_pdc2027x.c | 4 +-- > drivers/ata/pata_serverworks.c | 4 +-- > drivers/ata/pata_sis.c | 2 +- > drivers/ata/pata_via.c | 2 +- > include/linux/libata.h | 49 +++++++++++++++++----------------- > 13 files changed, 67 insertions(+), 68 deletions(-) > Are you going to resend this as a single patch ? -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-06-08 6:44 ` Damien Le Moal @ 2022-06-10 21:20 ` Sergey Shtylyov 2022-06-12 23:24 ` Damien Le Moal 0 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-06-10 21:20 UTC (permalink / raw) To: Damien Le Moal, linux-ide On 6/8/22 9:44 AM, Damien Le Moal wrote: >> The PATA transfer mode masks (direct and packed) in libata are sometimes >> declared as *unsigned int* and sometimes as *unsigned long* (which is a >> 64-bit type on 64-bit architectures), while the packed mask really only >> uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to >> the uniform 32-bit masks saves siginificant amount of the object code... >> >> Sergey Shtylyov (3): >> ata: make packed transfer mode masks *unsigned int* >> ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* >> ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* >> >> drivers/ata/libata-acpi.c | 8 +++--- >> drivers/ata/libata-core.c | 38 +++++++++++++------------- >> drivers/ata/pata_acpi.c | 2 +- >> drivers/ata/pata_ali.c | 2 +- >> drivers/ata/pata_amd.c | 14 +++++----- >> drivers/ata/pata_hpt366.c | 2 +- >> drivers/ata/pata_hpt37x.c | 6 ++--- >> drivers/ata/pata_hpt3x2n.c | 2 +- >> drivers/ata/pata_pdc2027x.c | 4 +-- >> drivers/ata/pata_serverworks.c | 4 +-- >> drivers/ata/pata_sis.c | 2 +- >> drivers/ata/pata_via.c | 2 +- >> include/linux/libata.h | 49 +++++++++++++++++----------------- >> 13 files changed, 67 insertions(+), 68 deletions(-) >> > > Are you going to resend this as a single patch ? No, I'd like to avoid that... Please merge as is. MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-06-10 21:20 ` Sergey Shtylyov @ 2022-06-12 23:24 ` Damien Le Moal 2022-06-13 19:20 ` Sergey Shtylyov 0 siblings, 1 reply; 19+ messages in thread From: Damien Le Moal @ 2022-06-12 23:24 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 6/11/22 06:20, Sergey Shtylyov wrote: > On 6/8/22 9:44 AM, Damien Le Moal wrote: > >>> The PATA transfer mode masks (direct and packed) in libata are sometimes >>> declared as *unsigned int* and sometimes as *unsigned long* (which is a >>> 64-bit type on 64-bit architectures), while the packed mask really only >>> uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to >>> the uniform 32-bit masks saves siginificant amount of the object code... >>> >>> Sergey Shtylyov (3): >>> ata: make packed transfer mode masks *unsigned int* >>> ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* >>> ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* >>> >>> drivers/ata/libata-acpi.c | 8 +++--- >>> drivers/ata/libata-core.c | 38 +++++++++++++------------- >>> drivers/ata/pata_acpi.c | 2 +- >>> drivers/ata/pata_ali.c | 2 +- >>> drivers/ata/pata_amd.c | 14 +++++----- >>> drivers/ata/pata_hpt366.c | 2 +- >>> drivers/ata/pata_hpt37x.c | 6 ++--- >>> drivers/ata/pata_hpt3x2n.c | 2 +- >>> drivers/ata/pata_pdc2027x.c | 4 +-- >>> drivers/ata/pata_serverworks.c | 4 +-- >>> drivers/ata/pata_sis.c | 2 +- >>> drivers/ata/pata_via.c | 2 +- >>> include/linux/libata.h | 49 +++++++++++++++++----------------- >>> 13 files changed, 67 insertions(+), 68 deletions(-) >>> >> >> Are you going to resend this as a single patch ? > > No, I'd like to avoid that... Please merge as is. Nope. I still have concerns about this patch structure. And reviewing again, I think some changes are still missing. E.g., patch 3 changes struct ata_port_info masks to unsigned int. This is used in ata_host_alloc_pinfo() to set the port masks, but I do not see where these are changed to unsigned int too. Which patch does that ? These should be in the same patch. I am OK with one patch for the packed mask, and one patch for the {pio|mwdma|udma}_mask fields. Patch 3 is weird and should at least be squashed into patch 2. But given that patch 1 and 2 both touch the same functions, one patch would be better. > > MBR, Sergey -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-06-12 23:24 ` Damien Le Moal @ 2022-06-13 19:20 ` Sergey Shtylyov 2022-06-14 5:43 ` Damien Le Moal 0 siblings, 1 reply; 19+ messages in thread From: Sergey Shtylyov @ 2022-06-13 19:20 UTC (permalink / raw) To: Damien Le Moal, linux-ide Hello! On 6/13/22 2:24 AM, Damien Le Moal wrote: >>>> The PATA transfer mode masks (direct and packed) in libata are sometimes >>>> declared as *unsigned int* and sometimes as *unsigned long* (which is a >>>> 64-bit type on 64-bit architectures), while the packed mask really only >>>> uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to >>>> the uniform 32-bit masks saves siginificant amount of the object code... >>>> >>>> Sergey Shtylyov (3): >>>> ata: make packed transfer mode masks *unsigned int* >>>> ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* >>>> ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* >>>> >>>> drivers/ata/libata-acpi.c | 8 +++--- >>>> drivers/ata/libata-core.c | 38 +++++++++++++------------- >>>> drivers/ata/pata_acpi.c | 2 +- >>>> drivers/ata/pata_ali.c | 2 +- >>>> drivers/ata/pata_amd.c | 14 +++++----- >>>> drivers/ata/pata_hpt366.c | 2 +- >>>> drivers/ata/pata_hpt37x.c | 6 ++--- >>>> drivers/ata/pata_hpt3x2n.c | 2 +- >>>> drivers/ata/pata_pdc2027x.c | 4 +-- >>>> drivers/ata/pata_serverworks.c | 4 +-- >>>> drivers/ata/pata_sis.c | 2 +- >>>> drivers/ata/pata_via.c | 2 +- >>>> include/linux/libata.h | 49 +++++++++++++++++----------------- >>>> 13 files changed, 67 insertions(+), 68 deletions(-) >>>> >>> >>> Are you going to resend this as a single patch ? >> >> No, I'd like to avoid that... Please merge as is. > > Nope. I still have concerns about this patch structure. And reviewing > again, I think some changes are still missing. > E.g., patch 3 changes struct ata_port_info masks to unsigned int. This is > used in ata_host_alloc_pinfo() to set the port masks, but I do not see > where these are changed to unsigned int too. Which patch does that ? These > should be in the same patch. Sigh... they always were *unsigned int*! And I explicitly wrote about that in the commit msg of that patch... :-/ > I am OK with one patch for the packed mask, and one patch for the > {pio|mwdma|udma}_mask fields. Patch 3 is weird and should at least be > squashed into patch 2. Why? What does *struct* ata_device have to do with *struct* ata_port_info? > > But given that patch 1 and 2 both touch the same functions, one patch > would be better. I specifically aimed at minimizing the patches.... MBR, Sergey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Make PATA transfer mode masks always being 32-bit 2022-06-13 19:20 ` Sergey Shtylyov @ 2022-06-14 5:43 ` Damien Le Moal 0 siblings, 0 replies; 19+ messages in thread From: Damien Le Moal @ 2022-06-14 5:43 UTC (permalink / raw) To: Sergey Shtylyov, linux-ide On 6/14/22 04:20, Sergey Shtylyov wrote: > Hello! > > On 6/13/22 2:24 AM, Damien Le Moal wrote: > >>>>> The PATA transfer mode masks (direct and packed) in libata are sometimes >>>>> declared as *unsigned int* and sometimes as *unsigned long* (which is a >>>>> 64-bit type on 64-bit architectures), while the packed mask really only >>>>> uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to >>>>> the uniform 32-bit masks saves siginificant amount of the object code... >>>>> >>>>> Sergey Shtylyov (3): >>>>> ata: make packed transfer mode masks *unsigned int* >>>>> ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int* >>>>> ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int* >>>>> >>>>> drivers/ata/libata-acpi.c | 8 +++--- >>>>> drivers/ata/libata-core.c | 38 +++++++++++++------------- >>>>> drivers/ata/pata_acpi.c | 2 +- >>>>> drivers/ata/pata_ali.c | 2 +- >>>>> drivers/ata/pata_amd.c | 14 +++++----- >>>>> drivers/ata/pata_hpt366.c | 2 +- >>>>> drivers/ata/pata_hpt37x.c | 6 ++--- >>>>> drivers/ata/pata_hpt3x2n.c | 2 +- >>>>> drivers/ata/pata_pdc2027x.c | 4 +-- >>>>> drivers/ata/pata_serverworks.c | 4 +-- >>>>> drivers/ata/pata_sis.c | 2 +- >>>>> drivers/ata/pata_via.c | 2 +- >>>>> include/linux/libata.h | 49 +++++++++++++++++----------------- >>>>> 13 files changed, 67 insertions(+), 68 deletions(-) >>>>> >>>> >>>> Are you going to resend this as a single patch ? >>> >>> No, I'd like to avoid that... Please merge as is. >> >> Nope. I still have concerns about this patch structure. And reviewing >> again, I think some changes are still missing. >> E.g., patch 3 changes struct ata_port_info masks to unsigned int. This is >> used in ata_host_alloc_pinfo() to set the port masks, but I do not see >> where these are changed to unsigned int too. Which patch does that ? These >> should be in the same patch. > > Sigh... they always were *unsigned int*! And I explicitly wrote about that > in the commit msg of that patch... :-/ Nobody is perfect. I miss/forget stuff from time to time. All you need to do is to remind me. So no need for the "sigh". > >> I am OK with one patch for the packed mask, and one patch for the >> {pio|mwdma|udma}_mask fields. Patch 3 is weird and should at least be >> squashed into patch 2. > > Why? What does *struct* ata_device have to do with *struct* ata_port_info? > >> >> But given that patch 1 and 2 both touch the same functions, one patch >> would be better. > > I specifically aimed at minimizing the patches.... I do not care about the number of patches it takes to fix/improve something. I care about what the patches do. And for this case, I prefer seeing a single patch cleaning up the mess of unsigned long/unsigned int for all these transfer speed masks so that everything becomes unsigned int in one go. With multiple patches we still have a messy situation of mixed unsigned long/unsigned int between patches. > > MBR, Sergey -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-06-14 5:43 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-08 20:41 [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 1/3] ata: make packed transfer mode masks *unsigned int* Sergey Shtylyov 2022-05-16 11:17 ` Damien Le Moal 2022-05-19 20:19 ` Sergey Shtylyov 2022-05-20 3:37 ` Damien Le Moal 2022-06-10 20:19 ` Sergey Shtylyov 2022-06-12 23:18 ` Damien Le Moal 2022-05-08 20:41 ` [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask " Sergey Shtylyov 2022-05-16 11:19 ` Damien Le Moal 2022-05-19 20:58 ` Sergey Shtylyov 2022-05-20 3:31 ` Damien Le Moal 2022-06-10 21:15 ` Sergey Shtylyov 2022-05-08 20:41 ` [PATCH 3/3] ata: make ata_port_info::{pio|mwdma|udma}_mask " Sergey Shtylyov 2022-05-09 17:57 ` [PATCH 0/3] Make PATA transfer mode masks always being 32-bit Sergey Shtylyov 2022-06-08 6:44 ` Damien Le Moal 2022-06-10 21:20 ` Sergey Shtylyov 2022-06-12 23:24 ` Damien Le Moal 2022-06-13 19:20 ` Sergey Shtylyov 2022-06-14 5:43 ` Damien Le Moal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.