Hello!
On 6/13/22 2:24 AM, Damien Le Moal wrote:
>>>> The PATA transfer mode masks (direct and packed) in libata are sometimes
>>>> declared as *unsigned int* and sometimes as *unsigned long* (which is a
>>>> 64-bit type on 64-bit architectures), while the packed mask really only
>>>> uses 20 bits and the PIO/MWDMA/UDMA masks use up to 8 bits. Switching to
>>>> the uniform 32-bit masks saves siginificant amount of the object code...
>>>>
>>>> Sergey Shtylyov (3):
>>>> ata: make packed transfer mode masks *unsigned int*
>>>> ata: make ata_device::{pio|mwdma|udma}_mask *unsigned int*
>>>> ata: make ata_port_info::{pio|mwdma|udma}_mask *unsigned int*
>>>>
>>>> drivers/ata/libata-acpi.c | 8 +++---
>>>> drivers/ata/libata-core.c | 38 +++++++++++++-------------
>>>> drivers/ata/pata_acpi.c | 2 +-
>>>> drivers/ata/pata_ali.c | 2 +-
>>>> drivers/ata/pata_amd.c | 14 +++++-----
>>>> drivers/ata/pata_hpt366.c | 2 +-
>>>> drivers/ata/pata_hpt37x.c | 6 ++---
>>>> drivers/ata/pata_hpt3x2n.c | 2 +-
>>>> drivers/ata/pata_pdc2027x.c | 4 +--
>>>> drivers/ata/pata_serverworks.c | 4 +--
>>>> drivers/ata/pata_sis.c | 2 +-
>>>> drivers/ata/pata_via.c | 2 +-
>>>> include/linux/libata.h | 49 +++++++++++++++++-----------------
>>>> 13 files changed, 67 insertions(+), 68 deletions(-)
>>>>
>>>
>>> Are you going to resend this as a single patch ?
>>
>> No, I'd like to avoid that... Please merge as is.
>
> Nope. I still have concerns about this patch structure. And reviewing
> again, I think some changes are still missing.
> E.g., patch 3 changes struct ata_port_info masks to unsigned int. This is
> used in ata_host_alloc_pinfo() to set the port masks, but I do not see
> where these are changed to unsigned int too. Which patch does that ? These
> should be in the same patch.
Sigh... they always were *unsigned int*! And I explicitly wrote about that
in the commit msg of that patch... :-/
> I am OK with one patch for the packed mask, and one patch for the
> {pio|mwdma|udma}_mask fields. Patch 3 is weird and should at least be
> squashed into patch 2.
Why? What does *struct* ata_device have to do with *struct* ata_port_info?
>
> But given that patch 1 and 2 both touch the same functions, one patch
> would be better.
I specifically aimed at minimizing the patches....
MBR, Sergey
