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@xxxxxxxx>
