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@xxxxxx> > [...] >>> 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
