Re: [PATCH v1 3/4] scsi - convert mvme146_scsi.c to platform device

Geert Uytterhoeven
2022-07-11T07:16:34Z

On Sun, Jul 10, 2022 at 6:12 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Sat, Jul 9, 2022 at 2:10 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> > Convert the mvme147_scsi driver to a platform device driver.
> > This is required for conversion of the driver to the DMA API.
> >
> > 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>
>
> The patch looks correct to me, but the type cast for the address doesn't
> seem right:
>
> > - regs.SASR = (volatile unsigned char *)0xfffe4000;
> > - regs.SCMD = (volatile unsigned char *)0xfffe4001;
> >
> > - hdata = shost_priv(mvme147_shost);
> > + mvme147_inst->base = mres->start;
> > + mvme147_inst->irq = ires->start;
> > +
> > + regs.SASR = (volatile unsigned char *)mres->start;
> > + regs.SCMD = (volatile unsigned char *)(mres->start)+0x1;
>
> A resource would pass a phys_addr_t token, but the driver expects a
> virtual address that should be an __iomem pointer. The MMIO area
> already gets mapped into virtual addresses in arch/m68k/kernel/head.S,
> so it makes sense to skip the extra ioremap() and just use the address,
> but then you can't pass it as an IORESOUCRE_MEM token and should
> use platform_data with the pointer instead.
>
> The alternative is to do it the normal way and pass the physical address
> as a resource, that you can pass into devm_platform_ioremap_resource()
> or a similar helper.

I would prefer the latter. While head.S sets up the mapping,
__ioremap() does not have support for this on the mvme platform,
so this has to be added first. Look at the amiga and virt platforms
for examples.

If you do want to pass the virtual address as platform data (for now
;-), please provide the physical memory resource too, so we don't
have to go through another synchronization step with the m68k and
scsi trees later.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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