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