[PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

In order to support putting the u-boot console on UART3 of the
ast2400, this commit adds support for setting bits 22 and 23 of SCU80
to enable TXD3 and RXD3 on pins C14 and B14, respectively.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2400.c b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
index 7a6c2892f469..a8a5ff517108 100644
--- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
+++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
@@ -125,6 +125,14 @@ static struct aspeed_sig_desc spi1_link[] = {
 	{ 0x70, BIT(12), 0},
 };
 
+static struct aspeed_sig_desc txd3_link[] = {
+	{ 0x80, BIT(22), 0},
+};
+
+static struct aspeed_sig_desc rxd3_link[] = {
+	{ 0x80, BIT(23), 0},
+};
+
 static const struct aspeed_group_config ast2400_groups[] = {
 	{ "MAC1LINK", 1, mac1_link },
 	{ "MAC2LINK", 1, mac2_link },
@@ -146,6 +154,8 @@ static const struct aspeed_group_config ast2400_groups[] = {
 	{ "SD1", 1, sdio1_link },
 	{ "SPI1", 1, spi1_link},
 	{ "SPI1CS1", 1, spi1cs1_link},
+	{ "TXD3", 1, txd3_link },
+	{ "RXD3", 1, rxd3_link },
 };
 
 static int ast2400_pinctrl_get_groups_count(struct udevice *dev)
-- 
2.34.1

RE: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Ryan Chen]

Hello,
	You may need claim for function group for link, not for pin link.
	Ex. 
	static struct aspeed_sig_desc uart3_link[] = {
		{ 0x80, BIT(22), 0},
		{ 0x80, BIT(23), 0},
	}

	ast2400_groups[] = {
	{ "UART3", 2, uart3_link },
Ryan
> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Tuesday, December 14, 2021 3:47 AM
> To: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>
> Cc: Zev Weiss <zev@bewilderbeest.net>; Ryan Chen
> <ryan_chen@aspeedtech.com>
> Subject: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
> support for TXD3/RXD3 pins
> 
> In order to support putting the u-boot console on UART3 of the ast2400, this
> commit adds support for setting bits 22 and 23 of SCU80 to enable TXD3 and
> RXD3 on pins C14 and B14, respectively.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> index 7a6c2892f469..a8a5ff517108 100644
> --- a/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> +++ b/drivers/pinctrl/aspeed/pinctrl_ast2400.c
> @@ -125,6 +125,14 @@ static struct aspeed_sig_desc spi1_link[] = {
>  	{ 0x70, BIT(12), 0},
>  };
> 
> +static struct aspeed_sig_desc txd3_link[] = {
> +	{ 0x80, BIT(22), 0},
> +};
> +
> +static struct aspeed_sig_desc rxd3_link[] = {
> +	{ 0x80, BIT(23), 0},
> +};
> +
>  static const struct aspeed_group_config ast2400_groups[] = {
>  	{ "MAC1LINK", 1, mac1_link },
>  	{ "MAC2LINK", 1, mac2_link },
> @@ -146,6 +154,8 @@ static const struct aspeed_group_config
> ast2400_groups[] = {
>  	{ "SD1", 1, sdio1_link },
>  	{ "SPI1", 1, spi1_link},
>  	{ "SPI1CS1", 1, spi1cs1_link},
> +	{ "TXD3", 1, txd3_link },
> +	{ "RXD3", 1, rxd3_link },
>  };
> 
>  static int ast2400_pinctrl_get_groups_count(struct udevice *dev)
> --
> 2.34.1

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
>Hello,
>	You may need claim for function group for link, not for pin link.
>	Ex.
>	static struct aspeed_sig_desc uart3_link[] = {
>		{ 0x80, BIT(22), 0},
>		{ 0x80, BIT(23), 0},
>	}
>
>	ast2400_groups[] = {
>	{ "UART3", 2, uart3_link },
>Ryan

Hi Ryan,

That possibility occurred to me, but the existing function/group names 
in arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378) made me 
think they should be separate.  I'm certainly not an expert on pinctrl 
stuff though...is there some other existing logic or mechanism to link a 
"UART3" to the separate "TXD3" and "RXD3" in the device tree?


Zev

RE: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Ryan Chen]

Hello,

> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Tuesday, December 14, 2021 9:33 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
> <chiawei_wang@aspeedtech.com>
> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
> support for TXD3/RXD3 pins
> 
> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
> >Hello,
> >	You may need claim for function group for link, not for pin link.
> >	Ex.
> >	static struct aspeed_sig_desc uart3_link[] = {
> >		{ 0x80, BIT(22), 0},
> >		{ 0x80, BIT(23), 0},
> >	}
> >
> >	ast2400_groups[] = {
> >	{ "UART3", 2, uart3_link },
> >Ryan
> 
> Hi Ryan,
> 
> That possibility occurred to me, but the existing function/group names in
> arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378) made me think
> they should be separate.  
This device tree is copied from Linux kernel.

> I'm certainly not an expert on pinctrl stuff
> though...is there some other existing logic or mechanism to link a "UART3" to
> the separate "TXD3" and "RXD3" in the device tree?
There is no separate in u-boot device tree.
May I know why you need separate?
> 
> 
> Zev

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
>Hello,
>
>> -----Original Message-----
>> From: Zev Weiss <zev@bewilderbeest.net>
>> Sent: Tuesday, December 14, 2021 9:33 AM
>> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
>> <chiawei_wang@aspeedtech.com>
>> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
>> support for TXD3/RXD3 pins
>>
>> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
>> >Hello,
>> >	You may need claim for function group for link, not for pin link.
>> >	Ex.
>> >	static struct aspeed_sig_desc uart3_link[] = {
>> >		{ 0x80, BIT(22), 0},
>> >		{ 0x80, BIT(23), 0},
>> >	}
>> >
>> >	ast2400_groups[] = {
>> >	{ "UART3", 2, uart3_link },
>> >Ryan
>>
>> Hi Ryan,
>>
>> That possibility occurred to me, but the existing function/group names in
>> arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378) made me think
>> they should be separate.
>This device tree is copied from Linux kernel.
>
>> I'm certainly not an expert on pinctrl stuff
>> though...is there some other existing logic or mechanism to link a "UART3" to
>> the separate "TXD3" and "RXD3" in the device tree?
>There is no separate in u-boot device tree.

Perhaps we're misunderstanding each other...

https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133

and

https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378

look fairly separate to me?

>May I know why you need separate?

In my particular case I don't need these two to be separate, but it 
seems conceivable that there might be other cases that would require a 
different set of signals to be enabled for a generic "UART3" group -- 
possibly more (sideband signals like CTS and such), or perhaps even 
fewer (e.g. if you had an output-only UART3 just for logging or 
something, and only needed TXD3 for that, but still wanted to use pin 
B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like it 
leaves things as flexible as possible, avoiding imposing any artificial 
constraints.


Zev

RE: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Ryan Chen]

Hello,
> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Tuesday, December 14, 2021 11:21 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
> <chiawei_wang@aspeedtech.com>
> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
> support for TXD3/RXD3 pins
> 
> On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
> >Hello,
> >
> >> -----Original Message-----
> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> Sent: Tuesday, December 14, 2021 9:33 AM
> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei
> >> Wang <chiawei_wang@aspeedtech.com>
> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
> >> add support for TXD3/RXD3 pins
> >>
> >> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
> >> >Hello,
> >> >	You may need claim for function group for link, not for pin link.
> >> >	Ex.
> >> >	static struct aspeed_sig_desc uart3_link[] = {
> >> >		{ 0x80, BIT(22), 0},
> >> >		{ 0x80, BIT(23), 0},
> >> >	}
> >> >
> >> >	ast2400_groups[] = {
> >> >	{ "UART3", 2, uart3_link },
> >> >Ryan
> >>
> >> Hi Ryan,
> >>
> >> That possibility occurred to me, but the existing function/group
> >> names in arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378)
> >> made me think they should be separate.
> >This device tree is copied from Linux kernel.
> >
> >> I'm certainly not an expert on pinctrl stuff though...is there some
> >> other existing logic or mechanism to link a "UART3" to the separate
> >> "TXD3" and "RXD3" in the device tree?
> >There is no separate in u-boot device tree.
> 
> Perhaps we're misunderstanding each other...
> 
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133
> 
> and
> 
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378
> 
The following is my point.
https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4fa5013d6dd/arch/arm/dts/ast2400.dtsi#L3

> look fairly separate to me?
> 
> >May I know why you need separate?
> 
> In my particular case I don't need these two to be separate, but it seems
> conceivable that there might be other cases that would require a different set
> of signals to be enabled for a generic "UART3" group -- possibly more
> (sideband signals like CTS and such), or perhaps even fewer (e.g. if you had an
> output-only UART3 just for logging or something, and only needed TXD3 for
> that, but still wanted to use pin
> B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like it leaves
> things as flexible as possible, avoiding imposing any artificial constraints.
> 
> 
> Zev

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Mon, Dec 13, 2021 at 07:29:48PM PST, Ryan Chen wrote:
>Hello,
>> -----Original Message-----
>> From: Zev Weiss <zev@bewilderbeest.net>
>> Sent: Tuesday, December 14, 2021 11:21 AM
>> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
>> <chiawei_wang@aspeedtech.com>
>> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
>> support for TXD3/RXD3 pins
>>
>> On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
>> >Hello,
>> >
>> >> -----Original Message-----
>> >> From: Zev Weiss <zev@bewilderbeest.net>
>> >> Sent: Tuesday, December 14, 2021 9:33 AM
>> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei
>> >> Wang <chiawei_wang@aspeedtech.com>
>> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
>> >> add support for TXD3/RXD3 pins
>> >>
>> >> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
>> >> >Hello,
>> >> >	You may need claim for function group for link, not for pin link.
>> >> >	Ex.
>> >> >	static struct aspeed_sig_desc uart3_link[] = {
>> >> >		{ 0x80, BIT(22), 0},
>> >> >		{ 0x80, BIT(23), 0},
>> >> >	}
>> >> >
>> >> >	ast2400_groups[] = {
>> >> >	{ "UART3", 2, uart3_link },
>> >> >Ryan
>> >>
>> >> Hi Ryan,
>> >>
>> >> That possibility occurred to me, but the existing function/group
>> >> names in arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378)
>> >> made me think they should be separate.
>> >This device tree is copied from Linux kernel.
>> >
>> >> I'm certainly not an expert on pinctrl stuff though...is there some
>> >> other existing logic or mechanism to link a "UART3" to the separate
>> >> "TXD3" and "RXD3" in the device tree?
>> >There is no separate in u-boot device tree.
>>
>> Perhaps we're misunderstanding each other...
>>
>> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
>> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133
>>
>> and
>>
>> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
>> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378
>>
>The following is my point.
>https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4fa5013d6dd/arch/arm/dts/ast2400.dtsi#L3
>

I'm afraid I don't follow...how does it being copied from the Linux 
kernel device tree affect whether or not we should group these two or 
keep them separate?


>> look fairly separate to me?
>>
>> >May I know why you need separate?
>>
>> In my particular case I don't need these two to be separate, but it seems
>> conceivable that there might be other cases that would require a different set
>> of signals to be enabled for a generic "UART3" group -- possibly more
>> (sideband signals like CTS and such), or perhaps even fewer (e.g. if you had an
>> output-only UART3 just for logging or something, and only needed TXD3 for
>> that, but still wanted to use pin
>> B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like it leaves
>> things as flexible as possible, avoiding imposing any artificial constraints.
>>
>>
>> Zev
>

RE: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Ryan Chen]

Hello,
> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Tuesday, December 14, 2021 11:43 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
> <chiawei_wang@aspeedtech.com>
> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
> support for TXD3/RXD3 pins
> 
> On Mon, Dec 13, 2021 at 07:29:48PM PST, Ryan Chen wrote:
> >Hello,
> >> -----Original Message-----
> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> Sent: Tuesday, December 14, 2021 11:21 AM
> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei
> >> Wang <chiawei_wang@aspeedtech.com>
> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
> >> add support for TXD3/RXD3 pins
> >>
> >> On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
> >> >Hello,
> >> >
> >> >> -----Original Message-----
> >> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> >> Sent: Tuesday, December 14, 2021 9:33 AM
> >> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>;
> >> >> ChiaWei Wang <chiawei_wang@aspeedtech.com>
> >> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
> >> >> add support for TXD3/RXD3 pins
> >> >>
> >> >> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
> >> >> >Hello,
> >> >> >	You may need claim for function group for link, not for pin link.
> >> >> >	Ex.
> >> >> >	static struct aspeed_sig_desc uart3_link[] = {
> >> >> >		{ 0x80, BIT(22), 0},
> >> >> >		{ 0x80, BIT(23), 0},
> >> >> >	}
> >> >> >
> >> >> >	ast2400_groups[] = {
> >> >> >	{ "UART3", 2, uart3_link },
> >> >> >Ryan
> >> >>
> >> >> Hi Ryan,
> >> >>
> >> >> That possibility occurred to me, but the existing function/group
> >> >> names in arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378)
> >> >> made me think they should be separate.
> >> >This device tree is copied from Linux kernel.
> >> >
> >> >> I'm certainly not an expert on pinctrl stuff though...is there
> >> >> some other existing logic or mechanism to link a "UART3" to the
> >> >> separate "TXD3" and "RXD3" in the device tree?
> >> >There is no separate in u-boot device tree.
> >>
> >> Perhaps we're misunderstanding each other...
> >>
> >>
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133
> >>
> >> and
> >>
> >>
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378
> >>
> >The following is my point.
> >https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a
> 4fa
> >5013d6dd/arch/arm/dts/ast2400.dtsi#L3
> >
> 
> I'm afraid I don't follow...how does it being copied from the Linux kernel device
> tree affect whether or not we should group these two or keep them separate?
My point is the original dtsi file is copy from kernel.
So that dtsi define is inherit. So that you see in currently u-boot.

> 
> 
> >> look fairly separate to me?
> >>
> >> >May I know why you need separate?
> >>
> >> In my particular case I don't need these two to be separate, but it
> >> seems conceivable that there might be other cases that would require
> >> a different set of signals to be enabled for a generic "UART3" group
> >> -- possibly more (sideband signals like CTS and such), or perhaps
> >> even fewer (e.g. if you had an output-only UART3 just for logging or
> >> something, and only needed TXD3 for that, but still wanted to use pin
> >> B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like it
> >> leaves things as flexible as possible, avoiding imposing any artificial
> constraints.
> >>
> >>
> >> Zev
> >

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Mon, Dec 13, 2021 at 09:21:36PM PST, Ryan Chen wrote:
>Hello,
>> -----Original Message-----
>> From: Zev Weiss <zev@bewilderbeest.net>
>> Sent: Tuesday, December 14, 2021 11:43 AM
>> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
>> <chiawei_wang@aspeedtech.com>
>> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
>> support for TXD3/RXD3 pins
>>
>> On Mon, Dec 13, 2021 at 07:29:48PM PST, Ryan Chen wrote:
>> >Hello,
>> >> -----Original Message-----
>> >> From: Zev Weiss <zev@bewilderbeest.net>
>> >> Sent: Tuesday, December 14, 2021 11:21 AM
>> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei
>> >> Wang <chiawei_wang@aspeedtech.com>
>> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
>> >> add support for TXD3/RXD3 pins
>> >>
>> >> On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
>> >> >Hello,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Zev Weiss <zev@bewilderbeest.net>
>> >> >> Sent: Tuesday, December 14, 2021 9:33 AM
>> >> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
>> >> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>;
>> >> >> ChiaWei Wang <chiawei_wang@aspeedtech.com>
>> >> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
>> >> >> add support for TXD3/RXD3 pins
>> >> >>
>> >> >> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
>> >> >> >Hello,
>> >> >> >	You may need claim for function group for link, not for pin link.
>> >> >> >	Ex.
>> >> >> >	static struct aspeed_sig_desc uart3_link[] = {
>> >> >> >		{ 0x80, BIT(22), 0},
>> >> >> >		{ 0x80, BIT(23), 0},
>> >> >> >	}
>> >> >> >
>> >> >> >	ast2400_groups[] = {
>> >> >> >	{ "UART3", 2, uart3_link },
>> >> >> >Ryan
>> >> >>
>> >> >> Hi Ryan,
>> >> >>
>> >> >> That possibility occurred to me, but the existing function/group
>> >> >> names in arch/arm/dts/ast2400.dtsi (lines 1130-1133 and 1375-1378)
>> >> >> made me think they should be separate.
>> >> >This device tree is copied from Linux kernel.
>> >> >
>> >> >> I'm certainly not an expert on pinctrl stuff though...is there
>> >> >> some other existing logic or mechanism to link a "UART3" to the
>> >> >> separate "TXD3" and "RXD3" in the device tree?
>> >> >There is no separate in u-boot device tree.
>> >>
>> >> Perhaps we're misunderstanding each other...
>> >>
>> >>
>> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
>> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133
>> >>
>> >> and
>> >>
>> >>
>> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
>> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378
>> >>
>> >The following is my point.
>> >https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a
>> 4fa
>> >5013d6dd/arch/arm/dts/ast2400.dtsi#L3
>> >
>>
>> I'm afraid I don't follow...how does it being copied from the Linux kernel device
>> tree affect whether or not we should group these two or keep them separate?
>My point is the original dtsi file is copy from kernel.
>So that dtsi define is inherit. So that you see in currently u-boot.
>

I mean, I saw that comment and I'm aware of the derivation of the file, 
but I'm still not sure what bearing it has on the question at hand.

Is your view that because the dts was initially just copied from the 
kernel, it's not necessarily the right fit for u-boot, and that we 
should change it to unify these two functions in a single group?  If so, 
I guess I'm still wondering what tangible benefit that would have, and 
about the flexibility issue I raised a few messages back.

If that's not what you're aiming to suggest, pardon me if I'm being 
dense here, but I'm going to need a more detailed explanation, because 
as it stands I'm still pretty mystified.


>>
>>
>> >> look fairly separate to me?
>> >>
>> >> >May I know why you need separate?
>> >>
>> >> In my particular case I don't need these two to be separate, but it
>> >> seems conceivable that there might be other cases that would require
>> >> a different set of signals to be enabled for a generic "UART3" group
>> >> -- possibly more (sideband signals like CTS and such), or perhaps
>> >> even fewer (e.g. if you had an output-only UART3 just for logging or
>> >> something, and only needed TXD3 for that, but still wanted to use pin
>> >> B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like it
>> >> leaves things as flexible as possible, avoiding imposing any artificial
>> constraints.
>> >>
>> >>
>> >> Zev
>> >

RE: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Ryan Chen]

Hello,
> -----Original Message-----
> From: Zev Weiss <zev@bewilderbeest.net>
> Sent: Wednesday, December 15, 2021 10:50 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei Wang
> <chiawei_wang@aspeedtech.com>
> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add
> support for TXD3/RXD3 pins
> 
> On Mon, Dec 13, 2021 at 09:21:36PM PST, Ryan Chen wrote:
> >Hello,
> >> -----Original Message-----
> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> Sent: Tuesday, December 14, 2021 11:43 AM
> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>; ChiaWei
> >> Wang <chiawei_wang@aspeedtech.com>
> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
> >> add support for TXD3/RXD3 pins
> >>
> >> On Mon, Dec 13, 2021 at 07:29:48PM PST, Ryan Chen wrote:
> >> >Hello,
> >> >> -----Original Message-----
> >> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> >> Sent: Tuesday, December 14, 2021 11:21 AM
> >> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>;
> >> >> ChiaWei Wang <chiawei_wang@aspeedtech.com>
> >> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400:
> >> >> add support for TXD3/RXD3 pins
> >> >>
> >> >> On Mon, Dec 13, 2021 at 05:39:17PM PST, Ryan Chen wrote:
> >> >> >Hello,
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Zev Weiss <zev@bewilderbeest.net>
> >> >> >> Sent: Tuesday, December 14, 2021 9:33 AM
> >> >> >> To: Ryan Chen <ryan_chen@aspeedtech.com>
> >> >> >> Cc: openbmc@lists.ozlabs.org; Joel Stanley <joel@jms.id.au>;
> >> >> >> ChiaWei Wang <chiawei_wang@aspeedtech.com>
> >> >> >> Subject: Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl:
> ast2400:
> >> >> >> add support for TXD3/RXD3 pins
> >> >> >>
> >> >> >> On Mon, Dec 13, 2021 at 05:22:52PM PST, Ryan Chen wrote:
> >> >> >> >Hello,
> >> >> >> >	You may need claim for function group for link, not for pin link.
> >> >> >> >	Ex.
> >> >> >> >	static struct aspeed_sig_desc uart3_link[] = {
> >> >> >> >		{ 0x80, BIT(22), 0},
> >> >> >> >		{ 0x80, BIT(23), 0},
> >> >> >> >	}
> >> >> >> >
> >> >> >> >	ast2400_groups[] = {
> >> >> >> >	{ "UART3", 2, uart3_link },
> >> >> >> >Ryan
> >> >> >>
> >> >> >> Hi Ryan,
> >> >> >>
> >> >> >> That possibility occurred to me, but the existing
> >> >> >> function/group names in arch/arm/dts/ast2400.dtsi (lines
> >> >> >> 1130-1133 and 1375-1378) made me think they should be separate.
> >> >> >This device tree is copied from Linux kernel.
> >> >> >
> >> >> >> I'm certainly not an expert on pinctrl stuff though...is there
> >> >> >> some other existing logic or mechanism to link a "UART3" to the
> >> >> >> separate "TXD3" and "RXD3" in the device tree?
> >> >> >There is no separate in u-boot device tree.
> >> >>
> >> >> Perhaps we're misunderstanding each other...
> >> >>
> >> >>
> >>
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> >> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1130-L1133
> >> >>
> >> >> and
> >> >>
> >> >>
> >>
> https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a4
> >> >> fa5013d6dd/arch/arm/dts/ast2400.dtsi#L1375-L1378
> >> >>
> >> >The following is my point.
> >>
> >https://github.com/openbmc/u-boot/blob/a570745a1a836e351bd4b1131f23a
> >> 4fa
> >> >5013d6dd/arch/arm/dts/ast2400.dtsi#L3
> >> >
> >>
> >> I'm afraid I don't follow...how does it being copied from the Linux
> >> kernel device tree affect whether or not we should group these two or keep
> them separate?
> >My point is the original dtsi file is copy from kernel.
> >So that dtsi define is inherit. So that you see in currently u-boot.
> >
> 
> I mean, I saw that comment and I'm aware of the derivation of the file, but I'm
> still not sure what bearing it has on the question at hand.
> 
> Is your view that because the dts was initially just copied from the kernel, it's
> not necessarily the right fit for u-boot, and that we should change it to unify
> these two functions in a single group?  If so, I guess I'm still wondering what
> tangible benefit that would have, and about the flexibility issue I raised a few
> messages back.
> 
Yes, it is my original through for dtsi file.
It is ok for this add for TXD3/RTX3 function for pinctrl.

> If that's not what you're aiming to suggest, pardon me if I'm being dense here,
> but I'm going to need a more detailed explanation, because as it stands I'm
> still pretty mystified.
> 
> 
> >>
> >>
> >> >> look fairly separate to me?
> >> >>
> >> >> >May I know why you need separate?
> >> >>
> >> >> In my particular case I don't need these two to be separate, but
> >> >> it seems conceivable that there might be other cases that would
> >> >> require a different set of signals to be enabled for a generic
> >> >> "UART3" group
> >> >> -- possibly more (sideband signals like CTS and such), or perhaps
> >> >> even fewer (e.g. if you had an output-only UART3 just for logging
> >> >> or something, and only needed TXD3 for that, but still wanted to
> >> >> use pin
> >> >> B14 as GPIOE7 instead of RXD3).  Keeping them separate seems like
> >> >> it leaves things as flexible as possible, avoiding imposing any
> >> >> artificial
> >> constraints.
> >> >>
> >> >>
> >> >> Zev
> >> >

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Mon, Dec 13, 2021 at 11:47:03AM PST, Zev Weiss wrote:
>In order to support putting the u-boot console on UART3 of the
>ast2400, this commit adds support for setting bits 22 and 23 of SCU80
>to enable TXD3 and RXD3 on pins C14 and B14, respectively.
>
>Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>---
> drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>

Ping...Joel?

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Joel Stanley]

On Tue, 25 Jan 2022 at 17:32, Zev Weiss <zweiss@equinix.com> wrote:
>
> On Mon, Dec 13, 2021 at 11:47:03AM PST, Zev Weiss wrote:
> >In order to support putting the u-boot console on UART3 of the
> >ast2400, this commit adds support for setting bits 22 and 23 of SCU80
> >to enable TXD3 and RXD3 on pins C14 and B14, respectively.
> >
> >Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> >---
> > drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
>
> Ping...Joel?


Pinctrl is not my jam. Did you resolve Ryan's concerns?

Andrew, does this change make sense?

Cheers,

Joel

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Zev Weiss]

On Sun, Jan 30, 2022 at 10:25:38PM PST, Joel Stanley wrote:
>On Tue, 25 Jan 2022 at 17:32, Zev Weiss <zweiss@equinix.com> wrote:
>>
>> On Mon, Dec 13, 2021 at 11:47:03AM PST, Zev Weiss wrote:
>> >In order to support putting the u-boot console on UART3 of the
>> >ast2400, this commit adds support for setting bits 22 and 23 of SCU80
>> >to enable TXD3 and RXD3 on pins C14 and B14, respectively.
>> >
>> >Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> >---
>> > drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
>> > 1 file changed, 10 insertions(+)
>> >
>>
>> Ping...Joel?
>
>
>Pinctrl is not my jam. Did you resolve Ryan's concerns?
>

I had interpreted his last message as indicating that he thought it was 
OK as is, though perhaps I misunderstood -- Ryan, can you confirm?

Zev

Re: [PATCH u-boot v2019.04-aspeed-openbmc] pinctrl: ast2400: add support for TXD3/RXD3 pins

[Joel Stanley]

On Mon, 31 Jan 2022 at 06:25, Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 25 Jan 2022 at 17:32, Zev Weiss <zweiss@equinix.com> wrote:
> >
> > On Mon, Dec 13, 2021 at 11:47:03AM PST, Zev Weiss wrote:
> > >In order to support putting the u-boot console on UART3 of the
> > >ast2400, this commit adds support for setting bits 22 and 23 of SCU80
> > >to enable TXD3 and RXD3 on pins C14 and B14, respectively.
> > >
> > >Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> > >---
> > > drivers/pinctrl/aspeed/pinctrl_ast2400.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> >
> > Ping...Joel?
>
>
> Pinctrl is not my jam. Did you resolve Ryan's concerns?
>
> Andrew, does this change make sense?

Given the lack of objection, I have merged this into the tree.

Cheers,

Joel