[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

The right check for conf_reg to be invalid it testing against -1 not 0
as is done in the rest of the driver.

This fixes an oops that can be triggered by:

	cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*

Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 52f2b9404fe0..448f10986c28 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -437,7 +437,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 	unsigned long config;
 
-	if (!pin_reg || !pin_reg->conf_reg) {
+	if (!pin_reg || pin_reg->conf_reg == -1) {
 		seq_printf(s, "N/A");
 		return;
 	}
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

The right check for conf_reg to be invalid it testing against -1 not 0
as is done in the rest of the driver.

This fixes an oops that can be triggered by:

	cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*

Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 52f2b9404fe0..448f10986c28 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -437,7 +437,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 	unsigned long config;
 
-	if (!pin_reg || !pin_reg->conf_reg) {
+	if (!pin_reg || pin_reg->conf_reg == -1) {
 		seq_printf(s, "N/A");
 		return;
 	}
-- 
2.1.4

Re: [PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

Hello,

please disregard the patch here, the email address of Dong Aisheng I
used isn't valid any more. I resend with the correct address.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

Hello,

please disregard the patch here, the email address of Dong Aisheng I
used isn't valid any more. I resend with the correct address.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Linus Walleij]

On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The right check for conf_reg to be invalid it testing against -1 not 0
> as is done in the rest of the driver.
>
> This fixes an oops that can be triggered by:
>
>         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>
> Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Patch applied.

But...

> -       if (!pin_reg || !pin_reg->conf_reg) {
> +       if (!pin_reg || pin_reg->conf_reg == -1) {

I can see that the other code checks for -1 in this variable.
But where is conf_reg ever set to -1?

freescale]$ git grep "\-1"
pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {

Only checked for, never assigned.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Linus Walleij]

On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:

> The right check for conf_reg to be invalid it testing against -1 not 0
> as is done in the rest of the driver.
>
> This fixes an oops that can be triggered by:
>
>         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>
> Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Patch applied.

But...

> -       if (!pin_reg || !pin_reg->conf_reg) {
> +       if (!pin_reg || pin_reg->conf_reg == -1) {

I can see that the other code checks for -1 in this variable.
But where is conf_reg ever set to -1?

freescale]$ git grep "\-1"
pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {

Only checked for, never assigned.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > The right check for conf_reg to be invalid it testing against -1 not 0
> > as is done in the rest of the driver.
> >
> > This fixes an oops that can be triggered by:
> >
> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
> >
> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Patch applied.
> 
> But...
> 
> > -       if (!pin_reg || !pin_reg->conf_reg) {
> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
> 
> I can see that the other code checks for -1 in this variable.
> But where is conf_reg ever set to -1?
> 
> freescale]$ git grep "\-1"
> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
Uah, ugly. This member gets -1 by means of:

	memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Uwe Kleine-König]

On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > The right check for conf_reg to be invalid it testing against -1 not 0
> > as is done in the rest of the driver.
> >
> > This fixes an oops that can be triggered by:
> >
> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
> >
> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> Patch applied.
> 
> But...
> 
> > -       if (!pin_reg || !pin_reg->conf_reg) {
> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
> 
> I can see that the other code checks for -1 in this variable.
> But where is conf_reg ever set to -1?
> 
> freescale]$ git grep "\-1"
> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
Uah, ugly. This member gets -1 by means of:

	memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Linus Walleij]

On Tue, Feb 3, 2015 at 8:01 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
>> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>
>> > The right check for conf_reg to be invalid it testing against -1 not 0
>> > as is done in the rest of the driver.
>> >
>> > This fixes an oops that can be triggered by:
>> >
>> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>> >
>> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>
>> Patch applied.
>>
>> But...
>>
>> > -       if (!pin_reg || !pin_reg->conf_reg) {
>> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
>>
>> I can see that the other code checks for -1 in this variable.
>> But where is conf_reg ever set to -1?
>>
>> freescale]$ git grep "\-1"
>> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
> Uah, ugly. This member gets -1 by means of:
>
>         memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);

OMG. Uwe can you make a patch changing that initialization
code to something sane? We can't have it like this.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Linus Walleij]

On Tue, Feb 3, 2015 at 8:01 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
>> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-K?nig
>> <u.kleine-koenig@pengutronix.de> wrote:
>>
>> > The right check for conf_reg to be invalid it testing against -1 not 0
>> > as is done in the rest of the driver.
>> >
>> > This fixes an oops that can be triggered by:
>> >
>> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>> >
>> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")
>> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>
>> Patch applied.
>>
>> But...
>>
>> > -       if (!pin_reg || !pin_reg->conf_reg) {
>> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
>>
>> I can see that the other code checks for -1 in this variable.
>> But where is conf_reg ever set to -1?
>>
>> freescale]$ git grep "\-1"
>> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
> Uah, ugly. This member gets -1 by means of:
>
>         memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);

OMG. Uwe can you make a patch changing that initialization
code to something sane? We can't have it like this.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Stefan Agner]

On 2015-02-05 14:53, Linus Walleij wrote:
> On Tue, Feb 3, 2015 at 8:01 PM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
>>> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-König
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>
>>> > The right check for conf_reg to be invalid it testing against -1 not 0
>>> > as is done in the rest of the driver.
>>> >
>>> > This fixes an oops that can be triggered by:
>>> >
>>> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>>> >
>>> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")

This was actually introduce by me in 3dac1918a491 ("pinctrl: imx: detect
uninitialized pins").

>>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>>
>>> Patch applied.
>>>
>>> But...
>>>
>>> > -       if (!pin_reg || !pin_reg->conf_reg) {
>>> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
>>>
>>> I can see that the other code checks for -1 in this variable.
>>> But where is conf_reg ever set to -1?
>>>
>>> freescale]$ git grep "\-1"
>>> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
>>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>>> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
>> Uah, ugly. This member gets -1 by means of:
>>
>>         memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);
> 
> OMG. Uwe can you make a patch changing that initialization
> code to something sane? We can't have it like this.

Oh sorry, this was actually me, in the commit mentioned above. That's
ugly, I agree. I'm wondering how I came to that idea....

Will send a patch for that.

--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] pinctrl: pinctrl-imx: don't use invalid value of conf_reg

[Stefan Agner]

On 2015-02-05 14:53, Linus Walleij wrote:
> On Tue, Feb 3, 2015 at 8:01 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
>> On Tue, Feb 03, 2015 at 02:01:09PM +0100, Linus Walleij wrote:
>>> On Tue, Jan 27, 2015 at 11:50 PM, Uwe Kleine-K?nig
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>
>>> > The right check for conf_reg to be invalid it testing against -1 not 0
>>> > as is done in the rest of the driver.
>>> >
>>> > This fixes an oops that can be triggered by:
>>> >
>>> >         cat /sys/kernel/debug/pinctrl/43fac000.iomuxc/*
>>> >
>>> > Fixes: ae75ff814538 ("pinctrl: pinctrl-imx: add imx pinctrl core driver")

This was actually introduce by me in 3dac1918a491 ("pinctrl: imx: detect
uninitialized pins").

>>> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>>
>>> Patch applied.
>>>
>>> But...
>>>
>>> > -       if (!pin_reg || !pin_reg->conf_reg) {
>>> > +       if (!pin_reg || pin_reg->conf_reg == -1) {
>>>
>>> I can see that the other code checks for -1 in this variable.
>>> But where is conf_reg ever set to -1?
>>>
>>> freescale]$ git grep "\-1"
>>> pinctrl-imx.c:          if (pin_reg->mux_reg == -1) {
>>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>>> pinctrl-imx.c:  if (pin_reg->mux_reg == -1)
>>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>>> pinctrl-imx.c:  if (pin_reg->conf_reg == -1) {
>>> pinctrl-imx.c:  if (!pin_reg || pin_reg->conf_reg == -1) {
>> Uah, ugly. This member gets -1 by means of:
>>
>>         memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);
> 
> OMG. Uwe can you make a patch changing that initialization
> code to something sane? We can't have it like this.

Oh sorry, this was actually me, in the commit mentioned above. That's
ugly, I agree. I'm wondering how I came to that idea....

Will send a patch for that.

--
Stefan