[PATCH 0/3] m68k: amiga: CS-Lab Warp ata support (and a small zorro id refactor)

[PATCH 0/3] m68k: amiga: CS-Lab Warp ata support (and a small zorro id refactor)

[Paolo Pisati]

Patch 01 is a small refactor of the CS-Lab Zorro ids
Patch 02 is the driver for the CS-Lab Warp pata/CF socket
Patch 03 enables the driver in amiga_defconfig

Paolo Pisati (3):
  include/uapi/linux/zorro_ids.h: add more cslab warp id
  amiga: cslab ata support
  m68k: amiga_defconfig: enable PATA_CSWARP

 arch/m68k/amiga/config.c          |   4 +-
 arch/m68k/configs/amiga_defconfig |   1 +
 drivers/ata/Kconfig               |  12 ++
 drivers/ata/Makefile              |   1 +
 drivers/ata/pata_cswarp.c         | 210 ++++++++++++++++++++++++++++++
 drivers/zorro/zorro.ids           |   5 +
 include/uapi/linux/zorro_ids.h    |  11 +-
 7 files changed, 241 insertions(+), 3 deletions(-)
 create mode 100644 drivers/ata/pata_cswarp.c

-- 
2.34.1

[PATCH 1/3] include/uapi/linux/zorro_ids.h: add more cslab warp id

[Paolo Pisati]

Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
 arch/m68k/amiga/config.c       | 4 ++--
 include/uapi/linux/zorro_ids.h | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/m68k/amiga/config.c b/arch/m68k/amiga/config.c
index 0147130dc34e..a76bc9daf38a 100644
--- a/arch/m68k/amiga/config.c
+++ b/arch/m68k/amiga/config.c
@@ -182,8 +182,8 @@ int __init amiga_parse_bootinfo(const struct bi_record *record)
 			dev->boardsize = be32_to_cpu(cd->cd_BoardSize);
 
 			/* CS-LAB Warp 1260 workaround */
