Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Konrad Dybcio]

On 4/10/24 17:57, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S

autosel has been reeaaaaaly going over the top lately, particularly
with dts patches.. I'm not sure adding support for a device is
something that should go to stable

Konrad

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Greg KH]

On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
> 
> 
> On 4/10/24 17:57, Sasha Levin wrote:
> > This is a note to let you know that I've just added the patch titled
> > 
> >      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
> 
> autosel has been reeaaaaaly going over the top lately, particularly
> with dts patches.. I'm not sure adding support for a device is
> something that should go to stable

Simple device ids and quirks have always been stable material.

thanks,

greg k-h

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Krzysztof Kozlowski]

On 10/04/2024 20:02, Greg KH wrote:
> On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
>>
>>
>> On 4/10/24 17:57, Sasha Levin wrote:
>>> This is a note to let you know that I've just added the patch titled
>>>
>>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
>>
>> autosel has been reeaaaaaly going over the top lately, particularly
>> with dts patches.. I'm not sure adding support for a device is
>> something that should go to stable
> 
> Simple device ids and quirks have always been stable material.
>

That's true, but maybe DTS should have an exception. I guess you think
this is trivial device ID, because the patch contents is small. But it
is or it can be misleading. The patch adds new small DTS file which
includes another file:

	#include "sm7125-xiaomi-common.dtsi"

Which includes another 7 files:

	#include <dt-bindings/arm/qcom,ids.h>
	#include <dt-bindings/firmware/qcom,scm.h>
	#include <dt-bindings/gpio/gpio.h>
	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
	#include "sm7125.dtsi"
	#include "pm6150.dtsi"
	#include "pm6150l.dtsi"

Out of which last three are likely to be changing as well.

This means that following workflow is reasonable and likely:
1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
2. Add some sm7125 board (out of scope here).
3. Release new kernel, e.g. v6.7.
4. Make more changes to sm7125.dtsi
5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.

Now if you backport only (5) above, without (4), it won't work. Might
compile, might not. Even if it compiles, might not work.

The step (4) here might be small, but might be big as well.

IOW, new DTS board is not a quirk, but new hardware description which
relies on other DTS files. Only if all other parts are the same, such
backport is reasonable.




Best regards,
Krzysztof

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Greg KH]

On Wed, Apr 10, 2024 at 08:24:49PM +0200, Krzysztof Kozlowski wrote:
> On 10/04/2024 20:02, Greg KH wrote:
> > On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
> >>
> >>
> >> On 4/10/24 17:57, Sasha Levin wrote:
> >>> This is a note to let you know that I've just added the patch titled
> >>>
> >>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
> >>
> >> autosel has been reeaaaaaly going over the top lately, particularly
> >> with dts patches.. I'm not sure adding support for a device is
> >> something that should go to stable
> > 
> > Simple device ids and quirks have always been stable material.
> >
> 
> That's true, but maybe DTS should have an exception. I guess you think
> this is trivial device ID, because the patch contents is small. But it
> is or it can be misleading. The patch adds new small DTS file which
> includes another file:
> 
> 	#include "sm7125-xiaomi-common.dtsi"
> 
> Which includes another 7 files:
> 
> 	#include <dt-bindings/arm/qcom,ids.h>
> 	#include <dt-bindings/firmware/qcom,scm.h>
> 	#include <dt-bindings/gpio/gpio.h>
> 	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> 	#include "sm7125.dtsi"
> 	#include "pm6150.dtsi"
> 	#include "pm6150l.dtsi"
> 
> Out of which last three are likely to be changing as well.
> 
> This means that following workflow is reasonable and likely:
> 1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
> 2. Add some sm7125 board (out of scope here).
> 3. Release new kernel, e.g. v6.7.
> 4. Make more changes to sm7125.dtsi
> 5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.
> 
> Now if you backport only (5) above, without (4), it won't work. Might
> compile, might not. Even if it compiles, might not work.
> 
> The step (4) here might be small, but might be big as well.

Fair enough.  So should we drop this change?

thanks,

greg k-h

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Krzysztof Kozlowski]

