From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751587AbdB0WfM (ORCPT ); Mon, 27 Feb 2017 17:35:12 -0500 Received: from muru.com ([72.249.23.125]:36656 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751454AbdB0WfI (ORCPT ); Mon, 27 Feb 2017 17:35:08 -0500 Date: Mon, 27 Feb 2017 10:45:35 -0800 From: Tony Lindgren To: Gary Bisson Cc: Mika =?utf-8?B?UGVudHRpbMOk?= , LKML , linus.walleij@linaro.org Subject: Re: [REGRESSION] pinctrl, of, unable to find hogs Message-ID: <20170227184535.GP21809@atomide.com> References: <4b93bdb8-2894-4c27-6256-21499110f08e@nextfour.com> <20170227155352.GL21809@atomide.com> <20170227162747.kxgkyqzjximtou4u@t450s.lan> <20170227164033.3ogiyrt2hqlsnfqn@t450s.lan> <20170227173718.GO21809@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170227173718.GO21809@atomide.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Tony Lindgren [170227 09:37]: > * Gary Bisson [170227 08:42]: > > > Not sure how to fix it though since we can't move the dt probing before > > > radix tree init. > > Yup looks like we still have an issue with pinctrl driver functions > getting called before driver probe has completed. > > How about we introduce something like: > > int pinctrl_claim_hogs(struct pinctrl_dev *pctldev); > > Then the drivers can call that at the end of the probe after > the pins have been parsed? > > This should be safe as no other driver can claim the pins either > before the pins have been parsed :) Below is an initial take on this solution. I've only briefly tested it so far but maybe give it a try and see if it helps. I'll take a look if we can make the error handling better for pinctrl_register_and_init(). Regards, Tony 8< --------------------------- >>From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 27 Feb 2017 10:15:22 -0800 Subject: [PATCH] Fix imx hogs --- drivers/pinctrl/core.c | 90 ++++++++++++++++++++++++--------- drivers/pinctrl/freescale/pinctrl-imx.c | 6 +++ drivers/pinctrl/pinctrl-single.c | 6 +++ drivers/pinctrl/sh-pfc/pinctrl.c | 12 ++++- drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 4 +- include/linux/pinctrl/pinctrl.h | 1 + 6 files changed, 93 insertions(+), 26 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -2010,30 +2010,14 @@ struct pinctrl_dev *pinctrl_init_controller(struct pinctrl_desc *pctldesc, return ERR_PTR(ret); } -static int pinctrl_create_and_start(struct pinctrl_dev *pctldev) +static int pinctrl_create(struct pinctrl_dev *pctldev) { + /* REVISIT: Can we bail out on errors here? */ pctldev->p = create_pinctrl(pctldev->dev, pctldev); - if (!IS_ERR(pctldev->p)) { - kref_get(&pctldev->p->users); - pctldev->hog_default = - pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT); - if (IS_ERR(pctldev->hog_default)) { - dev_dbg(pctldev->dev, - "failed to lookup the default state\n"); - } else { - if (pinctrl_select_state(pctldev->p, - pctldev->hog_default)) - dev_err(pctldev->dev, - "failed to select default state\n"); - } - - pctldev->hog_sleep = - pinctrl_lookup_state(pctldev->p, - PINCTRL_STATE_SLEEP); - if (IS_ERR(pctldev->hog_sleep)) - dev_dbg(pctldev->dev, - "failed to lookup the sleep state\n"); - } + if (IS_ERR(pctldev->p)) + dev_warn(pctldev->dev, + "creating incomplete pin controller: %li\n", + PTR_ERR(pctldev->p)); mutex_lock(&pinctrldev_list_mutex); list_add_tail(&pctldev->node, &pinctrldev_list); @@ -2044,6 +2028,66 @@ static int pinctrl_create_and_start(struct pinctrl_dev *pctldev) return 0; } +int pinctrl_claim_hogs(struct pinctrl_dev *pctldev) +{ + if (IS_ERR(pctldev->p)) { + dev_err(pctldev->dev, + "claim hogs for incomplete pin controller?: %li\n", + PTR_ERR(pctldev->p)); + + return PTR_ERR(pctldev->p); + } + + kref_get(&pctldev->p->users); + pctldev->hog_default = + pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT); + if (IS_ERR(pctldev->hog_default)) { + dev_dbg(pctldev->dev, + "failed to lookup the default state\n"); + } else { + if (pinctrl_select_state(pctldev->p, + pctldev->hog_default)) + dev_err(pctldev->dev, + "failed to select default state\n"); + } + + pctldev->hog_sleep = + pinctrl_lookup_state(pctldev->p, + PINCTRL_STATE_SLEEP); + if (IS_ERR(pctldev->hog_sleep)) + dev_dbg(pctldev->dev, + "failed to lookup the sleep state\n"); + + return 0; +} +EXPORT_SYMBOL_GPL(pinctrl_claim_hogs); + +/* REVISIT: Can we bail out on errors here? */ +static int pinctrl_create_and_start(struct pinctrl_dev *pctldev) +{ + int error; + + error = pinctrl_create(pctldev); + if (error) { + dev_warn(pctldev->dev, + "pin controller not claiming hogs: %i\n", + error); + + goto out_warn_only; + } + + error = pinctrl_claim_hogs(pctldev); + if (!error) + return 0; + +out_warn_only: + dev_warn(pctldev->dev, + "pin controller incomplete, hogs not claimed: %i\n", + error); + + return 0; +} + /** * pinctrl_register() - register a pin controller device * @pctldesc: descriptor for this pin controller @@ -2097,7 +2141,7 @@ int pinctrl_register_and_init(struct pinctrl_desc *pctldesc, */ *pctldev = p; - error = pinctrl_create_and_start(p); + error = pinctrl_create(p); if (error) { mutex_destroy(&p->mutex); kfree(p); diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c --- a/drivers/pinctrl/freescale/pinctrl-imx.c +++ b/drivers/pinctrl/freescale/pinctrl-imx.c @@ -788,6 +788,12 @@ int imx_pinctrl_probe(struct platform_device *pdev, goto free; } + ret = pinctrl_claim_hogs(ipctl->pctl); + if (ret) { + dev_err(&pdev->dev, "could not claim hogs: %i\n", ret); + goto free; + } + dev_info(&pdev->dev, "initialized IMX pinctrl driver\n"); return 0; 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 @@ -1765,6 +1765,12 @@ static int pcs_probe(struct platform_device *pdev) goto free; } + ret = pinctrl_claim_hogs(pcs->pctl); + if (ret) { + dev_err(pcs->dev, "could not claim hogs\n"); + 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 @@ -816,6 +816,14 @@ int sh_pfc_register_pinctrl(struct sh_pfc *pfc) pmx->pctl_desc.pins = pmx->pins; pmx->pctl_desc.npins = pfc->info->nr_pins; - return devm_pinctrl_register_and_init(pfc->dev, &pmx->pctl_desc, pmx, - &pmx->pctl); + ret = devm_pinctrl_register_and_init(pfc->dev, &pmx->pctl_desc, pmx, + &pmx->pctl); + if (ret) { + dev_error(pcf->dev, "could not claim hogs: %i\n", ret); + + return ret; + + } + + return pinctrl_claim_hogs(pmx->pctl); } diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c --- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c +++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c @@ -885,13 +885,15 @@ static int ti_iodelay_probe(struct platform_device *pdev) iod->desc.name = dev_name(dev); iod->desc.owner = THIS_MODULE; + platform_set_drvdata(pdev, iod); + ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl); if (ret) { dev_err(dev, "Failed to register pinctrl\n"); goto exit_out; } - platform_set_drvdata(pdev, iod); + return pinctrl_claim_hogs(iod->pctl); exit_out: of_node_put(np); 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 @@ -145,6 +145,7 @@ struct pinctrl_desc { extern int pinctrl_register_and_init(struct pinctrl_desc *pctldesc, struct device *dev, void *driver_data, struct pinctrl_dev **pctldev); +extern int pinctrl_claim_hogs(struct pinctrl_dev *pctldev); /* Please use pinctrl_register_and_init() instead */ extern struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc, -- 2.11.1