-			if (be16_to_cpu(dev->rom.er_Manufacturer) == ZORRO_MANUF(ZORRO_PROD_CSLAB_WARP_1260) &&
-			    dev->rom.er_Product == ZORRO_PROD(ZORRO_PROD_CSLAB_WARP_1260)) {
+			if (be16_to_cpu(dev->rom.er_Manufacturer) == ZORRO_MANUF(ZORRO_PROD_CSLAB_WARP_CTRL) &&
+			    dev->rom.er_Product == ZORRO_PROD(ZORRO_PROD_CSLAB_WARP_CTRL)) {
 
 				/* turn off all interrupts */
 				pr_info("Warp 1260 card detected: applying interrupt storm workaround\n");
diff --git a/include/uapi/linux/zorro_ids.h b/include/uapi/linux/zorro_ids.h
index 393f2ee9c042..0be1fb0c3915 100644
--- a/include/uapi/linux/zorro_ids.h
+++ b/include/uapi/linux/zorro_ids.h
@@ -450,7 +450,10 @@
 #define  ZORRO_PROD_VMC_HYPERCOM_4				ZORRO_ID(VMC, 0x02, 0)
 
 #define ZORRO_MANUF_CSLAB					0x1400
-#define  ZORRO_PROD_CSLAB_WARP_1260				ZORRO_ID(CSLAB, 0x65, 0)
+#define ZORRO_PROD_CSLAB_WARP_DDR3				ZORRO_ID(CSLAB, 0x3c, 0)
+#define ZORRO_PROD_CSLAB_WARP_VRAM				ZORRO_ID(CSLAB, 0x64, 0)
+#define ZORRO_PROD_CSLAB_WARP_CTRL				ZORRO_ID(CSLAB, 0x65, 0)
+#define ZORRO_PROD_CSLAB_WARP_XROM				ZORRO_ID(CSLAB, 0x66, 1)
 
 #define ZORRO_MANUF_INFORMATION					0x157C
 #define  ZORRO_PROD_INFORMATION_ISDN_ENGINE_I			ZORRO_ID(INFORMATION, 0x64, 0)
-- 
2.34.1

[PATCH 2/3] amiga: cslab ata support

[Paolo Pisati]

Driver for the on-board IDE interface on CS-Lab Warp Expansion Card
(NOTE that idemode=native has to be set in Warp Configuration)

Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
 drivers/ata/Kconfig            |  12 ++
 drivers/ata/Makefile           |   1 +
 drivers/ata/pata_cswarp.c      | 210 +++++++++++++++++++++++++++++++++
 drivers/zorro/zorro.ids        |   5 +
 include/uapi/linux/zorro_ids.h |   6 +
 5 files changed, 234 insertions(+)
 create mode 100644 drivers/ata/pata_cswarp.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e00536b49552..9ec4d3a7ec1e 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -1025,6 +1025,18 @@ config PATA_GAYLE
 
 	  If unsure, say N.
 
+config PATA_CSWARP
+	tristate "Amiga CS Warp PATA support"
+	depends on M68K && AMIGA
+	help
+	  This option enables support for the on-board IDE
+	  interface on CS-Lab Warp Expansion Card
+	  (NOTE that idemode=native has to be set in Warp
+	  Configuration)
+
+	  If unsure, say N.
+
+
 config PATA_BUDDHA
 	tristate "Buddha/Catweasel/X-Surf PATA support"
 	depends on ZORRO
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 20e6645ab737..7b9b87ebacea 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_PATA_WINBOND)	+= pata_sl82c105.o
 obj-$(CONFIG_PATA_CMD640_PCI)	+= pata_cmd640.o
 obj-$(CONFIG_PATA_FALCON)	+= pata_falcon.o
 obj-$(CONFIG_PATA_GAYLE)	+= pata_gayle.o
+obj-$(CONFIG_PATA_CSWARP)	+= pata_cswarp.o
 obj-$(CONFIG_PATA_BUDDHA)	+= pata_buddha.o
 obj-$(CONFIG_PATA_ISAPNP)	+= pata_isapnp.o
 obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
diff --git a/drivers/ata/pata_cswarp.c b/drivers/ata/pata_cswarp.c
new file mode 100644
index 000000000000..29d76b22f1b2
--- /dev/null
+++ b/drivers/ata/pata_cswarp.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Amiga CS Warp PATA controller driver
+ *
+ * Copyright (c) 2024 CS-Lab s.c.
+ *		http://www.cs-lab.eu
+ *
+ * Based on pata_gayle.c, pata_buddha.c and warpATA.device:
+ *
+ *     Created 2 Jun 2024 by Andrzej Rogozynski
+ */
+
+#include <linux/ata.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/libata.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/zorro.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_host.h>
+
+#include <asm/amigahw.h>
+#include <asm/amigaints.h>
+#include <asm/amigayle.h>
+#include <asm/setup.h>
+
+#define DRV_NAME "pata_cswarp"
+#define DRV_VERSION "0.1.0"
+
+#define WARP_OFFSET_ATA         0x6000
+#define REV16(x) ((uint16_t)((x << 8) | (x >> 8)))
+
+static const struct scsi_host_template pata_cswarp_sht = {
+	ATA_PIO_SHT(DRV_NAME),
+};
+
+/* FIXME: is this needed? */
+static unsigned int pata_cswarp_data_xfer(struct ata_queued_cmd *qc,
+					 unsigned char *buf,
+					 unsigned int buflen, int rw)
+{
+	struct ata_device *dev = qc->dev;
+	struct ata_port *ap = dev->link->ap;
+	void __iomem *data_addr = ap->ioaddr.data_addr;
+	unsigned int words = buflen >> 1;
+
+	/* Transfer multiple of 2 bytes */
+	if (rw == READ)
+		raw_insw((u16 *)data_addr, (u16 *)buf, words);
+	else
+		raw_outsw((u16 *)data_addr, (u16 *)buf, words);
+
+	/* Transfer trailing byte, if any. */
+	if (unlikely(buflen & 0x01)) {
+		unsigned char pad[2] = { };
+
+		/* Point buf to the tail of buffer */
+		buf += buflen - 1;
+
+		if (rw == READ) {
+			raw_insw((u16 *)data_addr, (u16 *)pad, 1);
+			*buf = pad[0];
+		} else {
+			pad[0] = *buf;
+			raw_outsw((u16 *)data_addr, (u16 *)pad, 1);
+		}
+		words++;
+	}
+
+	return words << 1;
+}
+
+/*
+ * Provide our own set_mode() as we don't want to change anything that has
+ * already been configured..
+ */
+static int pata_cswarp_set_mode(struct ata_link *link,
+			       struct ata_device **unused)
+{
+	struct ata_device *dev;
+
+	ata_for_each_dev(dev, link, ENABLED) {
+		/* We don't really care */
+		dev->pio_mode = dev->xfer_mode = XFER_PIO_0;
+		dev->xfer_shift = ATA_SHIFT_PIO;
+		dev->flags |= ATA_DFLAG_PIO;
+		ata_dev_info(dev, "configured for PIO\n");
+	}
+	return 0;
+}
+
+static struct ata_port_operations pata_cswarp_ops = {
+	.inherits	= &ata_sff_port_ops,
+	.sff_data_xfer	= pata_cswarp_data_xfer,
+	.cable_detect	= ata_cable_unknown,
+	.set_mode	= pata_cswarp_set_mode,
+};
+
+static int pata_cswarp_probe(struct zorro_dev *z,
+			     const struct zorro_device_id *ent)
+{
+	static const char board_name[] = "csWarp";
+	struct ata_host *host;
+	void __iomem *cswarp_ctrl_board;
+	unsigned long board;
+
+	board = z->resource.start;
+
+	dev_info(&z->dev, "%s IDE controller (board: 0x%lx)\n", board_name,
+		 board);
+
+	if (!devm_request_mem_region(&z->dev, board + WARP_OFFSET_ATA, 0x1800,
+				     DRV_NAME))
+		return -ENXIO;
+
+	/* allocate host */
+	host = ata_host_alloc(&z->dev, 1);
+	if (!host)
+		return -ENXIO;
+
+	cswarp_ctrl_board = (void *)board;
+
+	struct ata_port *ap = host->ports[0];
+	void __iomem *base = cswarp_ctrl_board + WARP_OFFSET_ATA;
+
+	ap->ops = &pata_cswarp_ops;
+
+	ap->pio_mask = ATA_PIO4;
+	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY |
+		ATA_FLAG_PIO_POLLING;
+
+	ap->ioaddr.data_addr		= base;
+	ap->ioaddr.error_addr		= base + 1 * 4;
+	ap->ioaddr.feature_addr		= base + 1 * 4;
+	ap->ioaddr.nsect_addr		= base + 2 * 4;
+	ap->ioaddr.lbal_addr		= base + 3 * 4;
+	ap->ioaddr.lbam_addr		= base + 4 * 4;
+	ap->ioaddr.lbah_addr		= base + 5 * 4;
+	ap->ioaddr.device_addr		= base + 6 * 4;
+	ap->ioaddr.status_addr		= base + 7 * 4;
+	ap->ioaddr.command_addr		= base + 7 * 4;
+
+	ap->ioaddr.altstatus_addr	= base + (0x1000 | (6UL << 2));
+	ap->ioaddr.ctl_addr			= base + (0x1000 | (6UL << 2));
+
+	ap->private_data = (void *)0;
+
+	ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
+		      (unsigned long)ap->ioaddr.ctl_addr);
+
+	ata_host_activate(host, 0, NULL,
+			  IRQF_SHARED, &pata_cswarp_sht);
+
+	return 0;
+}
+
+static void pata_cswarp_remove(struct zorro_dev *z)
+{
+	struct ata_host *host = dev_get_drvdata(&z->dev);
+
+	ata_host_detach(host);
+}
+
+static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
+	{ ZORRO_PROD_CSLAB_WARP_CTRL, 0},
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(zorro, pata_cswarp_zorro_tbl);
+
+static struct zorro_driver pata_cswarp_driver = {
+	.name           = "pata_cswarp",
+	.id_table       = pata_cswarp_zorro_tbl,
+	.probe          = pata_cswarp_probe,
+	.remove         = pata_cswarp_remove,
+};
+
+/*
+ * We cannot have a modalias for X-Surf boards, as it competes with the
+ * zorro8390 network driver. As a stopgap measure until we have proper
+ * MFD support for this board, we manually attach to it late after Zorro
+ * has enumerated its boards.
+ */
+static int __init pata_cswarp_late_init(void)
+{
+	struct zorro_dev *z = NULL;
+
+	/* Auto-bind to regular boards */
+	zorro_register_driver(&pata_cswarp_driver);
+
+	/* Manually bind to all boards */
+	while ((z = zorro_find_device(ZORRO_PROD_CSLAB_WARP_CTRL, z))) {
+		static struct zorro_device_id cswarp_ent = {
+			ZORRO_PROD_CSLAB_WARP_CTRL, 0
+		};
+
+		pata_cswarp_probe(z, &cswarp_ent);
+	}
+	return 0;
+}
+late_initcall(pata_cswarp_late_init);
+
+MODULE_AUTHOR("Andrzej Rogozynski");
+MODULE_DESCRIPTION("low-level driver for CSWarp PATA");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
diff --git a/drivers/zorro/zorro.ids b/drivers/zorro/zorro.ids
index 119abea8c6cb..33418af7488a 100644
--- a/drivers/zorro/zorro.ids
+++ b/drivers/zorro/zorro.ids
@@ -400,6 +400,11 @@
 	0100  ISDN Blaster Z2 [ISDN Interface]
 	0200  HyperCom 4 [Multi I/O]
 	0600  HyperCom 4+ [Multi I/O]
