All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ata: add Amiga Gayle PATA controller driver
       [not found] <CGME20180301110252epcas2p3ea455cfb08b2f7edb4af6fbc95addc88@epcas2p3.samsung.com>
@ 2018-03-01 11:02   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-03-01 11:02 UTC (permalink / raw)
  To: Tejun Heo
  Cc: John Paul Adrian Glaubitz, Michael Schmitz, Geert Uytterhoeven,
	Philippe Ombredanne, linux-ide, linux-m68k, linux-kernel

Add Amiga Gayle PATA controller driver. It enables libata support
for the on-board IDE interfaces on some Amiga models (A600, A1200,
A4000 and A4000T) and also for IDE interfaces on the Zorro expansion
bus (M-Tech E-Matrix 530 expansion card).

Thanks to John Paul Adrian Glaubitz and Michael Schmitz for help
with testing the driver.

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Michael Schmitz <schmitzmic@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
v2:
- clarify license version (it should be GPL 2.0)
- use SPDX header

 drivers/ata/Kconfig      |   12 ++
 drivers/ata/Makefile     |    1 
 drivers/ata/pata_gayle.c |  222 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+)

Index: b/drivers/ata/Kconfig
===================================================================
--- a/drivers/ata/Kconfig	2018-03-01 11:48:06.941525737 +0100
+++ b/drivers/ata/Kconfig	2018-03-01 11:48:06.921525736 +0100
@@ -963,6 +963,18 @@ config PATA_FALCON
 
 	  If unsure, say N.
 
+config PATA_GAYLE
+	tristate "Amiga Gayle PATA support"
+	depends on M68K && AMIGA
+	help
+	  This option enables support for the on-board IDE
+	  interfaces on some Amiga models (A600, A1200,
+	  A4000 and A4000T) and also for IDE interfaces on
+	  the Zorro expansion bus (M-Tech E-Matrix 530
+	  expansion card).
+
+	  If unsure, say N.
+
 config PATA_ISAPNP
 	tristate "ISA Plug and Play PATA support"
 	depends on ISAPNP
Index: b/drivers/ata/Makefile
===================================================================
--- a/drivers/ata/Makefile	2018-03-01 11:48:06.941525737 +0100
+++ b/drivers/ata/Makefile	2018-03-01 11:48:06.937525737 +0100
@@ -98,6 +98,7 @@ obj-$(CONFIG_PATA_WINBOND)	+= pata_sl82c
 # SFF PIO only
 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_ISAPNP)	+= pata_isapnp.o
 obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_MPIIX)	+= pata_mpiix.o
