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

Sergey Shtylyov
2022-06-10T21:15:59Z

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