[PATCH v2 0/5] Convert m68k MVME147 WD33C93 SCSI driver to DMA API

[PATCH v2 0/5] Convert m68k MVME147 WD33C93 SCSI driver to DMA API

[Michael Schmitz]

This series was precipitated by Arnd removing CONFIG_VIRT_TO_BUS. The
m68k WD33C93 still used virt_to_bus to convert virtual addresses to
physical addresses suitable for the DMA engines (note m68k does not
have an IOMMU and uses a direct mapping for DMA addresses). 

Arnd suggested to use dma_map_single() to set up dma mappings instead
of open-coding much the same in every driver dma_setup() function. An
earlier patch series has converted the three Amiga WD33C93 drivers,
this series now takes care of the sole remaining m68k WD33C93 driver.

The m68k VME mvme147 driver has no DMA addressing or alignment
restrictions and can be converted in the same way as the Amiga a3000
one in my previous series, but requires conversion to a platform
device driver first.

Changes from v1: 
- use of devm_platform_ioremap_resource() to convert phys. resource
to virtual address for chip registers.

Only compile tested so far, and hardware testing might be hard to do.
I'd appreciate someone giving this a thorough review.

Cheers,

   Michael

[PATCH v2 1/5] m68k - add MVME147 SCSI base address to mvme147hw.h

[Michael Schmitz]

The base address for the WD33C93 SCSI host adapter on mvme147
boards is missing from mvme147hw.h. This information will be
needed for platform device conversion of the mvme147_scsi
driver, so add it here.

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/include/asm/mvme147hw.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h
index e28eb1c0e0bf..fd8c1e4fc7be 100644
--- a/arch/m68k/include/asm/mvme147hw.h
+++ b/arch/m68k/include/asm/mvme147hw.h
@@ -93,6 +93,7 @@ struct pcc_regs {
 #define M147_SCC_B_ADDR		0xfffe3000
 #define M147_SCC_PCLK		5000000
 
+#define MVME147_SCSI_BASE	0xfffe4000
 #define MVME147_IRQ_SCSI_PORT	(IRQ_USER+0x45)
 #define MVME147_IRQ_SCSI_DMA	(IRQ_USER+0x46)
 
-- 
2.17.1

[PATCH v2 2/5] m68k - set up platform device for mvme147_scsi

[Michael Schmitz]

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(+)

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);
-- 
2.17.1

Re: [PATCH v2 2/5] m68k - set up platform device for mvme147_scsi

[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?

> +       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

Re: [PATCH v2 2/5] m68k - set up platform device for mvme147_scsi

[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).

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
>

Re: [PATCH v2 2/5] m68k - set up platform device for mvme147_scsi

[Michael Schmitz]

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

Re: [PATCH v2 2/5] m68k - set up platform device for mvme147_scsi

[Michael Schmitz]

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

[PATCH v2 3/5] m68k - add MMIO ioremap support for mvme147

[Michael Schmitz]

Converting the mvme147 SCSI driver to the DMA API requires use of
ioremap() in order to get the kernel virtual address of the WD
chip registers.

Add support for transparent mapping of the mvme147 MMIO region to
arch/m68k/mm/kmap.c to enable use of ioremap() in that driver.

Link: https://lore.kernel.org/r/6d1d88ee-1cf6-c735-1e6d-bafd2096e322@gmail.com
Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---
 arch/m68k/mm/kmap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/m68k/mm/kmap.c b/arch/m68k/mm/kmap.c
index 7594a945732b..2bcede2af902 100644
--- a/arch/m68k/mm/kmap.c
+++ b/arch/m68k/mm/kmap.c
@@ -185,6 +185,13 @@ void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 			return (void __iomem *)physaddr;
 	}
 #endif
+#ifdef CONFIG_MVME147
+	if (MACH_IS_MVME147) {
+		if (physaddr >= 0xe0000000 && cacheflag == IOMAP_NOCACHE_SER)
+			return (void __iomem *)physaddr;
+	}
+#endif
+
 #ifdef CONFIG_COLDFIRE
 	if (__cf_internalio(physaddr))
 		return (void __iomem *) physaddr;
@@ -308,6 +315,10 @@ void iounmap(void __iomem *addr)
 	if (MACH_IS_VIRT && (unsigned long)addr >= 0xff000000)
 		return;
 #endif
+#ifdef CONFIG_MVME147
+	if (MACH_IS_MVME147 && (unsigned long)addr >= 0xe000000)
+		return;
+#endif
 #ifdef CONFIG_COLDFIRE
 	if (cf_internalio(addr))
 		return;
-- 
2.17.1

[PATCH v2 4/5] scsi - convert mvme147.c driver to platform device

[Michael Schmitz]

Convert the mvme147.c driver to a platform device driver.
This 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>

--

Changes from v1:

- change patch index from 3 to 3 (due to insertion of m68k kmap patch)

