[PATCH v5 0/2] m68k: pata_cswarp: Add Amiga cslab ata support

[PATCH v5 0/2] m68k: pata_cswarp: Add Amiga cslab ata support

[Paolo Pisati]

Here is a driver (and the subsequent defconfig change) for the pata/CF socket in
the CS-Lab Warp series of Amiga boards.

This is a respin of v4 addressing Geert's review - apologies for the long delay.

Tested on real Warp hardware.

v4: https://lore.kernel.org/linux-ide/20250325155613.352680-1-p.pisati@gmail.com/

Changes in v5:
 - return the number of bytes actually consumed on the bus
 - fix the trailing byte in pata_cswarp_data_xfer()
 - drop a blank line in Kconfig
 - fix the remaining checkpatch complaints

Changes in v4:
 - refactor pata_cswarp_data_xfer()
 - ioremap WARP_OFFSET_ATA region

Changes in v3:
 - suppress a comment
 - properly return ata_host_activate() error code

Change in v2:
 - fix style, remove duplicate swap macro, driver version, etc
 - rework the zorro attach mechanism
 - remove the unnecessary zorro ids refactor

Paolo Pisati (2):
  ata: pata_cswarp: Add Amiga cslab ata support
  m68k: defconfig: enable PATA_CSWARP

 arch/m68k/configs/amiga_defconfig |   1 +
 arch/m68k/configs/multi_defconfig |   1 +
 drivers/ata/Kconfig               |  10 ++
 drivers/ata/Makefile              |   1 +
 drivers/ata/pata_cswarp.c         | 180 ++++++++++++++++++++++++++++++
 5 files changed, 193 insertions(+)
 create mode 100644 drivers/ata/pata_cswarp.c


base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.43.0

[PATCH v5 1/2] ata: pata_cswarp: Add 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)

Assisted-by: Claude_Code:claude-opus-5
Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
 drivers/ata/Kconfig       |  10 +++
 drivers/ata/Makefile      |   1 +
 drivers/ata/pata_cswarp.c | 180 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)
 create mode 100644 drivers/ata/pata_cswarp.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 28ca856ecc75..86af89a9bfce 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -1040,6 +1040,16 @@ config PATA_GAYLE
 
 	  If unsure, say N.
 
+config PATA_CSWARP
+	tristate "Amiga CS Warp PATA support"
+	depends on M68K && AMIGA && ZORRO
+	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 b96025abd45e..647e3d4b6f75 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -100,6 +100,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..55f000f42738
--- /dev/null
+++ b/drivers/ata/pata_cswarp.c
@@ -0,0 +1,180 @@
+// 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 WARP_OFFSET_ATA         0x6000
+
+static const struct scsi_host_template pata_cswarp_sht = {
+	ATA_PIO_SHT(DRV_NAME),
+};
+
+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;
+	u16 *buf16 = (u16 *)buf;
+
+	/* Transfer multiple of 2 bytes */
+	if (rw == READ)
+		raw_insw(data_addr, buf16, words);
+	else
+		raw_outsw(data_addr, buf16, words);
+
+	/* Transfer trailing byte, if any. */
+	if (unlikely(buflen & 0x01)) {
+		if (rw == READ)
+			buf[buflen - 1] = raw_inw(data_addr) >> 8;
+		else
+			raw_outw(buf[buflen - 1] << 8, data_addr);
+		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;
+	struct ata_port *ap;
+	void __iomem *base;
+	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;
+
+	host = ata_host_alloc(&z->dev, 1);
+	if (!host)
+		return -ENXIO;
+
+	ap = host->ports[0];
+	base = ioremap(board + WARP_OFFSET_ATA, 0x1800);
+
+	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));
+
+	ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
+		      (unsigned long)ap->ioaddr.ctl_addr);
+
+	return ata_host_activate(host, 0, NULL,
+			  IRQF_SHARED, &pata_cswarp_sht);
+}
+
+static void pata_cswarp_remove(struct zorro_dev *z)
+{
+	struct ata_host *host = dev_get_drvdata(&z->dev);
+
+	ata_host_detach(host);
+	iounmap(host->ports[0]->ioaddr.data_addr);
+}
+
+static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
+	{ ZORRO_PROD_CSLAB_WARP_1260, 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,
+};
+
+static int __init pata_cswarp_init(void)
+{
+	return zorro_register_driver(&pata_cswarp_driver);
+}
+
+static void __exit pata_cswarp_unregister(void)
+{
+	zorro_unregister_driver(&pata_cswarp_driver);
+}
+
+module_init(pata_cswarp_init);
+module_exit(pata_cswarp_unregister);
+
+MODULE_AUTHOR("Andrzej Rogozynski");
+MODULE_DESCRIPTION("low-level driver for CSWarp PATA");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH v5 2/2] m68k: defconfig: enable PATA_CSWARP

