Message-ID: <20241002135246.dvzciibdcdb2r4ni@education>
Date: Wed, 2 Oct 2024 08:52:46 -0500
From: Nishanth Menon <nm@...com>
To: Francesco Dolcini <francesco@...cini.it>
CC: Krzysztof Kozlowski <krzk@...nel.org>,
        João Paulo Gonçalves <jpaulo.silvagoncalves@...il.com>,
        Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
        Rob
 Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor
 Dooley <conor+dt@...nel.org>,
        João Paulo Gonçalves
	<joao.goncalves@...adex.com>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] arm64: dts: ti: k3-am62-verdin: Update tla2024 adc
 compatible

On 16:13-20241001, Francesco Dolcini wrote:
> On Tue, Oct 01, 2024 at 03:59:39PM +0200, Krzysztof Kozlowski wrote:
> > On 01/10/2024 15:01, Francesco Dolcini wrote:
> > > On Tue, Oct 01, 2024 at 01:54:56PM +0200, Krzysztof Kozlowski wrote:
> > >> On 01/10/2024 13:14, João Paulo Gonçalves wrote:
> > >>> From: João Paulo Gonçalves <joao.goncalves@...adex.com>
> > >>>
> > >>> With commit f1c9ce0ced2d ("iio: adc: ti-ads1015: Add TLA2024 support") a
> > >>> new compatible was introduced for TLA2024 ADC. Update the device
> > >>> tree to use the correct compatible for the Verdin-AM62 hardware.
> > >>>
> > >>> Signed-off-by: João Paulo Gonçalves <joao.goncalves@...adex.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi | 2 +-
> > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> > >>> index 5bef31b8577b..f201722d81b3 100644
> > >>> --- a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> > >>> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> > >>> @@ -1220,7 +1220,7 @@ sensor@48 {
> > >>>  	};
> > >>>
> > >>>  	adc@49 {
> > >>> -		compatible = "ti,ads1015";
> > >>> +		compatible = "ti,tla2024";
> > >>
> > >> So it is not always TI, who breaks their users. :) (as pointed out in
> > >> LPC DT BoF).
> > > 
> > > So, let's adjust what I said at that time, I think is important, and I
> > > appreciate you giving me an excuse for doing that :-)
> > > 
> > > Lately as Toradex we are working a lot with TI, and one of the reasons is
> > > that they have a great software support, backed-up by a great strategy
> > > on the way they contribute to the various upstream projects they build
> > > their SDK on top (Linux, U-Boot, and more).
> > > 
> > > With that is normal that while working so closely with them we find
> > > issues, everybody have those, it's just that those are the one we
> > > care the most at the moment :-). Not to mention that we started working
> > > with TI a couple of years ago, so TI is still somehow "new" to us and we
> > > are still "learning".
> > > 
> > > On this regards I was recently working on updating our BSP to the
> > > latest SDK from TI, that is based on a v6.6 stable kernel and looking at
> > > the patches we had to apply on top, the total counts of the patches we
> > > do not have in mainline to support the board subject of this patch is
> > > just _zero_. This to me is a great achievement.
> > > 
> > > Nishant: this is also for you, and feel free to "market" this
> > > internally/externally :-)
> > > 
> > > 
> > >> If you want to break users, sure, but at least explain in commit msg why.
> > > 
> > > Now, on this specific topic, the actual device that is assembled on this
> > > board is a TI TLA2024, and it's like that since ever, the board never
> > > changed. The current compatible is not matching what is assembled on
> > > board. It works because the device is close enough to TI ADS1015.
> > > 
> > > With that said, I do not think this is breaking any actual compatibility
> > > issue.
> > > 
> > >  - The old DTB will keep working with old and new kernel.
> > 
> > New DTB stops working with old kernel and this is what we talked about
> > during LPC.
> 
> My mind at that time was really on using old DTB with a new kernel, not that
> other way around.
> 
> In any case, I do not think that this comment applies on this specific case,
> as I wrote you cannot really run this board on a kernel that does not support
> the ti,tla2024 compatible.
> 
> > All out-of-tree users of this DTS, like other operating systems, will be
> > affected as well probably.
> 
> Well, yes. From what I know those user do not exist and this is just
> theoretical, but, I might be as well wrong and I see your point.
> 
> So, let me try to sum it up, I see 2 options:
> 
>  1 - we drop this change. this is fine for me.
>  2 - we add a comment in the commit message that this is a breaking change, and
>      while I am not aware of any impact with real software that is available today,
>      I might have incomplete information.

>From the discussion:
a) old dtb will continue to work on new kernel (using the older driver
   compatible)
b) new dtb will work on new kernel
c) new dtb with older kernel depends on the kernel support for the
   driver. Looking at compatible in drivers/iio/adc/ti-ads1015.c and
   commit f1c9ce0ced2d, I see the oldest kernel is around v5.18 or .19 kernel
   when at least this file was not yet in mainline.

So, it sounds like a fixes to indicate the correct compatible to me at
least.

I suggest (2) and update the commit message to indicate backward and
forward compatibility aspect - I do not see either broken here.

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

Powered by blists - more mailing lists