[v2,0/5] Convert m68k MVME147 WD33C93 SCSI driver to DMA API

This series was precipitated by Arnd removing CONFIG_VIRT_TO_BUS. The
m68k WD33C93 still used virt_to_bus to convert virtual addresses to
physical addresses suitable for the DMA engines (note m68k does not
have an IOMMU and uses a direct mapping for DMA addresses). 

Arnd suggested to use dma_map_single() to set up dma mappings instead
of open-coding much the same in every driver dma_setup() function. An
earlier patch series has converted the three Amiga WD33C93 drivers,
this series now takes care of the sole remaining m68k WD33C93 driver.

The m68k VME mvme147 driver has no DMA addressing or alignment
restrictions and can be converted in the same way as the Amiga a3000
one in my previous series, but requires conversion to a platform
device driver first.

Changes from v1: 
- use of devm_platform_ioremap_resource() to convert phys. resource
to virtual address for chip registers.

Only compile tested so far, and hardware testing might be hard to do.
I'd appreciate someone giving this a thorough review.

Cheers,

   Michael

On Tue, Jul 12, 2022 at 9:58 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> +
> +static const struct resource mvme147_scsi_rsrc[] __initconst = {
> +       DEFINE_RES_MEM(MVME147_SCSI_BASE, 0xff),

Still the wrong size?

> +       DEFINE_RES_IRQ(MVME147_IRQ_SCSI_PORT),
> +};
> +
> +int __init mvme147_platform_init(void)
> +{
> +       struct platform_device *pdev;
> +       int rv = 0;
> +
> +       pdev = platform_device_register_simple("mvme147-scsi", -1,
> +               mvme147_scsi_rsrc, ARRAY_SIZE(mvme147_scsi_rsrc));

I think you actually have to use platform_device_register_full() to pass
a DMA mask here: As I understand it, the dma_set_mask_and_coherent()
call in the driver fails if the device is not already marked as dma
capable by having an initial mask set.

The way this normally works is that the device gets created with a mask
that reflects the capabilities of the bus, while the driver sets a mask
based on what it wants to program into the device, and the dma-mapping
interfaces ensure that we only use the intersection of those.

        Arnd

Hi Arnd,

Am 12.07.2022 um 20:12 schrieb Arnd Bergmann:
> On Tue, Jul 12, 2022 at 9:58 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> +
>> +static const struct resource mvme147_scsi_rsrc[] __initconst = {
>> +       DEFINE_RES_MEM(MVME147_SCSI_BASE, 0xff),
>
> Still the wrong size?

Too true - forgot to fix that, sorry.

>
>> +       DEFINE_RES_IRQ(MVME147_IRQ_SCSI_PORT),
>> +};
>> +
>> +int __init mvme147_platform_init(void)
>> +{
>> +       struct platform_device *pdev;
>> +       int rv = 0;
>> +
>> +       pdev = platform_device_register_simple("mvme147-scsi", -1,
>> +               mvme147_scsi_rsrc, ARRAY_SIZE(mvme147_scsi_rsrc));
>
> I think you actually have to use platform_device_register_full() to pass
> a DMA mask here: As I understand it, the dma_set_mask_and_coherent()
> call in the driver fails if the device is not already marked as dma
> capable by having an initial mask set.

I'll take a look at that - if true, this requires the amiga-a3000-scsi 
platform device set-up be changed in the same way (the gvp11 and a2091 
drivers inherit the DMA mask from the Zorro bus default, so ought to 
work OK).

Cheers,

	Michael

> The way this normally works is that the device gets created with a mask
> that reflects the capabilities of the bus, while the driver sets a mask
> based on what it wants to program into the device, and the dma-mapping
> interfaces ensure that we only use the intersection of those.
>
>         Arnd
>

Hi Arnd,

On 12/07/22 21:28, Michael Schmitz wrote:
>
>>> I think you actually have to use platform_device_register_full() to 
>>> pass
>>> a DMA mask here: As I understand it, the dma_set_mask_and_coherent()
>>> call in the driver fails if the device is not already marked as dma
>>> capable by having an initial mask set.
>>
>> I'll take a look at that - if true, this requires the amiga-a3000-scsi
>> platform device set-up be changed in the same way (the gvp11 and a2091
>> drivers inherit the DMA mask from the Zorro bus default, so ought to
>> work OK).
>
> I think we are good with using platform_device_register_simple():
>
> setup_pdev_dma_masks() sets the DMA mask to 32 bit when allocating a 
> new platform device, and platform_device_register_full() only changes 
> that when passed in a non-zero mask in pdevinfo.
>
> platform_device_register_simple() leaves the pdevinfo mask zero, so 
> the 32 bit default set in setup_pdev_dma_masks() survives.

Verified by having pata_falcon print the default DMA mask (0xffffffff), 
then setting a 24 bit DMA mask and reading it back.

That did uncover an error I made in the gvp11 driver - the devices' 
default DMA mask there is given as a 32 bit integer, but the DMA API 
needs 64 bit. Will send a separate fix for that.

Cheers,

     Michael

>
> Cheers,
>
>     Michael
>
>>
>> Cheers,
>>
>>     Michael
>>
>>> The way this normally works is that the device gets created with a mask
>>> that reflects the capabilities of the bus, while the driver sets a mask
>>> based on what it wants to program into the device, and the dma-mapping
>>> interfaces ensure that we only use the intersection of those.
>>>
>>>         Arnd
>>>