Re: [PATCH] scsi: gvp11: add module parameter for DMA transfer bit mask

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Arnd,

thanks for your comments!

On 30/08/23 10:05, Arnd Bergmann wrote:
> On Tue, Aug 29, 2023, at 17:45, Michael Schmitz wrote:
>> SCSI boards on Amiga. There now is no way to set a non-default
>> DMA mask on these boards.
> It might help to mention here in which cases the default mask
> is actually wrong.

All I have is:

Probably it's needed on A2000 with an accelerator card and GVP II SCSI,
to prevent DMA to RAM banks that do not support fast DMA cycles.

from Geert's reply. I can add that. It just did sound a shade 
speculative...
>
>> +module_param(gvp11_xfer_mask,  int, 0444);
>> +MODULE_PARM_DESC(gvp11_xfer_mask, "DMA mask (0xff000000 == 24 bit DMA)");
>> +
> I think the comment is the wrong way round, it should be
> 0x00ffffff in this case, which also matches the default
> mask for ZORRO_PROD_GVP_SERIES_II, in the match table:
>
> static struct zorro_device_id gvp11_zorro_tbl[] = {
>          { ZORRO_PROD_GVP_COMBO_030_R3_SCSI,     ~0x00ffffff },
>          { ZORRO_PROD_GVP_SERIES_II,             ~0x00ffffff },
>          { ZORRO_PROD_GVP_GFORCE_030_SCSI,       ~0x01ffffff },
>          { ZORRO_PROD_GVP_A530_SCSI,             ~0x01ffffff },
>          { ZORRO_PROD_GVP_COMBO_030_R4_SCSI,     ~0x01ffffff },
>          { ZORRO_PROD_GVP_A1291,                 ~0x07ffffff },
>          { ZORRO_PROD_GVP_GFORCE_040_SCSI_1,     ~0x07ffffff },
>          { 0 }
> };

gvp11_xfer_mask works inverse to what you'd expect (and inverse to what 
a DMA mask usually is defined as). DMA can _not_ be used if (address & 
gvp11_xfer_mask) isn't zero. See code in dma_setup() for details.

All those definitions have a '~' prefix, for that very reason.

I agree it isn't intuitive, and caused a little head scratching when 
preparing this patch. But I believe it is correct.

Now you could argue to shift the bit mask inversion to gvp11_probe() or 
even dma_setup() instead to rule out such confusion in future, but that 
would be an actual code change and would benefit from testing on at 
least one of these boards IMO. Not sure how easy that will be.

Cheers,

     Michael

>        Arnd