linux-kernel - Re: [STLinux Kernel] [PATCH 3/8] serial: st-asc: Read in all Pinctrl states

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170201124751.f3r7l52vneqgilac@dell>
Date:   Wed, 1 Feb 2017 12:47:51 +0000
From:   Lee Jones <lee.jones@...aro.org>
To:     Peter Griffin <peter.griffin@...aro.org>
Cc:     gregkh@...uxfoundation.org, jslaby@...e.com,
        linux-serial@...r.kernel.org, robh+dt@...nel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, kernel@...inux.com
Subject: Re: [STLinux Kernel] [PATCH 3/8] serial: st-asc: Read in all Pinctrl
 states

> > > > Again, doesn't matter, since it's the DTB that provides the default
> > > > state.  So, back when it was authored, the default state was HW
> > > > flow-control disabled.  And in a newer DTB (again, until I follow-up
> > > > with more changes), the defaults for UART 1 and UART 2 are HW
> > > > flow-control disabled.
> > > > 
> > > > Your issue seems to be that you've assumed since we now provide the
> > > > possibility of a "manual-rts" state, then the "default" state should
> > > > *only* be HW flow-control capable, which is not the case.
> > > 
> > > No my feedback was that it would be clearer & simpler to make manual-rts the
> > > 'default' state, and 'hw-flow-control' the optional state.
> > 
> > Absolutely not.  The use of "manual-rts" is the corner-case here and
> > is not normally required.
> 
> See below.
> 
> > The "default" state should normally be
> > populated with whatever pins are available i.e. all 4 pins (including
> > "rts, cts") if they are wired up and only 2 pins (just "tx, rx") if
> > they are not.
> 
> Yep OK, I agree :)

\o/

> > The submission of "manual-rts" is only required if the
> > RTS pin is required for some other purpose e.g. resetting a uC on a
> > draughtboard.
> 
> All UARTs the SoC have the same st-asc IP, which suffers from the same
> hw flow control limitation. Also all instances on the SoC have rts/cts
> pins, the only limitation is board wiring.
> 
> So I can't see why would you ever *not* want to deploy this dynamic pin
> switching solution if rts/cts is wired up at board level now the facility
> exists?

Mainly because it's surplus to requirement, in that there is very
seldom any point in manually toggling the RTS line (at least to my
knowledge).  I figure we'd add >1 Pinctrl states only when the need
arises, thus keeping the DTS' as simple as possible.

> Also regarding the naming of the second pin group, 'manual-rts' seems like
> a bad name as a logical extension of this set is to also offer the same
> dynamic switching for the CTS line.
> 
> Maybe a better name would be 'tx-rx-only' or 'no-rts-cts'.

Works for me.  Will fix.

> > > > It's the
> > > > 'uart-has-rtscts' property which determines this *not* whether the
> > > > second state has been provided.
> > > 
> > > Yep, which is why IMO it makes more sense for the optional pin group to be the hw
> > > flow control pins which are obtained if the uart-has-rtscts property is present.
> > 
> > There would normally only be one pin group.  Your method would insist
> > we always provided 2, which would be surplus to requirement.
> 
> Yep OK, agree with your point.

\o/

> Yep OK, I agree.

\o/

> Yep OK, I agree.

\o/

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog