Skip to contentSkip to conversation

ARM: pinctrl: sunxi-pinctrl: fix pin funtion can not be match correctly.

PATCH In review hao_zhang 3 participants 5 messages Dec 13, 2017 – Jan 8, 2018

Subsystems (4)
Arm/Amlogic Meson SoC support Arm/Allwinner sunXi SoC support Pin control PWM
lore
Share & tools
lore

hao_zhang

author
#0 lore raw

PATCH v4 4/4 ARM: pinctrl: sunxi-pinctrl: fix pin funtion can not be match correctly.

Pin function can not be match correctly when SUNXI_PIN describe with mutiple variant and same function.

such as:
on pinctrl-sun4i-a10.c

SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),

		SUNXI_FUNCTION(0x0, "gpio_in"),
		SUNXI_FUNCTION(0x1, "gpio_out"),
		SUNXI_FUNCTION_VARIANT(0x2, "pwm",    /* PWM0 */
			PINCTRL_SUN4I_A10 |
			PINCTRL_SUN7I_A20),
		SUNXI_FUNCTION_VARIANT(0x3, "pwm",    /* PWM0 */
			PINCTRL_SUN8I_R40)),

it would always match to the first variant function (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)

so we should add variant compare on it.

Signed-off-by hao_zhang <[email protected]>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sunxi.c +4 -2
index 4b6cb25..f23e74e 100644
@@ -83,9 +83,11 @@ sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl,
 			struct sunxi_desc_function *func = pin->functions;
 
 			while (func->name) {
-				if (!strcmp(func->name, func_name))
+				if (!strcmp(func->name, func_name)) {
+					if (!(func->variant) ||
+					   (func->variant & pctl->variant))
 					return func;
-
+				}
 				func++;
 			}
 		}
Signature · 1 line
2.7.4
Acked-by on v4 4/4
#1 lore raw

Hi,

Thanks for your patch!

Quoted 30 lines · hao_zhang

Pin function can not be match correctly when SUNXI_PIN describe with mutiple variant and same function.

such as:
on pinctrl-sun4i-a10.c

SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),

		SUNXI_FUNCTION(0x0, "gpio_in"),
		SUNXI_FUNCTION(0x1, "gpio_out"),
		SUNXI_FUNCTION_VARIANT(0x2, "pwm",    /* PWM0 */
			PINCTRL_SUN4I_A10 |
			PINCTRL_SUN7I_A20),
		SUNXI_FUNCTION_VARIANT(0x3, "pwm",    /* PWM0 */
			PINCTRL_SUN8I_R40)),

it would always match to the first variant function (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)

so we should add variant compare on it.

Signed-off-by hao_zhang <[email protected]>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sunxi.c +0 -0
index 4b6cb25..f23e74e 100644

drivers/pinctrl/sunxi/pinctrl-sunxi.c sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl, · line 88

Quoted 7 lines
drivers/pinctrl/sunxi/pinctrl-sunxi.c +3 -1
@@ -83,9 +83,11 @@ sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl,
 			struct sunxi_desc_function *func = pin->functions;
 
 			while (func->name) {
-				if (!strcmp(func->name, func_name))
+				if (!strcmp(func->name, func_name)) {
+					if (!(func->variant) ||
+					   (func->variant & pctl->variant))

I guess it would be better to have:

	if (!strcmp(func->name, func_name) &&
	    (!func->variant || (func->variant & pctl->variant)))

Once fixed,

Acked-by Maxime Ripard <[email protected]>

Thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20171213/5c4188c3/attachment.sig>
author
#2 lore raw

2017-12-13 23:45 GMT+08:00 Maxime Ripard <[email protected]>:

Quoted 44 lines

Hi,

Thanks for your patch!

Quoted 30 lines · hao_zhang

Pin function can not be match correctly when SUNXI_PIN describe with mutiple variant and same function.

such as:
on pinctrl-sun4i-a10.c

SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),

              SUNXI_FUNCTION(0x0, "gpio_in"),
              SUNXI_FUNCTION(0x1, "gpio_out"),
              SUNXI_FUNCTION_VARIANT(0x2, "pwm",    /* PWM0 */
                      PINCTRL_SUN4I_A10 |
                      PINCTRL_SUN7I_A20),
              SUNXI_FUNCTION_VARIANT(0x3, "pwm",    /* PWM0 */
                      PINCTRL_SUN8I_R40)),

it would always match to the first variant function (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)

so we should add variant compare on it.

