| Message ID | 20220709001019.11149-2-schmitzmic@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert m68k MVME147 WD33C93 SCSI driver to DMA API | expand |
On Sat, Jul 9, 2022 at 2:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: > diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h > index e28eb1c0e0bf..fd8c1e4fc7be 100644 > --- a/arch/m68k/include/asm/mvme147hw.h > +++ b/arch/m68k/include/asm/mvme147hw.h > @@ -93,6 +93,7 @@ struct pcc_regs { > #define M147_SCC_B_ADDR 0xfffe3000 > #define M147_SCC_PCLK 5000000 > > +#define MVME147_SCSI_BASE 0xfffe4000 I think this should be an 'void *__iomem' token, not a plain integer. Apparently the driver internally uses a 'volatile void *', but some of the other front-ends are already converted to use __iomem. Arnd
Hi Arns, Am 11.07.2022 um 04:06 schrieb Arnd Bergmann: > On Sat, Jul 9, 2022 at 2:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: >> diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h >> index e28eb1c0e0bf..fd8c1e4fc7be 100644 >> --- a/arch/m68k/include/asm/mvme147hw.h >> +++ b/arch/m68k/include/asm/mvme147hw.h >> @@ -93,6 +93,7 @@ struct pcc_regs { >> #define M147_SCC_B_ADDR 0xfffe3000 >> #define M147_SCC_PCLK 5000000 >> >> +#define MVME147_SCSI_BASE 0xfffe4000 > > I think this should be an 'void *__iomem' token, not a plain integer. > Apparently the driver internally uses a 'volatile void *', but some of > the other front-ends are already converted to use __iomem. I'll pass the base address through a platform data struct in the next version to address your other concerns. Haven't seen __iomem types used in the other drivers - two are Zorro devices, and two platform devices (a3000 and sgiwd93). Found no other wd33c93 drivers... Cheers, Michael > > Arnd >
On Mon, Jul 11, 2022 at 6:16 AM Michael Schmitz <schmitzmic@gmail.com> wrote: > Am 11.07.2022 um 04:06 schrieb Arnd Bergmann: > > On Sat, Jul 9, 2022 at 2:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: > > I think this should be an 'void *__iomem' token, not a plain integer. > > Apparently the driver internally uses a 'volatile void *', but some of > > the other front-ends are already converted to use __iomem. > > I'll pass the base address through a platform data struct in the next > version to address your other concerns. Haven't seen __iomem types used > in the other drivers - two are Zorro devices, and two platform devices > (a3000 and sgiwd93). Found no other wd33c93 drivers... Right, I noticed this as well. The ideal way to do this would be to change all of these files to use __iomem tokens consistently, and then use ioread32be()/iowrite32be() in place of the volatile pointer dereference. Maybe leave that for a follow-up series, you'll probably uncover more of these issues once you take that step. Arnd
Am 11.07.2022 um 20:45 schrieb Arnd Bergmann: > On Mon, Jul 11, 2022 at 6:16 AM Michael Schmitz <schmitzmic@gmail.com> wrote: >> Am 11.07.2022 um 04:06 schrieb Arnd Bergmann: >>> On Sat, Jul 9, 2022 at 2:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote: >>> I think this should be an 'void *__iomem' token, not a plain integer. >>> Apparently the driver internally uses a 'volatile void *', but some of >>> the other front-ends are already converted to use __iomem. >> >> I'll pass the base address through a platform data struct in the next >> version to address your other concerns. Haven't seen __iomem types used >> in the other drivers - two are Zorro devices, and two platform devices >> (a3000 and sgiwd93). Found no other wd33c93 drivers... > > Right, I noticed this as well. The ideal way to do this would be to change > all of these files to use __iomem tokens consistently, and then use > ioread32be()/iowrite32be() in place of the volatile pointer dereference. On a second glance, the Amiga drivers _do_ use iomem types (ZTWO_VADDR() does the Right Thing, leaving this driver and SGI. Changes in wd33c93.c proper (which I read your mail to suggest) would require actual hardware regression testing IMO. Not sure I can do that. Cheers, Michael > > Maybe leave that for a follow-up series, you'll probably uncover > more of these issues once you take that step. > > Arnd >
diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h index e28eb1c0e0bf..fd8c1e4fc7be 100644 --- a/arch/m68k/include/asm/mvme147hw.h +++ b/arch/m68k/include/asm/mvme147hw.h @@ -93,6 +93,7 @@ struct pcc_regs { #define M147_SCC_B_ADDR 0xfffe3000 #define M147_SCC_PCLK 5000000 +#define MVME147_SCSI_BASE 0xfffe4000 #define MVME147_IRQ_SCSI_PORT (IRQ_USER+0x45) #define MVME147_IRQ_SCSI_DMA (IRQ_USER+0x46)
The base address for the WD33C93 SCSI host adapter on mvme147 boards is missing from mvme147hw.h. This information will be needed for platform device conversion of the mvme147_scsi driver, so add it here. CC: linux-scsi@vger.kernel.org Link: https://lore.kernel.org/r/6d1d88ee-1cf6-c735-1e6d-bafd2096e322@gmail.com Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- arch/m68k/include/asm/mvme147hw.h | 1 + 1 file changed, 1 insertion(+)