linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: mpc8xxx: Add ACPI support
@ 2021-03-12  6:58 Ran Wang
  2021-03-12 11:07 ` Bartosz Golaszewski
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ran Wang @ 2021-03-12  6:58 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski; +Cc: linux-gpio, linux-kernel, Ran Wang

Current implementation only supports DT, now add ACPI support.

Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
LS1088A's GPIO have no extra programming, so simplify related checking.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
 drivers/gpio/gpio-mpc8xxx.c | 50 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index 6dfca83bcd90..de5b7e17cde3 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -9,6 +9,7 @@
  * kind, whether express or implied.
  */
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
@@ -292,8 +293,6 @@ static const struct of_device_id mpc8xxx_gpio_ids[] = {
 	{ .compatible = "fsl,mpc5121-gpio", .data = &mpc512x_gpio_devtype, },
 	{ .compatible = "fsl,mpc5125-gpio", .data = &mpc5125_gpio_devtype, },
 	{ .compatible = "fsl,pq3-gpio",     },
-	{ .compatible = "fsl,ls1028a-gpio", },
-	{ .compatible = "fsl,ls1088a-gpio", },
 	{ .compatible = "fsl,qoriq-gpio",   },
 	{}
 };
@@ -303,10 +302,19 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 	struct gpio_chip	*gc;
-	const struct mpc8xxx_gpio_devtype *devtype =
-		of_device_get_match_data(&pdev->dev);
+	const struct mpc8xxx_gpio_devtype *devtype;
+	const struct acpi_device_id *acpi_id;
 	int ret;
 
+	if (pdev->dev.of_node) {
+		devtype = of_device_get_match_data(&pdev->dev);
+	} else {
+		acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
+						&pdev->dev);
+		if (acpi_id)
+			devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data;
+	}
+
 	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
 	if (!mpc8xxx_gc)
 		return -ENOMEM;
@@ -315,14 +323,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 
 	raw_spin_lock_init(&mpc8xxx_gc->lock);
 
-	mpc8xxx_gc->regs = of_iomap(np, 0);
+	mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (!mpc8xxx_gc->regs)
 		return -ENOMEM;
 
 	gc = &mpc8xxx_gc->gc;
 	gc->parent = &pdev->dev;
 
-	if (of_property_read_bool(np, "little-endian")) {
+	if (device_property_read_bool(&pdev->dev, "little-endian")) {
 		ret = bgpio_init(gc, &pdev->dev, 4,
 				 mpc8xxx_gc->regs + GPIO_DAT,
 				 NULL, NULL,
@@ -369,10 +377,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 	 * associated input enable must be set (GPIOxGPIE[IEn]=1) to propagate
 	 * the port value to the GPIO Data Register.
 	 */
-	if (of_device_is_compatible(np, "fsl,qoriq-gpio") ||
-	    of_device_is_compatible(np, "fsl,ls1028a-gpio") ||
-	    of_device_is_compatible(np, "fsl,ls1088a-gpio"))
-		gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
+	if (pdev->dev.of_node) {
+		if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
+			gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
+	} else {
+		if (acpi_match_device(pdev->dev.driver->acpi_match_table,
+					&pdev->dev))
+			gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
+	}
 
 	ret = gpiochip_add_data(gc, mpc8xxx_gc);
 	if (ret) {
@@ -381,12 +393,15 @@ static int mpc8xxx_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
+	mpc8xxx_gc->irqn = platform_get_irq(pdev, 0);
 	if (!mpc8xxx_gc->irqn)
 		return 0;
 
-	mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
-					&mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
+	mpc8xxx_gc->irq = irq_domain_create_linear(dev_fwnode(&pdev->dev),
+						   MPC8XXX_GPIO_PINS,
+						   &mpc8xxx_gpio_irq_ops,
+						   mpc8xxx_gc);
+
 	if (!mpc8xxx_gc->irq)
 		return 0;
 
@@ -425,12 +440,21 @@ static int mpc8xxx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id gpio_acpi_ids[] = {
+	{"NXP0031",},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
+#endif
+
 static struct platform_driver mpc8xxx_plat_driver = {
 	.probe		= mpc8xxx_probe,
 	.remove		= mpc8xxx_remove,
 	.driver		= {
 		.name = "gpio-mpc8xxx",
 		.of_match_table	= mpc8xxx_gpio_ids,
+		.acpi_match_table = ACPI_PTR(gpio_acpi_ids),
 	},
 };
 
-- 
2.25.1


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

* Re: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-12  6:58 [PATCH] gpio: mpc8xxx: Add ACPI support Ran Wang
@ 2021-03-12 11:07 ` Bartosz Golaszewski
  2021-03-14  0:10   ` Michael Walle
  2021-03-13  1:04 ` kernel test robot
  2021-03-14 13:51 ` Andy Shevchenko
  2 siblings, 1 reply; 7+ messages in thread
From: Bartosz Golaszewski @ 2021-03-12 11:07 UTC (permalink / raw)
  To: Ran Wang; +Cc: Linus Walleij, linux-gpio, LKML

On Fri, Mar 12, 2021 at 7:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>
> Current implementation only supports DT, now add ACPI support.
>
> Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
> LS1088A's GPIO have no extra programming, so simplify related checking.
>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
>  drivers/gpio/gpio-mpc8xxx.c | 50 +++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index 6dfca83bcd90..de5b7e17cde3 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -9,6 +9,7 @@
>   * kind, whether express or implied.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/spinlock.h>
> @@ -292,8 +293,6 @@ static const struct of_device_id mpc8xxx_gpio_ids[] = {
>         { .compatible = "fsl,mpc5121-gpio", .data = &mpc512x_gpio_devtype, },
>         { .compatible = "fsl,mpc5125-gpio", .data = &mpc5125_gpio_devtype, },
>         { .compatible = "fsl,pq3-gpio",     },
> -       { .compatible = "fsl,ls1028a-gpio", },
> -       { .compatible = "fsl,ls1088a-gpio", },

Why are you removing support for those?

Bart

>         { .compatible = "fsl,qoriq-gpio",   },
>         {}
>  };
> @@ -303,10 +302,19 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>         struct device_node *np = pdev->dev.of_node;
>         struct mpc8xxx_gpio_chip *mpc8xxx_gc;
>         struct gpio_chip        *gc;
> -       const struct mpc8xxx_gpio_devtype *devtype =
> -               of_device_get_match_data(&pdev->dev);
> +       const struct mpc8xxx_gpio_devtype *devtype;
> +       const struct acpi_device_id *acpi_id;
>         int ret;
>
> +       if (pdev->dev.of_node) {
> +               devtype = of_device_get_match_data(&pdev->dev);
> +       } else {
> +               acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> +                                               &pdev->dev);
> +               if (acpi_id)
> +                       devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data;
> +       }
> +
>         mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
>         if (!mpc8xxx_gc)
>                 return -ENOMEM;
> @@ -315,14 +323,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>
>         raw_spin_lock_init(&mpc8xxx_gc->lock);
>
> -       mpc8xxx_gc->regs = of_iomap(np, 0);
> +       mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
>         if (!mpc8xxx_gc->regs)
>                 return -ENOMEM;
>
>         gc = &mpc8xxx_gc->gc;
>         gc->parent = &pdev->dev;
>
> -       if (of_property_read_bool(np, "little-endian")) {
> +       if (device_property_read_bool(&pdev->dev, "little-endian")) {
>                 ret = bgpio_init(gc, &pdev->dev, 4,
>                                  mpc8xxx_gc->regs + GPIO_DAT,
>                                  NULL, NULL,
> @@ -369,10 +377,14 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>          * associated input enable must be set (GPIOxGPIE[IEn]=1) to propagate
>          * the port value to the GPIO Data Register.
>          */
> -       if (of_device_is_compatible(np, "fsl,qoriq-gpio") ||
> -           of_device_is_compatible(np, "fsl,ls1028a-gpio") ||
> -           of_device_is_compatible(np, "fsl,ls1088a-gpio"))
> -               gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> +       if (pdev->dev.of_node) {
> +               if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> +       } else {
> +               if (acpi_match_device(pdev->dev.driver->acpi_match_table,
> +                                       &pdev->dev))
> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> +       }
>
>         ret = gpiochip_add_data(gc, mpc8xxx_gc);
>         if (ret) {
> @@ -381,12 +393,15 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>                 goto err;
>         }
>
> -       mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
> +       mpc8xxx_gc->irqn = platform_get_irq(pdev, 0);
>         if (!mpc8xxx_gc->irqn)
>                 return 0;
>
> -       mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
> -                                       &mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
> +       mpc8xxx_gc->irq = irq_domain_create_linear(dev_fwnode(&pdev->dev),
> +                                                  MPC8XXX_GPIO_PINS,
> +                                                  &mpc8xxx_gpio_irq_ops,
> +                                                  mpc8xxx_gc);
> +
>         if (!mpc8xxx_gc->irq)
>                 return 0;
>
> @@ -425,12 +440,21 @@ static int mpc8xxx_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id gpio_acpi_ids[] = {
> +       {"NXP0031",},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
> +#endif
> +
>  static struct platform_driver mpc8xxx_plat_driver = {
>         .probe          = mpc8xxx_probe,
>         .remove         = mpc8xxx_remove,
>         .driver         = {
>                 .name = "gpio-mpc8xxx",
>                 .of_match_table = mpc8xxx_gpio_ids,
> +               .acpi_match_table = ACPI_PTR(gpio_acpi_ids),
>         },
>  };
>
> --
> 2.25.1
>

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

* Re: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-12  6:58 [PATCH] gpio: mpc8xxx: Add ACPI support Ran Wang
  2021-03-12 11:07 ` Bartosz Golaszewski