Index: b/drivers/ata/pata_gayle.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/ata/pata_gayle.c	2018-03-01 11:57:12.673539480 +0100
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Amiga Gayle PATA controller driver
+ *
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Based on gayle.c:
+ *
+ *     Created 12 Jul 1997 by Geert Uytterhoeven
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_cmnd.h>
+#include <linux/ata.h>
+#include <linux/libata.h>
+#include <linux/mm.h>
+#include <linux/interrupt.h>
+#include <linux/zorro.h>
+#include <linux/platform_device.h>
+
+#include <asm/setup.h>
+#include <asm/amigahw.h>
+#include <asm/amigaints.h>
+#include <asm/amigayle.h>
+#include <asm/ide.h>
+
+#define DRV_NAME "pata_gayle"
+#define DRV_VERSION "0.1.0"
+
+#define GAYLE_CONTROL	0x101a
+
+static struct scsi_host_template pata_gayle_sht = {
+	ATA_PIO_SHT(DRV_NAME),
+};
+
+/* FIXME: is this needed? */
+static unsigned int pata_gayle_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_gayle_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 bool pata_gayle_irq_check(struct ata_port *ap)
+{
+	u8 ch;
+
+	ch = z_readb((unsigned long)ap->private_data);
+	if (!(ch & GAYLE_IRQ_IDE))
+		return false;
+	return true;
+}
+
+static void pata_gayle_irq_clear(struct ata_port *ap)
+{
+	(void)z_readb((unsigned long)ap->ioaddr.status_addr);
+	z_writeb(0x7c, (unsigned long)ap->private_data);
+}
+
+static struct ata_port_operations pata_gayle_a1200_ops = {
+	.inherits	= &ata_sff_port_ops,
+	.sff_data_xfer	= pata_gayle_data_xfer,
+	.sff_irq_check	= pata_gayle_irq_check,
+	.sff_irq_clear	= pata_gayle_irq_clear,
+	.cable_detect	= ata_cable_unknown,
+	.set_mode	= pata_gayle_set_mode,
+};
+
+static struct ata_port_operations pata_gayle_a4000_ops = {
+	.inherits	= &ata_sff_port_ops,
+	.sff_data_xfer	= pata_gayle_data_xfer,
+	.cable_detect	= ata_cable_unknown,
+	.set_mode	= pata_gayle_set_mode,
+};
+
+static int __init pata_gayle_init_one(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct gayle_ide_platform_data *pdata;
+	struct ata_host *host;
+	struct ata_port *ap;
+	void __iomem *base;
+	int ret;
+
+	pdata = dev_get_platdata(&pdev->dev);
+
+	pr_info(DRV_NAME ": Amiga Gayle IDE controller (A%u style)\n",
+		pdata->explicit_ack ? 1200 : 4000);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     resource_size(res), DRV_NAME)) {
+		pr_err(DRV_NAME ": resources busy\n");
+		return -EBUSY;
+	}
+
+	/* allocate host */
+	host = ata_host_alloc(&pdev->dev, 1);
+	if (!host)
+		return -ENOMEM;
+
+	ap = host->ports[0];
+
+	if (pdata->explicit_ack)
+		ap->ops = &pata_gayle_a1200_ops;
+	else
+		ap->ops = &pata_gayle_a4000_ops;
+
+	ap->pio_mask = ATA_PIO4;
+	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY;
+
+	base = ZTWO_VADDR(pdata->base);
+	ap->ioaddr.data_addr		= base;
+	ap->ioaddr.error_addr		= base + 2 + 1 * 4;
+	ap->ioaddr.feature_addr		= base + 2 + 1 * 4;
+	ap->ioaddr.nsect_addr		= base + 2 + 2 * 4;
+	ap->ioaddr.lbal_addr		= base + 2 + 3 * 4;
+	ap->ioaddr.lbam_addr		= base + 2 + 4 * 4;
+	ap->ioaddr.lbah_addr		= base + 2 + 5 * 4;
+	ap->ioaddr.device_addr		= base + 2 + 6 * 4;
+	ap->ioaddr.status_addr		= base + 2 + 7 * 4;
+	ap->ioaddr.command_addr		= base + 2 + 7 * 4;
+
+	ap->ioaddr.altstatus_addr	= base + GAYLE_CONTROL;
+	ap->ioaddr.ctl_addr		= base + GAYLE_CONTROL;
+
+	ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
+
+	ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
+		      (unsigned long)base + GAYLE_CONTROL);
+
+	/* activate */
+	ret = ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
+				IRQF_SHARED, &pata_gayle_sht);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, host);
+
+	return 0;
+}
+
+static int __exit pata_gayle_remove_one(struct platform_device *pdev)
+{
+	struct ata_host *host = platform_get_drvdata(pdev);
+
+	ata_host_detach(host);
+
+	return 0;
+}
+
+static struct platform_driver pata_gayle_driver = {
+	.remove = __exit_p(pata_gayle_remove_one),
+	.driver   = {
+		.name	= "amiga-gayle-ide",
+	},
+};
+
+module_platform_driver_probe(pata_gayle_driver, pata_gayle_init_one);
+
+MODULE_AUTHOR("Bartlomiej Zolnierkiewicz");
+MODULE_DESCRIPTION("low-level driver for Amiga Gayle PATA");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:amiga-gayle-ide");
+MODULE_VERSION(DRV_VERSION);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] ata: add Amiga Gayle PATA controller driver
@ 2018-03-01 11:02   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-03-01 11:02 UTC (permalink / raw)
  To: Tejun Heo
  Cc: John Paul Adrian Glaubitz, Michael Schmitz, Geert Uytterhoeven,
	Philippe Ombredanne, linux-ide, linux-m68k, linux-kernel