Arnd Bergmann:
- use devm_platform_ioremap_resource() for wd33c93 base address
---
 drivers/scsi/mvme147.c | 89 +++++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/mvme147.c b/drivers/scsi/mvme147.c
index 472fa043094f..ea8afeec8e56 100644
--- a/drivers/scsi/mvme147.c
+++ b/drivers/scsi/mvme147.c
@@ -3,6 +3,7 @@
 #include <linux/mm.h>
 #include <linux/blkdev.h>
 #include <linux/interrupt.h>
+#include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -85,40 +86,52 @@ static struct scsi_host_template mvme147_host_template = {
 	.cmd_size		= sizeof(struct scsi_pointer),
 };
 
-static struct Scsi_Host *mvme147_shost;
-
-static int __init mvme147_init(void)
+static int __init mvme147_scsi_probe(struct platform_device *pdev)
 {
+	struct resource *mres, *ires;
+	struct Scsi_Host *mvme147_inst;
 	wd33c93_regs regs;
 	struct WD33C93_hostdata *hdata;
+	void __iomem *base;
 	int error = -ENOMEM;
 
-	if (!MACH_IS_MVME147)
-		return 0;
+	mres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mres)
+		return -ENODEV;
+
+	ires = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!ires)
+		return -ENODEV;
+
+	if (!request_mem_region(mres->start, resource_size(mres), "wd33c93"))
+		return -EBUSY;
 
-	mvme147_shost = scsi_host_alloc(&mvme147_host_template,
+	mvme147_inst = scsi_host_alloc(&mvme147_host_template,
 			sizeof(struct WD33C93_hostdata));
-	if (!mvme147_shost)
+	if (!mvme147_inst)
 		goto err_out;
-	mvme147_shost->base = 0xfffe4000;
-	mvme147_shost->irq = MVME147_IRQ_SCSI_PORT;
 
-	regs.SASR = (volatile unsigned char *)0xfffe4000;
-	regs.SCMD = (volatile unsigned char *)0xfffe4001;
+	base = devm_platform_ioremap_resource(pdev, 0);
 
-	hdata = shost_priv(mvme147_shost);
+	mvme147_inst->base = (unsigned int) base;
+	mvme147_inst->irq = ires->start;
+
+	regs.SASR = (volatile unsigned char *)base;
+	regs.SCMD = (volatile unsigned char *)(base+1);
+
+	hdata = shost_priv(mvme147_inst);
 	hdata->no_sync = 0xff;
 	hdata->fast = 0;
 	hdata->dma_mode = CTRL_DMA;
 
-	wd33c93_init(mvme147_shost, regs, dma_setup, dma_stop, WD33C93_FS_8_10);
+	wd33c93_init(mvme147_inst, regs, dma_setup, dma_stop, WD33C93_FS_8_10);
 
-	error = request_irq(MVME147_IRQ_SCSI_PORT, mvme147_intr, 0,
-			"MVME147 SCSI PORT", mvme147_shost);
+	error = request_irq(ires->start, mvme147_intr, 0,
+			"MVME147 SCSI PORT", mvme147_inst);
 	if (error)
 		goto err_unregister;
-	error = request_irq(MVME147_IRQ_SCSI_DMA, mvme147_intr, 0,
-			"MVME147 SCSI DMA", mvme147_shost);
+	error = request_irq(ires->start+1, mvme147_intr, 0,
+			"MVME147 SCSI DMA", mvme147_inst);
 	if (error)
 		goto err_free_irq;
 #if 0	/* Disabled; causes problems booting */
@@ -133,30 +146,50 @@ static int __init mvme147_init(void)
 	m147_pcc->dma_cntrl = 0x00;	/* ensure DMA is stopped */
 	m147_pcc->dma_intr = 0x89;	/* Ack and enable ints */
 
-	error = scsi_add_host(mvme147_shost, NULL);
+	error = scsi_add_host(mvme147_inst, &pdev->dev);
 	if (error)
 		goto err_free_irq;
-	scsi_scan_host(mvme147_shost);
+
+	platform_set_drvdata(pdev, mvme147_inst);
+
+	scsi_scan_host(mvme147_inst);
 	return 0;
 
 err_free_irq:
-	free_irq(MVME147_IRQ_SCSI_PORT, mvme147_shost);
+	free_irq(ires->start, mvme147_inst);
 err_unregister:
-	scsi_host_put(mvme147_shost);
+	scsi_host_put(mvme147_inst);
 err_out:
 	return error;
 }
 
