From patchwork Tue Jan 10 19:19:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Lindgren X-Patchwork-Id: 9508429 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 880FD60231 for ; Tue, 10 Jan 2017 19:22:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7B7CC28553 for ; Tue, 10 Jan 2017 19:22:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 704382859E; Tue, 10 Jan 2017 19:22:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4C12228596 for ; Tue, 10 Jan 2017 19:22:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S940830AbdAJTTt (ORCPT ); Tue, 10 Jan 2017 14:19:49 -0500 Received: from muru.com ([72.249.23.125]:56240 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933330AbdAJTTQ (ORCPT ); Tue, 10 Jan 2017 14:19:16 -0500 Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 3E54B82CA; Tue, 10 Jan 2017 19:19:52 +0000 (UTC) Date: Tue, 10 Jan 2017 11:19:09 -0800 From: Tony Lindgren To: Geert Uytterhoeven Cc: Linus Walleij , Haojian Zhuang , Masahiro Yamada , Grygorii Strashko , Nishanth Menon , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-omap@vger.kernel.org" , Linux-Renesas Subject: Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs Message-ID: <20170110191908.GV2630@atomide.com> References: <20161227172003.6517-1-tony@atomide.com> <20161227172003.6517-2-tony@atomide.com> <20170110153045.GS2630@atomide.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170110153045.GS2630@atomide.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP * Tony Lindgren [170110 07:32]: > * Geert Uytterhoeven [170110 06:09]: > > Hi Tony, > > > > On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren wrote: > > > Having the pin control framework call pin controller functions > > > before it's probe has finished is not nice as the pin controller > > > device driver does not yet have struct pinctrl_dev handle. > > > > > > Let's fix this issue by adding deferred work for late init. This is > > > needed to be able to add pinctrl generic helper functions that expect > > > to know struct pinctrl_dev handle. Note that we now need to call > > > create_pinctrl() directly as we don't want to add the pin controller > > > to the list of controllers until the hogs are claimed. We also need > > > to pass the pinctrl_dev to the device tree parser functions as they > > > otherwise won't find the right controller at this point. > > > > > > Signed-off-by: Tony Lindgren > > > > I believe this patch causes a regression on r8a7740/armadillo, where the > > pin controller is also a GPIO controller, and lcd0 needs a hog > > (cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts): > > > > -GPIO line 176 (lcd0) hogged as output/high > > -sh-pfc e6050000.pfc: r8a7740_pfc handling gpio 0 -> 211 > > +gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register > > +sh-pfc e6050000.pfc: failed to init GPIO chip, ignoring... > > sh-pfc e6050000.pfc: r8a7740_pfc support registered > > > > Hence all drivers using GPIOs fail to initialize because their GPIOs never > > become available. > > > > Adding debug prints to the failure paths shows that the call to > > of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER. > > Adding a debug print to the top of gpiochip_add_data() makes the problem go > > away, presumably because it introduces a delay that allows the delayed work > > to kick in... > > OK. What if we added also an optional pinctrl function that the pin > controller driver could call to initialize hogs? Then the pin controller > driver could call it during or after probe as needed. That is after > there's a valid struct pinctrl_dev handle. ... > We could also pass some flag if should always call pinctrl_late_init() > directly. But that does not remove the problem of struct pinctrl_dev handle > being uninitialized when the pin controller driver functionas are called. Looks like we need both a flag and a way for the pin controller driver to start things. Below is an experimental fix to intorduce pinctrl_start() that I've tested with pinctrl-single. Then we should probably make all pin controller drivers call pinctrl_start() to properly fix the issue of struct pinctrl_dev handle not being initialized before driver functions are called. Or do you guys have any better ideas? Regards, Tony 8< -------------------------------- diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -1962,6 +1962,17 @@ static void pinctrl_late_init(struct work_struct *work) pinctrl_init_device_debugfs(pctldev); } +int pinctrl_start(struct pinctrl_dev *pctldev) +{ + if (!IS_ERR(pctldev->p)) + return -EEXIST; + + pinctrl_late_init(&pctldev->late_init.work); + + return 0; +} +EXPORT_SYMBOL_GPL(pinctrl_start); + /** * pinctrl_register() - register a pin controller device * @pctldesc: descriptor for this pin controller @@ -2035,9 +2046,14 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc, /* * If the device has hogs we want the probe() function of the driver * to complete before we go in and hog them and add the pin controller - * to the list of controllers. If it has no hogs, we can just complete - * the registration immediately. + * to the list of controllers. If the pin controller driver initializes + * hogs, or the pin controller instance has no hogs, we can just + * complete the registration immediately. */ + + if (pctldesc->flags & PINCTRL_DRIVER_START) + return pctldev; + if (pinctrl_dt_has_hogs(pctldev)) schedule_delayed_work(&pctldev->late_init, 0); else diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -1741,6 +1741,7 @@ static int pcs_probe(struct platform_device *pdev) pcs->desc.pmxops = &pcs_pinmux_ops; if (PCS_HAS_PINCONF) pcs->desc.confops = &pcs_pinconf_ops; + pcs->desc.flags = PINCTRL_DRIVER_START; pcs->desc.owner = THIS_MODULE; ret = pcs_allocate_pin_table(pcs); @@ -1754,6 +1755,10 @@ static int pcs_probe(struct platform_device *pdev) goto free; } + ret = pinctrl_start(pcs->pctl); + if (ret) + goto free; + ret = pcs_add_gpio_func(np, pcs); if (ret < 0) goto free; diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -815,7 +815,15 @@ int sh_pfc_register_pinctrl(struct sh_pfc *pfc) pmx->pctl_desc.confops = &sh_pfc_pinconf_ops; pmx->pctl_desc.pins = pmx->pins; pmx->pctl_desc.npins = pfc->info->nr_pins; + pmx->pctl_desc.flags = PINCTRL_DRIVER_START; pmx->pctl = devm_pinctrl_register(pfc->dev, &pmx->pctl_desc, pmx); - return PTR_ERR_OR_ZERO(pmx->pctl); + if (IS_ERR(pmx->pctl)) + return PTR_ERR(pmx->pctl); + + ret = pinctrl_start(pmx->pctl); + if (ret) + return ret; + + return 0; } diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h --- a/include/linux/pinctrl/pinctrl.h +++ b/include/linux/pinctrl/pinctrl.h @@ -104,6 +104,8 @@ struct pinctrl_ops { struct pinctrl_map *map, unsigned num_maps); }; +#define PINCTRL_DRIVER_START BIT(0) + /** * struct pinctrl_desc - pin controller descriptor, register this to pin * control subsystem @@ -112,6 +114,8 @@ struct pinctrl_ops { * this pin controller * @npins: number of descriptors in the array, usually just ARRAY_SIZE() * of the pins field above + * @flags: Optional pin controller feature flags + * handling is needed in the pin controller driver. * @pctlops: pin control operation vtable, to support global concepts like * grouping of pins, this is optional. * @pmxops: pinmux operations vtable, if you support pinmuxing in your driver @@ -129,6 +133,7 @@ struct pinctrl_desc { const char *name; const struct pinctrl_pin_desc *pins; unsigned int npins; + unsigned int flags; const struct pinctrl_ops *pctlops; const struct pinmux_ops *pmxops; const struct pinconf_ops *confops; @@ -149,6 +154,7 @@ extern struct pinctrl_dev *devm_pinctrl_register(struct device *dev, void *driver_data); extern void devm_pinctrl_unregister(struct device *dev, struct pinctrl_dev *pctldev); +extern int pinctrl_start(struct pinctrl_dev *pctldev); extern bool pin_is_valid(struct pinctrl_dev *pctldev, int pin); extern void pinctrl_add_gpio_range(struct pinctrl_dev *pctldev,