Add Amiga Gayle PATA controller driver. It enables libata support
for the on-board IDE interfaces on some Amiga models (A600, A1200,
A4000 and A4000T) and also for IDE interfaces on the Zorro expansion
bus (M-Tech E-Matrix 530 expansion card).

Thanks to John Paul Adrian Glaubitz and Michael Schmitz for help
with testing the driver.

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Michael Schmitz <schmitzmic@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
v2:
- clarify license version (it should be GPL 2.0)
- use SPDX header

 drivers/ata/Kconfig      |   12 ++
 drivers/ata/Makefile     |    1 
 drivers/ata/pata_gayle.c |  222 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+)

Index: b/drivers/ata/Kconfig
===================================================================
--- a/drivers/ata/Kconfig	2018-03-01 11:48:06.941525737 +0100
+++ b/drivers/ata/Kconfig	2018-03-01 11:48:06.921525736 +0100
@@ -963,6 +963,18 @@ config PATA_FALCON
 
 	  If unsure, say N.
 
+config PATA_GAYLE
+	tristate "Amiga Gayle PATA support"
+	depends on M68K && AMIGA
+	help
+	  This option enables support for the on-board IDE
+	  interfaces on some Amiga models (A600, A1200,
+	  A4000 and A4000T) and also for IDE interfaces on
+	  the Zorro expansion bus (M-Tech E-Matrix 530
+	  expansion card).
+
+	  If unsure, say N.
+
 config PATA_ISAPNP
 	tristate "ISA Plug and Play PATA support"
 	depends on ISAPNP
Index: b/drivers/ata/Makefile
===================================================================
--- a/drivers/ata/Makefile	2018-03-01 11:48:06.941525737 +0100
+++ b/drivers/ata/Makefile	2018-03-01 11:48:06.937525737 +0100
@@ -98,6 +98,7 @@ obj-$(CONFIG_PATA_WINBOND)	+= pata_sl82c
 # SFF PIO only
 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_ISAPNP)	+= pata_isapnp.o
 obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_MPIIX)	+= pata_mpiix.o