+1400 CSLab
+	6000  Warp DDR3 Memory
+	0001  Warp Video Memory
+	0101  Warp Control Registers
+	0201  Warp Flash ROM
 157c  Information
 	6400  ISDN Engine I [ISDN Interface]
 2017  Vortex
diff --git a/include/uapi/linux/zorro_ids.h b/include/uapi/linux/zorro_ids.h
index 0be1fb0c3915..5736d2bf0295 100644
--- a/include/uapi/linux/zorro_ids.h
+++ b/include/uapi/linux/zorro_ids.h
@@ -455,6 +455,12 @@
 #define ZORRO_PROD_CSLAB_WARP_CTRL				ZORRO_ID(CSLAB, 0x65, 0)
 #define ZORRO_PROD_CSLAB_WARP_XROM				ZORRO_ID(CSLAB, 0x66, 1)
 
+#define ZORRO_MANUF_CSLAB                   0x1400
+#define ZORRO_PROD_CSLAB_WARP_DDR3          ZORRO_ID(CSLAB, 0x3c, 0)
+#define ZORRO_PROD_CSLAB_WARP_VRAM          ZORRO_ID(CSLAB, 0x64, 0)
+#define ZORRO_PROD_CSLAB_WARP_CTRL          ZORRO_ID(CSLAB, 0x65, 0)
+#define ZORRO_PROD_CSLAB_WARP_XROM          ZORRO_ID(CSLAB, 0x66, 1)
+
 #define ZORRO_MANUF_INFORMATION					0x157C
 #define  ZORRO_PROD_INFORMATION_ISDN_ENGINE_I			ZORRO_ID(INFORMATION, 0x64, 0)
 
