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>
