[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 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.

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
>> >