[Paolo Pisati]

Enable the new CS-Lab Warp PATA driver in the Amiga and multi defconfigs.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Assisted-by: Claude_Code:claude-opus-5
Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
 arch/m68k/configs/amiga_defconfig | 1 +
 arch/m68k/configs/multi_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
index 5ed08e0fdf88..b769be5a0cbb 100644
--- a/arch/m68k/configs/amiga_defconfig
+++ b/arch/m68k/configs/amiga_defconfig
@@ -284,6 +284,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_MD_LINEAR=m
diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig
index 615e2dcad3d3..078d71e9b7cb 100644
--- a/arch/m68k/configs/multi_defconfig
+++ b/arch/m68k/configs/multi_defconfig
@@ -310,6 +310,7 @@ CONFIG_ATA=y
 # CONFIG_ATA_BMDMA is not set
 CONFIG_PATA_FALCON=y
 CONFIG_PATA_GAYLE=y
+CONFIG_PATA_CSWARP=y
 CONFIG_PATA_BUDDHA=y
 CONFIG_PATA_PLATFORM=y
 CONFIG_MD=y
-- 
2.43.0

Re: [PATCH v5 2/2] m68k: defconfig: enable PATA_CSWARP

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing check for ioremap() failure in pata_cswarp_probe().
- [Medium] Missing iounmap() on the ata_host_activate() error path.
--

--- Patch [2]: [PATCH v5 2/2] m68k: defconfig: enable PATA_CSWARP ---
commit 50c3007b9ddccb0c0a488126c545810ad538a593
Author: Paolo Pisati <p.pisati@gmail.com>

ata: pata_cswarp: Add Amiga cslab ata support

This commit introduces the pata_cswarp driver to provide support for
the on-board IDE interface on the CS-Lab Warp Expansion Card.

> diff --git a/drivers/ata/pata_cswarp.c b/drivers/ata/pata_cswarp.c
> new file mode 100644
> index 0000000000000..55f000f427384
> --- /dev/null
> +++ b/drivers/ata/pata_cswarp.c
> @@ -0,0 +1,180 @@

[ ... ]

> +static int pata_cswarp_probe(struct zorro_dev *z,
> +			     const struct zorro_device_id *ent)
> +{

[ ... ]

> +	host = ata_host_alloc(&z->dev, 1);
> +	if (!host)
> +		return -ENXIO;
> +
> +	ap = host->ports[0];
> +	base = ioremap(board + WARP_OFFSET_ATA, 0x1800);
> +
> +	ap->ops = &pata_cswarp_ops;

[Severity: High]
Is it possible for ioremap() to fail and return a NULL pointer here?

If memory mapping fails during device probe, base would be NULL.

This NULL pointer is then used to calculate and assign various ATA port I/O
addresses (e.g., ap->ioaddr.error_addr = base + 1 * 4).

When the block layer or ATA framework later attempts to access these I/O
ports, will this result in a kernel panic due to an invalid pointer
dereference?

> +
> +	ap->pio_mask = ATA_PIO4;

[ ... ]

> +	ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +		      (unsigned long)ap->ioaddr.ctl_addr);
> +
> +	return ata_host_activate(host, 0, NULL,
> +			  IRQF_SHARED, &pata_cswarp_sht);
> +}

[Severity: Medium]
Does this error path leak the I/O memory mapping?

If ata_host_activate() fails, it returns a non-zero error code which is
immediately returned by pata_cswarp_probe().

Since this driver uses manual memory mapping rather than devm_ioremap()
(as seen by the manual iounmap() call in pata_cswarp_remove()), should
iounmap(base) be called before returning the error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824153339.512487-1-p.pisati@gmail.com?part=2

Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for ioremap() return value.
- [Medium] Resource leak of I/O mapping on probe error path.
--

--- Patch [1]: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support ---
Note: The format of this report is altered due to recitation restrictions. I
will not quote the original patch code directly, but will instead provide a
free-form summary of the findings.

commit 50c3007b9ddccb0c0a488126c545810ad538a593
Author: Paolo Pisati <p.pisati@gmail.com>

ata: pata_cswarp: Add Amiga cslab ata support

