pinctrl: imx: do not implicitly set pin regs to -1
diff mbox

Message ID 1423240256-27210-1-git-send-email-stefan@agner.ch
State New
Headers show

Commit Message

Stefan Agner Feb. 6, 2015, 4:30 p.m. UTC
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(-)

Comments

Uwe Kleine-König Feb. 6, 2015, 7:21 p.m. UTC | #1
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
Stefan Agner Feb. 6, 2015, 7:40 p.m. UTC | #2
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
--
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
Shawn Guo March 2, 2015, 11:45 a.m. UTC | #3
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>
--
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
Uwe Kleine-König March 2, 2015, 12:59 p.m. UTC | #4
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
Stefan Agner March 2, 2015, 1:42 p.m. UTC | #5
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

--
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
Uwe Kleine-König March 2, 2015, 3:53 p.m. UTC | #6
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
Stefan Agner March 4, 2015, 11:23 p.m. UTC | #7
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
--
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
Linus Walleij March 9, 2015, 4:18 p.m. UTC | #8
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
--
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
Linus Walleij March 9, 2015, 4:18 p.m. UTC | #9
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
--
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
diff mbox

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);