All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] pinctrl: Don't create a pinctrl handle if no pinctrl entries exist
Date: Thu, 16 Jun 2016 16:20:17 +0100	[thread overview]
Message-ID: <5762C3B1.7000800@nvidia.com> (raw)
In-Reply-To: <1466080749-23567-1-git-send-email-jonathanh@nvidia.com>


On 16/06/16 13:39, Jon Hunter wrote:
> When pinctrl_get() is called for a device, it will return a valid handle
> even if the device itself has no pinctrl state entries defined in
> device-tree. This is caused by the function pinctrl_dt_to_map() which
> will return success even if the first pinctrl state, 'pinctrl-0', is not
> found in the device-tree node for a device.
> 
> According to the pinctrl device-tree binding documentation, pinctrl
> states must be numbered starting from 0 and so 'pinctrl-0' should always
> be present if a device uses pinctrl and therefore, if 'pinctrl-0' is not
> present it seems valid that we should not return a valid pinctrl handle.
> 
> Fix this by returning an error code if the property 'pinctrl-0' is not
> present for a device.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> I was wondering if this meant we are creating pinctrl handles for
> devices on boot that don't use pinctrl (when
> calling pinctrl_bind_pins()). However, although devm_pinctrl_get()
> does return successful for all devices, the subsequent call to
> pinctrl_lookup_state() (to get the default state) will fail and so
> we will destroy the pinctrl handle afterall.
>
>  drivers/pinctrl/devicetree.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
> index fe04e748dfe4..f41c16e11a11 100644
> --- a/drivers/pinctrl/devicetree.c
> +++ b/drivers/pinctrl/devicetree.c
> @@ -195,8 +195,11 @@ int pinctrl_dt_to_map(struct pinctrl *p)
>  		propname = kasprintf(GFP_KERNEL, "pinctrl-%d", state);
>  		prop = of_find_property(np, propname, &size);
>  		kfree(propname);
> -		if (!prop)
> +		if (!prop) {
> +			if (state == 0)

I think there should be a of_node_put() here. Will resend.

> +				return -ENODEV;
>  			break;
> +		}

Cheers
Jon

-- 
nvpublic

WARNING: multiple messages have this Message-ID (diff)
From: Jon Hunter <jonathanh@nvidia.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: <linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] pinctrl: Don't create a pinctrl handle if no pinctrl entries exist
Date: Thu, 16 Jun 2016 16:20:17 +0100	[thread overview]
Message-ID: <5762C3B1.7000800@nvidia.com> (raw)
In-Reply-To: <1466080749-23567-1-git-send-email-jonathanh@nvidia.com>


On 16/06/16 13:39, Jon Hunter wrote:
> When pinctrl_get() is called for a device, it will return a valid handle
> even if the device itself has no pinctrl state entries defined in
> device-tree. This is caused by the function pinctrl_dt_to_map() which
> will return success even if the first pinctrl state, 'pinctrl-0', is not
> found in the device-tree node for a device.
> 
> According to the pinctrl device-tree binding documentation, pinctrl
> states must be numbered starting from 0 and so 'pinctrl-0' should always
> be present if a device uses pinctrl and therefore, if 'pinctrl-0' is not
> present it seems valid that we should not return a valid pinctrl handle.
> 
> Fix this by returning an error code if the property 'pinctrl-0' is not
> present for a device.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> I was wondering if this meant we are creating pinctrl handles for
> devices on boot that don't use pinctrl (when
> calling pinctrl_bind_pins()). However, although devm_pinctrl_get()
> does return successful for all devices, the subsequent call to
> pinctrl_lookup_state() (to get the default state) will fail and so
> we will destroy the pinctrl handle afterall.
>
>  drivers/pinctrl/devicetree.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
> index fe04e748dfe4..f41c16e11a11 100644
> --- a/drivers/pinctrl/devicetree.c
> +++ b/drivers/pinctrl/devicetree.c
> @@ -195,8 +195,11 @@ int pinctrl_dt_to_map(struct pinctrl *p)
>  		propname = kasprintf(GFP_KERNEL, "pinctrl-%d", state);
>  		prop = of_find_property(np, propname, &size);
>  		kfree(propname);
> -		if (!prop)
> +		if (!prop) {
> +			if (state == 0)

I think there should be a of_node_put() here. Will resend.

> +				return -ENODEV;
>  			break;
> +		}

Cheers
Jon

-- 
nvpublic

  reply	other threads:[~2016-06-16 15:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 12:39 [PATCH] pinctrl: Don't create a pinctrl handle if no pinctrl entries exist Jon Hunter
2016-06-16 12:39 ` Jon Hunter
2016-06-16 15:20 ` Jon Hunter [this message]
2016-06-16 15:20   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5762C3B1.7000800@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.