This commit adds support for the on-board IDE interface on the CS-Lab Warp
Expansion Card. It introduces the pata_cswarp driver handling initialization,
data transfer, and configuration for PIO mode.

[Severity: High]
In drivers/ata/pata_cswarp.c:pata_cswarp_probe(), the code calls ioremap()
but does not check the return value before using it.

Could ioremap() return NULL under memory pressure or if the vmalloc area is
exhausted? If so, using the returned base pointer without validation to 
calculate static offsets (such as base + 1 * 4) and assigning them to the 
ATA port's IO address structure might lead to an immediate kernel oops or 
exception vector table corruption on m68k systems. Should there be a NULL
check immediately after the ioremap() call?

[Severity: Medium]
In drivers/ata/pata_cswarp.c:pata_cswarp_probe(), there is a potential
resource leak on the error path for ata_host_activate().

If the call to ata_host_activate() fails and returns an error code, the
function exits directly by returning that error. Because the base pointer was 
mapped manually with ioremap() rather than a managed API like devm_ioremap(), 
does this leave the I/O mapping permanently leaked? There appears to be no 
explicit iounmap(base) in this specific error path to clean up the mapping.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824153339.512487-1-p.pisati@gmail.com?part=1

Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Geert Uytterhoeven]

Hi Paolo,

On Mon, 24 Aug 2026 at 17:33, 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)
>
> Assisted-by: Claude_Code:claude-opus-5
> Signed-off-by: Paolo Pisati <p.pisati@gmail.com>

Thanks for the update!

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

> +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;
> +       u16 *buf16 = (u16 *)buf;
> +
> +       /* Transfer multiple of 2 bytes */
> +       if (rw == READ)
> +               raw_insw(data_addr, buf16, words);
> +       else
> +               raw_outsw(data_addr, buf16, words);
> +
> +       /* Transfer trailing byte, if any. */
> +       if (unlikely(buflen & 0x01)) {
> +               if (rw == READ)
> +                       buf[buflen - 1] = raw_inw(data_addr) >> 8;
> +               else
> +                       raw_outw(buf[buflen - 1] << 8, data_addr);
> +               words++;
> +       }
> +
> +       return words << 1;

This may be one less than the actual number of bytes
Why not buflen?

> +}

> +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;
> +       struct ata_port *ap;
> +       void __iomem *base;
> +       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;
> +
> +       host = ata_host_alloc(&z->dev, 1);
> +       if (!host)
> +               return -ENXIO;
> +
> +       ap = host->ports[0];
> +       base = ioremap(board + WARP_OFFSET_ATA, 0x1800);
> +
> +       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));
> +
> +       ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +                     (unsigned long)ap->ioaddr.ctl_addr);

Both printed addresses are virtual addresses hence not really useful.
If you want to print something, please print board or z->resource
instead.

> +static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
> +       { ZORRO_PROD_CSLAB_WARP_1260, 0},
> +       { 0 }

Please use named initializers, and drop unneeded zeroes, like Uwe
just did in all existing Zorro drivers:

    { .id = ZORRO_PROD_CSLAB_WARP_1260 },
    { }

> +};

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 v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Geert Uytterhoeven]

Hi Paolo,

On Tue, 25 Aug 2026 at 09:53, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, 24 Aug 2026 at 17:33, 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)
> >
> > Assisted-by: Claude_Code:claude-opus-5
> > Signed-off-by: Paolo Pisati <p.pisati@gmail.com>

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

> > +static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
> > +       { ZORRO_PROD_CSLAB_WARP_1260, 0},

FTR, this doesn't build anymore.
You also should include <linux/device-id/zorro.h>.

> > +       { 0 }
>
> Please use named initializers, and drop unneeded zeroes, like Uwe
> just did in all existing Zorro drivers:
>
>     { .id = ZORRO_PROD_CSLAB_WARP_1260 },
>     { }

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 v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Paolo Pisati]

On Tue, Aug 25, 2026 at 09:53:18AM +0200, Geert Uytterhoeven wrote:
> > +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;
> > +       u16 *buf16 = (u16 *)buf;
> > +
> > +       /* Transfer multiple of 2 bytes */
> > +       if (rw == READ)
> > +               raw_insw(data_addr, buf16, words);
> > +       else
> > +               raw_outsw(data_addr, buf16, words);
> > +
> > +       /* Transfer trailing byte, if any. */
> > +       if (unlikely(buflen & 0x01)) {
> > +               if (rw == READ)
> > +                       buf[buflen - 1] = raw_inw(data_addr) >> 8;
> > +               else
> > +                       raw_outw(buf[buflen - 1] << 8, data_addr);
> > +               words++;
> > +       }
> > +
> > +       return words << 1;
> 
> This may be one less than the actual number of bytes
> Why not buflen?

