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

["Arnd Bergmann" <arnd@arndb.de>]

On Tue, Aug 29, 2023, at 18:25, Michael Schmitz wrote:
>
>>> +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.

Ok, I see now. Let's leave the patch as it is then.

Acked-by: Arnd Bergmann <arnd@arndb.de>