| Message ID | 20241001111413.10390-1-jpaulo.silvagoncalves@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] arm64: dts: ti: k3-am62-verdin: Update tla2024 adc compatible | expand |
On Tue, Oct 01, 2024 at 08:14:13AM -0300, João Paulo Gonçalves wrote: > From: João Paulo Gonçalves <joao.goncalves@toradex.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@toradex.com> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
On 01/10/2024 13:14, João Paulo Gonçalves wrote: > From: João Paulo Gonçalves <joao.goncalves@toradex.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@toradex.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). If you want to break users, sure, but at least explain in commit msg why. Best regards, Krzysztof
Hello Krzysztof, 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@toradex.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@toradex.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. - The commit adding support for TI TLA2024 is in v5.19, and this board is supported only from kernel v6.5, so you cannot run an older kernel with this board. In addition to that I do not think that is reasonable to have someone using a "new" dtb with an "old" kernel. - The firmware, U-Boot, the only one that currently supports this board, is not making any use of this compatible nor of this ADC. Francesco
On 01/10/2024 15:01, Francesco Dolcini wrote: > Hello Krzysztof, > > 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@toradex.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@toradex.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. All out-of-tree users of this DTS, like other operating systems, will be affected as well probably. > - The commit adding support for TI TLA2024 is in v5.19, and this board > is supported only from kernel v6.5, so you cannot run an older kernel > with this board. In addition to that I do not think that is > reasonable to have someone using a "new" dtb with an "old" kernel. > - The firmware, U-Boot, the only one that currently supports this board, > is not making any use of this compatible nor of this ADC. Best regards, Krzysztof
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@toradex.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@toradex.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. Francesco
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@toradex.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@toradex.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.
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"; reg = <0x49>; #address-cells = <1>; #size-cells = <0>;