-- 
2.34.1

[PATCH 3/3] m68k: amiga_defconfig: enable PATA_CSWARP

[Paolo Pisati]

Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
 arch/m68k/configs/amiga_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
index c705247e7b5b..feed92c95394 100644
--- a/arch/m68k/configs/amiga_defconfig
+++ b/arch/m68k/configs/amiga_defconfig
@@ -334,6 +334,7 @@ CONFIG_ATA=y
 # CONFIG_ATA_VERBOSE_ERROR is not set
 # CONFIG_ATA_BMDMA is not set
 CONFIG_PATA_GAYLE=y
+CONFIG_PATA_CSWARP=y
 CONFIG_PATA_BUDDHA=y
 CONFIG_MD=y
 CONFIG_BLK_DEV_DM=m
-- 
2.34.1

Re: [PATCH 2/3] amiga: cslab ata support

[Damien Le Moal]

On 2025/03/21 11:14, Paolo Pisati wrote:
> Driver for the on-board IDE interface on CS-Lab Warp Expansion Card
> (NOTE that idemode=native has to be set in Warp Configuration)
> 
> Signed-off-by: Paolo Pisati <p.pisati@gmail.com>


Please change the commit title to something like:

ata: pata_cswarp: Add Amiga cslab ata support

> ---
>  drivers/ata/Kconfig            |  12 ++
>  drivers/ata/Makefile           |   1 +
>  drivers/ata/pata_cswarp.c      | 210 +++++++++++++++++++++++++++++++++
>  drivers/zorro/zorro.ids        |   5 +
>  include/uapi/linux/zorro_ids.h |   6 +
>  5 files changed, 234 insertions(+)
>  create mode 100644 drivers/ata/pata_cswarp.c
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index e00536b49552..9ec4d3a7ec1e 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -1025,6 +1025,18 @@ config PATA_GAYLE
>  
>  	  If unsure, say N.
>  
> +config PATA_CSWARP
> +	tristate "Amiga CS Warp PATA support"
> +	depends on M68K && AMIGA
> +	help
> +	  This option enables support for the on-board IDE
> +	  interface on CS-Lab Warp Expansion Card
> +	  (NOTE that idemode=native has to be set in Warp
> +	  Configuration)

Looks like the lines are short. Please use up to 80 chars per line here.

> +
> +	  If unsure, say N.
> +
> +
>  config PATA_BUDDHA
>  	tristate "Buddha/Catweasel/X-Surf PATA support"
>  	depends on ZORRO
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 20e6645ab737..7b9b87ebacea 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -98,6 +98,7 @@ obj-$(CONFIG_PATA_WINBOND)	+= pata_sl82c105.o
>  obj-$(CONFIG_PATA_CMD640_PCI)	+= pata_cmd640.o
>  obj-$(CONFIG_PATA_FALCON)	+= pata_falcon.o
>  obj-$(CONFIG_PATA_GAYLE)	+= pata_gayle.o
> +obj-$(CONFIG_PATA_CSWARP)	+= pata_cswarp.o
>  obj-$(CONFIG_PATA_BUDDHA)	+= pata_buddha.o
>  obj-$(CONFIG_PATA_ISAPNP)	+= pata_isapnp.o
>  obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
> diff --git a/drivers/ata/pata_cswarp.c b/drivers/ata/pata_cswarp.c
> new file mode 100644
> index 000000000000..29d76b22f1b2
> --- /dev/null
> +++ b/drivers/ata/pata_cswarp.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Amiga CS Warp PATA controller driver
> + *
> + * Copyright (c) 2024 CS-Lab s.c.
> + *		http://www.cs-lab.eu
> + *
> + * Based on pata_gayle.c, pata_buddha.c and warpATA.device:
> + *
> + *     Created 2 Jun 2024 by Andrzej Rogozynski
> + */
> +
> +#include <linux/ata.h>
> +#include <linux/blkdev.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/libata.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/zorro.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <scsi/scsi_host.h>
> +
> +#include <asm/amigahw.h>
> +#include <asm/amigaints.h>
> +#include <asm/amigayle.h>
> +#include <asm/setup.h>
> +
> +#define DRV_NAME "pata_cswarp"
> +#define DRV_VERSION "0.1.0"

There is no need for a driver version. The driver comes with the kernel, it is
not external.

> +
> +#define WARP_OFFSET_ATA         0x6000
> +#define REV16(x) ((uint16_t)((x << 8) | (x >> 8)))

Very generic macrom name... And that is just a byte swap. We have functions for
that already, so why define this ?

> +
> +static const struct scsi_host_template pata_cswarp_sht = {
> +	ATA_PIO_SHT(DRV_NAME),
> +};
> +
> +/* FIXME: is this needed? */

If you need to fix it, then please do it. Otherwise, remove this comment please.

