| Message ID | 20230926-vf610-gpio-v4-4-b57b7f6e8368@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/7] dt-bindings: gpio: vf610: update gpio-ranges | expand |
On 23-09-26, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > i.MX8ULP/93 GPIO supports similar feature as i.MX7ULP GPIO, but i.MX8ULP is > actually not hardware compatible with i.MX7ULP. i.MX8ULP only has one > register base, not two bases. i.MX8ULP and i.MX93 actually has two interrupts > for each gpio controller, one for Trustzone non-secure world, one for > secure world. > > Although the Linux Kernel driver gpio-vf610.c could work with > fsl,imx7ulp-gpio compatible, it is based on some tricks did in device tree > with some offset added to base address. > > Add a new of_device_id entry for i.MX8ULP. But to make the driver could > also support old bindings, check the compatible string first, before > check the device data. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/gpio/gpio-vf610.c | 40 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c > index dbc7ba0ee72c..49867d5db642 100644 > --- a/drivers/gpio/gpio-vf610.c > +++ b/drivers/gpio/gpio-vf610.c > @@ -25,6 +25,7 @@ > struct fsl_gpio_soc_data { > /* SoCs has a Port Data Direction Register (PDDR) */ > bool have_paddr; > + bool have_dual_base; > }; > > struct vf610_gpio_port { > @@ -60,13 +61,22 @@ struct vf610_gpio_port { > #define PORT_INT_EITHER_EDGE 0xb > #define PORT_INT_LOGIC_ONE 0xc > > +#define IMX8ULP_GPIO_BASE_OFF 0x40 > +#define IMX8ULP_BASE_OFF 0x80 > + > static const struct fsl_gpio_soc_data imx_data = { > .have_paddr = true, > + .have_dual_base = true, > +}; > + > +static const struct fsl_gpio_soc_data imx8ulp_data = { > + .have_paddr = true, > }; > > static const struct of_device_id vf610_gpio_dt_ids[] = { > { .compatible = "fsl,vf610-gpio", .data = NULL, }, > { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, > + { .compatible = "fsl,imx8ulp-gpio", .data = &imx8ulp_data, }, > { /* sentinel */ } > }; > > @@ -263,19 +273,37 @@ static int vf610_gpio_probe(struct platform_device *pdev) > struct gpio_irq_chip *girq; > int i; > int ret; > + bool dual_base = false; > > port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); > if (!port) > return -ENOMEM; > > port->sdata = of_device_get_match_data(dev); > - port->base = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(port->base)) > - return PTR_ERR(port->base); > > - port->gpio_base = devm_platform_ioremap_resource(pdev, 1); > - if (IS_ERR(port->gpio_base)) > - return PTR_ERR(port->gpio_base); > + /* support old compatible strings */ > + if (device_is_compatible(dev, "fsl,imx7ulp-gpio") && > + (device_is_compatible(dev, "fsl,imx93-gpio") || > + (device_is_compatible(dev, "fsl,imx8ulp-gpio")))) > + dual_base = true; Could be simplified even further, if we would add the have_dual_base for the vf610 as well within this patch. dual_base = port->sdata->have_dual_base; /* support old bindings */ if (device_is_compatible(dev, "fsl,imx7ulp-gpio") && (device_is_compatible(dev, "fsl,imx93-gpio") || (device_is_compatible(dev, "fsl,imx8ulp-gpio")))) dual_base = true; if (dual_base) { ... Regards, Marco > + if ((port->sdata && port->sdata->have_dual_base) || dual_base) { > + port->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(port->base)) > + return PTR_ERR(port->base); > + > + port->gpio_base = devm_platform_ioremap_resource(pdev, 1); > + if (IS_ERR(port->gpio_base)) > + return PTR_ERR(port->gpio_base); > + } else { > + port->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(port->base)) > + return PTR_ERR(port->base); > + > + port->gpio_base = port->base + IMX8ULP_GPIO_BASE_OFF; > + port->base = port->base + IMX8ULP_BASE_OFF; > + } > + > > port->irq = platform_get_irq(pdev, 0); > if (port->irq < 0) > > -- > 2.37.1 > >
> Subject: Re: [PATCH v4 4/7] gpio: vf610: add i.MX8ULP of_device_id entry > > On 23-09-26, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > i.MX8ULP/93 GPIO supports similar feature as i.MX7ULP GPIO, but > > i.MX8ULP is actually not hardware compatible with i.MX7ULP. i.MX8ULP > > only has one register base, not two bases. i.MX8ULP and i.MX93 > > actually has two interrupts for each gpio controller, one for > > Trustzone non-secure world, one for secure world. > > > > Although the Linux Kernel driver gpio-vf610.c could work with > > fsl,imx7ulp-gpio compatible, it is based on some tricks did in device > > tree with some offset added to base address. > > > > Add a new of_device_id entry for i.MX8ULP. But to make the driver > > could also support old bindings, check the compatible string first, > > before check the device data. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/gpio/gpio-vf610.c | 40 > > ++++++++++++++++++++++++++++++++++------ > > 1 file changed, 34 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c > > index dbc7ba0ee72c..49867d5db642 100644 > > --- a/drivers/gpio/gpio-vf610.c > > +++ b/drivers/gpio/gpio-vf610.c > > @@ -25,6 +25,7 @@ > > struct fsl_gpio_soc_data { > > /* SoCs has a Port Data Direction Register (PDDR) */ > > bool have_paddr; > > + bool have_dual_base; > > }; > > > > struct vf610_gpio_port { > > @@ -60,13 +61,22 @@ struct vf610_gpio_port { > > #define PORT_INT_EITHER_EDGE 0xb > > #define PORT_INT_LOGIC_ONE 0xc > > > > +#define IMX8ULP_GPIO_BASE_OFF 0x40 > > +#define IMX8ULP_BASE_OFF 0x80 > > + > > static const struct fsl_gpio_soc_data imx_data = { > > .have_paddr = true, > > + .have_dual_base = true, > > +}; > > + > > +static const struct fsl_gpio_soc_data imx8ulp_data = { > > + .have_paddr = true, > > }; > > > > static const struct of_device_id vf610_gpio_dt_ids[] = { > > { .compatible = "fsl,vf610-gpio", .data = NULL, }, > > { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, > > + { .compatible = "fsl,imx8ulp-gpio", .data = &imx8ulp_data, }, > > { /* sentinel */ } > > }; > > > > @@ -263,19 +273,37 @@ static int vf610_gpio_probe(struct > platform_device *pdev) > > struct gpio_irq_chip *girq; > > int i; > > int ret; > > + bool dual_base = false; > > > > port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); > > if (!port) > > return -ENOMEM; > > > > port->sdata = of_device_get_match_data(dev); > > - port->base = devm_platform_ioremap_resource(pdev, 0); > > - if (IS_ERR(port->base)) > > - return PTR_ERR(port->base); > > > > - port->gpio_base = devm_platform_ioremap_resource(pdev, 1); > > - if (IS_ERR(port->gpio_base)) > > - return PTR_ERR(port->gpio_base); > > + /* support old compatible strings */ > > + if (device_is_compatible(dev, "fsl,imx7ulp-gpio") && > > + (device_is_compatible(dev, "fsl,imx93-gpio") || > > + (device_is_compatible(dev, "fsl,imx8ulp-gpio")))) > > + dual_base = true; > > Could be simplified even further, if we would add the have_dual_base for the > vf610 as well within this patch. ok, need move part of patch 5 into patch 4, Then patch 5 just drop the port->sdata check. Will wait a few days before post V5 in case people has comments on other parts. Thanks, Peng. > > dual_base = port->sdata->have_dual_base; > > /* support old bindings */ > if (device_is_compatible(dev, "fsl,imx7ulp-gpio") && > (device_is_compatible(dev, "fsl,imx93-gpio") || > (device_is_compatible(dev, "fsl,imx8ulp-gpio")))) > dual_base = true; > > if (dual_base) { > ... > > Regards, > Marco > > > + if ((port->sdata && port->sdata->have_dual_base) || dual_base) { > > + port->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(port->base)) > > + return PTR_ERR(port->base); > > + > > + port->gpio_base = devm_platform_ioremap_resource(pdev, > 1); > > + if (IS_ERR(port->gpio_base)) > > + return PTR_ERR(port->gpio_base); > > + } else { > > + port->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(port->base)) > > + return PTR_ERR(port->base); > > + > > + port->gpio_base = port->base + IMX8ULP_GPIO_BASE_OFF; > > + port->base = port->base + IMX8ULP_BASE_OFF; > > + } > > + > > > > port->irq = platform_get_irq(pdev, 0); > > if (port->irq < 0) > > > > -- > > 2.37.1 > > > >
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c index dbc7ba0ee72c..49867d5db642 100644 --- a/drivers/gpio/gpio-vf610.c +++ b/drivers/gpio/gpio-vf610.c @@ -25,6 +25,7 @@ struct fsl_gpio_soc_data { /* SoCs has a Port Data Direction Register (PDDR) */ bool have_paddr; + bool have_dual_base; }; struct vf610_gpio_port { @@ -60,13 +61,22 @@ struct vf610_gpio_port { #define PORT_INT_EITHER_EDGE 0xb #define PORT_INT_LOGIC_ONE 0xc +#define IMX8ULP_GPIO_BASE_OFF 0x40 +#define IMX8ULP_BASE_OFF 0x80 + static const struct fsl_gpio_soc_data imx_data = { .have_paddr = true, + .have_dual_base = true, +}; + +static const struct fsl_gpio_soc_data imx8ulp_data = { + .have_paddr = true, }; static const struct of_device_id vf610_gpio_dt_ids[] = { { .compatible = "fsl,vf610-gpio", .data = NULL, }, { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, + { .compatible = "fsl,imx8ulp-gpio", .data = &imx8ulp_data, }, { /* sentinel */ } }; @@ -263,19 +273,37 @@ static int vf610_gpio_probe(struct platform_device *pdev) struct gpio_irq_chip *girq; int i; int ret; + bool dual_base = false; port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); if (!port) return -ENOMEM; port->sdata = of_device_get_match_data(dev); - port->base = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(port->base)) - return PTR_ERR(port->base); - port->gpio_base = devm_platform_ioremap_resource(pdev, 1); - if (IS_ERR(port->gpio_base)) - return PTR_ERR(port->gpio_base); + /* support old compatible strings */ + if (device_is_compatible(dev, "fsl,imx7ulp-gpio") && + (device_is_compatible(dev, "fsl,imx93-gpio") || + (device_is_compatible(dev, "fsl,imx8ulp-gpio")))) + dual_base = true; + + if ((port->sdata && port->sdata->have_dual_base) || dual_base) { + port->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(port->base)) + return PTR_ERR(port->base); + + port->gpio_base = devm_platform_ioremap_resource(pdev, 1); + if (IS_ERR(port->gpio_base)) + return PTR_ERR(port->gpio_base); + } else { + port->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(port->base)) + return PTR_ERR(port->base); + + port->gpio_base = port->base + IMX8ULP_GPIO_BASE_OFF; + port->base = port->base + IMX8ULP_BASE_OFF; + } + port->irq = platform_get_irq(pdev, 0); if (port->irq < 0)