This fixes a semantic ordering issue as we need to add pin ranges before adding gpiochips when gpiochips use pin control as a backend: as it needs to talk to the pin control backend during initialization, the backend needs to be there already. Other drivers in the tree using pincontrol as backend do not necessarily have this problem, as they might not need to access the pincontrol portions during initialization. Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> --- Hans: would be great if you could test this. I can't even compiletest right now because of a slow machine as workhorse... --- drivers/gpio/gpio-merrifield.c | 18 +++++++++++------- drivers/pinctrl/intel/pinctrl-baytrail.c | 14 +++++++++----- drivers/pinctrl/intel/pinctrl-cherryview.c | 16 ++++++++++------ drivers/pinctrl/intel/pinctrl-intel.c | 16 ++++++++++------ drivers/pinctrl/pinctrl-amd.c | 14 +++++++++----- 5 files changed, 49 insertions(+), 29 deletions(-) diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c index 2f1e9da81c1e..195e253cb561 100644 --- a/drivers/gpio/gpio-merrifield.c +++ b/drivers/gpio/gpio-merrifield.c @@ -463,13 +463,10 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id girq->default_type = IRQ_TYPE_NONE; girq->handler = handle_bad_irq; - pci_set_drvdata(pdev, priv); - retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv); - if (retval) { - dev_err(&pdev->dev, "gpiochip_add error %d\n", retval); - return retval; - } - + /* + * Needs to happen first since the gpiochip is using pin + * control as back-end. + */ pinctrl_dev_name = mrfld_gpio_get_pinctrl_dev_name(priv); for (i = 0; i < ARRAY_SIZE(mrfld_gpio_ranges); i++) { range = &mrfld_gpio_ranges[i]; @@ -484,6 +481,13 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id } } + pci_set_drvdata(pdev, priv); + retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv); + if (retval) { + dev_err(&pdev->dev, "gpiochip_add error %d\n", retval); + return retval; + } + return 0; } diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index beb26550c25f..b308567c5153 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -1549,16 +1549,20 @@ static int byt_gpio_probe(struct byt_gpio *vg) girq->handler = handle_bad_irq; } - ret = devm_gpiochip_add_data(&vg->pdev->dev, gc, vg); + /* + * Needs to happen first since the gpiochip is using pin + * control as back-end. + */ + ret = gpiochip_add_pin_range(gc, dev_name(&vg->pdev->dev), + 0, 0, vg->soc_data->npins); if (ret) { - dev_err(&vg->pdev->dev, "failed adding byt-gpio chip\n"); + dev_err(&vg->pdev->dev, "failed to add GPIO pin range\n"); return ret; } - ret = gpiochip_add_pin_range(&vg->chip, dev_name(&vg->pdev->dev), - 0, 0, vg->soc_data->npins); + ret = devm_gpiochip_add_data(&vg->pdev->dev, gc, vg); if (ret) { - dev_err(&vg->pdev->dev, "failed to add GPIO pin range\n"); + dev_err(&vg->pdev->dev, "failed adding byt-gpio chip\n"); return ret; } diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index dff2a81250b6..13144e192c1f 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -1572,12 +1572,10 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) if (need_valid_mask) chip->irq.init_valid_mask = chv_init_irq_valid_mask; - ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl); - if (ret) { - dev_err(pctrl->dev, "Failed to register gpiochip\n"); - return ret; - } - + /* + * Needs to happen first since the gpiochip is using pin + * control as back-end. + */ for (i = 0; i < community->ngpio_ranges; i++) { range = &community->gpio_ranges[i]; ret = gpiochip_add_pin_range(chip, dev_name(pctrl->dev), @@ -1589,6 +1587,12 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) } } + ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl); + if (ret) { + dev_err(pctrl->dev, "Failed to register gpiochip\n"); + return ret; + } + /* * The same set of machines in chv_no_valid_mask[] have incorrectly * configured GPIOs that generate spurious interrupts so we use diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c index b54b27228ad9..78afcf13c444 100644 --- a/drivers/pinctrl/intel/pinctrl-intel.c +++ b/drivers/pinctrl/intel/pinctrl-intel.c @@ -1225,12 +1225,10 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq) pctrl->irqchip.irq_set_wake = intel_gpio_irq_wake; pctrl->irqchip.flags = IRQCHIP_MASK_ON_SUSPEND; - ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); - if (ret) { - dev_err(pctrl->dev, "failed to register gpiochip\n"); - return ret; - } - + /* + * Needs to happen first since the gpiochip is using pin + * control as back-end. + */ for (i = 0; i < pctrl->ncommunities; i++) { struct intel_community *community = &pctrl->communities[i]; @@ -1241,6 +1239,12 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq) } } + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); + if (ret) { + dev_err(pctrl->dev, "failed to register gpiochip\n"); + return ret; + } + /* * We need to request the interrupt here (instead of providing chip * to the irq directly) because on some platforms several GPIO diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c index 2c61141519f8..3637059083ff 100644 --- a/drivers/pinctrl/pinctrl-amd.c +++ b/drivers/pinctrl/pinctrl-amd.c @@ -912,17 +912,21 @@ static int amd_gpio_probe(struct platform_device *pdev) return PTR_ERR(gpio_dev->pctrl); } - ret = gpiochip_add_data(&gpio_dev->gc, gpio_dev); - if (ret) - return ret; - + /* + * Needs to happen first since the gpiochip is using pin + * control as back-end. + */ ret = gpiochip_add_pin_range(&gpio_dev->gc, dev_name(&pdev->dev), 0, 0, gpio_dev->gc.ngpio); if (ret) { dev_err(&pdev->dev, "Failed to add pin range\n"); - goto out2; + return ret; } + ret = gpiochip_add_data(&gpio_dev->gc, gpio_dev); + if (ret) + return ret; + ret = gpiochip_irqchip_add(&gpio_dev->gc, &amd_gpio_irqchip, 0, -- 2.21.0