> +static unsigned int pata_cswarp_data_xfer(struct ata_queued_cmd *qc,
> +					 unsigned char *buf,
> +					 unsigned int buflen, int rw)
> +{
> +	struct ata_device *dev = qc->dev;
> +	struct ata_port *ap = dev->link->ap;
> +	void __iomem *data_addr = ap->ioaddr.data_addr;
> +	unsigned int words = buflen >> 1;
> +
> +	/* Transfer multiple of 2 bytes */
> +	if (rw == READ)
> +		raw_insw((u16 *)data_addr, (u16 *)buf, words);
> +	else
> +		raw_outsw((u16 *)data_addr, (u16 *)buf, words);
> +
> +	/* Transfer trailing byte, if any. */
> +	if (unlikely(buflen & 0x01)) {
> +		unsigned char pad[2] = { };
> +
> +		/* Point buf to the tail of buffer */
> +		buf += buflen - 1;
> +
> +		if (rw == READ) {
> +			raw_insw((u16 *)data_addr, (u16 *)pad, 1);
> +			*buf = pad[0];
> +		} else {
> +			pad[0] = *buf;
> +			raw_outsw((u16 *)data_addr, (u16 *)pad, 1);
> +		}
> +		words++;
> +	}
> +
> +	return words << 1;
> +}
> +
> +/*
> + * Provide our own set_mode() as we don't want to change anything that has
> + * already been configured..
> + */
> +static int pata_cswarp_set_mode(struct ata_link *link,
> +			       struct ata_device **unused)
> +{
> +	struct ata_device *dev;
> +
> +	ata_for_each_dev(dev, link, ENABLED) {
> +		/* We don't really care */
> +		dev->pio_mode = dev->xfer_mode = XFER_PIO_0;
> +		dev->xfer_shift = ATA_SHIFT_PIO;
> +		dev->flags |= ATA_DFLAG_PIO;
> +		ata_dev_info(dev, "configured for PIO\n");
> +	}
> +	return 0;
> +}
> +
> +static struct ata_port_operations pata_cswarp_ops = {
> +	.inherits	= &ata_sff_port_ops,
> +	.sff_data_xfer	= pata_cswarp_data_xfer,
> +	.cable_detect	= ata_cable_unknown,
> +	.set_mode	= pata_cswarp_set_mode,
> +};
> +
> +static int pata_cswarp_probe(struct zorro_dev *z,
> +			     const struct zorro_device_id *ent)
> +{
> +	static const char board_name[] = "csWarp";
> +	struct ata_host *host;
> +	void __iomem *cswarp_ctrl_board;
> +	unsigned long board;
> +
> +	board = z->resource.start;

Please fold this above with the declaration:

	unsigned long board = z->resource.start;

> +
> +	dev_info(&z->dev, "%s IDE controller (board: 0x%lx)\n", board_name,
> +		 board);
> +
> +	if (!devm_request_mem_region(&z->dev, board + WARP_OFFSET_ATA, 0x1800,
> +				     DRV_NAME))
> +		return -ENXIO;
> +
> +	/* allocate host */
> +	host = ata_host_alloc(&z->dev, 1);
> +	if (!host)
> +		return -ENXIO;
> +
> +	cswarp_ctrl_board = (void *)board;
> +
> +	struct ata_port *ap = host->ports[0];
> +	void __iomem *base = cswarp_ctrl_board + WARP_OFFSET_ATA;

Please do not mix code and declarations. Move the declarations at the top of the
functions. (yes, C allows this, but we do not use this style in the kernel).

> +
> +	ap->ops = &pata_cswarp_ops;
> +
> +	ap->pio_mask = ATA_PIO4;
> +	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY |
> +		ATA_FLAG_PIO_POLLING;
> +
> +	ap->ioaddr.data_addr		= base;
> +	ap->ioaddr.error_addr		= base + 1 * 4;
> +	ap->ioaddr.feature_addr		= base + 1 * 4;
> +	ap->ioaddr.nsect_addr		= base + 2 * 4;
> +	ap->ioaddr.lbal_addr		= base + 3 * 4;
> +	ap->ioaddr.lbam_addr		= base + 4 * 4;
> +	ap->ioaddr.lbah_addr		= base + 5 * 4;
> +	ap->ioaddr.device_addr		= base + 6 * 4;
> +	ap->ioaddr.status_addr		= base + 7 * 4;
> +	ap->ioaddr.command_addr		= base + 7 * 4;
> +
> +	ap->ioaddr.altstatus_addr	= base + (0x1000 | (6UL << 2));
> +	ap->ioaddr.ctl_addr			= base + (0x1000 | (6UL << 2));

It would be nice to have macro definitions for all the magic numbers you use for
offsets into base, to document these values.

> +
> +	ap->private_data = (void *)0;

= NULL please. And that should not be needed as this is the inital value anyway.

> +
> +	ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +		      (unsigned long)ap->ioaddr.ctl_addr);
> +
> +	ata_host_activate(host, 0, NULL,
> +			  IRQF_SHARED, &pata_cswarp_sht);
> +
> +	return 0;
> +}
> +
> +static void pata_cswarp_remove(struct zorro_dev *z)
> +{
> +	struct ata_host *host = dev_get_drvdata(&z->dev);
> +
> +	ata_host_detach(host);
> +}
> +
> +static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
> +	{ ZORRO_PROD_CSLAB_WARP_CTRL, 0},
> +	{ 0 }
> +};
> +MODULE_DEVICE_TABLE(zorro, pata_cswarp_zorro_tbl);
> +
> +static struct zorro_driver pata_cswarp_driver = {
> +	.name           = "pata_cswarp",
> +	.id_table       = pata_cswarp_zorro_tbl,
> +	.probe          = pata_cswarp_probe,
> +	.remove         = pata_cswarp_remove,
> +};
> +
> +/*
> + * We cannot have a modalias for X-Surf boards, as it competes with the
> + * zorro8390 network driver. As a stopgap measure until we have proper
> + * MFD support for this board, we manually attach to it late after Zorro
> + * has enumerated its boards.
> + */
> +static int __init pata_cswarp_late_init(void)
> +{
> +	struct zorro_dev *z = NULL;
> +
> +	/* Auto-bind to regular boards */
> +	zorro_register_driver(&pata_cswarp_driver);
> +
> +	/* Manually bind to all boards */
> +	while ((z = zorro_find_device(ZORRO_PROD_CSLAB_WARP_CTRL, z))) {
> +		static struct zorro_device_id cswarp_ent = {
> +			ZORRO_PROD_CSLAB_WARP_CTRL, 0
> +		};
> +
> +		pata_cswarp_probe(z, &cswarp_ent);
> +	}
> +	return 0;
> +}
> +late_initcall(pata_cswarp_late_init);
> +
> +MODULE_AUTHOR("Andrzej Rogozynski");
> +MODULE_DESCRIPTION("low-level driver for CSWarp PATA");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> diff --git a/drivers/zorro/zorro.ids b/drivers/zorro/zorro.ids
> index 119abea8c6cb..33418af7488a 100644
> --- a/drivers/zorro/zorro.ids
> +++ b/drivers/zorro/zorro.ids
> @@ -400,6 +400,11 @@
>  	0100  ISDN Blaster Z2 [ISDN Interface]
>  	0200  HyperCom 4 [Multi I/O]
>  	0600  HyperCom 4+ [Multi I/O]
> +1400 CSLab
> +	6000  Warp DDR3 Memory
> +	0001  Warp Video Memory
> +	0101  Warp Control Registers
> +	0201  Warp Flash ROM
>  157c  Information
>  	6400  ISDN Engine I [ISDN Interface]
>  2017  Vortex
> diff --git a/include/uapi/linux/zorro_ids.h b/include/uapi/linux/zorro_ids.h
> index 0be1fb0c3915..5736d2bf0295 100644
> --- a/include/uapi/linux/zorro_ids.h
> +++ b/include/uapi/linux/zorro_ids.h
> @@ -455,6 +455,12 @@
>  #define ZORRO_PROD_CSLAB_WARP_CTRL				ZORRO_ID(CSLAB, 0x65, 0)
>  #define ZORRO_PROD_CSLAB_WARP_XROM				ZORRO_ID(CSLAB, 0x66, 1)
>  
> +#define ZORRO_MANUF_CSLAB                   0x1400
> +#define ZORRO_PROD_CSLAB_WARP_DDR3          ZORRO_ID(CSLAB, 0x3c, 0)
> +#define ZORRO_PROD_CSLAB_WARP_VRAM          ZORRO_ID(CSLAB, 0x64, 0)
> +#define ZORRO_PROD_CSLAB_WARP_CTRL          ZORRO_ID(CSLAB, 0x65, 0)
> +#define ZORRO_PROD_CSLAB_WARP_XROM          ZORRO_ID(CSLAB, 0x66, 1)
> +
>  #define ZORRO_MANUF_INFORMATION					0x157C
>  #define  ZORRO_PROD_INFORMATION_ISDN_ENGINE_I			ZORRO_ID(INFORMATION, 0x64, 0)
>  


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/3] include/uapi/linux/zorro_ids.h: add more cslab warp id

