| Message ID | 20210913224926.1260726-5-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gpio/pinctrl-rockchip: Fixes for the recently separated gpio/pinctrl driver | expand |
On Tue, Sep 14, 2021 at 12:49 AM Heiko Stuebner <heiko@sntech.de> wrote: > Fetch the output settings the pinctrl driver may have created > for pinctrl hogs and set the relevant pins as requested. > > Fixes: 9ce9a02039de ("pinctrl/rockchip: drop the gpio related codes") > Signed-off-by: Heiko Stuebner <heiko@sntech.de> Since this patch depends on patch 4/4 I applied this to the pinctrl tree as well. I still think this looks a bit kludgy but can't think of anything better right now and we need a fix for the problem so this goes in. But we need to think of something better, Yours, Linus Walleij
Hi Linus, Am Samstag, 18. September 2021, 01:38:08 CEST schrieb Linus Walleij: > On Tue, Sep 14, 2021 at 12:49 AM Heiko Stuebner <heiko@sntech.de> wrote: > > > Fetch the output settings the pinctrl driver may have created > > for pinctrl hogs and set the relevant pins as requested. > > > > Fixes: 9ce9a02039de ("pinctrl/rockchip: drop the gpio related codes") > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > Since this patch depends on patch 4/4 I applied this to the pinctrl > tree as well. > > I still think this looks a bit kludgy but can't think of anything better > right now and we need a fix for the problem so this goes in. > > But we need to think of something better, I'm all ears :-) . And yes I do agree with you that this is not very elegant right now. The issue is that the pinconf part for PIN_CONFIG_OUTPUT is actually using the gpio controller to realize this setting. So when this ends up in a pinctrl-hog, stuff explodes while probing the first pinctrl part. I guess one way would be to somehow only do the pinctrl-hogs _after_ all parts have probed. Thinking about this, the component framework may be one option? And then adding a pinctr-register / init+enable variant where the pinctrl hogs can be aquired separately, not as part of pinctrl_enable? Or maybe I'm thinking way too complex and a way easier solution is around the corner ;-) . Heiko
On Sat, Sep 18, 2021 at 2:00 AM Heiko Stübner <heiko@sntech.de> wrote: > The issue is that the pinconf part for PIN_CONFIG_OUTPUT is actually > using the gpio controller to realize this setting. So when this ends up > in a pinctrl-hog, stuff explodes while probing the first pinctrl part. The Nomadik driver has something similar, I came up with a solution ages ago which isn't elegant either, so it's not like I'm any better :/ commit ab4a936247561cd998913bab5f15e3d3eaed1f9e "pinctrl: nomadik: assure GPIO chips are populated" > Thinking about this, the component framework may be one option? > And then adding a pinctr-register / init+enable variant where the > pinctrl hogs can be aquired separately, not as part of pinctrl_enable? Check out my commit, but the component framework is what we should ideally use (IMO) when drivers depend on each other so I think you are right. Yours, Linus Walleij
diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c index 3335bd57761d..cf1b465db8c3 100644 --- a/drivers/gpio/gpio-rockchip.c +++ b/drivers/gpio/gpio-rockchip.c @@ -689,6 +689,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev) struct device_node *pctlnp = of_get_parent(np); struct pinctrl_dev *pctldev = NULL; struct rockchip_pin_bank *bank = NULL; + struct rockchip_pin_output_deferred *cfg; static int gpio; int id, ret; @@ -716,12 +717,33 @@ static int rockchip_gpio_probe(struct platform_device *pdev) if (ret) return ret; + /* + * Prevent clashes with a deferred output setting + * being added right at this moment. + */ + mutex_lock(&bank->deferred_lock); + ret = rockchip_gpiolib_register(bank); if (ret) { clk_disable_unprepare(bank->clk); + mutex_unlock(&bank->deferred_lock); return ret; } + while (!list_empty(&bank->deferred_output)) { + cfg = list_first_entry(&bank->deferred_output, + struct rockchip_pin_output_deferred, head); + list_del(&cfg->head); + + ret = rockchip_gpio_direction_output(&bank->gpio_chip, cfg->pin, cfg->arg); + if (ret) + dev_warn(dev, "setting output pin %u to %u failed\n", cfg->pin, cfg->arg); + + kfree(cfg); + } + + mutex_unlock(&bank->deferred_lock); + platform_set_drvdata(pdev, bank); dev_info(dev, "probed %pOF\n", np);
Fetch the output settings the pinctrl driver may have created for pinctrl hogs and set the relevant pins as requested. Fixes: 9ce9a02039de ("pinctrl/rockchip: drop the gpio related codes") Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpio/gpio-rockchip.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)