| Message ID | 20220712075832.23793-3-schmitzmic@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert m68k MVME147 WD33C93 SCSI driver to DMA API | expand |
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, Am 12.07.2022 um 21:07 schrieb Michael Schmitz: > 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). 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. 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 >>
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 >>>
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c index 4e6218115f43..c6e7dfe3eb54 100644 --- a/arch/m68k/mvme147/config.c +++ b/arch/m68k/mvme147/config.c @@ -21,6 +21,7 @@ #include <linux/console.h> #include <linux/linkage.h> #include <linux/init.h> +#include <linux/platform_device.h> #include <linux/major.h> #include <linux/rtc.h> #include <linux/interrupt.h> @@ -188,3 +189,23 @@ int mvme147_hwclk(int op, struct rtc_time *t) } return 0; } + +static const struct resource mvme147_scsi_rsrc[] __initconst = { + DEFINE_RES_MEM(MVME147_SCSI_BASE, 0xff), + 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)); + if (IS_ERR(pdev)) + rv = PTR_ERR(pdev); + + return rv; +} + +arch_initcall(mvme147_platform_init);
Set up a platform device for the mvme147_scsi driver. The platform device is required for conversion of the driver to the DMA API. CC: linux-scsi@vger.kernel.org Link: https://lore.kernel.org/r/6d1d88ee-1cf6-c735-1e6d-bafd2096e322@gmail.com Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- arch/m68k/mvme147/config.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)