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@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.
>
> 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
