diff mbox

[1/4] pinctrl: generic: scan for "pins" and "groups" properties in sub-nodes

Message ID 1471340990-16247-2-git-send-email-b.galvani@gmail.com
State Not Applicable
Headers show

Commit Message

Beniamino Galvani Aug. 16, 2016, 9:49 a.m. UTC
In cases where the pins and groups definitions are in a sub-node, as:

	uart_a {
		mux {
			groups = "uart_tx_a", "uart_rx_a";
			function = "uart_a";
		};
	};

pinctrl_generic_set_state_subnode() returns an error for the top-level
node and pinctrl_generic_set_state() fails. Instead, return success so
that the child nodes are tried.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 drivers/pinctrl/pinctrl-generic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Simon Glass Aug. 18, 2016, 3:44 a.m. UTC | #1
On 16 August 2016 at 03:49, Beniamino Galvani <b.galvani@gmail.com> wrote:
> In cases where the pins and groups definitions are in a sub-node, as:
>
>         uart_a {
>                 mux {
>                         groups = "uart_tx_a", "uart_rx_a";
>                         function = "uart_a";
>                 };
>         };
>
> pinctrl_generic_set_state_subnode() returns an error for the top-level
> node and pinctrl_generic_set_state() fails. Instead, return success so
> that the child nodes are tried.
>
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---
>  drivers/pinctrl/pinctrl-generic.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Masahiro, are you happy with this change?

We really need some pinctrl tests.

>
> diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
> index e86b72a..baff40f 100644
> --- a/drivers/pinctrl/pinctrl-generic.c
> +++ b/drivers/pinctrl/pinctrl-generic.c
> @@ -312,8 +312,10 @@ static int pinctrl_generic_set_state_subnode(struct udevice *dev,
>                 is_group = true;
>                 strings_count = fdt_count_strings(fdt, node,
>                                                   subnode_target_type);
> -               if (strings_count < 0)
> -                       return -EINVAL;
> +               if (strings_count < 0) {
> +                       /* skip this node; may contain config child nodes */
> +                       return 0;
> +               }
>         }
>
>         for (i = 0; i < strings_count; i++) {
> --
> 2.7.4
>

Regards,
Simon
Masahiro Yamada Aug. 19, 2016, 9:19 a.m. UTC | #2
2016-08-16 18:49 GMT+09:00 Beniamino Galvani <b.galvani@gmail.com>:
> In cases where the pins and groups definitions are in a sub-node, as:
>
>         uart_a {
>                 mux {
>                         groups = "uart_tx_a", "uart_rx_a";
>                         function = "uart_a";
>                 };
>         };
>
> pinctrl_generic_set_state_subnode() returns an error for the top-level
> node and pinctrl_generic_set_state() fails. Instead, return success so
> that the child nodes are tried.
>
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---


Looks good to me.  Thanks!

Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Tom Rini Sept. 7, 2016, 5:57 p.m. UTC | #3
On Tue, Aug 16, 2016 at 11:49:47AM +0200, Beniamino Galvani wrote:

> In cases where the pins and groups definitions are in a sub-node, as:
> 
> 	uart_a {
> 		mux {
> 			groups = "uart_tx_a", "uart_rx_a";
> 			function = "uart_a";
> 		};
> 	};
> 
> pinctrl_generic_set_state_subnode() returns an error for the top-level
> node and pinctrl_generic_set_state() fails. Instead, return success so
> that the child nodes are tried.
> 
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index e86b72a..baff40f 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -312,8 +312,10 @@  static int pinctrl_generic_set_state_subnode(struct udevice *dev,
 		is_group = true;
 		strings_count = fdt_count_strings(fdt, node,
 						  subnode_target_type);
-		if (strings_count < 0)
-			return -EINVAL;
+		if (strings_count < 0) {
+			/* skip this node; may contain config child nodes */
+			return 0;
+		}
 	}
 
 	for (i = 0; i < strings_count; i++) {