Re: [PATCH v4 1/2] ata: pata_falcon: fix IO base selection for Q40

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

On Wed, Aug 23, 2023 at 12:14 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
> With commit 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver
> with pata_falcon and falconide"), the Q40 IDE driver was
> replaced by pata_falcon.c.
>
> Both IO and memory resources were defined for the Q40 IDE
> platform device, but definition of the IDE register addresses
> was modeled after the Falcon case, both in use of the memory
> resources and in including register shift and byte vs. word
> offset in the address.
>
> This was correct for the Falcon case, which does not apply
> any address translation to the register addresses. In the
> Q40 case, all of device base address, byte access offset
> and register shift is included in the platform specific
> ISA access translation (in asm/mm_io.h).
>
> As a consequence, such address translation gets applied
> twice, and register addresses are mangled.
>
> Use the device base address from the platform IO resource
> for Q40 (the IO address translation will then add the correct
> ISA window base address and byte access offset), with register
> shift 1. Use MMIO base address and register shift 2 as before
> for Falcon.
>
> Encode PIO_OFFSET into IO port addresses for all registers
> for Q40 except the data transfer register. Encode the MMIO
> offset there (pata_falcon_data_xfer() directly uses raw IO
> with no address translation).
>
> Reported-by: William R Sowerbutts <will@xxxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@xxxxxxxxxxxxxx
> Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@xxxxxxxxxxxxxx
> Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Finn Thain <fthain@xxxxxxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Tested-by: William R Sowerbutts <will@xxxxxxxxxxxxxx>
> Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>
> ---
>
> Changes from v3:
>
> Sergey Shtylyov:
> - change use of reg_scale to reg_shift
>
> Geert Uytterhoeven:
> - factor out ata_port_desc() from platform specific code

Thanks for the update!

> --- a/drivers/ata/pata_falcon.c
> +++ b/drivers/ata/pata_falcon.c

> +       ata_port_desc(ap, "cmd %px ctl %px data %pa",
> +                     base, ctl_base, &ap->ioaddr.data_addr);

%px and ap->ioaddr.data_addr

The rest LGTM, so with the above fixed
Reviewed-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux