| Message ID | 20230817221232.22035-2-schmitzmic@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Q40 IDE fixes | expand |
On 2023/08/18 7:12, Michael Schmitz wrote: > With commit 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver > with pata_falcon and falconide"), the Q40 IDE driver was > replaced by pata_falcon.c. Please change the patch title to: ata: pata_falcon: fix IO base selection for Q40 > > 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 scale 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 scaling 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, > and use standard register offsets from that base in order > to calculate register addresses (the IO address translation > will then apply the correct ISA window base and scaling). > > Encode PIO_OFFSET into IO port addresses for all registers > 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@sowerbutts.com> > Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com > Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com > Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide") > Cc: <stable@vger.kernel.org> # 5.14 5.14+ ? But I do not think you need to specify anything anyway since you have the Fixes tag. > Cc: Finn Thain <fthain@linux-m68k.org> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > > --- > > Changes from RFC v3: > > - split off byte swap option into separate patch > > Geert Uytterhoeven: > - review comments > > Changes from RFC v2: > - add driver parameter 'data_swap' as bit mask for drives to swap > > Changes from RFC v1: > > Finn Thain: > - take care to supply IO address suitable for ioread8/iowrite8 > - use MMIO address for data transfer > --- > drivers/ata/pata_falcon.c | 55 ++++++++++++++++++++++++--------------- > 1 file changed, 34 insertions(+), 21 deletions(-) > > diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c > index 996516e64f13..346259e3bbc8 100644 > --- a/drivers/ata/pata_falcon.c > +++ b/drivers/ata/pata_falcon.c > @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) > struct resource *base_res, *ctl_res, *irq_res; > struct ata_host *host; > struct ata_port *ap; > - void __iomem *base; > - int irq = 0; > + void __iomem *base, *ctl_base; > + int irq = 0, io_offset = 1, reg_scale = 4; > > dev_info(&pdev->dev, "Atari Falcon and Q40/Q60 PATA controller\n"); > > @@ -165,26 +165,39 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) > ap->pio_mask = ATA_PIO4; > ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY; > > - base = (void __iomem *)base_mem_res->start; > /* N.B. this assumes data_addr will be used for word-sized I/O only */ > - ap->ioaddr.data_addr = base + 0 + 0 * 4; > - ap->ioaddr.error_addr = base + 1 + 1 * 4; > - ap->ioaddr.feature_addr = base + 1 + 1 * 4; > - ap->ioaddr.nsect_addr = base + 1 + 2 * 4; > - ap->ioaddr.lbal_addr = base + 1 + 3 * 4; > - ap->ioaddr.lbam_addr = base + 1 + 4 * 4; > - ap->ioaddr.lbah_addr = base + 1 + 5 * 4; > - ap->ioaddr.device_addr = base + 1 + 6 * 4; > - ap->ioaddr.status_addr = base + 1 + 7 * 4; > - ap->ioaddr.command_addr = base + 1 + 7 * 4; > - > - base = (void __iomem *)ctl_mem_res->start; > - ap->ioaddr.altstatus_addr = base + 1; > - ap->ioaddr.ctl_addr = base + 1; > - > - ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", > - (unsigned long)base_mem_res->start, > - (unsigned long)ctl_mem_res->start); > + ap->ioaddr.data_addr = (void __iomem *)base_mem_res->start; > + > + if (base_res) { /* only Q40 has IO resources */ > + io_offset = 0x10000; > + reg_scale = 1; > + base = (void __iomem *)base_res->start; > + ctl_base = (void __iomem *)ctl_res->start; > + > + ata_port_desc(ap, "cmd %pa ctl %pa", > + &base_res->start, > + &ctl_res->start); > + } else { > + base = (void __iomem *)base_mem_res->start; > + ctl_base = (void __iomem *)ctl_mem_res->start; > + > + ata_port_desc(ap, "cmd %pa ctl %pa", > + &base_mem_res->start, > + &ctl_mem_res->start); > + } > + > + ap->ioaddr.error_addr = base + io_offset + 1 * reg_scale; > + ap->ioaddr.feature_addr = base + io_offset + 1 * reg_scale; > + ap->ioaddr.nsect_addr = base + io_offset + 2 * reg_scale; > + ap->ioaddr.lbal_addr = base + io_offset + 3 * reg_scale; > + ap->ioaddr.lbam_addr = base + io_offset + 4 * reg_scale; > + ap->ioaddr.lbah_addr = base + io_offset + 5 * reg_scale; > + ap->ioaddr.device_addr = base + io_offset + 6 * reg_scale; > + ap->ioaddr.status_addr = base + io_offset + 7 * reg_scale; > + ap->ioaddr.command_addr = base + io_offset + 7 * reg_scale; > + > + ap->ioaddr.altstatus_addr = ctl_base + io_offset; > + ap->ioaddr.ctl_addr = ctl_base + io_offset; > > irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > if (irq_res && irq_res->start > 0) {
Hi Damien, thanks for your review! Am 18.08.2023 um 12:42 schrieb Damien Le Moal: > On 2023/08/18 7:12, Michael Schmitz wrote: >> With commit 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver >> with pata_falcon and falconide"), the Q40 IDE driver was >> replaced by pata_falcon.c. > > Please change the patch title to: > > ata: pata_falcon: fix IO base selection for Q40 Will do. > >> >> 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 scale 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 scaling 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, >> and use standard register offsets from that base in order >> to calculate register addresses (the IO address translation >> will then apply the correct ISA window base and scaling). >> >> Encode PIO_OFFSET into IO port addresses for all registers >> 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@sowerbutts.com> >> Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com >> Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com >> Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide") >> Cc: <stable@vger.kernel.org> # 5.14 > > 5.14+ ? But I do not think you need to specify anything anyway since you have > the Fixes tag. 5.14+ perhaps. I'll check the docs again to see whether Fixes: obsoletes the stable backport tag. I've so far used both together... Cheers, Michael > >> Cc: Finn Thain <fthain@linux-m68k.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> >> >> --- >> >> Changes from RFC v3: >> >> - split off byte swap option into separate patch >> >> Geert Uytterhoeven: >> - review comments >> >> Changes from RFC v2: >> - add driver parameter 'data_swap' as bit mask for drives to swap >> >> Changes from RFC v1: >> >> Finn Thain: >> - take care to supply IO address suitable for ioread8/iowrite8 >> - use MMIO address for data transfer >> --- >> drivers/ata/pata_falcon.c | 55 ++++++++++++++++++++++++--------------- >> 1 file changed, 34 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c >> index 996516e64f13..346259e3bbc8 100644 >> --- a/drivers/ata/pata_falcon.c >> +++ b/drivers/ata/pata_falcon.c >> @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) >> struct resource *base_res, *ctl_res, *irq_res; >> struct ata_host *host; >> struct ata_port *ap; >> - void __iomem *base; >> - int irq = 0; >> + void __iomem *base, *ctl_base; >> + int irq = 0, io_offset = 1, reg_scale = 4; >> >> dev_info(&pdev->dev, "Atari Falcon and Q40/Q60 PATA controller\n"); >> >> @@ -165,26 +165,39 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) >> ap->pio_mask = ATA_PIO4; >> ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY; >> >> - base = (void __iomem *)base_mem_res->start; >> /* N.B. this assumes data_addr will be used for word-sized I/O only */ >> - ap->ioaddr.data_addr = base + 0 + 0 * 4; >> - ap->ioaddr.error_addr = base + 1 + 1 * 4; >> - ap->ioaddr.feature_addr = base + 1 + 1 * 4; >> - ap->ioaddr.nsect_addr = base + 1 + 2 * 4; >> - ap->ioaddr.lbal_addr = base + 1 + 3 * 4; >> - ap->ioaddr.lbam_addr = base + 1 + 4 * 4; >> - ap->ioaddr.lbah_addr = base + 1 + 5 * 4; >> - ap->ioaddr.device_addr = base + 1 + 6 * 4; >> - ap->ioaddr.status_addr = base + 1 + 7 * 4; >> - ap->ioaddr.command_addr = base + 1 + 7 * 4; >> - >> - base = (void __iomem *)ctl_mem_res->start; >> - ap->ioaddr.altstatus_addr = base + 1; >> - ap->ioaddr.ctl_addr = base + 1; >> - >> - ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", >> - (unsigned long)base_mem_res->start, >> - (unsigned long)ctl_mem_res->start); >> + ap->ioaddr.data_addr = (void __iomem *)base_mem_res->start; >> + >> + if (base_res) { /* only Q40 has IO resources */ >> + io_offset = 0x10000; >> + reg_scale = 1; >> + base = (void __iomem *)base_res->start; >> + ctl_base = (void __iomem *)ctl_res->start; >> + >> + ata_port_desc(ap, "cmd %pa ctl %pa", >> + &base_res->start, >> + &ctl_res->start); >> + } else { >> + base = (void __iomem *)base_mem_res->start; >> + ctl_base = (void __iomem *)ctl_mem_res->start; >> + >> + ata_port_desc(ap, "cmd %pa ctl %pa", >> + &base_mem_res->start, >> + &ctl_mem_res->start); >> + } >> + >> + ap->ioaddr.error_addr = base + io_offset + 1 * reg_scale; >> + ap->ioaddr.feature_addr = base + io_offset + 1 * reg_scale; >> + ap->ioaddr.nsect_addr = base + io_offset + 2 * reg_scale; >> + ap->ioaddr.lbal_addr = base + io_offset + 3 * reg_scale; >> + ap->ioaddr.lbam_addr = base + io_offset + 4 * reg_scale; >> + ap->ioaddr.lbah_addr = base + io_offset + 5 * reg_scale; >> + ap->ioaddr.device_addr = base + io_offset + 6 * reg_scale; >> + ap->ioaddr.status_addr = base + io_offset + 7 * reg_scale; >> + ap->ioaddr.command_addr = base + io_offset + 7 * reg_scale; >> + >> + ap->ioaddr.altstatus_addr = ctl_base + io_offset; >> + ap->ioaddr.ctl_addr = ctl_base + io_offset; >> >> irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> if (irq_res && irq_res->start > 0) { >
On Fri, 18 Aug 2023, Michael Schmitz wrote: > >> Cc: <stable@vger.kernel.org> # 5.14 > > > > 5.14+ ? But I do not think you need to specify anything anyway since > > you have the Fixes tag. > > 5.14+ perhaps. I'll check the docs again to see whether Fixes: obsoletes > the stable backport tag. I've so far used both together... > You'd specify a "# x.y+" limit along with a "Fixes" tag if you don't want to backport as far back as the buggy commit (because some other pre-requisite isn't present on the older branches). But that does not apply in this case. Writing "# 5.14" is surprising because (according to www.kernel.org landing page) that branch was abandoned, and no live branch was named. But in "git log" you can see that people write this anyway. Writing "# 5.14+" or "# 5.15+" is clear enough but is normally omitted when it can be inferred from the Fixes tag. That's been my experience, at least.
Hello! On 8/18/23 1:12 AM, Michael Schmitz 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 scale 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 scaling 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, > and use standard register offsets from that base in order > to calculate register addresses (the IO address translation > will then apply the correct ISA window base and scaling). > > Encode PIO_OFFSET into IO port addresses for all registers > 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@sowerbutts.com> > Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com > Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com > Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide") > Cc: <stable@vger.kernel.org> # 5.14 > Cc: Finn Thain <fthain@linux-m68k.org> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] > diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c > index 996516e64f13..346259e3bbc8 100644 > --- a/drivers/ata/pata_falcon.c > +++ b/drivers/ata/pata_falcon.c > @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) > struct resource *base_res, *ctl_res, *irq_res; > struct ata_host *host; > struct ata_port *ap; > - void __iomem *base; > - int irq = 0; > + void __iomem *base, *ctl_base; > + int irq = 0, io_offset = 1, reg_scale = 4; Maybe reg_step? [...] MBR, Sergey
Hi Sergey, thanks for your review! On 20/08/23 08:29, Sergey Shtylyov wrote: > Hello! > > On 8/18/23 1:12 AM, Michael Schmitz 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 scale 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 scaling 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, >> and use standard register offsets from that base in order >> to calculate register addresses (the IO address translation >> will then apply the correct ISA window base and scaling). >> >> Encode PIO_OFFSET into IO port addresses for all registers >> 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@sowerbutts.com> >> Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com >> Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com >> Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide") >> Cc: <stable@vger.kernel.org> # 5.14 >> Cc: Finn Thain <fthain@linux-m68k.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > [...] >> diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c >> index 996516e64f13..346259e3bbc8 100644 >> --- a/drivers/ata/pata_falcon.c >> +++ b/drivers/ata/pata_falcon.c >> @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) >> struct resource *base_res, *ctl_res, *irq_res; >> struct ata_host *host; >> struct ata_port *ap; >> - void __iomem *base; >> - int irq = 0; >> + void __iomem *base, *ctl_base; >> + int irq = 0, io_offset = 1, reg_scale = 4; > Maybe reg_step? Could name it that, too. I can't recall where I picked up the term 'register scaling'... I'll see what's the consensus (if any) in drivers/. Cheers, Michael > > [...] > > MBR, Sergey
Hi Sergey, Am 21.08.2023 um 07:19 schrieb Michael Schmitz: >>> diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c >>> index 996516e64f13..346259e3bbc8 100644 >>> --- a/drivers/ata/pata_falcon.c >>> +++ b/drivers/ata/pata_falcon.c >>> @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct >>> platform_device *pdev) >>> struct resource *base_res, *ctl_res, *irq_res; >>> struct ata_host *host; >>> struct ata_port *ap; >>> - void __iomem *base; >>> - int irq = 0; >>> + void __iomem *base, *ctl_base; >>> + int irq = 0, io_offset = 1, reg_scale = 4; >> Maybe reg_step? > > Could name it that, too. I can't recall where I picked up the term > 'register scaling'... > > I'll see what's the consensus (if any) in drivers/. I've seen some use of 'step' but mostly use of 'shift'. Rewriting pata_falcon_init_one() to use register shift instead of register step is trivial, so unless anyone objects, I'll send that version as v4. Cheers, Michael > > Cheers, > > Michael > > >> >> [...] >> >> MBR, Sergey
diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c index 996516e64f13..346259e3bbc8 100644 --- a/drivers/ata/pata_falcon.c +++ b/drivers/ata/pata_falcon.c @@ -123,8 +123,8 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) struct resource *base_res, *ctl_res, *irq_res; struct ata_host *host; struct ata_port *ap; - void __iomem *base; - int irq = 0; + void __iomem *base, *ctl_base; + int irq = 0, io_offset = 1, reg_scale = 4; dev_info(&pdev->dev, "Atari Falcon and Q40/Q60 PATA controller\n"); @@ -165,26 +165,39 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) ap->pio_mask = ATA_PIO4; ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY; - base = (void __iomem *)base_mem_res->start; /* N.B. this assumes data_addr will be used for word-sized I/O only */ - ap->ioaddr.data_addr = base + 0 + 0 * 4; - ap->ioaddr.error_addr = base + 1 + 1 * 4; - ap->ioaddr.feature_addr = base + 1 + 1 * 4; - ap->ioaddr.nsect_addr = base + 1 + 2 * 4; - ap->ioaddr.lbal_addr = base + 1 + 3 * 4; - ap->ioaddr.lbam_addr = base + 1 + 4 * 4; - ap->ioaddr.lbah_addr = base + 1 + 5 * 4; - ap->ioaddr.device_addr = base + 1 + 6 * 4; - ap->ioaddr.status_addr = base + 1 + 7 * 4; - ap->ioaddr.command_addr = base + 1 + 7 * 4; - - base = (void __iomem *)ctl_mem_res->start; - ap->ioaddr.altstatus_addr = base + 1; - ap->ioaddr.ctl_addr = base + 1; - - ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", - (unsigned long)base_mem_res->start, - (unsigned long)ctl_mem_res->start); + ap->ioaddr.data_addr = (void __iomem *)base_mem_res->start; + + if (base_res) { /* only Q40 has IO resources */ + io_offset = 0x10000; + reg_scale = 1; + base = (void __iomem *)base_res->start; + ctl_base = (void __iomem *)ctl_res->start; + + ata_port_desc(ap, "cmd %pa ctl %pa", + &base_res->start, + &ctl_res->start); + } else { + base = (void __iomem *)base_mem_res->start; + ctl_base = (void __iomem *)ctl_mem_res->start; + + ata_port_desc(ap, "cmd %pa ctl %pa", + &base_mem_res->start, + &ctl_mem_res->start); + } + + ap->ioaddr.error_addr = base + io_offset + 1 * reg_scale; + ap->ioaddr.feature_addr = base + io_offset + 1 * reg_scale; + ap->ioaddr.nsect_addr = base + io_offset + 2 * reg_scale; + ap->ioaddr.lbal_addr = base + io_offset + 3 * reg_scale; + ap->ioaddr.lbam_addr = base + io_offset + 4 * reg_scale; + ap->ioaddr.lbah_addr = base + io_offset + 5 * reg_scale; + ap->ioaddr.device_addr = base + io_offset + 6 * reg_scale; + ap->ioaddr.status_addr = base + io_offset + 7 * reg_scale; + ap->ioaddr.command_addr = base + io_offset + 7 * reg_scale; + + ap->ioaddr.altstatus_addr = ctl_base + io_offset; + ap->ioaddr.ctl_addr = ctl_base + io_offset; irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (irq_res && irq_res->start > 0) {
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 scale 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 scaling 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, and use standard register offsets from that base in order to calculate register addresses (the IO address translation will then apply the correct ISA window base and scaling). Encode PIO_OFFSET into IO port addresses for all registers 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@sowerbutts.com> Closes: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com Link: https://lore.kernel.org/r/CAMuHMdUU62jjunJh9cqSqHT87B0H0A4udOOPs=WN7WZKpcagVA@mail.gmail.com Fixes: 44b1fbc0f5f3 ("m68k/q40: Replace q40ide driver with pata_falcon and falconide") Cc: <stable@vger.kernel.org> # 5.14 Cc: Finn Thain <fthain@linux-m68k.org> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- Changes from RFC v3: - split off byte swap option into separate patch Geert Uytterhoeven: - review comments Changes from RFC v2: - add driver parameter 'data_swap' as bit mask for drives to swap Changes from RFC v1: Finn Thain: - take care to supply IO address suitable for ioread8/iowrite8 - use MMIO address for data transfer --- drivers/ata/pata_falcon.c | 55 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 21 deletions(-)