Re: [PATCH 1/3] ata: make packed transfer mode masks *unsigned int*

[Sergey Shtylyov <s.shtylyov@omp.ru>]

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