The top-level MFD driver now passes the device properties to the GPIO
cell via the software node. Use generic device property accessors and
stop using platform data. We can ignore the "ngpios" property here now
as it will be retrieved internally by GPIO core.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpio-timberdale.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-timberdale.c b/drivers/gpio/gpio-timberdale.c
index f488939dd00a8a7f332d3af27962a38a3b7e6ecf..7c34ea8a0ececf9432dbb16881eb53ee95d58441 100644
--- a/drivers/gpio/gpio-timberdale.c
+++ b/drivers/gpio/gpio-timberdale.c
@@ -14,7 +14,6 @@
#include <linux/platform_device.h>
#include <linux/irq.h>
#include <linux/io.h>
-#include <linux/timb_gpio.h>
#include <linux/interrupt.h>
#include <linux/slab.h>
@@ -225,19 +224,21 @@ static int timbgpio_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct gpio_chip *gc;
struct timbgpio *tgpio;
- struct timbgpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
int irq = platform_get_irq(pdev, 0);
- if (!pdata || pdata->nr_pins > 32) {
- dev_err(dev, "Invalid platform data\n");
- return -EINVAL;
- }
-
tgpio = devm_kzalloc(dev, sizeof(*tgpio), GFP_KERNEL);
if (!tgpio)
return -EINVAL;
- tgpio->irq_base = pdata->irq_base;
+ gc = &tgpio->gpio;
+
+ err = device_property_read_u32(dev, "intel,irq-base", &tgpio->irq_base);
+ if (err)
+ return err;
+
+ err = device_property_read_u32(dev, "intel,gpio-base", &gc->base);
+ if (err)
+ return err;
spin_lock_init(&tgpio->lock);
@@ -245,8 +246,6 @@ static int timbgpio_probe(struct platform_device *pdev)
if (IS_ERR(tgpio->membase))
return PTR_ERR(tgpio->membase);
- gc = &tgpio->gpio;
-
gc->label = dev_name(&pdev->dev);
gc->owner = THIS_MODULE;
gc->parent = &pdev->dev;
@@ -256,21 +255,22 @@ static int timbgpio_probe(struct platform_device *pdev)
gc->set = timbgpio_gpio_set;
gc->to_irq = (irq >= 0 && tgpio->irq_base > 0) ? timbgpio_to_irq : NULL;
gc->dbg_show = NULL;
- gc->base = pdata->gpio_base;
- gc->ngpio = pdata->nr_pins;
gc->can_sleep = false;
err = devm_gpiochip_add_data(&pdev->dev, gc, tgpio);
if (err)
return err;
+ if (gc->ngpio > 32)
+ return dev_err_probe(dev, -EINVAL, "Invalid number of pins\n");
+
/* make sure to disable interrupts */
iowrite32(0x0, tgpio->membase + TGPIO_IER);
if (irq < 0 || tgpio->irq_base <= 0)
return 0;
- for (i = 0; i < pdata->nr_pins; i++) {
+ for (i = 0; i < gc->ngpio; i++) {
irq_set_chip_and_handler(tgpio->irq_base + i,
&timbgpio_irqchip, handle_simple_irq);
irq_set_chip_data(tgpio->irq_base + i, tgpio);
--
2.47.3Hi Bartosz, On Fri, Mar 13, 2026 at 11:05 AM Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> wrote: > + err = device_property_read_u32(dev, "intel,gpio-base", &gc->base); I guess there is some background to why we do this crazy thing, like legacy. I guess if I want to change the world away from this I need to send a separate patch to set gc->base to -1 and see what happens... Otherwise the patch looks good to me! Yours, Linus Walleij
On Fri, Mar 13, 2026 at 11:04:49AM +0100, Bartosz Golaszewski wrote:
> The top-level MFD driver now passes the device properties to the GPIO
> cell via the software node. Use generic device property accessors and
> stop using platform data. We can ignore the "ngpios" property here now
> as it will be retrieved internally by GPIO core.
...
> + err = device_property_read_u32(dev, "intel,gpio-base", &gc->base);
In drivers/mfd/intel_quark_i2c_gpio.c we use 'gpio-base' and I prefer to have
it common since it's Linux only property for now. Alternatively patch that to
have a snps prefix.
> + if (err)
> + return err;
...
> err = devm_gpiochip_add_data(&pdev->dev, gc, tgpio);
> if (err)
> return err;
>
> + if (gc->ngpio > 32)
> + return dev_err_probe(dev, -EINVAL, "Invalid number of pins\n");
> +
> /* make sure to disable interrupts */
> iowrite32(0x0, tgpio->membase + TGPIO_IER);
>
> if (irq < 0 || tgpio->irq_base <= 0)
> return 0;
>
> - for (i = 0; i < pdata->nr_pins; i++) {
> + for (i = 0; i < gc->ngpio; i++) {
> irq_set_chip_and_handler(tgpio->irq_base + i,
> &timbgpio_irqchip, handle_simple_irq);
> irq_set_chip_data(tgpio->irq_base + i, tgpio);
Shouldn't this be done in the respective callbacks before the
devm_gpiochip_add_data() finishes? (Yes, it's not related to this
patch, but it is related to seems racy driver initialisation.
--
With Best Regards,
Andy Shevchenko
On Fri, Mar 13, 2026 at 11:21 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Mar 13, 2026 at 11:04:49AM +0100, Bartosz Golaszewski wrote:
> > The top-level MFD driver now passes the device properties to the GPIO
> > cell via the software node. Use generic device property accessors and
> > stop using platform data. We can ignore the "ngpios" property here now
> > as it will be retrieved internally by GPIO core.
>
> ...
>
> > + err = device_property_read_u32(dev, "intel,gpio-base", &gc->base);
>
> In drivers/mfd/intel_quark_i2c_gpio.c we use 'gpio-base' and I prefer to have
> it common since it's Linux only property for now. Alternatively patch that to
> have a snps prefix.
>
So "gpio-base" and "snps,irq-base" for the properties?
> > + if (err)
> > + return err;
>
> ...
>
> > err = devm_gpiochip_add_data(&pdev->dev, gc, tgpio);
> > if (err)
> > return err;
> >
> > + if (gc->ngpio > 32)
> > + return dev_err_probe(dev, -EINVAL, "Invalid number of pins\n");
> > +
> > /* make sure to disable interrupts */
> > iowrite32(0x0, tgpio->membase + TGPIO_IER);
> >
> > if (irq < 0 || tgpio->irq_base <= 0)
> > return 0;
> >
> > - for (i = 0; i < pdata->nr_pins; i++) {
> > + for (i = 0; i < gc->ngpio; i++) {
> > irq_set_chip_and_handler(tgpio->irq_base + i,
> > &timbgpio_irqchip, handle_simple_irq);
> > irq_set_chip_data(tgpio->irq_base + i, tgpio);
>
> Shouldn't this be done in the respective callbacks before the
> devm_gpiochip_add_data() finishes? (Yes, it's not related to this
> patch, but it is related to seems racy driver initialisation.
>
Sure, let's not it but I'm not going to do this as part of this
series, let me clean up all the headers first.
Bart
On Fri, Mar 13, 2026 at 02:27:36PM +0100, Bartosz Golaszewski wrote: > On Fri, Mar 13, 2026 at 11:21 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Mar 13, 2026 at 11:04:49AM +0100, Bartosz Golaszewski wrote: ... > > > + err = device_property_read_u32(dev, "intel,gpio-base", &gc->base); > > > > In drivers/mfd/intel_quark_i2c_gpio.c we use 'gpio-base' and I prefer to have > > it common since it's Linux only property for now. Alternatively patch that to > > have a snps prefix. > > So "gpio-base" and "snps,irq-base" for the properties? Either 'gpio-base' in all drivers that use it only as Linux property (currently 2 + potentially this one), OR fixing _there_ (in two drivers) by having 'snps,gpio-base' _there_, not in this driver. And if you choose the former (dropping the vendor prefix here), the 'irq-base' with it (vendor prefix) will look inconsistent. > > > + if (err) > > > + return err; -- With Best Regards, Andy Shevchenko
On Fri, Mar 13, 2026 at 12:21:54PM +0200, Andy Shevchenko wrote: > On Fri, Mar 13, 2026 at 11:04:49AM +0100, Bartosz Golaszewski wrote: ... > Shouldn't this be done in the respective callbacks before the > devm_gpiochip_add_data() finishes? (Yes, it's not related to this > patch, but it is related to seems racy driver initialisation. And with that being said, wouldn't be better to move the driver to one of the generic library (gpio-regmap / gpio-mmio)? Also it mentions FPGA in the header, I am wondering what that FPGA is. Perhaps we have already similar driver in the kernel from the FPGA vendor? -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.