| Message ID | 20250503-pinctrl-msm-fix-v1-3-da9b7f6408f4@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: qcom: several fixes for the pinctrl-msm code | expand |
On 03/05/2025 08:32, Dmitry Baryshkov wrote: > In order to simplify cleanup actions, use devres-enabled version of > gpiochip_add_data(). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > index 88dd462516c76d58b43d49accbddeea38af8f1ec..b2e8f7b3f3e3d5d232b2bd60e5cace62b21ffb03 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > @@ -1449,7 +1449,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > girq->handler = handle_bad_irq; > girq->parents[0] = pctrl->irq; > > - ret = gpiochip_add_data(&pctrl->chip, pctrl); > + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); > if (ret) { > dev_err(pctrl->dev, "Failed register gpiochip\n"); > return ret; > @@ -1470,7 +1470,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > dev_name(pctrl->dev), 0, 0, chip->ngpio); > if (ret) { > dev_err(pctrl->dev, "Failed to add pin range\n"); > - gpiochip_remove(&pctrl->chip); > return ret; > } > } > @@ -1608,9 +1607,6 @@ EXPORT_SYMBOL(msm_pinctrl_probe); > > void msm_pinctrl_remove(struct platform_device *pdev) > { > - struct msm_pinctrl *pctrl = platform_get_drvdata(pdev); > - > - gpiochip_remove(&pctrl->chip); > } > EXPORT_SYMBOL(msm_pinctrl_remove); I suppose this should also make the time window when the GPIO chip has gone, but pinmux is still there (to call the 'gpiochip_line_is_valid()') smaller. FWIW: Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> Yours, -- Matti
On Sat, 3 May 2025 at 07:32, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > In order to simplify cleanup actions, use devres-enabled version of > gpiochip_add_data(). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > index 88dd462516c76d58b43d49accbddeea38af8f1ec..b2e8f7b3f3e3d5d232b2bd60e5cace62b21ffb03 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > @@ -1449,7 +1449,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > girq->handler = handle_bad_irq; > girq->parents[0] = pctrl->irq; > > - ret = gpiochip_add_data(&pctrl->chip, pctrl); > + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); > if (ret) { > dev_err(pctrl->dev, "Failed register gpiochip\n"); > return ret; > @@ -1470,7 +1470,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > dev_name(pctrl->dev), 0, 0, chip->ngpio); > if (ret) { > dev_err(pctrl->dev, "Failed to add pin range\n"); > - gpiochip_remove(&pctrl->chip); > return ret; > } > } > @@ -1608,9 +1607,6 @@ EXPORT_SYMBOL(msm_pinctrl_probe); > > void msm_pinctrl_remove(struct platform_device *pdev) > { > - struct msm_pinctrl *pctrl = platform_get_drvdata(pdev); > - > - gpiochip_remove(&pctrl->chip); > } > EXPORT_SYMBOL(msm_pinctrl_remove); > > > -- > 2.39.5 > If you're at it then why not remove this function here and the callback assignment throughout the pinctrl/qcom/ directory? Bart
On Tue, 6 May 2025 at 19:18, Bartosz Golaszewski <bartosz.golaszewski@linaro.org> wrote: > > On Sat, 3 May 2025 at 07:32, Dmitry Baryshkov > <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > In order to simplify cleanup actions, use devres-enabled version of > > gpiochip_add_data(). > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > > --- > > drivers/pinctrl/qcom/pinctrl-msm.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > > index 88dd462516c76d58b43d49accbddeea38af8f1ec..b2e8f7b3f3e3d5d232b2bd60e5cace62b21ffb03 100644 > > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > > @@ -1449,7 +1449,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > > girq->handler = handle_bad_irq; > > girq->parents[0] = pctrl->irq; > > > > - ret = gpiochip_add_data(&pctrl->chip, pctrl); > > + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); > > if (ret) { > > dev_err(pctrl->dev, "Failed register gpiochip\n"); > > return ret; > > @@ -1470,7 +1470,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > > dev_name(pctrl->dev), 0, 0, chip->ngpio); > > if (ret) { > > dev_err(pctrl->dev, "Failed to add pin range\n"); > > - gpiochip_remove(&pctrl->chip); > > return ret; > > } > > } > > @@ -1608,9 +1607,6 @@ EXPORT_SYMBOL(msm_pinctrl_probe); > > > > void msm_pinctrl_remove(struct platform_device *pdev) > > { > > - struct msm_pinctrl *pctrl = platform_get_drvdata(pdev); > > - > > - gpiochip_remove(&pctrl->chip); > > } > > EXPORT_SYMBOL(msm_pinctrl_remove); > > > > > > -- > > 2.39.5 > > > > If you're at it then why not remove this function here and the > callback assignment throughout the pinctrl/qcom/ directory? > > Bart Ah, it's in the next patch. I'd make it one commit though, no reason to split it IMO. Bartosz
On Tue, May 06, 2025 at 07:23:10PM +0200, Bartosz Golaszewski wrote: > On Tue, 6 May 2025 at 19:18, Bartosz Golaszewski > <bartosz.golaszewski@linaro.org> wrote: > > > > On Sat, 3 May 2025 at 07:32, Dmitry Baryshkov > > <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > > > In order to simplify cleanup actions, use devres-enabled version of > > > gpiochip_add_data(). > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > > > --- > > > drivers/pinctrl/qcom/pinctrl-msm.c | 6 +----- > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > > > index 88dd462516c76d58b43d49accbddeea38af8f1ec..b2e8f7b3f3e3d5d232b2bd60e5cace62b21ffb03 100644 > > > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > > > @@ -1449,7 +1449,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > > > girq->handler = handle_bad_irq; > > > girq->parents[0] = pctrl->irq; > > > > > > - ret = gpiochip_add_data(&pctrl->chip, pctrl); > > > + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); > > > if (ret) { > > > dev_err(pctrl->dev, "Failed register gpiochip\n"); > > > return ret; > > > @@ -1470,7 +1470,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) > > > dev_name(pctrl->dev), 0, 0, chip->ngpio); > > > if (ret) { > > > dev_err(pctrl->dev, "Failed to add pin range\n"); > > > - gpiochip_remove(&pctrl->chip); > > > return ret; > > > } > > > } > > > @@ -1608,9 +1607,6 @@ EXPORT_SYMBOL(msm_pinctrl_probe); > > > > > > void msm_pinctrl_remove(struct platform_device *pdev) > > > { > > > - struct msm_pinctrl *pctrl = platform_get_drvdata(pdev); > > > - > > > - gpiochip_remove(&pctrl->chip); > > > } > > > EXPORT_SYMBOL(msm_pinctrl_remove); > > > > > > > > > -- > > > 2.39.5 > > > > > > > If you're at it then why not remove this function here and the > > callback assignment throughout the pinctrl/qcom/ directory? > > > > Bart > > Ah, it's in the next patch. I'd make it one commit though, no reason > to split it IMO. Up to you, but from my POV it's cleaner this way: first patch removes the contents, second one removes the function. Otherwise it's too easy to loose the functional changes (of gpiochip_remove() removal) in the noise of updating all the platform files. If you wish, I can add a note to the commit message telling that the actual function will be dropped in the next commit.
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 88dd462516c76d58b43d49accbddeea38af8f1ec..b2e8f7b3f3e3d5d232b2bd60e5cace62b21ffb03 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -1449,7 +1449,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) girq->handler = handle_bad_irq; girq->parents[0] = pctrl->irq; - ret = gpiochip_add_data(&pctrl->chip, pctrl); + ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl); if (ret) { dev_err(pctrl->dev, "Failed register gpiochip\n"); return ret; @@ -1470,7 +1470,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) dev_name(pctrl->dev), 0, 0, chip->ngpio); if (ret) { dev_err(pctrl->dev, "Failed to add pin range\n"); - gpiochip_remove(&pctrl->chip); return ret; } } @@ -1608,9 +1607,6 @@ EXPORT_SYMBOL(msm_pinctrl_probe); void msm_pinctrl_remove(struct platform_device *pdev) { - struct msm_pinctrl *pctrl = platform_get_drvdata(pdev); - - gpiochip_remove(&pctrl->chip); } EXPORT_SYMBOL(msm_pinctrl_remove);
In order to simplify cleanup actions, use devres-enabled version of gpiochip_add_data(). Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/pinctrl/qcom/pinctrl-msm.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)