Index: b/drivers/ata/pata_gayle.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/ata/pata_gayle.c	2018-03-01 11:57:12.673539480 +0100
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Amiga Gayle PATA controller driver
+ *
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Based on gayle.c:
+ *
+ *     Created 12 Jul 1997 by Geert Uytterhoeven
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_cmnd.h>
+#include <linux/ata.h>
+#include <linux/libata.h>
+#include <linux/mm.h>
+#include <linux/interrupt.h>
+#include <linux/zorro.h>
+#include <linux/platform_device.h>
+
+#include <asm/setup.h>
+#include <asm/amigahw.h>
+#include <asm/amigaints.h>
+#include <asm/amigayle.h>
+#include <asm/ide.h>
+
+#define DRV_NAME "pata_gayle"
+#define DRV_VERSION "0.1.0"
+
+#define GAYLE_CONTROL	0x101a
+
+static struct scsi_host_template pata_gayle_sht = {
+	ATA_PIO_SHT(DRV_NAME),
+};
+
+/* FIXME: is this needed? */
+static unsigned int pata_gayle_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_gayle_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 bool pata_gayle_irq_check(struct ata_port *ap)
+{
+	u8 ch;
+
+	ch = z_readb((unsigned long)ap->private_data);
+	if (!(ch & GAYLE_IRQ_IDE))
+		return false;
+	return true;
+}
+
+static void pata_gayle_irq_clear(struct ata_port *ap)
+{
+	(void)z_readb((unsigned long)ap->ioaddr.status_addr);
+	z_writeb(0x7c, (unsigned long)ap->private_data);
+}
+
+static struct ata_port_operations pata_gayle_a1200_ops = {
+	.inherits	= &ata_sff_port_ops,
+	.sff_data_xfer	= pata_gayle_data_xfer,
+	.sff_irq_check	= pata_gayle_irq_check,
+	.sff_irq_clear	= pata_gayle_irq_clear,
+	.cable_detect	= ata_cable_unknown,
+	.set_mode	= pata_gayle_set_mode,
+};
+
+static struct ata_port_operations pata_gayle_a4000_ops = {
+	.inherits	= &ata_sff_port_ops,
+	.sff_data_xfer	= pata_gayle_data_xfer,
+	.cable_detect	= ata_cable_unknown,
+	.set_mode	= pata_gayle_set_mode,
+};
+
+static int __init pata_gayle_init_one(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct gayle_ide_platform_data *pdata;
+	struct ata_host *host;
+	struct ata_port *ap;
+	void __iomem *base;
+	int ret;
+
+	pdata = dev_get_platdata(&pdev->dev);
+
+	pr_info(DRV_NAME ": Amiga Gayle IDE controller (A%u style)\n",
+		pdata->explicit_ack ? 1200 : 4000);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     resource_size(res), DRV_NAME)) {
+		pr_err(DRV_NAME ": resources busy\n");
+		return -EBUSY;
+	}
+
+	/* allocate host */
+	host = ata_host_alloc(&pdev->dev, 1);
+	if (!host)
+		return -ENOMEM;
+
+	ap = host->ports[0];
+
+	if (pdata->explicit_ack)
+		ap->ops = &pata_gayle_a1200_ops;
+	else
+		ap->ops = &pata_gayle_a4000_ops;
+
+	ap->pio_mask = ATA_PIO4;
+	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY;
+
+	base = ZTWO_VADDR(pdata->base);
+	ap->ioaddr.data_addr		= base;
+	ap->ioaddr.error_addr		= base + 2 + 1 * 4;
+	ap->ioaddr.feature_addr		= base + 2 + 1 * 4;
+	ap->ioaddr.nsect_addr		= base + 2 + 2 * 4;
+	ap->ioaddr.lbal_addr		= base + 2 + 3 * 4;
+	ap->ioaddr.lbam_addr		= base + 2 + 4 * 4;
+	ap->ioaddr.lbah_addr		= base + 2 + 5 * 4;
+	ap->ioaddr.device_addr		= base + 2 + 6 * 4;
+	ap->ioaddr.status_addr		= base + 2 + 7 * 4;
+	ap->ioaddr.command_addr		= base + 2 + 7 * 4;
+
+	ap->ioaddr.altstatus_addr	= base + GAYLE_CONTROL;
+	ap->ioaddr.ctl_addr		= base + GAYLE_CONTROL;
+
+	ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
+
+	ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
+		      (unsigned long)base + GAYLE_CONTROL);
+
+	/* activate */
+	ret = ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
+				IRQF_SHARED, &pata_gayle_sht);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, host);
+
+	return 0;
+}
+
+static int __exit pata_gayle_remove_one(struct platform_device *pdev)
+{
+	struct ata_host *host = platform_get_drvdata(pdev);
+
+	ata_host_detach(host);
+
+	return 0;
+}
+
+static struct platform_driver pata_gayle_driver = {
+	.remove = __exit_p(pata_gayle_remove_one),
+	.driver   = {
+		.name	= "amiga-gayle-ide",
+	},
+};
+
+module_platform_driver_probe(pata_gayle_driver, pata_gayle_init_one);
+
+MODULE_AUTHOR("Bartlomiej Zolnierkiewicz");
+MODULE_DESCRIPTION("low-level driver for Amiga Gayle PATA");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:amiga-gayle-ide");
+MODULE_VERSION(DRV_VERSION);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: add Amiga Gayle PATA controller driver
  2018-03-01 11:02   ` Bartlomiej Zolnierkiewicz
@ 2018-03-01 12:32     ` Andy Shevchenko
  -1 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-03-01 12:32 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Tejun Heo, John Paul Adrian Glaubitz, Michael Schmitz,
	Geert Uytterhoeven, Philippe Ombredanne, linux-ide, linux-m68k,
	Linux Kernel Mailing List