@ 2021-03-13  1:04 ` kernel test robot
  2021-03-14 13:51 ` Andy Shevchenko
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-03-13  1:04 UTC (permalink / raw)
  To: Ran Wang, Linus Walleij, Bartosz Golaszewski
  Cc: kbuild-all, linux-gpio, linux-kernel, Ran Wang

[-- Attachment #1: Type: text/plain, Size: 16402 bytes --]

Hi Ran,

I love your patch! Perhaps something to improve:

[auto build test WARNING on gpio/for-next]
[also build test WARNING on v5.12-rc2 next-20210312]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ran-Wang/gpio-mpc8xxx-Add-ACPI-support/20210312-145412
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: arm-randconfig-m031-20210312 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
drivers/gpio/gpio-mpc8xxx.c:356 mpc8xxx_probe() error: uninitialized symbol 'devtype'.

vim +/devtype +356 drivers/gpio/gpio-mpc8xxx.c

e39d5ef678045d arch/powerpc/sysdev/mpc8xxx_gpio.c Anatolij Gustschin       2010-08-09  299  
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  300  static int mpc8xxx_probe(struct platform_device *pdev)
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  301  {
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  302  	struct device_node *np = pdev->dev.of_node;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  303  	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  304  	struct gpio_chip	*gc;
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  305  	const struct mpc8xxx_gpio_devtype *devtype;
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  306  	const struct acpi_device_id *acpi_id;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  307  	int ret;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  308  
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  309  	if (pdev->dev.of_node) {
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  310  		devtype = of_device_get_match_data(&pdev->dev);
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  311  	} else {
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  312  		acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  313  						&pdev->dev);
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  314  		if (acpi_id)
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  315  			devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data;
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  316  	}
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  317  
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  318  	mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  319  	if (!mpc8xxx_gc)
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  320  		return -ENOMEM;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  321  
257e10752c13f2 drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  322  	platform_set_drvdata(pdev, mpc8xxx_gc);
257e10752c13f2 drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  323  
505936131ea71e drivers/gpio/gpio-mpc8xxx.c        Alexander Stein          2015-07-21  324  	raw_spin_lock_init(&mpc8xxx_gc->lock);
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  325  
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  326  	mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  327  	if (!mpc8xxx_gc->regs)
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  328  		return -ENOMEM;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  329  
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  330  	gc = &mpc8xxx_gc->gc;
322f6a3182d42d drivers/gpio/gpio-mpc8xxx.c        Johnson CH Chen (陳昭勳  2019-11-26  331) 	gc->parent = &pdev->dev;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  332  
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  333  	if (device_property_read_bool(&pdev->dev, "little-endian")) {
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  334  		ret = bgpio_init(gc, &pdev->dev, 4,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  335  				 mpc8xxx_gc->regs + GPIO_DAT,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  336  				 NULL, NULL,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  337  				 mpc8xxx_gc->regs + GPIO_DIR, NULL,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  338  				 BGPIOF_BIG_ENDIAN);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  339  		if (ret)
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  340  			goto err;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  341  		dev_dbg(&pdev->dev, "GPIO registers are LITTLE endian\n");
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  342  	} else {
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  343  		ret = bgpio_init(gc, &pdev->dev, 4,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  344  				 mpc8xxx_gc->regs + GPIO_DAT,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  345  				 NULL, NULL,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  346  				 mpc8xxx_gc->regs + GPIO_DIR, NULL,
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  347  				 BGPIOF_BIG_ENDIAN
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  348  				 | BGPIOF_BIG_ENDIAN_BYTE_ORDER);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  349  		if (ret)
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  350  			goto err;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  351  		dev_dbg(&pdev->dev, "GPIO registers are BIG endian\n");
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  352  	}
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  353  
fa4007ca06e4c8 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  354  	mpc8xxx_gc->direction_output = gc->direction_output;
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  355  
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16 @356  	if (!devtype)
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  357  		devtype = &mpc8xxx_gpio_devtype_default;
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  358  
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  359  	/*
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  360  	 * It's assumed that only a single type of gpio controller is available
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  361  	 * on the current machine, so overwriting global data is fine.
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  362  	 */
4e50573f39229d drivers/gpio/gpio-mpc8xxx.c        Vladimir Oltean          2019-11-15  363  	if (devtype->irq_set_type)
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  364  		mpc8xxx_irq_chip.irq_set_type = devtype->irq_set_type;
82e39b0d8566fa drivers/gpio/gpio-mpc8xxx.c        Uwe Kleine-König         2015-07-16  365  
adf32eaa053234 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  366  	if (devtype->gpio_dir_out)
adf32eaa053234 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  367  		gc->direction_output = devtype->gpio_dir_out;
adf32eaa053234 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  368  	if (devtype->gpio_get)
adf32eaa053234 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  369  		gc->get = devtype->gpio_get;
adf32eaa053234 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  370  
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  371  	gc->to_irq = mpc8xxx_gpio_to_irq;
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  372  
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  373  	/*
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  374  	 * The GPIO Input Buffer Enable register(GPIO_IBE) is used to control
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  375  	 * the input enable of each individual GPIO port.  When an individual
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  376  	 * GPIO port’s direction is set to input (GPIO_GPDIR[DRn=0]), the
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  377  	 * associated input enable must be set (GPIOxGPIE[IEn]=1) to propagate
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  378  	 * the port value to the GPIO Data Register.
3795d7cc4fe132 drivers/gpio/gpio-mpc8xxx.c        Michael Walle            2020-09-30  379  	 */
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  380  	if (pdev->dev.of_node) {
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  381  		if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
787b64a43f7aca drivers/gpio/gpio-mpc8xxx.c        Russell King             2019-11-19  382  			gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  383  	} else {
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  384  		if (acpi_match_device(pdev->dev.driver->acpi_match_table,
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  385  					&pdev->dev))
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  386  			gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  387  	}
787b64a43f7aca drivers/gpio/gpio-mpc8xxx.c        Russell King             2019-11-19  388  
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  389  	ret = gpiochip_add_data(gc, mpc8xxx_gc);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  390  	if (ret) {
7eb6ce2f272336 drivers/gpio/gpio-mpc8xxx.c        Rob Herring              2017-07-18  391  		pr_err("%pOF: GPIO chip registration failed with status %d\n",
7eb6ce2f272336 drivers/gpio/gpio-mpc8xxx.c        Rob Herring              2017-07-18  392  		       np, ret);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  393  		goto err;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  394  	}
1e16dfc1baa745 arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2008-09-23  395  
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  396  	mpc8xxx_gc->irqn = platform_get_irq(pdev, 0);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  397  	if (!mpc8xxx_gc->irqn)
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  398  		return 0;
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  399  
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  400  	mpc8xxx_gc->irq = irq_domain_create_linear(dev_fwnode(&pdev->dev),
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  401  						   MPC8XXX_GPIO_PINS,
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  402  						   &mpc8xxx_gpio_irq_ops,
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  403  						   mpc8xxx_gc);
f6481b7d46fa21 drivers/gpio/gpio-mpc8xxx.c        Ran Wang                 2021-03-12  404  
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  405  	if (!mpc8xxx_gc->irq)
98686d9a52eeea drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  406  		return 0;
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  407  
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  408  	/* ack and mask all irqs */
cd0d3f58a0ca05 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  409  	gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, 0xffffffff);
cd0d3f58a0ca05 drivers/gpio/gpio-mpc8xxx.c        Axel Lin                 2016-02-22  410  	gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, 0);
345e5c8a1cc30e arch/powerpc/sysdev/mpc8xxx_gpio.c Peter Korsgaard          2010-01-07  411  
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  412  	ret = devm_request_irq(&pdev->dev, mpc8xxx_gc->irqn,
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  413  			       mpc8xxx_gpio_irq_cascade,
3d5bfbd9716318 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2020-06-11  414  			       IRQF_SHARED, "gpio-cascade",
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  415  			       mpc8xxx_gc);
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  416  	if (ret) {
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  417  		dev_err(&pdev->dev, "%s: failed to devm_request_irq(%d), ret = %d\n",
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  418  			np->full_name, mpc8xxx_gc->irqn, ret);
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  419  		goto err;
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  420  	}
698b8eeaed7287 drivers/gpio/gpio-mpc8xxx.c        Song Hui                 2019-10-11  421  
257e10752c13f2 drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  422  	return 0;
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  423  err:
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  424  	iounmap(mpc8xxx_gc->regs);
42178e2a1e42b4 drivers/gpio/gpio-mpc8xxx.c        Liu Gang                 2016-02-03  425  	return ret;
257e10752c13f2 drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  426  }
257e10752c13f2 drivers/gpio/gpio-mpc8xxx.c        Ricardo Ribalda          2015-01-18  427  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47417 bytes --]

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

