[PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Stefan Agner]

Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
the values in struct imx_pin_reg to be -1. This has been done in a
rather unorthodox way by setting the memory to 0xff using memset...
Use a proper for loop to initialize the whole array with -1.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 52f2b94..95041b6 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -645,7 +645,7 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 {
 	struct imx_pinctrl *ipctl;
 	struct resource *res;
-	int ret;
+	int ret, i;
 
 	if (!info || !info->pins || !info->npins) {
 		dev_err(&pdev->dev, "wrong pinctrl info\n");
@@ -662,7 +662,11 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 				      info->npins, GFP_KERNEL);
 	if (!info->pin_regs)
 		return -ENOMEM;
-	memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);
+
+	for (i = 0; i < info->npins; i++) {
+		info->pin_regs[i].mux_reg = -1;
+		info->pin_regs[i].conf_reg = -1;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ipctl->base = devm_ioremap_resource(&pdev->dev, res);
-- 
2.2.2

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Uwe Kleine-König]

Hallo Stefan,

On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
> -	memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);
> +
> +	for (i = 0; i < info->npins; i++) {
> +		info->pin_regs[i].mux_reg = -1;
> +		info->pin_regs[i].conf_reg = -1;
> +	}
looks definitely better. Just out of interest, did you check if it
changes the generated code?

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

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

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Stefan Agner]

On 2015-02-06 20:21, Uwe Kleine-König wrote:
> Hallo Stefan,
> 
> On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
>> -	memset(info->pin_regs, 0xff, sizeof(*info->pin_regs) * info->npins);
>> +
>> +	for (i = 0; i < info->npins; i++) {
>> +		info->pin_regs[i].mux_reg = -1;
>> +		info->pin_regs[i].conf_reg = -1;
>> +	}
> looks definitely better. Just out of interest, did you check if it
> changes the generated code?

Just checked quickly, before it branched to memset. The object file is
24 bytes larger. I guess memset might be a bit faster due to optimized
memory access. But not worth for some error handling relevant
initialization code IMHO...

--
Stefan

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Shawn Guo]

On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
> Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
> the values in struct imx_pin_reg to be -1. This has been done in a
> rather unorthodox way by setting the memory to 0xff using memset...
> Use a proper for loop to initialize the whole array with -1.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Uwe Kleine-König]

Hello Shawn

On Mon, Mar 02, 2015 at 07:45:17PM +0800, Shawn Guo wrote:
> On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
> > Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
> > the values in struct imx_pin_reg to be -1. This has been done in a
> > rather unorthodox way by setting the memory to 0xff using memset...
> > Use a proper for loop to initialize the whole array with -1.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
too late. This patch is part of 4.0-rc1 (4ff0f034e95d).

Best regards
Uwe

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

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Stefan Agner]

On 2015-03-02 13:59, Uwe Kleine-König wrote:
> Hello Shawn
> 
> On Mon, Mar 02, 2015 at 07:45:17PM +0800, Shawn Guo wrote:
>> On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
>> > Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
>> > the values in struct imx_pin_reg to be -1. This has been done in a
>> > rather unorthodox way by setting the memory to 0xff using memset...
>> > Use a proper for loop to initialize the whole array with -1.
>> >
>> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>>
>> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> too late. This patch is part of 4.0-rc1 (4ff0f034e95d).

This is not the same patch. The patch you are mentioning is actually
fixing a bug introduce in the change where we set -1 for uninitialized
pins. This patch is solving the weird assignment of the initial value...

--
Stefan

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Uwe Kleine-König]

Hello,

On Mon, Mar 02, 2015 at 02:42:01PM +0100, Stefan Agner wrote:
> On 2015-03-02 13:59, Uwe Kleine-König wrote:
> > On Mon, Mar 02, 2015 at 07:45:17PM +0800, Shawn Guo wrote:
> >> On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
> >> > Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
> >> > the values in struct imx_pin_reg to be -1. This has been done in a
> >> > rather unorthodox way by setting the memory to 0xff using memset...
> >> > Use a proper for loop to initialize the whole array with -1.
> >> >
> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> >>
> >> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > too late. This patch is part of 4.0-rc1 (4ff0f034e95d).
> 
> This is not the same patch. The patch you are mentioning is actually
> fixing a bug introduce in the change where we set -1 for uninitialized
> pins. This patch is solving the weird assignment of the initial value...
ah right.

Best regards
Uwe

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

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Stefan Agner]

On 2015-03-02 16:53, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Mar 02, 2015 at 02:42:01PM +0100, Stefan Agner wrote:
>> On 2015-03-02 13:59, Uwe Kleine-König wrote:
>> > On Mon, Mar 02, 2015 at 07:45:17PM +0800, Shawn Guo wrote:
>> >> On Fri, Feb 06, 2015 at 05:30:56PM +0100, Stefan Agner wrote:
>> >> > Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
>> >> > the values in struct imx_pin_reg to be -1. This has been done in a
>> >> > rather unorthodox way by setting the memory to 0xff using memset...
>> >> > Use a proper for loop to initialize the whole array with -1.
>> >> >
>> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>> >>
>> >> Acked-by: Shawn Guo <shawn.guo@linaro.org>
>> > too late. This patch is part of 4.0-rc1 (4ff0f034e95d).
>>
>> This is not the same patch. The patch you are mentioning is actually
>> fixing a bug introduce in the change where we set -1 for uninitialized
>> pins. This patch is solving the weird assignment of the initial value...
> ah right.

Do you generally agree to that change?

@Linus Walleij, anything holding this patch back from getting merged?

--
Stefan

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Linus Walleij]

On Fri, Feb 6, 2015 at 5:30 PM, Stefan Agner <stefan@agner.ch> wrote:

> Commit 3dac1918a491 ("pinctrl: imx: detect uninitialized pins") needs
> the values in struct imx_pin_reg to be -1. This has been done in a
> rather unorthodox way by setting the memory to 0xff using memset...
> Use a proper for loop to initialize the whole array with -1.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Patch applied with the ACKs.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: imx: do not implicitly set pin regs to -1

[Linus Walleij]

On Thu, Mar 5, 2015 at 12:23 AM, Stefan Agner <stefan@agner.ch> wrote:

> Do you generally agree to that change?
>
> @Linus Walleij, anything holding this patch back from getting merged?

Nothing apart from me being overloaded.

Merged now, thanks.

Yours,
Linus Walleij