Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1392243 > unrolled thread

[PATCH] pinctrl: tegra: Correctly check the supported configuration

Started byLaxman Dewangan <ldewangan@nvidia.com>
First post2016-05-02 16:50 +0200
Last post2016-05-02 18:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH] pinctrl: tegra: Correctly check the supported configuration Laxman Dewangan <ldewangan@nvidia.com> - 2016-05-02 16:50 +0200
    Re: [PATCH] pinctrl: tegra: Correctly check the supported  configuration Stephen Warren <swarren@wwwdotorg.org> - 2016-05-02 18:50 +0200

#1392243 — [PATCH] pinctrl: tegra: Correctly check the supported configuration

FromLaxman Dewangan <ldewangan@nvidia.com>
Date2016-05-02 16:50 +0200
Subject[PATCH] pinctrl: tegra: Correctly check the supported configuration
Message-ID<runkC-58W-17@gated-at.bofh.it>
The pincontrol registers of Tegra chips has multiple filed per
registers. There is two type of registers mux and drive. All
configurations belongs to one of these registers.

If any configurations are supported then <config>_bit is set to
bit position of these registers otherwise -1 to not support it.
The member is defined as
	s32 <config>_bit:6;

So if config is not supported ifor given SoC then it is set to -1
in soc pinmmux table.
In common driver code, to find out that given config is supported
or not, it is checked as:

s8 bit = <config>_bit;
if (bit > 31) {
	/* Not supported */
}

But in this case, bit is s8 and hence for non supporting it is -1.

Correct the check as:
if (bit < 0 || bit > 31) {
	/* Not supported */
}

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
I think it should go on stable.

 drivers/pinctrl/tegra/pinctrl-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index fb00129..cc117f7 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -417,7 +417,7 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx,
 		return -ENOTSUPP;
 	}
 
-	if (*reg < 0 || *bit > 31) {
+	if (*reg < 0 || *bit < 0 || *bit > 31)  {
 		if (report_err) {
 			const char *prop = "unknown";
 			int i;
-- 
2.1.4

[toc] | [next] | [standalone]


#1392376 — Re: [PATCH] pinctrl: tegra: Correctly check the supported configuration

FromStephen Warren <swarren@wwwdotorg.org>
Date2016-05-02 18:50 +0200
SubjectRe: [PATCH] pinctrl: tegra: Correctly check the supported configuration
Message-ID<rupcK-6XH-9@gated-at.bofh.it>
In reply to#1392243
On 05/02/2016 08:28 AM, Laxman Dewangan wrote:
> The pincontrol registers of Tegra chips has multiple filed per
> registers. There is two type of registers mux and drive. All
> configurations belongs to one of these registers.
>
> If any configurations are supported then <config>_bit is set to
> bit position of these registers otherwise -1 to not support it.
> The member is defined as
> 	s32 <config>_bit:6;
>
> So if config is not supported ifor given SoC then it is set to -1
> in soc pinmmux table.
> In common driver code, to find out that given config is supported
> or not, it is checked as:
>
> s8 bit = <config>_bit;
> if (bit > 31) {
> 	/* Not supported */
> }
>
> But in this case, bit is s8 and hence for non supporting it is -1.
>
> Correct the check as:
> if (bit < 0 || bit > 31) {
> 	/* Not supported */
> }
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> I think it should go on stable.
>
>   drivers/pinctrl/tegra/pinctrl-tegra.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index fb00129..cc117f7 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -417,7 +417,7 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx,
>   		return -ENOTSUPP;
>   	}
>
> -	if (*reg < 0 || *bit > 31) {
> +	if (*reg < 0 || *bit < 0 || *bit > 31)  {

May as well only test (*bit < 0)? Same for your other patch.

It'd be good to add the following tag to the commit description:

Fixes: e4c02dced975 ("pinctrl: tegra: use signed bitfields for optional 
fields")

This patch has the potential to suddenly catch more invalid 
configurations than it used to, and thus break some boards. However, I 
think this actually is safe; any errors that will be caught after this 
patch would have been caught before the "signed bitfields" patch above, 
and briefly looking at the (subject lines of) patches that have gone in 
since the "signed bitfields" patch was applied, I don't think we'll see 
any new issues.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web