Re: [PATCH 2/3] ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int*

Damien Le Moal
2022-05-20T03:31:44Z

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