Signed-off-by hao_zhang <[email protected]>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sunxi.c +0 -0
index 4b6cb25..f23e74e 100644

drivers/pinctrl/sunxi/pinctrl-sunxi.c sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl, · line 88

Quoted 7 lines
drivers/pinctrl/sunxi/pinctrl-sunxi.c +3 -1
@@ -83,9 +83,11 @@ sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl,
                      struct sunxi_desc_function *func = pin->functions;
 
                      while (func->name) {
-                             if (!strcmp(func->name, func_name))
+                             if (!strcmp(func->name, func_name)) {
+                                     if (!(func->variant) ||
+                                        (func->variant & pctl->variant))

I guess it would be better to have:

Quoted 2 lines
        if (!strcmp(func->name, func_name) &&
            (!func->variant || (func->variant & pctl->variant)))

It would over 80 characters, can i change it by this ? if (!strcmp(func->name, func_name) &&

         (func->variant & pctl->variant ||
          !func->variant))
Quoted 11 lines

Once fixed,

Acked-by Maxime Ripard <[email protected]>

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

#3 lore raw
Quoted 48 lines · Hao Zhang

2017-12-13 23:45 GMT+08:00 Maxime Ripard <[email protected]>:

Quoted 44 lines

Hi,

Thanks for your patch!

Quoted 30 lines · hao_zhang

Pin function can not be match correctly when SUNXI_PIN describe with mutiple variant and same function.

such as:
on pinctrl-sun4i-a10.c

SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),

              SUNXI_FUNCTION(0x0, "gpio_in"),
              SUNXI_FUNCTION(0x1, "gpio_out"),
              SUNXI_FUNCTION_VARIANT(0x2, "pwm",    /* PWM0 */
                      PINCTRL_SUN4I_A10 |
                      PINCTRL_SUN7I_A20),
              SUNXI_FUNCTION_VARIANT(0x3, "pwm",    /* PWM0 */
                      PINCTRL_SUN8I_R40)),

it would always match to the first variant function (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)

so we should add variant compare on it.

Signed-off-by hao_zhang <[email protected]>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/pinctrl/sunxi/pinctrl-sunxi.c +0 -0
index 4b6cb25..f23e74e 100644

drivers/pinctrl/sunxi/pinctrl-sunxi.c sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl, · line 88

Quoted 7 lines
drivers/pinctrl/sunxi/pinctrl-sunxi.c +3 -1
@@ -83,9 +83,11 @@ sunxi_pinctrl_desc_find_function_by_name(struct sunxi_pinctrl *pctl,
                      struct sunxi_desc_function *func = pin->functions;
 
                      while (func->name) {
-                             if (!strcmp(func->name, func_name))
+                             if (!strcmp(func->name, func_name)) {
+                                     if (!(func->variant) ||
+                                        (func->variant & pctl->variant))

I guess it would be better to have:

Quoted 2 lines
        if (!strcmp(func->name, func_name) &&
            (!func->variant || (func->variant & pctl->variant)))
Quoted 4 lines

It would over 80 characters, can i change it by this ? if (!strcmp(func->name, func_name) &&

         (func->variant & pctl->variant ||
          !func->variant))

It feels more natural to have !func->variant first, but feel free to have it split that way yes.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180108/a819570d/attachment-0001.sig>
#4 lore raw
In reply to hao_zhang
Quoted 15 lines · hao_zhang

Pin function can not be match correctly when SUNXI_PIN describe with mutiple variant and same function.

such as:
on pinctrl-sun4i-a10.c

SUNXI_PIN(SUNXI_PINCTRL_PIN(B, 2),

                SUNXI_FUNCTION(0x0, "gpio_in"),
                SUNXI_FUNCTION(0x1, "gpio_out"),
                SUNXI_FUNCTION_VARIANT(0x2, "pwm",    /* PWM0 */
                        PINCTRL_SUN4I_A10 |
                        PINCTRL_SUN7I_A20),
                SUNXI_FUNCTION_VARIANT(0x3, "pwm",    /* PWM0 */
                        PINCTRL_SUN8I_R40)),
Quoted 6 lines

it would always match to the first variant function (PINCTRL_SUN4I_A10, PINCTRL_SUN7I_A20)

so we should add variant compare on it.

Signed-off-by hao_zhang <[email protected]>

Please resend patch with Maxime's suggestions fixed and his ACK added so I can apply it.

I can take this patch separatelt, it does not need to be part of the PWM series.

Yours,
Linus Walleij