[v1,1/4] m68k - add MVME147 SCSI base address to mvme147hw.h

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(+)

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
>