diff mbox series

[v2] pinctrl: Ingenic: Add missing UART2 group C for X1000/E

Message ID fd813c7d-888a-ce53-b1e5-d9b41003b58b@sudomaker.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v2] pinctrl: Ingenic: Add missing UART2 group C for X1000/E | expand

Commit Message

Mike Yang March 24, 2022, 1:33 p.m. UTC
v2: Define PC31 pin only once, noted by Paul Cercueil <paul@crapouillou.net>
    Confirmed to work on hardware. Although the Ingenic folks did this twice
    in their 4.4 kernel fork; not sure why.

X1000/E has a third UART2 pin group selection, which uses the TDI(G2) as RX
and TDO(G1) as TX. This configuration is becoming increasingly popular in
newer core boards, such as the Halley2 v4.1. This is done by enabling
function 1 of a "virtual pin" PC31. See section 19.3.3 of the X1000
Programming Manual for details.

Signed-off-by: Yunian Yang <reimu@sudomaker.com>
---
 drivers/pinctrl/pinctrl-ingenic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Paul Cercueil March 24, 2022, 2:54 p.m. UTC | #1
Le jeu., mars 24 2022 at 21:33:56 +0800, Yunian Yang 
<reimu@sudomaker.com> a écrit :
> v2: Define PC31 pin only once, noted by Paul Cercueil 
> <paul@crapouillou.net>
>     Confirmed to work on hardware. Although the Ingenic folks did 
> this twice
>     in their 4.4 kernel fork; not sure why.
> 
> X1000/E has a third UART2 pin group selection, which uses the TDI(G2) 
> as RX
> and TDO(G1) as TX. This configuration is becoming increasingly 
> popular in
> newer core boards, such as the Halley2 v4.1. This is done by enabling
> function 1 of a "virtual pin" PC31. See section 19.3.3 of the X1000
> Programming Manual for details.
> 
> Signed-off-by: Yunian Yang <reimu@sudomaker.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/pinctrl/pinctrl-ingenic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c 
> b/drivers/pinctrl/pinctrl-ingenic.c
> index 2712f51eb238..29709059d62b 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -1982,6 +1982,7 @@ static int x1000_uart1_data_a_pins[] = { 0x04, 
> 0x05, };
>  static int x1000_uart1_data_d_pins[] = { 0x62, 0x63, };
>  static int x1000_uart1_hwflow_pins[] = { 0x64, 0x65, };
>  static int x1000_uart2_data_a_pins[] = { 0x02, 0x03, };
> +static int x1000_uart2_data_c_pins[] = { 0x5f, };
>  static int x1000_uart2_data_d_pins[] = { 0x65, 0x64, };
>  static int x1000_sfc_data_pins[] = { 0x1d, 0x1c, 0x1e, 0x1f, };
>  static int x1000_sfc_clk_pins[] = { 0x1a, };
> @@ -2058,6 +2059,7 @@ static const struct group_desc x1000_groups[] = 
> {
>         INGENIC_PIN_GROUP("uart1-data-d", x1000_uart1_data_d, 1),
>         INGENIC_PIN_GROUP("uart1-hwflow", x1000_uart1_hwflow, 1),
>         INGENIC_PIN_GROUP("uart2-data-a", x1000_uart2_data_a, 2),
> +       INGENIC_PIN_GROUP("uart2-data-c", x1000_uart2_data_c, 1),
>         INGENIC_PIN_GROUP("uart2-data-d", x1000_uart2_data_d, 0),
>         INGENIC_PIN_GROUP("sfc-data", x1000_sfc_data, 1),
>         INGENIC_PIN_GROUP("sfc-clk", x1000_sfc_clk, 1),
> @@ -2115,7 +2117,7 @@ static const char *x1000_uart0_groups[] = { 
> "uart0-data", "uart0-hwflow", };
>  static const char *x1000_uart1_groups[] = {
>         "uart1-data-a", "uart1-data-d", "uart1-hwflow",
>  };
> -static const char *x1000_uart2_groups[] = { "uart2-data-a", 
> "uart2-data-d", };
> +static const char *x1000_uart2_groups[] = { "uart2-data-a", 
> "uart2-data-c", "uart2-data-d", };
>  static const char *x1000_sfc_groups[] = { "sfc-data", "sfc-clk", 
> "sfc-ce", };
>  static const char *x1000_ssi_groups[] = {
>         "ssi-dt-a-22", "ssi-dt-a-29", "ssi-dt-d",
> --
> 2.30.2
Zhou Yanjie April 7, 2022, 7:07 p.m. UTC | #2
On 2022/3/24 下午9:33, Yunian Yang wrote:
> v2: Define PC31 pin only once, noted by Paul Cercueil <paul@crapouillou.net>
>      Confirmed to work on hardware. Although the Ingenic folks did this twice
>      in their 4.4 kernel fork; not sure why.
>
> X1000/E has a third UART2 pin group selection, which uses the TDI(G2) as RX
> and TDO(G1) as TX. This configuration is becoming increasingly popular in
> newer core boards, such as the Halley2 v4.1. This is done by enabling
> function 1 of a "virtual pin" PC31. See section 19.3.3 of the X1000
> Programming Manual for details.
>
> Signed-off-by: Yunian Yang <reimu@sudomaker.com>


A similar situation exists on JZ4780, except that its virtual pin is PA31.



Tested-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>


Thanks and best regards!


> ---
>   drivers/pinctrl/pinctrl-ingenic.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
> index 2712f51eb238..29709059d62b 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -1982,6 +1982,7 @@ static int x1000_uart1_data_a_pins[] = { 0x04, 0x05, };
>   static int x1000_uart1_data_d_pins[] = { 0x62, 0x63, };
>   static int x1000_uart1_hwflow_pins[] = { 0x64, 0x65, };
>   static int x1000_uart2_data_a_pins[] = { 0x02, 0x03, };
> +static int x1000_uart2_data_c_pins[] = { 0x5f, };
>   static int x1000_uart2_data_d_pins[] = { 0x65, 0x64, };
>   static int x1000_sfc_data_pins[] = { 0x1d, 0x1c, 0x1e, 0x1f, };
>   static int x1000_sfc_clk_pins[] = { 0x1a, };
> @@ -2058,6 +2059,7 @@ static const struct group_desc x1000_groups[] = {
>          INGENIC_PIN_GROUP("uart1-data-d", x1000_uart1_data_d, 1),
>          INGENIC_PIN_GROUP("uart1-hwflow", x1000_uart1_hwflow, 1),
>          INGENIC_PIN_GROUP("uart2-data-a", x1000_uart2_data_a, 2),
> +       INGENIC_PIN_GROUP("uart2-data-c", x1000_uart2_data_c, 1),
>          INGENIC_PIN_GROUP("uart2-data-d", x1000_uart2_data_d, 0),
>          INGENIC_PIN_GROUP("sfc-data", x1000_sfc_data, 1),
>          INGENIC_PIN_GROUP("sfc-clk", x1000_sfc_clk, 1),
> @@ -2115,7 +2117,7 @@ static const char *x1000_uart0_groups[] = { "uart0-data", "uart0-hwflow", };
>   static const char *x1000_uart1_groups[] = {
>          "uart1-data-a", "uart1-data-d", "uart1-hwflow",
>   };
> -static const char *x1000_uart2_groups[] = { "uart2-data-a", "uart2-data-d", };
> +static const char *x1000_uart2_groups[] = { "uart2-data-a", "uart2-data-c", "uart2-data-d", };
>   static const char *x1000_sfc_groups[] = { "sfc-data", "sfc-clk", "sfc-ce", };
>   static const char *x1000_ssi_groups[] = {
>          "ssi-dt-a-22", "ssi-dt-a-29", "ssi-dt-d",
Linus Walleij April 20, 2022, 10:41 p.m. UTC | #3
On Thu, Mar 24, 2022 at 2:32 PM Yunian Yang <reimu@sudomaker.com> wrote:

> v2: Define PC31 pin only once, noted by Paul Cercueil <paul@crapouillou.net>
>     Confirmed to work on hardware. Although the Ingenic folks did this twice
>     in their 4.4 kernel fork; not sure why.

Put the changelog after the commit text at least please.

> X1000/E has a third UART2 pin group selection, which uses the TDI(G2) as RX
> and TDO(G1) as TX. This configuration is becoming increasingly popular in
> newer core boards, such as the Halley2 v4.1. This is done by enabling
> function 1 of a "virtual pin" PC31. See section 19.3.3 of the X1000
> Programming Manual for details.
>
> Signed-off-by: Yunian Yang <reimu@sudomaker.com>

> @@ -2058,6 +2059,7 @@ static const struct group_desc x1000_groups[] = {
>         INGENIC_PIN_GROUP("uart1-data-d", x1000_uart1_data_d, 1),
>         INGENIC_PIN_GROUP("uart1-hwflow", x1000_uart1_hwflow, 1),
>         INGENIC_PIN_GROUP("uart2-data-a", x1000_uart2_data_a, 2),
> +       INGENIC_PIN_GROUP("uart2-data-c", x1000_uart2_data_c, 1),

This doesn't apply to the current mainline kernel, which doesn't
even have the uart2-data-a designation.

Clearly this patch depend on something that I haven't
yet applied? Something I missed?

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index 2712f51eb238..29709059d62b 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -1982,6 +1982,7 @@  static int x1000_uart1_data_a_pins[] = { 0x04, 0x05, };
 static int x1000_uart1_data_d_pins[] = { 0x62, 0x63, };
 static int x1000_uart1_hwflow_pins[] = { 0x64, 0x65, };
 static int x1000_uart2_data_a_pins[] = { 0x02, 0x03, };
+static int x1000_uart2_data_c_pins[] = { 0x5f, };
 static int x1000_uart2_data_d_pins[] = { 0x65, 0x64, };
 static int x1000_sfc_data_pins[] = { 0x1d, 0x1c, 0x1e, 0x1f, };
 static int x1000_sfc_clk_pins[] = { 0x1a, };
@@ -2058,6 +2059,7 @@  static const struct group_desc x1000_groups[] = {
        INGENIC_PIN_GROUP("uart1-data-d", x1000_uart1_data_d, 1),
        INGENIC_PIN_GROUP("uart1-hwflow", x1000_uart1_hwflow, 1),
        INGENIC_PIN_GROUP("uart2-data-a", x1000_uart2_data_a, 2),
+       INGENIC_PIN_GROUP("uart2-data-c", x1000_uart2_data_c, 1),
        INGENIC_PIN_GROUP("uart2-data-d", x1000_uart2_data_d, 0),
        INGENIC_PIN_GROUP("sfc-data", x1000_sfc_data, 1),
        INGENIC_PIN_GROUP("sfc-clk", x1000_sfc_clk, 1),
@@ -2115,7 +2117,7 @@  static const char *x1000_uart0_groups[] = { "uart0-data", "uart0-hwflow", };
 static const char *x1000_uart1_groups[] = {
        "uart1-data-a", "uart1-data-d", "uart1-hwflow",
 };
-static const char *x1000_uart2_groups[] = { "uart2-data-a", "uart2-data-d", };
+static const char *x1000_uart2_groups[] = { "uart2-data-a", "uart2-data-c", "uart2-data-d", };
 static const char *x1000_sfc_groups[] = { "sfc-data", "sfc-clk", "sfc-ce", };
 static const char *x1000_ssi_groups[] = {
        "ssi-dt-a-22", "ssi-dt-a-29", "ssi-dt-d",