On 11/04/2024 09:22, Greg KH wrote:
> On Wed, Apr 10, 2024 at 08:24:49PM +0200, Krzysztof Kozlowski wrote:
>> On 10/04/2024 20:02, Greg KH wrote:
>>> On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 4/10/24 17:57, Sasha Levin wrote:
>>>>> This is a note to let you know that I've just added the patch titled
>>>>>
>>>>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
>>>>
>>>> autosel has been reeaaaaaly going over the top lately, particularly
>>>> with dts patches.. I'm not sure adding support for a device is
>>>> something that should go to stable
>>>
>>> Simple device ids and quirks have always been stable material.
>>>
>>
>> That's true, but maybe DTS should have an exception. I guess you think
>> this is trivial device ID, because the patch contents is small. But it
>> is or it can be misleading. The patch adds new small DTS file which
>> includes another file:
>>
>> 	#include "sm7125-xiaomi-common.dtsi"
>>
>> Which includes another 7 files:
>>
>> 	#include <dt-bindings/arm/qcom,ids.h>
>> 	#include <dt-bindings/firmware/qcom,scm.h>
>> 	#include <dt-bindings/gpio/gpio.h>
>> 	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> 	#include "sm7125.dtsi"
>> 	#include "pm6150.dtsi"
>> 	#include "pm6150l.dtsi"
>>
>> Out of which last three are likely to be changing as well.
>>
>> This means that following workflow is reasonable and likely:
>> 1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
>> 2. Add some sm7125 board (out of scope here).
>> 3. Release new kernel, e.g. v6.7.
>> 4. Make more changes to sm7125.dtsi
>> 5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.
>>
>> Now if you backport only (5) above, without (4), it won't work. Might
>> compile, might not. Even if it compiles, might not work.
>>
>> The step (4) here might be small, but might be big as well.
> 
> Fair enough.  So should we drop this change?

I vote for dropping. Also, I think such DTS patches should not be picked
automatically via AUTOSEL. Manual backports or targetted Cc-stable,
assuming that backporter investigated it, seem ok.

Best regards,
Krzysztof

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Greg KH]

On Thu, Apr 11, 2024 at 09:27:28AM +0200, Krzysztof Kozlowski wrote:
> On 11/04/2024 09:22, Greg KH wrote:
> > On Wed, Apr 10, 2024 at 08:24:49PM +0200, Krzysztof Kozlowski wrote:
> >> On 10/04/2024 20:02, Greg KH wrote:
> >>> On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
> >>>>
> >>>>
> >>>> On 4/10/24 17:57, Sasha Levin wrote:
> >>>>> This is a note to let you know that I've just added the patch titled
> >>>>>
> >>>>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
> >>>>
> >>>> autosel has been reeaaaaaly going over the top lately, particularly
> >>>> with dts patches.. I'm not sure adding support for a device is
> >>>> something that should go to stable
> >>>
> >>> Simple device ids and quirks have always been stable material.
> >>>
> >>
> >> That's true, but maybe DTS should have an exception. I guess you think
> >> this is trivial device ID, because the patch contents is small. But it
> >> is or it can be misleading. The patch adds new small DTS file which
> >> includes another file:
> >>
> >> 	#include "sm7125-xiaomi-common.dtsi"
> >>
> >> Which includes another 7 files:
> >>
> >> 	#include <dt-bindings/arm/qcom,ids.h>
> >> 	#include <dt-bindings/firmware/qcom,scm.h>
> >> 	#include <dt-bindings/gpio/gpio.h>
> >> 	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> >> 	#include "sm7125.dtsi"
> >> 	#include "pm6150.dtsi"
> >> 	#include "pm6150l.dtsi"
> >>
> >> Out of which last three are likely to be changing as well.
> >>
> >> This means that following workflow is reasonable and likely:
> >> 1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
> >> 2. Add some sm7125 board (out of scope here).
> >> 3. Release new kernel, e.g. v6.7.
> >> 4. Make more changes to sm7125.dtsi
> >> 5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.
> >>
> >> Now if you backport only (5) above, without (4), it won't work. Might
> >> compile, might not. Even if it compiles, might not work.
> >>
> >> The step (4) here might be small, but might be big as well.
> > 
> > Fair enough.  So should we drop this change?
> 
> I vote for dropping. Also, I think such DTS patches should not be picked
> automatically via AUTOSEL. Manual backports or targetted Cc-stable,
> assuming that backporter investigated it, seem ok.

Sasha now dropped this, thanks.

Sasha, want to add dts changes to the AUTOSEL "deny-list"?

thanks,

greg k-h

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Sasha Levin]