On Thu, Mar 1, 2018 at 1:02 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> Add Amiga Gayle PATA controller driver. It enables libata support
> for the on-board IDE interfaces on some Amiga models (A600, A1200,
> A4000 and A4000T) and also for IDE interfaces on the Zorro expansion
> bus (M-Tech E-Matrix 530 expansion card).
>
> Thanks to John Paul Adrian Glaubitz and Michael Schmitz for help
> with testing the driver.




> +#include <linux/kernel.h>

> +#include <linux/module.h>
> +#include <linux/init.h>

Choose one of those two.

> +#include <linux/blkdev.h>
> +#include <linux/delay.h>
> +#include <scsi/scsi_host.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <linux/ata.h>
> +#include <linux/libata.h>
> +#include <linux/mm.h>
> +#include <linux/interrupt.h>
> +#include <linux/zorro.h>
> +#include <linux/platform_device.h>

Better to keep in order.

> +#include <asm/setup.h>
> +#include <asm/amigahw.h>
> +#include <asm/amigaints.h>
> +#include <asm/amigayle.h>
> +#include <asm/ide.h>

Ditto.

> +static bool pata_gayle_irq_check(struct ata_port *ap)
> +{
> +       u8 ch;
> +
> +       ch = z_readb((unsigned long)ap->private_data);

> +       if (!(ch & GAYLE_IRQ_IDE))
> +               return false;
> +       return true;

return !!(ch & GAYLE_IRQ_IDE);

?

> +}

> +static int __init pata_gayle_init_one(struct platform_device *pdev)
> +{

> +       pr_info(DRV_NAME ": Amiga Gayle IDE controller (A%u style)\n",
> +               pdata->explicit_ack ? 1200 : 4000);

What's wrong with dev_info() ?

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> +       if (!res)
> +               return -ENODEV;

> +
> +       if (!devm_request_mem_region(&pdev->dev, res->start,
> +                                    resource_size(res), DRV_NAME)) {
> +               pr_err(DRV_NAME ": resources busy\n");
> +               return -EBUSY;
> +       }
> +

> +       base = ZTWO_VADDR(pdata->base);

Hmm... Can't you use devm_ioremap_resources() to get the virtual
address for I/O ?

> +       ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
> +
> +       ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +                     (unsigned long)base + GAYLE_CONTROL);

When you use explicit casting in printf() you are doing in 99.9% cases
something wrong.

> +       /* activate */

Noise.

> +       ret = ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
> +                               IRQF_SHARED, &pata_gayle_sht);
> +       if (ret)
> +               return ret;

> +}

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: add Amiga Gayle PATA controller driver
@ 2018-03-01 12:32     ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-03-01 12:32 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Tejun Heo, John Paul Adrian Glaubitz, Michael Schmitz,
	Geert Uytterhoeven, Philippe Ombredanne, linux-ide, linux-m68k,
	Linux Kernel Mailing List

On Thu, Mar 1, 2018 at 1:02 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> Add Amiga Gayle PATA controller driver. It enables libata support
> for the on-board IDE interfaces on some Amiga models (A600, A1200,
> A4000 and A4000T) and also for IDE interfaces on the Zorro expansion
> bus (M-Tech E-Matrix 530 expansion card).
>
> Thanks to John Paul Adrian Glaubitz and Michael Schmitz for help
> with testing the driver.




> +#include <linux/kernel.h>

> +#include <linux/module.h>
> +#include <linux/init.h>

Choose one of those two.

> +#include <linux/blkdev.h>
> +#include <linux/delay.h>
> +#include <scsi/scsi_host.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <linux/ata.h>
> +#include <linux/libata.h>
> +#include <linux/mm.h>
> +#include <linux/interrupt.h>
> +#include <linux/zorro.h>
> +#include <linux/platform_device.h>

Better to keep in order.

> +#include <asm/setup.h>
> +#include <asm/amigahw.h>
> +#include <asm/amigaints.h>
> +#include <asm/amigayle.h>
> +#include <asm/ide.h>

Ditto.