* Re: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-12 11:07 ` Bartosz Golaszewski
@ 2021-03-14  0:10   ` Michael Walle
  2021-03-15  6:12     ` Ran Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Walle @ 2021-03-14  0:10 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: Ran Wang, Linus Walleij, linux-gpio, LKML

Am 2021-03-12 12:07, schrieb Bartosz Golaszewski:
> On Fri, Mar 12, 2021 at 7:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>> 
>> Current implementation only supports DT, now add ACPI support.
>> 
>> Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
>> LS1088A's GPIO have no extra programming, so simplify related 
>> checking.
>> 
>> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
>> ---
>>  drivers/gpio/gpio-mpc8xxx.c | 50 
>> +++++++++++++++++++++++++++----------
>>  1 file changed, 37 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
>> index 6dfca83bcd90..de5b7e17cde3 100644
>> --- a/drivers/gpio/gpio-mpc8xxx.c
>> +++ b/drivers/gpio/gpio-mpc8xxx.c
>> @@ -9,6 +9,7 @@
>>   * kind, whether express or implied.
>>   */
>> 
>> +#include <linux/acpi.h>
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>>  #include <linux/spinlock.h>
>> @@ -292,8 +293,6 @@ static const struct of_device_id 
>> mpc8xxx_gpio_ids[] = {
>>         { .compatible = "fsl,mpc5121-gpio", .data = 
>> &mpc512x_gpio_devtype, },
>>         { .compatible = "fsl,mpc5125-gpio", .data = 
>> &mpc5125_gpio_devtype, },
>>         { .compatible = "fsl,pq3-gpio",     },
>> -       { .compatible = "fsl,ls1028a-gpio", },
>> -       { .compatible = "fsl,ls1088a-gpio", },
> 
> Why are you removing support for those?

I guess because it doesn't matter because usually
  compatible = "fsl,ls1028a-gpio", "fsl,qoriq-gpio";
is used. But I just had a look at the dt binding and it doesn't
mandate it. So please keep it.

-michael

> 
> Bart
> 
>>         { .compatible = "fsl,qoriq-gpio",   },
>>         {}
>>  };
>> @@ -303,10 +302,19 @@ static int mpc8xxx_probe(struct platform_device 
>> *pdev)
>>         struct device_node *np = pdev->dev.of_node;
>>         struct mpc8xxx_gpio_chip *mpc8xxx_gc;
>>         struct gpio_chip        *gc;
>> -       const struct mpc8xxx_gpio_devtype *devtype =
>> -               of_device_get_match_data(&pdev->dev);
>> +       const struct mpc8xxx_gpio_devtype *devtype;
>> +       const struct acpi_device_id *acpi_id;
>>         int ret;
>> 
>> +       if (pdev->dev.of_node) {
>> +               devtype = of_device_get_match_data(&pdev->dev);
>> +       } else {
>> +               acpi_id = 
>> acpi_match_device(pdev->dev.driver->acpi_match_table,
>> +                                               &pdev->dev);
>> +               if (acpi_id)
>> +                       devtype = (struct mpc8xxx_gpio_devtype *) 
>> acpi_id->driver_data;
>> +       }
>> +
>>         mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), 
>> GFP_KERNEL);
>>         if (!mpc8xxx_gc)
>>                 return -ENOMEM;
>> @@ -315,14 +323,14 @@ static int mpc8xxx_probe(struct platform_device 
>> *pdev)
>> 
>>         raw_spin_lock_init(&mpc8xxx_gc->lock);
>> 
>> -       mpc8xxx_gc->regs = of_iomap(np, 0);
>> +       mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
>>         if (!mpc8xxx_gc->regs)
>>                 return -ENOMEM;
>> 
>>         gc = &mpc8xxx_gc->gc;
>>         gc->parent = &pdev->dev;
>> 
>> -       if (of_property_read_bool(np, "little-endian")) {
>> +       if (device_property_read_bool(&pdev->dev, "little-endian")) {
>>                 ret = bgpio_init(gc, &pdev->dev, 4,
>>                                  mpc8xxx_gc->regs + GPIO_DAT,
>>                                  NULL, NULL,
>> @@ -369,10 +377,14 @@ static int mpc8xxx_probe(struct platform_device 
>> *pdev)
>>          * associated input enable must be set (GPIOxGPIE[IEn]=1) to 
>> propagate
>>          * the port value to the GPIO Data Register.
>>          */
>> -       if (of_device_is_compatible(np, "fsl,qoriq-gpio") ||
>> -           of_device_is_compatible(np, "fsl,ls1028a-gpio") ||
>> -           of_device_is_compatible(np, "fsl,ls1088a-gpio"))
>> -               gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 
>> 0xffffffff);
>> +       if (pdev->dev.of_node) {
>> +               if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
>> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 
>> 0xffffffff);
>> +       } else {
>> +               if 
>> (acpi_match_device(pdev->dev.driver->acpi_match_table,
>> +                                       &pdev->dev))
>> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 
>> 0xffffffff);
>> +       }
>> 
>>         ret = gpiochip_add_data(gc, mpc8xxx_gc);
>>         if (ret) {
>> @@ -381,12 +393,15 @@ static int mpc8xxx_probe(struct platform_device 
>> *pdev)
>>                 goto err;
>>         }
>> 
>> -       mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0);
>> +       mpc8xxx_gc->irqn = platform_get_irq(pdev, 0);
>>         if (!mpc8xxx_gc->irqn)
>>                 return 0;
>> 
>> -       mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
>> -                                       &mpc8xxx_gpio_irq_ops, 
>> mpc8xxx_gc);
>> +       mpc8xxx_gc->irq = 
>> irq_domain_create_linear(dev_fwnode(&pdev->dev),
>> +                                                  MPC8XXX_GPIO_PINS,
>> +                                                  
>> &mpc8xxx_gpio_irq_ops,
>> +                                                  mpc8xxx_gc);
>> +
>>         if (!mpc8xxx_gc->irq)
>>                 return 0;
>> 
>> @@ -425,12 +440,21 @@ static int mpc8xxx_remove(struct platform_device 
>> *pdev)
>>         return 0;
>>  }
>> 
>> +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id gpio_acpi_ids[] = {
>> +       {"NXP0031",},
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
>> +#endif
>> +
>>  static struct platform_driver mpc8xxx_plat_driver = {
>>         .probe          = mpc8xxx_probe,
>>         .remove         = mpc8xxx_remove,
>>         .driver         = {
>>                 .name = "gpio-mpc8xxx",
>>                 .of_match_table = mpc8xxx_gpio_ids,
>> +               .acpi_match_table = ACPI_PTR(gpio_acpi_ids),
>>         },
>>  };
>> 
>> --
>> 2.25.1
>> 

-- 
-michael

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

* Re: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-12  6:58 [PATCH] gpio: mpc8xxx: Add ACPI support Ran Wang
  2021-03-12 11:07 ` Bartosz Golaszewski
  2021-03-13  1:04 ` kernel test robot
@ 2021-03-14 13:51 ` Andy Shevchenko
  2021-03-15  6:02   ` Ran Wang
  2 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2021-03-14 13:51 UTC (permalink / raw)
  To: Ran Wang
  Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List

On Fri, Mar 12, 2021 at 8:53 AM Ran Wang <ran.wang_1@nxp.com> wrote:

First of all, please add me to the Cc list for the next version of the patch.

> Current implementation only supports DT, now add ACPI support.
>
> Note that compared to device of 'fsl,qoriq-gpio', LS1028A and

to devices

> LS1088A's GPIO have no extra programming, so simplify related checking.

...

> +#include <linux/acpi.h>

Nope, you rather need property.h and mod_devicetable.h.

...

> +       if (pdev->dev.of_node) {
> +               devtype = of_device_get_match_data(&pdev->dev);
> +       } else {
> +               acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> +                                               &pdev->dev);
> +               if (acpi_id)
> +                       devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data;
> +       }

No, please use device_get_match_data() instead of the entire conditional block.


> +       if (pdev->dev.of_node) {
> +               if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> +       } else {
> +               if (acpi_match_device(pdev->dev.driver->acpi_match_table,
> +                                       &pdev->dev))
> +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> +       }

Yeah, no need to call acpi_match_device() here.
Instead use stuff from OF, like

if (of_device_is_compatible() || !(IS_ERR_OR_NULL(fwnode) ||
is_of_node(fwnode)))
(check the logic)

...

> +#ifdef CONFIG_ACPI

No ugly ifdeffery, please.

> +static const struct acpi_device_id gpio_acpi_ids[] = {
> +       {"NXP0031",},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
> +#endif
> +
>  static struct platform_driver mpc8xxx_plat_driver = {
>         .probe          = mpc8xxx_probe,
>         .remove         = mpc8xxx_remove,
>         .driver         = {
>                 .name = "gpio-mpc8xxx",
>                 .of_match_table = mpc8xxx_gpio_ids,

> +               .acpi_match_table = ACPI_PTR(gpio_acpi_ids),

Drop ACPI_PTR().

>         },
>  };


-- 
With Best Regards,
Andy Shevchenko

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

* RE: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-14 13:51 ` Andy Shevchenko
@ 2021-03-15  6:02   ` Ran Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Ran Wang @ 2021-03-15  6:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List

Hi Andy,

On Sunday, March 14, 2021 9:52 PM, Andy Shevchenko wrote:
> 
> On Fri, Mar 12, 2021 at 8:53 AM Ran Wang <ran.wang_1@nxp.com> wrote:
> 
> First of all, please add me to the Cc list for the next version of the patch.
> 
> > Current implementation only supports DT, now add ACPI support.
> >
> > Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
> 
> to devices
> 
> > LS1088A's GPIO have no extra programming, so simplify related checking.
> 
> ...
> 
> > +#include <linux/acpi.h>
> 
> Nope, you rather need property.h and mod_devicetable.h.

If I replace acpi.h, will encounter below error (even have added property.h mod_devicetable.h):

CC      drivers/gpio/gpio-mpc8xxx.o
drivers/gpio/gpio-mpc8xxx.c:439:1: warning: data definition has no type or storage class
  439 | MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
      | ^~~~~~~~~~~~~~~~~~~
drivers/gpio/gpio-mpc8xxx.c:439:1: error: type defaults to ‘int’ in declaration of ‘MODULE_DEVICE_TABLE’ [-Werror=implicit-int]
drivers/gpio/gpio-mpc8xxx.c:439:1: warning: parameter names (without types) in function declaration
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:271: drivers/gpio/gpio-mpc8xxx.o] Error 1
make[1]: *** [scripts/Makefile.build:514: drivers/gpio] Error 2
make: *** [Makefile:1849: drivers] Error 2
make: *** Waiting for unfinished jobs....

> ...
> 
> > +       if (pdev->dev.of_node) {
> > +               devtype = of_device_get_match_data(&pdev->dev);
> > +       } else {
> > +               acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table,
> > +                                               &pdev->dev);
> > +               if (acpi_id)
> > +                       devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data;
> > +       }
> 
> No, please use device_get_match_data() instead of the entire conditional block.

OK

> 
> > +       if (pdev->dev.of_node) {
> > +               if (of_device_is_compatible(np, "fsl,qoriq-gpio"))
> > +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> > +       } else {
> > +               if (acpi_match_device(pdev->dev.driver->acpi_match_table,
> > +                                       &pdev->dev))
> > +                       gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff);
> > +       }
> 
> Yeah, no need to call acpi_match_device() here.
> Instead use stuff from OF, like
> 
> if (of_device_is_compatible() || !(IS_ERR_OR_NULL(fwnode) ||
> is_of_node(fwnode)))
> (check the logic)

Got it!

> ...
> 
> > +#ifdef CONFIG_ACPI
> 
> No ugly ifdeffery, please.

Remove it will cause same compile error above.

> > +static const struct acpi_device_id gpio_acpi_ids[] = {
> > +       {"NXP0031",},
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids); #endif
> > +
> >  static struct platform_driver mpc8xxx_plat_driver = {
> >         .probe          = mpc8xxx_probe,
> >         .remove         = mpc8xxx_remove,m
> >         .driver         = {
> >                 .name = "gpio-mpc8xxx",
> >                 .of_match_table = mpc8xxx_gpio_ids,
> 
> > +               .acpi_match_table = ACPI_PTR(gpio_acpi_ids),
> 
> Drop ACPI_PTR().

Will encounter below error if !CONFIG_ACPI:

CC      drivers/gpio/gpio-mpc8xxx.o
drivers/gpio/gpio-mpc8xxx.c:449:23: error: ‘gpio_acpi_ids’ undeclared here (not in a function)
  449 |   .acpi_match_table = gpio_acpi_ids,
      |                       ^~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:271: drivers/gpio/gpio-mpc8xxx.o] Error 1
make[1]: *** [scripts/Makefile.build:514: drivers/gpio] Error 2
make[1]: *** Waiting for unfinished jobs....
  CC      net/core/page_pool.o
make: *** [Makefile:1849: drivers] Error 2
make: *** Waiting for unfinished jobs....

Thanks & Regards,
Ran



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

* RE: [PATCH] gpio: mpc8xxx: Add ACPI support
  2021-03-14  0:10   ` Michael Walle
