Thread (10 messages) 10 messages, 4 authors, 2021-12-18

Re: [PATCH 2/2] ata: pata_platform: Merge pata_of_platform into pata_platform

flat view

From: Rob Herring <robh+dt@kernel.org>
Date: 2021-12-17 14:50:22
Also in: lkml

On Fri, Dec 17, 2021 at 8:17 AM Lad Prabhakar
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
Merge the OF pata_of_platform driver into pata_platform.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/ata/Kconfig            |   3 +-
 drivers/ata/Makefile           |   1 -
 drivers/ata/pata_of_platform.c |  90 ---------------
 drivers/ata/pata_platform.c    | 199 +++++++++++++++++++++++++--------
 include/linux/ata_platform.h   |   9 --
 5 files changed, 153 insertions(+), 149 deletions(-)
 delete mode 100644 drivers/ata/pata_of_platform.c
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index a7da8ea7b3ed..0fab5cae45d5 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -1122,7 +1122,8 @@ config PATA_PLATFORM

 config PATA_OF_PLATFORM
        tristate "OpenFirmware platform device PATA support"
-       depends on PATA_PLATFORM && OF
+       depends on OF
+       select PATA_PLATFORM
Can't we just kill the kconfig option?
        help
          This option enables support for generic directly connected ATA
          devices commonly found on embedded systems with OpenFirmware
quoted hunk ↗ jump to hunk
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index cb3134bf88eb..b8d8d51bc562 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -11,21 +11,42 @@
  * License.  See the file "COPYING" in the main directory of this archive
  * for more details.
  */
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/blkdev.h>
-#include <scsi/scsi_host.h>
 #include <linux/ata.h>
+#include <linux/ata_platform.h>
+#include <linux/blkdev.h>
+#include <linux/kernel.h>
 #include <linux/libata.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
-#include <linux/ata_platform.h>
+#include <scsi/scsi_host.h>

 #define DRV_NAME "pata_platform"
 #define DRV_VERSION "1.2"

 static int pio_mask = 1;
 module_param(pio_mask, int, 0);
-MODULE_PARM_DESC(pio_mask, "PIO modes supported, mode 0 only by default");
+MODULE_PARM_DESC(pio_mask, "PIO modes supported, mode 0 only by default (param valid only for non DT users)");
+
+/**
+ * struct pata_platform_priv - Private info
+ * @io_res: Resource representing I/O base
+ * @ctl_res: Resource representing CTL base
+ * @irq_res: Resource representing IRQ and its flags
+ * @ioport_shift: I/O port shift
+ * @mask: PIO mask
+ * @sht: scsi_host_template to use when registering
+ * @use16bit: Flag to indicate 16-bit IO instead of 32-bit
+ */
+struct pata_platform_priv {
+       struct resource *io_res;
+       struct resource *ctl_res;
+       struct resource *irq_res;
+       unsigned int ioport_shift;
+       int mask;
+       struct scsi_host_template *sht;
+       bool use16bit;
+};

 /*
  * Provide our own set_mode() as we don't want to change anything that has
@@ -66,15 +87,9 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
 }

 /**
- *     __pata_platform_probe           -       attach a platform interface
+ *     pata_platform_host_activate - attach a platform interface
  *     @dev: device
- *     @io_res: Resource representing I/O base
- *     @ctl_res: Resource representing CTL base
- *     @irq_res: Resource representing IRQ and its flags
- *     @ioport_shift: I/O port shift
- *     @__pio_mask: PIO mask
- *     @sht: scsi_host_template to use when registering
- *     @use16bit: Flag to indicate 16-bit IO instead of 32-bit
+ *     @priv: Pointer to struct pata_platform_priv
  *
  *     Register a platform bus IDE interface. Such interfaces are PIO and we
  *     assume do not support IRQ sharing.
@@ -94,10 +109,7 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
  *
  *     If no IRQ resource is present, PIO polling mode is used instead.
  */
