On Fri, Dec 17, 2021 at 5:04 PM Rob Herring [off-list ref] wrote:

On Fri, Dec 17, 2021 at 10:42 AM Lad, Prabhakar
[off-list ref] wrote:

quoted

Hi Rob,

On Fri, Dec 17, 2021 at 2:50 PM Rob Herring [off-list ref] wrote:

quoted

On Fri, Dec 17, 2021 at 8:17 AM Lad Prabhakar
[off-list ref] wrote:

quoted

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?

No, as there are a couple defconfigs which have this option enabled.
So the plan is once this patch is merged for the send in the patches
for defconfig files for next release and a release later we can safely
drop this. So that we don't break anything.

Wouldn't they all be fine since they already have
CONFIG_PATA_PLATFORM=y as well?

I didn't realize all those defconfigs already have
CONFIG_PATA_PLATFORM enabled. So in that case PATA_OF_PLATFORM can be
wiped out.

Either way, that's fine.

quoted

quoted

quoted

        help
          This option enables support for generic directly connected ATA
          devices commonly found on embedded systems with OpenFirmware

quoted

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.

sorry you mean to drop the flags?

Right.

Ok, I will drop it.

Cheers,
Prabhakar