Message ID | 20210312065855.37072-1-ran.wang_1@nxp.com |
---|---|
State | Superseded |
Headers | show |
Series | gpio: mpc8xxx: Add ACPI support | expand |
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 >
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
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
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
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
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 > >
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), }, };
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(-)