@ 2021-03-15  6:12     ` Ran Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Ran Wang @ 2021-03-15  6:12 UTC (permalink / raw)
  To: Michael Walle, Bartosz Golaszewski; +Cc: Linus Walleij, linux-gpio, LKML

Hi Michael, Bartosz,

On Sunday, March 14, 2021 8:11 AM, Michael Walle wrote:
> 
> Am 2021-03-12 12:07, schrieb Bartosz Golaszewski:
> > On Fri, Mar 12, 2021 at 7:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
> >>
> >> Current implementation only supports DT, now add ACPI support.
> >>
> >> Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
> >> LS1088A's GPIO have no extra programming, so simplify related
> >> checking.
> >>
> >> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> >> ---
> >>  drivers/gpio/gpio-mpc8xxx.c | 50
> >> +++++++++++++++++++++++++++----------
> >>  1 file changed, 37 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpio-mpc8xxx.c
> >> b/drivers/gpio/gpio-mpc8xxx.c index 6dfca83bcd90..de5b7e17cde3 100644
> >> --- a/drivers/gpio/gpio-mpc8xxx.c
> >> +++ b/drivers/gpio/gpio-mpc8xxx.c
> >> @@ -9,6 +9,7 @@
> >>   * kind, whether express or implied.
> >>   */
> >>
> >> +#include <linux/acpi.h>
> >>  #include <linux/kernel.h>
> >>  #include <linux/init.h>
> >>  #include <linux/spinlock.h>
> >> @@ -292,8 +293,6 @@ static const struct of_device_id
> >> mpc8xxx_gpio_ids[] = {
> >>         { .compatible = "fsl,mpc5121-gpio", .data =
> >> &mpc512x_gpio_devtype, },
> >>         { .compatible = "fsl,mpc5125-gpio", .data =
> >> &mpc5125_gpio_devtype, },
> >>         { .compatible = "fsl,pq3-gpio",     },
> >> -       { .compatible = "fsl,ls1028a-gpio", },
> >> -       { .compatible = "fsl,ls1088a-gpio", },
> >
> > Why are you removing support for those?
> 
> I guess because it doesn't matter because usually
>   compatible = "fsl,ls1028a-gpio", "fsl,qoriq-gpio"; is used.

Yes,

>But I just had a look at the dt binding and it doesn't mandate it. So please keep it.

For now, strictly speaking, QorIQ pressors belong to Power Architecture
and Layerscape processor (LS1012A, LS1021A, LS1043A, LS1046A, LS1088A,
LS2088A, LX2160A, etc) belong to Arm Architecture

Although they are integrated the same GPIO controller IP with
minor difference (endian perspective), it would be find to use
SoC specific compatible + "qoriq-gpio" to make it work for all
Layerscape platforms (with "little-endian" accordingly).

But for mpc8xxx_gpio_ids, I think it would not be necessary
to list all LS/LX compatible strings. what do you say?

Regards,
Ran

> -michael
> 
> >
> > Bart
> >

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

end of thread, other threads:[~2021-03-15  6:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12  6:58 [PATCH] gpio: mpc8xxx: Add ACPI support Ran Wang
2021-03-12 11:07 ` Bartosz Golaszewski
2021-03-14  0:10   ` Michael Walle
2021-03-15  6:12     ` Ran Wang
2021-03-13  1:04 ` kernel test robot
2021-03-14 13:51 ` Andy Shevchenko
2021-03-15  6:02   ` Ran Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).