Hello!
On 12/21/21 7:26 PM, Lad Prabhakar wrote:
> Merge the OF pata_of_platform driver into pata_platform. While doing so
> drop usage of platform_get_resource() and use platform_get_irq_optional().
Sounds like you need a separate patch for doing that...
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> v1-->v2
> * Dropped check for IRQ0
> * Dropped setting the irqflags as suggested by Rob
> * Set just the irq start
> * Fixed freeing up irq_res when not present in DT
> * Dropped PATA_OF_PLATFORM entry
> * Split up sorting of headers in separate patch
> * Dropped sht from struct pata_platform_priv
[...]
> diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
> index cb3134bf88eb..d1f7f4d65316 100644
> --- a/drivers/ata/pata_platform.c
> +++ b/drivers/ata/pata_platform.c
[...]
[...]
> @@ -108,15 +118,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) {
Can this start < 0 ever happen?
> + irq = priv->irq_res->start;
> + irq_flags = (priv->irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
> }
>
> /*
[...]
> @@ -168,23 +178,81 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
[...]
> -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 device_node *dn = ofdev->dev.of_node;
> struct resource *io_res;
> struct resource *ctl_res;
> struct resource *irq_res;
> + int pio_mode = 0;
> + int irq;
> + int ret;
> +
> + io_res = devm_kzalloc(&ofdev->dev, sizeof(*io_res), GFP_KERNEL);
> + ctl_res = devm_kzalloc(&ofdev->dev, sizeof(*ctl_res), GFP_KERNEL);
> + irq_res = devm_kzalloc(&ofdev->dev, sizeof(*irq_res), GFP_KERNEL);
> + if (!io_res || !ctl_res || !irq_res)
> + return -ENOMEM;
> +
> + ret = of_address_to_resource(dn, 0, io_res);
Why not switch to platform_mem_or_io() -- either before or after this patch?
> + 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);
Ditto.
> + 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);
Contrariwise, either keep the old code and update it in pata_of_platform before
this patch or do it after this patch. Otherwise it seems somewhat inconsistent WRT
the of_address_to_resource() calls which you don't convert...
[...]
> @@ -198,32 +266,60 @@ 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))
As Damien said, you can drop unlikely() here...
> 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))
Ditto.
[...]
MBR, Sergey