On Thu, Apr 11, 2024 at 09:34:39AM +0200, Greg KH wrote:
>On Thu, Apr 11, 2024 at 09:27:28AM +0200, Krzysztof Kozlowski wrote:
>> On 11/04/2024 09:22, Greg KH wrote:
>> > On Wed, Apr 10, 2024 at 08:24:49PM +0200, Krzysztof Kozlowski wrote:
>> >> On 10/04/2024 20:02, Greg KH wrote:
>> >>> On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
>> >>>>
>> >>>>
>> >>>> On 4/10/24 17:57, Sasha Levin wrote:
>> >>>>> This is a note to let you know that I've just added the patch titled
>> >>>>>
>> >>>>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
>> >>>>
>> >>>> autosel has been reeaaaaaly going over the top lately, particularly
>> >>>> with dts patches.. I'm not sure adding support for a device is
>> >>>> something that should go to stable
>> >>>
>> >>> Simple device ids and quirks have always been stable material.
>> >>>
>> >>
>> >> That's true, but maybe DTS should have an exception. I guess you think
>> >> this is trivial device ID, because the patch contents is small. But it
>> >> is or it can be misleading. The patch adds new small DTS file which
>> >> includes another file:
>> >>
>> >> 	#include "sm7125-xiaomi-common.dtsi"
>> >>
>> >> Which includes another 7 files:
>> >>
>> >> 	#include <dt-bindings/arm/qcom,ids.h>
>> >> 	#include <dt-bindings/firmware/qcom,scm.h>
>> >> 	#include <dt-bindings/gpio/gpio.h>
>> >> 	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> >> 	#include "sm7125.dtsi"
>> >> 	#include "pm6150.dtsi"
>> >> 	#include "pm6150l.dtsi"
>> >>
>> >> Out of which last three are likely to be changing as well.
>> >>
>> >> This means that following workflow is reasonable and likely:
>> >> 1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
>> >> 2. Add some sm7125 board (out of scope here).
>> >> 3. Release new kernel, e.g. v6.7.
>> >> 4. Make more changes to sm7125.dtsi
>> >> 5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.
>> >>
>> >> Now if you backport only (5) above, without (4), it won't work. Might
>> >> compile, might not. Even if it compiles, might not work.
>> >>
>> >> The step (4) here might be small, but might be big as well.
>> >
>> > Fair enough.  So should we drop this change?
>>
>> I vote for dropping. Also, I think such DTS patches should not be picked
>> automatically via AUTOSEL. Manual backports or targetted Cc-stable,
>> assuming that backporter investigated it, seem ok.
>
>Sasha now dropped this, thanks.
>
>Sasha, want to add dts changes to the AUTOSEL "deny-list"?

Sure, this makes sense.

-- 
Thanks,
Sasha

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3307 bytes --]

On Thu, Apr 11, 2024 at 03:57:16AM -0400, Sasha Levin wrote:
> On Thu, Apr 11, 2024 at 09:34:39AM +0200, Greg KH wrote:
> > On Thu, Apr 11, 2024 at 09:27:28AM +0200, Krzysztof Kozlowski wrote:
> > > On 11/04/2024 09:22, Greg KH wrote:
> > > > On Wed, Apr 10, 2024 at 08:24:49PM +0200, Krzysztof Kozlowski wrote:
> > > >> On 10/04/2024 20:02, Greg KH wrote:
> > > >>> On Wed, Apr 10, 2024 at 07:58:40PM +0200, Konrad Dybcio wrote:
> > > >>>>
> > > >>>>
> > > >>>> On 4/10/24 17:57, Sasha Levin wrote:
> > > >>>>> This is a note to let you know that I've just added the patch titled
> > > >>>>>
> > > >>>>>      arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S
> > > >>>>
> > > >>>> autosel has been reeaaaaaly going over the top lately, particularly
> > > >>>> with dts patches.. I'm not sure adding support for a device is
> > > >>>> something that should go to stable
> > > >>>
> > > >>> Simple device ids and quirks have always been stable material.
> > > >>>
> > > >>
> > > >> That's true, but maybe DTS should have an exception. I guess you think
> > > >> this is trivial device ID, because the patch contents is small. But it
> > > >> is or it can be misleading. The patch adds new small DTS file which
> > > >> includes another file:
> > > >>
> > > >> 	#include "sm7125-xiaomi-common.dtsi"
> > > >>
> > > >> Which includes another 7 files:
> > > >>
> > > >> 	#include <dt-bindings/arm/qcom,ids.h>
> > > >> 	#include <dt-bindings/firmware/qcom,scm.h>
> > > >> 	#include <dt-bindings/gpio/gpio.h>
> > > >> 	#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > > >> 	#include "sm7125.dtsi"
> > > >> 	#include "pm6150.dtsi"
> > > >> 	#include "pm6150l.dtsi"
> > > >>
> > > >> Out of which last three are likely to be changing as well.
> > > >>
> > > >> This means that following workflow is reasonable and likely:
> > > >> 1. Add sm7125.dtsi (or pm6150.dtsi or pm6150l.dtsi)
> > > >> 2. Add some sm7125 board (out of scope here).
> > > >> 3. Release new kernel, e.g. v6.7.
> > > >> 4. Make more changes to sm7125.dtsi
> > > >> 5. The patch discussed here, so one adding sm7125-xiaomi-curtana.dts.
> > > >>
> > > >> Now if you backport only (5) above, without (4), it won't work. Might
> > > >> compile, might not. Even if it compiles, might not work.
> > > >>
> > > >> The step (4) here might be small, but might be big as well.
> > > >
> > > > Fair enough.  So should we drop this change?
> > > 
> > > I vote for dropping. Also, I think such DTS patches should not be picked
> > > automatically via AUTOSEL. Manual backports or targetted Cc-stable,
> > > assuming that backporter investigated it, seem ok.
> > 
> > Sasha now dropped this, thanks.
> > 
> > Sasha, want to add dts changes to the AUTOSEL "deny-list"?
> 
> Sure, this makes sense.