Cause every other driver (e.g. buddha, falcon, gayle, etc) is actually returning
words << 1: i think we are returning the "whole number of bytes" read
from the bus/ATA register here, or at least that's how i understood it.

Anyhow, thanks for the review, i'll send a V6 addressing all comments.
-- 
bye,
p.

Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Michael Schmitz]

Hi Geert,

On 25/08/26 19:53, Geert Uytterhoeven wrote:
>> +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;
>> +       u16 *buf16 = (u16 *)buf;
>> +
>> +       /* Transfer multiple of 2 bytes */
>> +       if (rw == READ)
>> +               raw_insw(data_addr, buf16, words);
>> +       else
>> +               raw_outsw(data_addr, buf16, words);
>> +
>> +       /* Transfer trailing byte, if any. */
>> +       if (unlikely(buflen & 0x01)) {
>> +               if (rw == READ)
>> +                       buf[buflen - 1] = raw_inw(data_addr) >> 8;
>> +               else
>> +                       raw_outw(buf[buflen - 1] << 8, data_addr);
>> +               words++;
>> +       }
>> +
>> +       return words << 1;
> This may be one less than the actual number of bytes
> Why not buflen?
>
words = buflen >> 1;

followed by

if (buflen & 0x01) words++;

makes 'words' the correct (i.e. rounded upwards if buflen was odd) 
number of words transferred.

The return value is then either correct, or one larger than the actual 
number of bytes?

I believe the template for these functions was 
drivers/ata/libata-sff.c:ata_sff_data_xfer() which follows the exact 
same logic.

Cheers,

MIchael

>> +}
>> +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;
>> +       struct ata_port *ap;
>> +       void __iomem *base;
>> +       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;
>> +
>> +       host = ata_host_alloc(&z->dev, 1);
>> +       if (!host)
>> +               return -ENXIO;
>> +
>> +       ap = host->ports[0];
>> +       base = ioremap(board + WARP_OFFSET_ATA, 0x1800);
>> +
>> +       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));
>> +
>> +       ata_port_desc(ap, "  cmd 0x%lx ctl 0x%lx", (unsigned long)base,
>> +                     (unsigned long)ap->ioaddr.ctl_addr);
> Both printed addresses are virtual addresses hence not really useful.
> If you want to print something, please print board or z->resource
> instead.
>
>> +static const struct zorro_device_id pata_cswarp_zorro_tbl[] = {
>> +       { ZORRO_PROD_CSLAB_WARP_1260, 0},
>> +       { 0 }
> Please use named initializers, and drop unneeded zeroes, like Uwe
> just did in all existing Zorro drivers:
>
>      { .id = ZORRO_PROD_CSLAB_WARP_1260 },
>      { }
>
>> +};
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support

[Geert Uytterhoeven]

Hi Michael,

On Wed, 26 Aug 2026 at 21:31, Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 25/08/26 19:53, Geert Uytterhoeven wrote:
> >> +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;
> >> +       u16 *buf16 = (u16 *)buf;
> >> +
> >> +       /* Transfer multiple of 2 bytes */
> >> +       if (rw == READ)
> >> +               raw_insw(data_addr, buf16, words);
> >> +       else
> >> +               raw_outsw(data_addr, buf16, words);
> >> +
> >> +       /* Transfer trailing byte, if any. */
> >> +       if (unlikely(buflen & 0x01)) {
> >> +               if (rw == READ)
> >> +                       buf[buflen - 1] = raw_inw(data_addr) >> 8;
> >> +               else
> >> +                       raw_outw(buf[buflen - 1] << 8, data_addr);
> >> +               words++;
> >> +       }
> >> +
> >> +       return words << 1;
> > This may be one less than the actual number of bytes
> > Why not buflen?
> >
> words = buflen >> 1;
>
> followed by
>
> if (buflen & 0x01) words++;
>
> makes 'words' the correct (i.e. rounded upwards if buflen was odd)
> number of words transferred.

Thanks, I had completely missed that increment.

> The return value is then either correct, or one larger than the actual
> number of bytes?
>
> I believe the template for these functions was
> drivers/ata/libata-sff.c:ata_sff_data_xfer() which follows the exact
> same logic.

Then Life's Good ;-)

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