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

[Ryan Chen]

Hello,
> -----Original Message-----
> From: Zev Weiss <zev at bewilderbeest.net>
> Sent: Wednesday, December 15, 2021 10:50 AM
> To: Ryan Chen <ryan_chen at aspeedtech.com>
> Cc: openbmc at lists.ozlabs.org; Joel Stanley <joel at jms.id.au>; ChiaWei Wang
> <chiawei_wang at 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 at bewilderbeest.net>
> >> Sent: Tuesday, December 14, 2021 11:43 AM
> >> To: Ryan Chen <ryan_chen at aspeedtech.com>
> >> Cc: openbmc at lists.ozlabs.org; Joel Stanley <joel at jms.id.au>; ChiaWei
> >> Wang <chiawei_wang at 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 at bewilderbeest.net>
> >> >> Sent: Tuesday, December 14, 2021 11:21 AM
> >> >> To: Ryan Chen <ryan_chen at aspeedtech.com>
> >> >> Cc: openbmc at lists.ozlabs.org; Joel Stanley <joel at jms.id.au>;
> >> >> ChiaWei Wang <chiawei_wang at 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 at bewilderbeest.net>
> >> >> >> Sent: Tuesday, December 14, 2021 9:33 AM
> >> >> >> To: Ryan Chen <ryan_chen at aspeedtech.com>
> >> >> >> Cc: openbmc at lists.ozlabs.org; Joel Stanley <joel at jms.id.au>;
> >> >> >> ChiaWei Wang <chiawei_wang at 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
> >> >