[Geert Uytterhoeven]

Hi Paolo,

On Fri, 21 Mar 2025 at 16:14, Paolo Pisati <p.pisati@gmail.com> wrote:
> Signed-off-by: Paolo Pisati <p.pisati@gmail.com>

Thanks for your patch!

> --- a/include/uapi/linux/zorro_ids.h
> +++ b/include/uapi/linux/zorro_ids.h
> @@ -450,7 +450,10 @@
>  #define  ZORRO_PROD_VMC_HYPERCOM_4                             ZORRO_ID(VMC, 0x02, 0)
>
>  #define ZORRO_MANUF_CSLAB                                      0x1400
> -#define  ZORRO_PROD_CSLAB_WARP_1260                            ZORRO_ID(CSLAB, 0x65, 0)

This removes a definition; was it incorrect? Please explain in the
patch description, especially as this is a uapi header file.

> +#define ZORRO_PROD_CSLAB_WARP_DDR3                             ZORRO_ID(CSLAB, 0x3c, 0)
> +#define ZORRO_PROD_CSLAB_WARP_VRAM                             ZORRO_ID(CSLAB, 0x64, 0)
> +#define ZORRO_PROD_CSLAB_WARP_CTRL                             ZORRO_ID(CSLAB, 0x65, 0)
> +#define ZORRO_PROD_CSLAB_WARP_XROM                             ZORRO_ID(CSLAB, 0x66, 1)

