Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5058591a3e0f0cb82b26cffe888d87b4d1bdd713.camel@ew.tq-group.com>
Subject: Re: [PATCH v2 1/2] arm64: dts: ti: k3-am65: disable optional
 peripherals by default
From:   Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To:     Tony Lindgren <tony@atomide.com>
Cc:     Rob Herring <robh+dt@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Olof Johansson <olof@lixom.net>, soc@kernel.org,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Tero Kristo <kristo@kernel.org>, jan.kiszka@siemens.com,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, Nishanth Menon <nm@ti.com>
Date:   Wed, 09 Mar 2022 12:10:50 +0100
In-Reply-To: <YihvWFdr1cT7cyk5@atomide.com>
References: <20220203140240.973690-1-matthias.schiffer@ew.tq-group.com>
         <20220204143108.653qk2ihnlhsr5aa@prior> <YgDCLaBHA3DDQAUd@atomide.com>
         <5944ba0ce568eaf507917799b1dfd89a3d0ca492.camel@ew.tq-group.com>
         <YgEBml9HvFzSl289@atomide.com>
         <9923df6525212389b86cb635624bcfb5c27a8bc5.camel@ew.tq-group.com>
         <YgJZbdOlazrde7O/@atomide.com>
         <1356e93cd5b101c3d896e35250c66959ed631544.camel@ew.tq-group.com>
         <YihvWFdr1cT7cyk5@atomide.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2022-03-09 at 11:11 +0200, Tony Lindgren wrote:
> Hi,
> 
> * Matthias Schiffer <matthias.schiffer@ew.tq-group.com> [220228
> 10:29]:
> > AFAICT, disabling non-operatational devices in the board DTS
> > instead of
> > the SoC DTSI is worse than the alternatives in every way:
> > 
> > - Verbose board DTS: You have to think about all the devices that
> > exist
> > in the SoC, not just the ones you want to use
> > - Adding new nodes without `status = "disabled" to SoC DTSI can
> > potentially cause issues on dependent boards
> > - It doesn't solve the issues that not having `status = "disabled"`
> > in
> > the DTSI is supposed to solve
> 
> My preference is the least amount of tinkering in the dts files
> naturally :) It really does not matter if the extra dts churn is to
> enable or disable devices, it should not be needed at all.
> 
> To summarize, my main point really is the following:
> 
> There should not be any need to tag the SoC internal devices with
> anything
> in the dts files. The device drivers should be able to just deal with
> the
> situation. IMO devices should be tagged with disabled or reserved
> when
> they are not accessible for example because of being used by secure
> mode
> for example. If the the status needs to be set to anything, it really
> is
> a symptom of incomplete handling somewhere.
> 
> Regards,
> 
> Tony


Hi Tony,

while I agree that it would be great if drivers could just detect when
hardware is not available, this is simply not how most drivers work -
when you instantiate the driver via a non-disabled(/reserved/...) DT
node, the driver expects a usable device.

Especially for busses like I2C, there is no way for a driver to
reliably detect whether the bus is usable or not. (There are several
states that can't really be distinguished: Is pinmuxing missing, or
does the device not need any muxing? Is a line low because it is not
actually connected to anything, or is there another master currently
using the bus, or is the bus stuck due to a faulty device?)

Which is why it is the convention for SoC DTSI files disable nodes for
devices that may be unusable. Taking UARTs as an example, a quick grep
for "serial@" nodes in arch/arm/boot/dts/*.dtsi and
arch/arm64/boot/dts/*/*.dtsi shows that the vast majority of these
nodes is disabled by default.

Regards,
Matthias