Message-ID: <20201106214638.amgltswy6dygnyee@tubular>
Date:   Fri, 6 Nov 2020 15:46:38 -0600
From:   Nishanth Menon <nm@...com>
To:     Peter Ujfalusi <peter.ujfalusi@...com>
CC:     Roger Quadros <rogerq@...com>, Keerthy <j-keerthy@...com>,
        Jyri Sarha <jsarha@...com>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        Lokesh Vutla <lokeshvutla@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Tony Lindgren <tony@...mide.com>,
        Tero Kristo <t-kristo@...com>, <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at
 SoC dtsi level

On 13:32-20201106, Peter Ujfalusi wrote:
[...]
> > 
> >>
> >>> default power management functionality etc
> >>
> >> Right, so how does that helps with devices present in the SoC, but no
> >> node at all? First thing which comes to mind is AASRC, we don't have
> >> Linux driver for it (and no DT binding document), but that does not mean
> >> that it is not present. How PM would take that into account?
> > 
> > I think we are mixing topics here -> I was stating the motivation why
> > devicetree chose such as default.
> 
> I don't question the fact that 'okay' is the default status if it is not
> explicitly present. There is no better default than that.

^^ -> Alright, that is all we are trying to do here: defaults in the
SoC.dtsi and specific cleanups (firmware reserved / board unused
disables) be done in a common board.dtsi (for now, there is no such
specific need, I guess).

[..]

> 
> >> The reason why we kept McASP nodes (and dss) disabled in the soc dtsi
> >> file is that they are not operation in the form they present in there.
> >> They _need_ additional properties to be operational and those properties
> >> can only be added in the board dts file.
> > 
> > I dont think we are changing anything in the output dtb files,
> 
> Correct, the resulted dtb is identical. If the developer for upcoming
> boards did check the schematics vs TRM vs dtsi and spot the things that
> is not configured.

Yes.

> 
> > we are
> > just leaving the defaults as dt defaults and set the disable state in
> > board dts OR common board dtsi.
> 
> Yes, we leave the non working/configured node 'okay' in dtsi and expect
> that the board file author will know which node must be disabled because
> it is incomplete.

Yes - I understand(and empathise) the implicit omission error risk we
are incurring here. I will add that to the commit message as well.

> >> This is not remotely a subjective view, this is the opposite of
> >> subjectivity.
> > 
> > the usage of McASP was'nt meant as (c).. it is (b). is there a better way
> > to describe this in a generic manner?
> 
> I had my saying on that ever since I have been taking care of audio on
> TI SoCs ;)
> 
> I used similar analogy in a private thread around this, but imho it fits
> the case neatly:
> car == McASP
> 
> you don't put an 'okay' (as is ready, operational) stamp on the car in
> the middle of the production line when the engine is not even installed.

Completely agree with you. we are just insisting that this be done in
either common board.dtsi OR board.dtsi where applicable.

> 

[..]

> > Alright - what do we suggest we do?
> 
> Not sure, I'm 'whatever' after [1] makes it to mainline or next.
[....]
> [1]
> https://lore.kernel.org/alsa-devel/20201106072551.689-1-peter.ujfalusi@ti.com/


I don't see the relationship between the series.. I think this series
brings no change in dtb, hence with OR without your driver cleanup
series, there is no practical regressions.
> 
> > Tony, Rob - I need some guidance here.
> 
> I'm fine whatever way we take, but I think it is up to you to make the
> call as the maintainer of the TI dts files... ;)

Yep - I have'nt seen a reason yet that must cause us to change from the
Device tree default approach in our debates.


As Tony already pointed out.. if we start seeing a lot more boards for
an SoC..
Instead of (reverse issue- where we have a lot of places where people are
doing "okay" problem):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=12afc0cf81210969756daecd7eb48b307f08faed

We should look at ways to consolidate in a common-board.dtsi

> 
> >>
> >>>  &serdes0 {
> > 	[...]
> >>>  
> >>>  	watchdog0: watchdog@...0000 {
> >>>
> >>
> >> There is no such a tag, but:
> >> whatever-by: Peter Ujfalusi <peter.ujfalusi@...com>
> > 
> > OK - I have no idea how B4 or patchworks pick that one as :D
> 
> If we take this road, than I'm okay with it, but I'm going to take
> silent protest (not sending acked-by or revired-by).
> That should not stop you doing what you believe is best for the future!

OK - thanks for your review and the discussions, always appreciate
getting our views out there.

if there are no other comments, I will try and post a v2 over the
weekend.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Powered by blists - more mailing lists