-int __pata_platform_probe(struct device *dev, struct resource *io_res,
-                         struct resource *ctl_res, struct resource *irq_res,
-                         unsigned int ioport_shift, int __pio_mask,
-                         struct scsi_host_template *sht, bool use16bit)
+static int pata_platform_host_activate(struct device *dev, struct pata_platform_priv *priv)
 {
        struct ata_host *host;
        struct ata_port *ap;
@@ -108,15 +120,15 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
        /*
         * Check for MMIO
         */
-       mmio = (( io_res->flags == IORESOURCE_MEM) &&
-               (ctl_res->flags == IORESOURCE_MEM));
+       mmio = ((priv->io_res->flags == IORESOURCE_MEM) &&
+               (priv->ctl_res->flags == IORESOURCE_MEM));

        /*
         * And the IRQ
         */
-       if (irq_res && irq_res->start > 0) {
-               irq = irq_res->start;
-               irq_flags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+       if (priv->irq_res && priv->irq_res->start > 0) {
+               irq = priv->irq_res->start;
+               irq_flags = (priv->irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
        }

        /*
@@ -131,12 +143,12 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
        ap->ops->inherits = &ata_sff_port_ops;
        ap->ops->cable_detect = ata_cable_unknown;
        ap->ops->set_mode = pata_platform_set_mode;
-       if (use16bit)
+       if (priv->use16bit)
                ap->ops->sff_data_xfer = ata_sff_data_xfer;
        else
                ap->ops->sff_data_xfer = ata_sff_data_xfer32;

-       ap->pio_mask = __pio_mask;
+       ap->pio_mask = priv->mask;
        ap->flags |= ATA_FLAG_SLAVE_POSS;

        /*
@@ -151,15 +163,15 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
         * Handle the MMIO case
         */
        if (mmio) {
-               ap->ioaddr.cmd_addr = devm_ioremap(dev, io_res->start,
-                               resource_size(io_res));
-               ap->ioaddr.ctl_addr = devm_ioremap(dev, ctl_res->start,
-                               resource_size(ctl_res));
+               ap->ioaddr.cmd_addr = devm_ioremap(dev, priv->io_res->start,
+                                                  resource_size(priv->io_res));
+               ap->ioaddr.ctl_addr = devm_ioremap(dev, priv->ctl_res->start,
+                                                  resource_size(priv->ctl_res));
        } else {
-               ap->ioaddr.cmd_addr = devm_ioport_map(dev, io_res->start,
-                               resource_size(io_res));
-               ap->ioaddr.ctl_addr = devm_ioport_map(dev, ctl_res->start,
-                               resource_size(ctl_res));
+               ap->ioaddr.cmd_addr = devm_ioport_map(dev, priv->io_res->start,
+                                                     resource_size(priv->io_res));
+               ap->ioaddr.ctl_addr = devm_ioport_map(dev, priv->ctl_res->start,
+                                                     resource_size(priv->ctl_res));
        }
        if (!ap->ioaddr.cmd_addr || !ap->ioaddr.ctl_addr) {
                dev_err(dev, "failed to map IO/CTL base\n");
@@ -168,23 +180,83 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,

        ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr;

-       pata_platform_setup_port(&ap->ioaddr, ioport_shift);
+       pata_platform_setup_port(&ap->ioaddr, priv->ioport_shift);

        ata_port_desc(ap, "%s cmd 0x%llx ctl 0x%llx", mmio ? "mmio" : "ioport",
-                     (unsigned long long)io_res->start,
-                     (unsigned long long)ctl_res->start);
+                     (unsigned long long)priv->io_res->start,
+                     (unsigned long long)priv->ctl_res->start);

        /* activate */
        return ata_host_activate(host, irq, irq ? ata_sff_interrupt : NULL,
-                                irq_flags, sht);
+                                irq_flags, priv->sht);
 }
-EXPORT_SYMBOL_GPL(__pata_platform_probe);

-static int pata_platform_probe(struct platform_device *pdev)
+static int pata_of_platform_get_pdata(struct platform_device *ofdev,
+                                     struct pata_platform_priv *priv)
 {
-       struct resource *io_res;
+       struct device_node *dn = ofdev->dev.of_node;
        struct resource *ctl_res;
        struct resource *irq_res;
+       struct resource *io_res;
+       int pio_mode = 0;
+       int irq;
+       int ret;
+
+       ctl_res = devm_kzalloc(&ofdev->dev, sizeof(*ctl_res), GFP_KERNEL);
+       io_res = devm_kzalloc(&ofdev->dev, sizeof(*io_res), GFP_KERNEL);
+       irq_res = devm_kzalloc(&ofdev->dev, sizeof(*irq_res), GFP_KERNEL);
+       if (!ctl_res || !io_res || !irq_res)
+               return -ENOMEM;
+
+       ret = of_address_to_resource(dn, 0, io_res);
+       if (ret) {
+               dev_err(&ofdev->dev, "can't get IO address from device tree\n");
+               return -EINVAL;
+       }
+       priv->io_res = io_res;
+
+       ret = of_address_to_resource(dn, 1, ctl_res);
+       if (ret) {
+               dev_err(&ofdev->dev, "can't get CTL address from device tree\n");
+               return -EINVAL;
+       }
+       priv->ctl_res = ctl_res;
+
+       irq = platform_get_irq_optional(ofdev, 0);
+       if (irq <= 0 && irq != -ENXIO)
+               return irq ? irq : -ENXIO;
+
+       if (irq > 0) {
+               irq_res->start = irq;
+               irq_res->end = irq;
+               irq_res->flags = IORESOURCE_IRQ | irq_get_trigger_type(irq);
Though of_irq_to_resource() does the same thing, I think the irq code
handles the flags automatically for DT.
quoted hunk ↗ jump to hunk
+               priv->irq_res = irq_res;
+       } else {
+               devm_kfree(&ofdev->dev, io_res);
Is this supposed to be irq_res? Otherwise, there shouldn't be any need
to free a devres allocation.
quoted hunk ↗ jump to hunk
+       }
+
+       of_property_read_u32(dn, "reg-shift", &priv->ioport_shift);
+
+       if (!of_property_read_u32(dn, "pio-mode", &pio_mode)) {
+               if (pio_mode > 6) {
+                       dev_err(&ofdev->dev, "invalid pio-mode\n");
+                       return -EINVAL;
+               }
+       } else {
+               dev_info(&ofdev->dev, "pio-mode unspecified, assuming PIO0\n");
+       }
+
+       priv->use16bit = of_property_read_bool(dn, "ata-generic,use16bit");
+
+       priv->mask = 1 << pio_mode;
+       priv->mask |= (1 << pio_mode) - 1;
+
+       return 0;
+}
+
+static int pata_platform_get_pdata(struct platform_device *pdev,
+                                  struct pata_platform_priv *priv)
+{
        struct pata_platform_info *pp_info = dev_get_platdata(&pdev->dev);

        /*
@@ -198,32 +270,63 @@ static int pata_platform_probe(struct platform_device *pdev)
        /*
         * Get the I/O base first
         */
-       io_res = platform_get_mem_or_io(pdev, 0);
-       if (unlikely(!io_res))
+       priv->io_res = platform_get_mem_or_io(pdev, 0);
+       if (unlikely(!priv->io_res))
                return -EINVAL;

        /*
         * Then the CTL base
         */
-       ctl_res = platform_get_mem_or_io(pdev, 1);
-       if (unlikely(!ctl_res))
+       priv->ctl_res = platform_get_mem_or_io(pdev, 1);
+       if (unlikely(!priv->ctl_res))
                return -EINVAL;

        /*
         * And the IRQ
         */
-       irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+       priv->irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+       priv->ioport_shift = pp_info ? pp_info->ioport_shift : 0;
+       priv->mask = pio_mask;
+       priv->use16bit = false;

-       return __pata_platform_probe(&pdev->dev, io_res, ctl_res, irq_res,
-                                    pp_info ? pp_info->ioport_shift : 0,
-                                    pio_mask, &pata_platform_sht, false);
+       return 0;
+}
+
+static int pata_platform_probe(struct platform_device *pdev)
+{
+       struct pata_platform_priv *priv;
+       int ret;
+
+       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+       if (!priv)
+               return -ENOMEM;
+
+       if (!dev_of_node(&pdev->dev))
+               ret = pata_platform_get_pdata(pdev, priv);
+       else
+               ret = pata_of_platform_get_pdata(pdev, priv);
+
+       if (ret)
+               return ret;
+
+       priv->sht = &pata_platform_sht;
+
+       return pata_platform_host_activate(&pdev->dev, priv);
 }

+static const struct of_device_id pata_of_platform_match[] = {
+       { .compatible = "ata-generic", },
+       { },
+};
+MODULE_DEVICE_TABLE(of, pata_of_platform_match);
+
 static struct platform_driver pata_platform_driver = {
        .probe          = pata_platform_probe,
        .remove         = ata_platform_remove_one,
        .driver = {
                .name           = DRV_NAME,
+               .of_match_table = pata_of_platform_match,
        },
 };
diff --git a/include/linux/ata_platform.h b/include/linux/ata_platform.h
index 9cafec92282d..91b8529e6712 100644
--- a/include/linux/ata_platform.h
+++ b/include/linux/ata_platform.h
@@ -13,15 +13,6 @@ struct pata_platform_info {

 struct scsi_host_template;

-extern int __pata_platform_probe(struct device *dev,
-                                struct resource *io_res,
-                                struct resource *ctl_res,
-                                struct resource *irq_res,
-                                unsigned int ioport_shift,
-                                int __pio_mask,
-                                struct scsi_host_template *sht,
-                                bool use16bit);
-
 /*
  * Marvell SATA private data
  */
--
2.17.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help