-static void __exit mvme147_exit(void)
+static int __exit mvme147_scsi_remove(struct platform_device *pdev)
 {
-	scsi_remove_host(mvme147_shost);
+	struct Scsi_Host *mvme147_inst = platform_get_drvdata(pdev);
+	struct resource *mres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct resource *ires = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+	scsi_remove_host(mvme147_inst);
 
 	/* XXX Make sure DMA is stopped! */
-	free_irq(MVME147_IRQ_SCSI_PORT, mvme147_shost);
-	free_irq(MVME147_IRQ_SCSI_DMA, mvme147_shost);
+	free_irq(ires->start, mvme147_inst);
+	free_irq(ires->start+1, mvme147_inst);
 
-	scsi_host_put(mvme147_shost);
+	iounmap((void __iomem *)mvme147_inst->base);
+	scsi_host_put(mvme147_inst);
+	release_mem_region(mres->start, resource_size(mres));
+	return 0;
 }
 
-module_init(mvme147_init);
-module_exit(mvme147_exit);
+static struct platform_driver mvme147_scsi_driver = {
+	.remove = __exit_p(mvme147_scsi_remove),
+	.driver   = {
+		.name	= "mvme147-scsi",
+	},
+};
+
+module_platform_driver_probe(mvme147_scsi_driver, mvme147_scsi_probe);
+
+MODULE_DESCRIPTION("MVME147 built-in SCSI");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mvme147-scsi");
-- 
2.17.1

[PATCH v2 5/5] scsi - convert mvme147.c driver to DMA API

[Michael Schmitz]

Convert mvme147.c to DMA API to eliminate one of the last
usages of virt_to_bus().

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>

--

Changes from v1:
- change patch index from 3 to 4 (due to insertion of m68k kmap patch)
---
 drivers/scsi/mvme147.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/mvme147.c b/drivers/scsi/mvme147.c
index ea8afeec8e56..166248bef6cf 100644
--- a/drivers/scsi/mvme147.c
+++ b/drivers/scsi/mvme147.c
@@ -21,6 +21,8 @@
 #include "wd33c93.h"
 #include "mvme147.h"
 
+#define DMA_DIR(d)   ((d == DATA_OUT_DIR) ? DMA_TO_DEVICE : DMA_FROM_DEVICE)
+
 static irqreturn_t mvme147_intr(int irq, void *data)
 {
 	struct Scsi_Host *instance = data;
@@ -35,10 +37,20 @@ static irqreturn_t mvme147_intr(int irq, void *data)
 static int dma_setup(struct scsi_cmnd *cmd, int dir_in)
 {
 	struct scsi_pointer *scsi_pointer = WD33C93_scsi_pointer(cmd);
+	unsigned long len = scsi_pointer->this_residual;
 	struct Scsi_Host *instance = cmd->device->host;
 	struct WD33C93_hostdata *hdata = shost_priv(instance);
 	unsigned char flags = 0x01;
-	unsigned long addr = virt_to_bus(scsi_pointer->ptr);
+	dma_addr_t addr;
+
+	addr = dma_map_single(instance->dma_dev, scsi_pointer->ptr,
+			      len, DMA_DIR(dir_in));
+	if (dma_mapping_error(instance->dma_dev, addr)) {
+		dev_warn(instance->dma_dev, "cannot map SCSI data block %p\n",
+			 scsi_pointer->ptr);
+		return 1;
+	}
+	scsi_pointer->dma_handle = addr;
 
 	/* setup dma direction */
 	if (!dir_in)
@@ -47,14 +59,6 @@ static int dma_setup(struct scsi_cmnd *cmd, int dir_in)
 	/* remember direction */
 	hdata->dma_dir = dir_in;
 
-	if (dir_in) {
-		/* invalidate any cache */
-		cache_clear(addr, scsi_pointer->this_residual);
-	} else {
-		/* push any dirty cache */
-		cache_push(addr, scsi_pointer->this_residual);
-	}
-
 	/* start DMA */
 	m147_pcc->dma_bcr = scsi_pointer->this_residual | (1 << 24);
 	m147_pcc->dma_dadr = addr;
@@ -67,7 +71,13 @@ static int dma_setup(struct scsi_cmnd *cmd, int dir_in)
 static void dma_stop(struct Scsi_Host *instance, struct scsi_cmnd *SCpnt,
 		     int status)
 {
+	struct scsi_pointer *scsi_pointer = WD33C93_scsi_pointer(SCpnt);
+	struct WD33C93_hostdata *hdata = shost_priv(instance);
+
 	m147_pcc->dma_cntrl = 0;
+	dma_unmap_single(instance->dma_dev, scsi_pointer->dma_handle,
+			 scsi_pointer->this_residual,
+			 DMA_DIR(hdata->dma_dir));
 }
 
 static struct scsi_host_template mvme147_host_template = {
@@ -95,6 +105,11 @@ static int __init mvme147_scsi_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int error = -ENOMEM;
 
+	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) {
+		dev_warn(&pdev->dev, "cannot use 32 bit DMA\n");
+		return -ENODEV;
+	}
+
 	mres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mres)
 		return -ENODEV;
-- 
2.17.1