> +static bool pata_gayle_irq_check(struct ata_port *ap)
> +{
> +       u8 ch;
> +
> +       ch = z_readb((unsigned long)ap->private_data);

> +       if (!(ch & GAYLE_IRQ_IDE))
> +               return false;
> +       return true;

return !!(ch & GAYLE_IRQ_IDE);

?

> +}

> +static int __init pata_gayle_init_one(struct platform_device *pdev)
> +{

> +       pr_info(DRV_NAME ": Amiga Gayle IDE controller (A%u style)\n",
> +               pdata->explicit_ack ? 1200 : 4000);

What's wrong with dev_info() ?

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> +       if (!res)
> +               return -ENODEV;

> +
> +       if (!devm_request_mem_region(&pdev->dev, res->start,
> +                                    resource_size(res), DRV_NAME)) {
> +               pr_err(DRV_NAME ": resources busy\n");
> +               return -EBUSY;
> +       }
> +

> +       base = ZTWO_VADDR(pdata->base);

Hmm... Can't you use devm_ioremap_resources() to get the virtual
address for I/O ?

> +       ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
> +
> +       ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> +                     (unsigned long)base + GAYLE_CONTROL);

When you use explicit casting in printf() you are doing in 99.9% cases
something wrong.

> +       /* activate */

Noise.

> +       ret = ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
> +                               IRQF_SHARED, &pata_gayle_sht);
> +       if (ret)
> +               return ret;

> +}

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: add Amiga Gayle PATA controller driver
  2018-03-01 12:32     ` Andy Shevchenko
@ 2018-03-01 12:42       ` Geert Uytterhoeven
  -1 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-03-01 12:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, John Paul Adrian Glaubitz,
	Michael Schmitz, Philippe Ombredanne, linux-ide, linux-m68k,
	Linux Kernel Mailing List

Hi Andy,

On Thu, Mar 1, 2018 at 1:32 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Mar 1, 2018 at 1:02 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>> +       base = ZTWO_VADDR(pdata->base);
>
> Hmm... Can't you use devm_ioremap_resources() to get the virtual
> address for I/O ?

Zorro II address space is always mapped, and address conversion done
through ZTWO_VADDR()/ZTWO_PADDR().

>> +       ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
>> +
>> +       ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
>> +                     (unsigned long)base + GAYLE_CONTROL);
>
> When you use explicit casting in printf() you are doing in 99.9% cases
> something wrong.

Indeed. This should print the physical addresses, using pdata->base instead
of base.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ata: add Amiga Gayle PATA controller driver
@ 2018-03-01 12:42       ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-03-01 12:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartlomiej Zolnierkiewicz, Tejun Heo, John Paul Adrian Glaubitz,
	Michael Schmitz, Philippe Ombredanne, linux-ide, linux-m68k,
	Linux Kernel Mailing List

Hi Andy,

On Thu, Mar 1, 2018 at 1:32 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Mar 1, 2018 at 1:02 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>> +       base = ZTWO_VADDR(pdata->base);
>
> Hmm... Can't you use devm_ioremap_resources() to get the virtual
> address for I/O ?

Zorro II address space is always mapped, and address conversion done
through ZTWO_VADDR()/ZTWO_PADDR().

>> +       ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
>> +
>> +       ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
>> +                     (unsigned long)base + GAYLE_CONTROL);
>
> When you use explicit casting in printf() you are doing in 99.9% cases
> something wrong.

Indeed. This should print the physical addresses, using pdata->base instead
of base.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-01 12:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20180301110252epcas2p3ea455cfb08b2f7edb4af6fbc95addc88@epcas2p3.samsung.com>
2018-03-01 11:02 ` [PATCH v2] ata: add Amiga Gayle PATA controller driver Bartlomiej Zolnierkiewicz
2018-03-01 11:02   ` Bartlomiej Zolnierkiewicz
2018-03-01 12:32   ` Andy Shevchenko
2018-03-01 12:32     ` Andy Shevchenko
2018-03-01 12:42     ` Geert Uytterhoeven
2018-03-01 12:42       ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.