What do these represent? Please explain in the patch description.

>
>  #define ZORRO_MANUF_INFORMATION                                        0x157C
>  #define  ZORRO_PROD_INFORMATION_ISDN_ENGINE_I                  ZORRO_ID(INFORMATION, 0x64, 0)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] amiga: cslab ata support

[Paolo Pisati]

On Sat, Mar 22, 2025 at 09:41:44AM -0400, Damien Le Moal wrote:
> > +	ap->ioaddr.data_addr		= base;
> > +	ap->ioaddr.error_addr		= base + 1 * 4;
> > +	ap->ioaddr.feature_addr		= base + 1 * 4;
> > +	ap->ioaddr.nsect_addr		= base + 2 * 4;
> > +	ap->ioaddr.lbal_addr		= base + 3 * 4;
> > +	ap->ioaddr.lbam_addr		= base + 4 * 4;
> > +	ap->ioaddr.lbah_addr		= base + 5 * 4;
> > +	ap->ioaddr.device_addr		= base + 6 * 4;
> > +	ap->ioaddr.status_addr		= base + 7 * 4;
> > +	ap->ioaddr.command_addr		= base + 7 * 4;
> > +
> > +	ap->ioaddr.altstatus_addr	= base + (0x1000 | (6UL << 2));
> > +	ap->ioaddr.ctl_addr			= base + (0x1000 | (6UL << 2));
> 
> It would be nice to have macro definitions for all the magic numbers you use for
> offsets into base, to document these values.

Thanks for the review Damien, unfortunately the original firmware this driver is
based off doesn't define any macro for these regs/bits meaning.

Other than that, i'll fix the rest and send a v2.
-- 
bye,
p.

Re: [PATCH 2/3] amiga: cslab ata support

[Geert Uytterhoeven]

Hi Paolo,

On Fri, 21 Mar 2025 at 16:14, Paolo Pisati <p.pisati@gmail.com> wrote:
> Driver for the on-board IDE interface on CS-Lab Warp Expansion Card
> (NOTE that idemode=native has to be set in Warp Configuration)
>
> Signed-off-by: Paolo Pisati <p.pisati@gmail.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/ata/pata_cswarp.c

> +/*
> + * We cannot have a modalias for X-Surf boards, as it competes with the
> + * zorro8390 network driver. As a stopgap measure until we have proper
> + * MFD support for this board, we manually attach to it late after Zorro
> + * has enumerated its boards.

I started working on Zorro MFD a long time ago, but never finished...
https://web.git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git/log/?h=zorro-mfd

> + */
> +static int __init pata_cswarp_late_init(void)
> +{
> +       struct zorro_dev *z = NULL;
> +
> +       /* Auto-bind to regular boards */
> +       zorro_register_driver(&pata_cswarp_driver);
> +
> +       /* Manually bind to all boards */
> +       while ((z = zorro_find_device(ZORRO_PROD_CSLAB_WARP_CTRL, z))) {
> +               static struct zorro_device_id cswarp_ent = {
> +                       ZORRO_PROD_CSLAB_WARP_CTRL, 0
> +               };
> +
> +               pata_cswarp_probe(z, &cswarp_ent);

This looks very hackish to me...
I guess it blows up when trying to unbind either the zorro8390 or
cswarp driver?

> +       }
> +       return 0;
> +}
> +late_initcall(pata_cswarp_late_init);
> +
> +MODULE_AUTHOR("Andrzej Rogozynski");
> +MODULE_DESCRIPTION("low-level driver for CSWarp PATA");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> diff --git a/drivers/zorro/zorro.ids b/drivers/zorro/zorro.ids
> index 119abea8c6cb..33418af7488a 100644
> --- a/drivers/zorro/zorro.ids
> +++ b/drivers/zorro/zorro.ids
> @@ -400,6 +400,11 @@
>         0100  ISDN Blaster Z2 [ISDN Interface]
>         0200  HyperCom 4 [Multi I/O]
>         0600  HyperCom 4+ [Multi I/O]
> +1400 CSLab
> +       6000  Warp DDR3 Memory
> +       0001  Warp Video Memory
> +       0101  Warp Control Registers
> +       0201  Warp Flash ROM

This change belongs to the previous patch.
Why don't the product IDs match with the definitions below?