Does it? Seems like a rather big hammer to me. I totally understand
blocking the addition of new dts files to stable, but there's a whole
load of different people maintaining dts files with differing levels of
remembering to cc stable explicitly.

That said, often a dts backport depends on a driver (or binding) change
too, so backporting one without the other may have no effect. I have no
idea whether or not AUTOSEL is capable of picking out those sort of
dependencies.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Krzysztof Kozlowski]

On 11/04/2024 12:23, Conor Dooley wrote:
>>>>>> Now if you backport only (5) above, without (4), it won't work. Might
>>>>>> compile, might not. Even if it compiles, might not work.
>>>>>>
>>>>>> The step (4) here might be small, but might be big as well.
>>>>>
>>>>> Fair enough.  So should we drop this change?
>>>>
>>>> I vote for dropping. Also, I think such DTS patches should not be picked
>>>> automatically via AUTOSEL. Manual backports or targetted Cc-stable,
>>>> assuming that backporter investigated it, seem ok.
>>>
>>> Sasha now dropped this, thanks.
>>>
>>> Sasha, want to add dts changes to the AUTOSEL "deny-list"?
>>
>> Sure, this makes sense.
> 
> Does it? Seems like a rather big hammer to me. I totally understand
> blocking the addition of new dts files to stable, but there's a whole
> load of different people maintaining dts files with differing levels of
> remembering to cc stable explicitly.
> 
> That said, often a dts backport depends on a driver (or binding) change
> too, so backporting one without the other may have no effect. I have no
> idea whether or not AUTOSEL is capable of picking out those sort of
> dependencies.


Uh, yes, I understood as "trivial quirks and new device ID" AUTOSEL
behavior. Not AUTOSEL for DTS patches in general.

Best regards,
Krzysztof

Re: Patch "arm64: dts: qcom: Add support for Xiaomi Redmi Note 9S" has been added to the 6.8-stable tree

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

On Thu, Apr 11, 2024 at 11:23:20AM +0100, Conor Dooley wrote:
> On Thu, Apr 11, 2024 at 03:57:16AM -0400, Sasha Levin wrote:
> > On Thu, Apr 11, 2024 at 09:34:39AM +0200, Greg KH wrote:
> > > On Thu, Apr 11, 2024 at 09:27:28AM +0200, Krzysztof Kozlowski wrote:

> > > > I vote for dropping. Also, I think such DTS patches should not be picked
> > > > automatically via AUTOSEL. Manual backports or targetted Cc-stable,
> > > > assuming that backporter investigated it, seem ok.

> > > Sasha, want to add dts changes to the AUTOSEL "deny-list"?
> > 
> > Sure, this makes sense.
> 
> Does it? Seems like a rather big hammer to me. I totally understand
> blocking the addition of new dts files to stable, but there's a whole
> load of different people maintaining dts files with differing levels of
> remembering to cc stable explicitly.
> 
> That said, often a dts backport depends on a driver (or binding) change
> too, so backporting one without the other may have no effect. I have no
> idea whether or not AUTOSEL is capable of picking out those sort of
> dependencies.

In the best case backporting a dts change as no effect, but it can also
break the driver completely unless the corresponding driver changes are
also backported. 

And such breaking dts changes are currently being pulled in as
dependencies:

	https://lore.kernel.org/all/ZgEpI31-OJkNchPF@hovoldconsulting.com/

I'm all for not backporting any dts changes that lack an explicit CC
stable tag.

(And people will never learn to add the CC stable tag when everything
with just a Fixes tag is being pulled in anyway.)

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]