| Message ID | 1431707940-19372-5-git-send-email-jenskuske@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 15, 2015 at 06:38:54PM +0200, Jens Kuske wrote: > Add a remove function and export the init and remove function > to allow us to build the SoC specific drivers as modules. > > Signed-off-by: Jens Kuske <jenskuske@gmail.com> > --- > drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ > drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > index f8e171b..4ef6b3d 100644 > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > struct sunxi_pinctrl *pctl; > struct resource *res; > int i, ret, last_pin; > - struct clk *clk; > > pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); > if (!pctl) > @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > goto gpiochip_error; > } > > - clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(clk)) { > - ret = PTR_ERR(clk); > + pctl->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(pctl->clk)) { > + ret = PTR_ERR(pctl->clk); > goto gpiochip_error; > } > > - ret = clk_prepare_enable(clk); > + ret = clk_prepare_enable(pctl->clk); > if (ret) > goto gpiochip_error; > > @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > return 0; > > clk_error: > - clk_disable_unprepare(clk); > + clk_disable_unprepare(pctl->clk); > gpiochip_error: > gpiochip_remove(pctl->chip); > pinctrl_error: > pinctrl_unregister(pctl->pctl_dev); > return ret; > } > +EXPORT_SYMBOL(sunxi_pinctrl_init); > + > +int sunxi_pinctrl_remove(struct platform_device *pdev) > +{ > + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); > + > + gpiochip_remove(pctl->chip); > + pinctrl_unregister(pctl->pctl_dev); > + > + clk_disable_unprepare(pctl->clk); We should also remove the domain and the interrupt mapping here. Maxime
Hi, On 05/17/15 16:19, Maxime Ripard wrote: > On Fri, May 15, 2015 at 06:38:54PM +0200, Jens Kuske wrote: >> Add a remove function and export the init and remove function >> to allow us to build the SoC specific drivers as modules. >> >> Signed-off-by: Jens Kuske <jenskuske@gmail.com> >> --- >> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ >> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ >> 2 files changed, 21 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> index f8e171b..4ef6b3d 100644 >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> struct sunxi_pinctrl *pctl; >> struct resource *res; >> int i, ret, last_pin; >> - struct clk *clk; >> >> pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); >> if (!pctl) >> @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> goto gpiochip_error; >> } >> >> - clk = devm_clk_get(&pdev->dev, NULL); >> - if (IS_ERR(clk)) { >> - ret = PTR_ERR(clk); >> + pctl->clk = devm_clk_get(&pdev->dev, NULL); >> + if (IS_ERR(pctl->clk)) { >> + ret = PTR_ERR(pctl->clk); >> goto gpiochip_error; >> } >> >> - ret = clk_prepare_enable(clk); >> + ret = clk_prepare_enable(pctl->clk); >> if (ret) >> goto gpiochip_error; >> >> @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> return 0; >> >> clk_error: >> - clk_disable_unprepare(clk); >> + clk_disable_unprepare(pctl->clk); >> gpiochip_error: >> gpiochip_remove(pctl->chip); >> pinctrl_error: >> pinctrl_unregister(pctl->pctl_dev); >> return ret; >> } >> +EXPORT_SYMBOL(sunxi_pinctrl_init); >> + >> +int sunxi_pinctrl_remove(struct platform_device *pdev) >> +{ >> + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); >> + >> + gpiochip_remove(pctl->chip); >> + pinctrl_unregister(pctl->pctl_dev); >> + >> + clk_disable_unprepare(pctl->clk); > > We should also remove the domain and the interrupt mapping here. Ouch, I missed that. Only looked at the *_error: labels. Apart from that, currently the kernel panics some seconds after removing the pinctrl module because mmc wants to access a gpio. Can this be prevented somehow? I think pinctrl must not be removed once other devices use any pin-related things. Jens
On Mon, May 18, 2015 at 11:32:31AM +0200, Jens Kuske wrote: > Hi, > > On 05/17/15 16:19, Maxime Ripard wrote: > > On Fri, May 15, 2015 at 06:38:54PM +0200, Jens Kuske wrote: > >> Add a remove function and export the init and remove function > >> to allow us to build the SoC specific drivers as modules. > >> > >> Signed-off-by: Jens Kuske <jenskuske@gmail.com> > >> --- > >> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ > >> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ > >> 2 files changed, 21 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> index f8e171b..4ef6b3d 100644 > >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> struct sunxi_pinctrl *pctl; > >> struct resource *res; > >> int i, ret, last_pin; > >> - struct clk *clk; > >> > >> pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); > >> if (!pctl) > >> @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> goto gpiochip_error; > >> } > >> > >> - clk = devm_clk_get(&pdev->dev, NULL); > >> - if (IS_ERR(clk)) { > >> - ret = PTR_ERR(clk); > >> + pctl->clk = devm_clk_get(&pdev->dev, NULL); > >> + if (IS_ERR(pctl->clk)) { > >> + ret = PTR_ERR(pctl->clk); > >> goto gpiochip_error; > >> } > >> > >> - ret = clk_prepare_enable(clk); > >> + ret = clk_prepare_enable(pctl->clk); > >> if (ret) > >> goto gpiochip_error; > >> > >> @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> return 0; > >> > >> clk_error: > >> - clk_disable_unprepare(clk); > >> + clk_disable_unprepare(pctl->clk); > >> gpiochip_error: > >> gpiochip_remove(pctl->chip); > >> pinctrl_error: > >> pinctrl_unregister(pctl->pctl_dev); > >> return ret; > >> } > >> +EXPORT_SYMBOL(sunxi_pinctrl_init); > >> + > >> +int sunxi_pinctrl_remove(struct platform_device *pdev) > >> +{ > >> + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); > >> + > >> + gpiochip_remove(pctl->chip); > >> + pinctrl_unregister(pctl->pctl_dev); > >> + > >> + clk_disable_unprepare(pctl->clk); > > > > We should also remove the domain and the interrupt mapping here. > > Ouch, I missed that. Only looked at the *_error: labels. > > Apart from that, currently the kernel panics some seconds after removing > the pinctrl module because mmc wants to access a gpio. Can this be > prevented somehow? I think pinctrl must not be removed once other > devices use any pin-related things. pinctrl_unregister doesn't look like it cares about whether or not there's users left in the system. Maybe the easiest path would be to just make this builtin like Paul suggested then ... :/ Maxime
On Tue, May 19, 2015 at 3:55 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, May 18, 2015 at 11:32:31AM +0200, Jens Kuske wrote: >> Hi, >> >> On 05/17/15 16:19, Maxime Ripard wrote: >> > On Fri, May 15, 2015 at 06:38:54PM +0200, Jens Kuske wrote: >> >> Add a remove function and export the init and remove function >> >> to allow us to build the SoC specific drivers as modules. >> >> >> >> Signed-off-by: Jens Kuske <jenskuske@gmail.com> >> >> --- >> >> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ >> >> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ >> >> 2 files changed, 21 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> >> index f8e171b..4ef6b3d 100644 >> >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c >> >> @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> >> struct sunxi_pinctrl *pctl; >> >> struct resource *res; >> >> int i, ret, last_pin; >> >> - struct clk *clk; >> >> >> >> pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); >> >> if (!pctl) >> >> @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> >> goto gpiochip_error; >> >> } >> >> >> >> - clk = devm_clk_get(&pdev->dev, NULL); >> >> - if (IS_ERR(clk)) { >> >> - ret = PTR_ERR(clk); >> >> + pctl->clk = devm_clk_get(&pdev->dev, NULL); >> >> + if (IS_ERR(pctl->clk)) { >> >> + ret = PTR_ERR(pctl->clk); >> >> goto gpiochip_error; >> >> } >> >> >> >> - ret = clk_prepare_enable(clk); >> >> + ret = clk_prepare_enable(pctl->clk); >> >> if (ret) >> >> goto gpiochip_error; >> >> >> >> @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, >> >> return 0; >> >> >> >> clk_error: >> >> - clk_disable_unprepare(clk); >> >> + clk_disable_unprepare(pctl->clk); >> >> gpiochip_error: >> >> gpiochip_remove(pctl->chip); >> >> pinctrl_error: >> >> pinctrl_unregister(pctl->pctl_dev); >> >> return ret; >> >> } >> >> +EXPORT_SYMBOL(sunxi_pinctrl_init); >> >> + >> >> +int sunxi_pinctrl_remove(struct platform_device *pdev) >> >> +{ >> >> + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); >> >> + >> >> + gpiochip_remove(pctl->chip); >> >> + pinctrl_unregister(pctl->pctl_dev); >> >> + >> >> + clk_disable_unprepare(pctl->clk); >> > >> > We should also remove the domain and the interrupt mapping here. >> >> Ouch, I missed that. Only looked at the *_error: labels. >> >> Apart from that, currently the kernel panics some seconds after removing >> the pinctrl module because mmc wants to access a gpio. Can this be >> prevented somehow? I think pinctrl must not be removed once other >> devices use any pin-related things. > > pinctrl_unregister doesn't look like it cares about whether or not > there's users left in the system. > > Maybe the easiest path would be to just make this builtin like Paul > suggested then ... :/ Is there a way to mark modules as not removable? At least we can keep the multi-platform kernel image small.
On Tue, May 19, 2015 at 04:02:39PM +0800, Chen-Yu Tsai wrote: > On Tue, May 19, 2015 at 3:55 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Mon, May 18, 2015 at 11:32:31AM +0200, Jens Kuske wrote: > >> Hi, > >> > >> On 05/17/15 16:19, Maxime Ripard wrote: > >> > On Fri, May 15, 2015 at 06:38:54PM +0200, Jens Kuske wrote: > >> >> Add a remove function and export the init and remove function > >> >> to allow us to build the SoC specific drivers as modules. > >> >> > >> >> Signed-off-by: Jens Kuske <jenskuske@gmail.com> > >> >> --- > >> >> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ > >> >> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ > >> >> 2 files changed, 21 insertions(+), 6 deletions(-) > >> >> > >> >> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> >> index f8e171b..4ef6b3d 100644 > >> >> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> >> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > >> >> @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> >> struct sunxi_pinctrl *pctl; > >> >> struct resource *res; > >> >> int i, ret, last_pin; > >> >> - struct clk *clk; > >> >> > >> >> pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); > >> >> if (!pctl) > >> >> @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> >> goto gpiochip_error; > >> >> } > >> >> > >> >> - clk = devm_clk_get(&pdev->dev, NULL); > >> >> - if (IS_ERR(clk)) { > >> >> - ret = PTR_ERR(clk); > >> >> + pctl->clk = devm_clk_get(&pdev->dev, NULL); > >> >> + if (IS_ERR(pctl->clk)) { > >> >> + ret = PTR_ERR(pctl->clk); > >> >> goto gpiochip_error; > >> >> } > >> >> > >> >> - ret = clk_prepare_enable(clk); > >> >> + ret = clk_prepare_enable(pctl->clk); > >> >> if (ret) > >> >> goto gpiochip_error; > >> >> > >> >> @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, > >> >> return 0; > >> >> > >> >> clk_error: > >> >> - clk_disable_unprepare(clk); > >> >> + clk_disable_unprepare(pctl->clk); > >> >> gpiochip_error: > >> >> gpiochip_remove(pctl->chip); > >> >> pinctrl_error: > >> >> pinctrl_unregister(pctl->pctl_dev); > >> >> return ret; > >> >> } > >> >> +EXPORT_SYMBOL(sunxi_pinctrl_init); > >> >> + > >> >> +int sunxi_pinctrl_remove(struct platform_device *pdev) > >> >> +{ > >> >> + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); > >> >> + > >> >> + gpiochip_remove(pctl->chip); > >> >> + pinctrl_unregister(pctl->pctl_dev); > >> >> + > >> >> + clk_disable_unprepare(pctl->clk); > >> > > >> > We should also remove the domain and the interrupt mapping here. > >> > >> Ouch, I missed that. Only looked at the *_error: labels. > >> > >> Apart from that, currently the kernel panics some seconds after removing > >> the pinctrl module because mmc wants to access a gpio. Can this be > >> prevented somehow? I think pinctrl must not be removed once other > >> devices use any pin-related things. > > > > pinctrl_unregister doesn't look like it cares about whether or not > > there's users left in the system. > > > > Maybe the easiest path would be to just make this builtin like Paul > > suggested then ... :/ > > Is there a way to mark modules as not removable? At least we can keep > the multi-platform kernel image small. If there's no module_exit, the module will only be removable through a rmmod -f, which seems like an acceptable solution. Maxime
On Tue, May 19, 2015 at 9:55 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: >> Apart from that, currently the kernel panics some seconds after removing >> the pinctrl module because mmc wants to access a gpio. Can this be >> prevented somehow? I think pinctrl must not be removed once other >> devices use any pin-related things. > > pinctrl_unregister doesn't look like it cares about whether or not > there's users left in the system. Both GPIO and pinctrl has this problem, Johan Hovold has been looking at fixing the GPIO side of things. Patches welcome. Yours, Linus Walleij
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index f8e171b..4ef6b3d 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -856,7 +856,6 @@ int sunxi_pinctrl_init(struct platform_device *pdev, struct sunxi_pinctrl *pctl; struct resource *res; int i, ret, last_pin; - struct clk *clk; pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL); if (!pctl) @@ -954,13 +953,13 @@ int sunxi_pinctrl_init(struct platform_device *pdev, goto gpiochip_error; } - clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(clk)) { - ret = PTR_ERR(clk); + pctl->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(pctl->clk)) { + ret = PTR_ERR(pctl->clk); goto gpiochip_error; } - ret = clk_prepare_enable(clk); + ret = clk_prepare_enable(pctl->clk); if (ret) goto gpiochip_error; @@ -1015,10 +1014,24 @@ int sunxi_pinctrl_init(struct platform_device *pdev, return 0; clk_error: - clk_disable_unprepare(clk); + clk_disable_unprepare(pctl->clk); gpiochip_error: gpiochip_remove(pctl->chip); pinctrl_error: pinctrl_unregister(pctl->pctl_dev); return ret; } +EXPORT_SYMBOL(sunxi_pinctrl_init); + +int sunxi_pinctrl_remove(struct platform_device *pdev) +{ + struct sunxi_pinctrl *pctl = platform_get_drvdata(pdev); + + gpiochip_remove(pctl->chip); + pinctrl_unregister(pctl->pctl_dev); + + clk_disable_unprepare(pctl->clk); + + return 0; +} +EXPORT_SYMBOL(sunxi_pinctrl_remove); diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h index e248e81..4442676 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h @@ -126,6 +126,7 @@ struct sunxi_pinctrl { unsigned *irq_array; spinlock_t lock; struct pinctrl_dev *pctl_dev; + struct clk *clk; }; #define SUNXI_PIN(_pin, ...) \ @@ -285,5 +286,6 @@ static inline u32 sunxi_irq_status_offset(u16 irq) int sunxi_pinctrl_init(struct platform_device *pdev, const struct sunxi_pinctrl_desc *desc); +int sunxi_pinctrl_remove(struct platform_device *pdev); #endif /* __PINCTRL_SUNXI_H */
Add a remove function and export the init and remove function to allow us to build the SoC specific drivers as modules. Signed-off-by: Jens Kuske <jenskuske@gmail.com> --- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 25 +++++++++++++++++++------ drivers/pinctrl/sunxi/pinctrl-sunxi.h | 2 ++ 2 files changed, 21 insertions(+), 6 deletions(-)