>  157c  Information
>         6400  ISDN Engine I [ISDN Interface]
>  2017  Vortex
> diff --git a/include/uapi/linux/zorro_ids.h b/include/uapi/linux/zorro_ids.h
> index 0be1fb0c3915..5736d2bf0295 100644
> --- a/include/uapi/linux/zorro_ids.h
> +++ b/include/uapi/linux/zorro_ids.h
> @@ -455,6 +455,12 @@
>  #define ZORRO_PROD_CSLAB_WARP_CTRL                             ZORRO_ID(CSLAB, 0x65, 0)
>  #define ZORRO_PROD_CSLAB_WARP_XROM                             ZORRO_ID(CSLAB, 0x66, 1)
>
> +#define ZORRO_MANUF_CSLAB                   0x1400
> +#define ZORRO_PROD_CSLAB_WARP_DDR3          ZORRO_ID(CSLAB, 0x3c, 0)
> +#define ZORRO_PROD_CSLAB_WARP_VRAM          ZORRO_ID(CSLAB, 0x64, 0)
> +#define ZORRO_PROD_CSLAB_WARP_CTRL          ZORRO_ID(CSLAB, 0x65, 0)
> +#define ZORRO_PROD_CSLAB_WARP_XROM          ZORRO_ID(CSLAB, 0x66, 1)
> +

This does not belong in this patch, and adds duplicates?

>  #define ZORRO_MANUF_INFORMATION                                        0x157C
>  #define  ZORRO_PROD_INFORMATION_ISDN_ENGINE_I                  ZORRO_ID(INFORMATION, 0x64, 0)



Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] amiga: cslab ata support

[Paolo Pisati]

On Mon, Mar 24, 2025 at 09:17:57AM +0100, Geert Uytterhoeven wrote:

> I started working on Zorro MFD a long time ago, but never finished...
> https://web.git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git/log/?h=zorro-mfd

Uhm, that could be useful for this multi-device board, thanks!

> > +       /* Manually bind to all boards */
> > +       while ((z = zorro_find_device(ZORRO_PROD_CSLAB_WARP_CTRL, z))) {
> > +               static struct zorro_device_id cswarp_ent = {
> > +                       ZORRO_PROD_CSLAB_WARP_CTRL, 0
> > +               };
> > +
> > +               pata_cswarp_probe(z, &cswarp_ent);
> 
> This looks very hackish to me...
> I guess it blows up when trying to unbind either the zorro8390 or
> cswarp driver?

Yes, this was clearly wrong.
 
> >  157c  Information
> >         6400  ISDN Engine I [ISDN Interface]
> >  2017  Vortex
> > diff --git a/include/uapi/linux/zorro_ids.h b/include/uapi/linux/zorro_ids.h
> > index 0be1fb0c3915..5736d2bf0295 100644
> > --- a/include/uapi/linux/zorro_ids.h
> > +++ b/include/uapi/linux/zorro_ids.h
> > @@ -455,6 +455,12 @@
> >  #define ZORRO_PROD_CSLAB_WARP_CTRL                             ZORRO_ID(CSLAB, 0x65, 0)
> >  #define ZORRO_PROD_CSLAB_WARP_XROM                             ZORRO_ID(CSLAB, 0x66, 1)
> >
> > +#define ZORRO_MANUF_CSLAB                   0x1400
> > +#define ZORRO_PROD_CSLAB_WARP_DDR3          ZORRO_ID(CSLAB, 0x3c, 0)
> > +#define ZORRO_PROD_CSLAB_WARP_VRAM          ZORRO_ID(CSLAB, 0x64, 0)
> > +#define ZORRO_PROD_CSLAB_WARP_CTRL          ZORRO_ID(CSLAB, 0x65, 0)
> > +#define ZORRO_PROD_CSLAB_WARP_XROM          ZORRO_ID(CSLAB, 0x66, 1)
> > +
> 
> This does not belong in this patch, and adds duplicates?

After submitting i noticed the duplication... anyhow, these ids changes are not
necessary for this submission and i'll remove them in v2.
-- 
bye,
p.

Re: [PATCH 2/3] amiga: cslab ata support

[Geert Uytterhoeven]

Hi Paolo,

On Mon, 24 Mar 2025 at 10:10, Paolo Pisati <p.pisati@gmail.com> wrote:
> On Mon, Mar 24, 2025 at 09:17:57AM +0100, Geert Uytterhoeven wrote:
> > I started working on Zorro MFD a long time ago, but never finished...
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git/log/?h=zorro-mfd
>
> Uhm, that could be useful for this multi-device board, thanks!

As I still keep up-to-date patches in my current tree, I have rebased
the zorro-mfd branch.

> > > +       /* Manually bind to all boards */
> > > +       while ((z = zorro_find_device(ZORRO_PROD_CSLAB_WARP_CTRL, z))) {
> > > +               static struct zorro_device_id cswarp_ent = {
> > > +                       ZORRO_PROD_CSLAB_WARP_CTRL, 0
> > > +               };
> > > +
> > > +               pata_cswarp_probe(z, &cswarp_ent);
> >
> > This looks very hackish to me...
> > I guess it blows up when trying to unbind either the zorro8390 or
> > cswarp driver?
>
> Yes, this was clearly wrong.

Oh, this was copied from drivers/ata/pata_buddha.c, which does the
same thing.  How could I have missed that? Ah, Summer holidays...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds