[PATCH v3 1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade

[PATCH v3 1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade

[Yunlong Jia]

SKU6 is LTE(w/o eSIM)+WIFI+Parade

Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>
---

Changes in v3:
- Bindings and dts in the same series.

Changes in v2:
- Put sku6 before sku4.

 arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts
index 764c451c1a85..767cb7450c0d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts
@@ -14,7 +14,7 @@
 
 / {
 	model = "Google Pazquel (Parade,LTE)";
-	compatible = "google,pazquel-sku4", "qcom,sc7180";
+	compatible = "google,pazquel-sku6", "google,pazquel-sku4", "qcom,sc7180";
 };
 
 &ap_sar_sensor_i2c {
-- 
2.17.1

[PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Yunlong Jia]

The difference between sku6 and sku4 is that there is no esim

 The different SKUs are:

   LTE with physical SIM _and_ eSIM
   LTE with only a physical SIM
   WiFi only
 Both sku4 and sku6 are LTE SKUs.
 One has the eSIM stuffed and one doesn't.
 There is a single shared device tree for the two.

Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>
---

Changes in v3:
- Bindings and dts in the same series.

 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index fb1d00bcc847..ff0ed8d4d232 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -450,6 +450,7 @@ properties:
 
       - description: Google Pazquel with LTE and Parade (newest rev)
         items:
+          - const: google,pazquel-sku6
           - const: google,pazquel-sku4
           - const: qcom,sc7180
 
-- 
2.17.1

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 21/07/2022 05:58, Yunlong Jia wrote:
> The difference between sku6 and sku4 is that there is no esim
> 
>  The different SKUs are:
> 
>    LTE with physical SIM _and_ eSIM
>    LTE with only a physical SIM
>    WiFi only
>  Both sku4 and sku6 are LTE SKUs.
>  One has the eSIM stuffed and one doesn't.
>  There is a single shared device tree for the two.
> 
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Wed, Jul 20, 2022 at 8:59 PM Yunlong Jia
<yunlong.jia@ecs.corp-partner.google.com> wrote:
>
> The difference between sku6 and sku4 is that there is no esim
>
>  The different SKUs are:
>
>    LTE with physical SIM _and_ eSIM
>    LTE with only a physical SIM
>    WiFi only
>  Both sku4 and sku6 are LTE SKUs.
>  One has the eSIM stuffed and one doesn't.
>  There is a single shared device tree for the two.
>
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>
> ---
>
> Changes in v3:
> - Bindings and dts in the same series.
>
>  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
>  1 file changed, 1 insertion(+)

Not worth sending a new version for, but normally I expect the
bindings to be patch #1 and the dts change to be patch #2. In any
case:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 21/07/2022 15:37, Doug Anderson wrote:
> 
> Not worth sending a new version for, but normally I expect the
> bindings to be patch #1 and the dts change to be patch #2. In any
> case:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

I would say worth v4, because otherwise patches is not bisectable.


Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/07/2022 15:37, Doug Anderson wrote:
> >
> > Not worth sending a new version for, but normally I expect the
> > bindings to be patch #1 and the dts change to be patch #2. In any
> > case:
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I would say worth v4, because otherwise patches is not bisectable.

You're saying because `dtbs_check` will fail between the two? How does
flipping the order help? If `dtbs_check` needs to be bisectable then
these two need to be one patch, but I was always under the impression
that we wanted bindings patches separate from dts patches.

-Doug

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 21/07/2022 18:43, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/07/2022 15:37, Doug Anderson wrote:
>>>
>>> Not worth sending a new version for, but normally I expect the
>>> bindings to be patch #1 and the dts change to be patch #2. In any
>>> case:
>>>
>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>
>> I would say worth v4, because otherwise patches is not bisectable.
> 
> You're saying because `dtbs_check` will fail between the two?

Yes

> How does
> flipping the order help? If `dtbs_check` needs to be bisectable then
> these two need to be one patch, but I was always under the impression
> that we wanted bindings patches separate from dts patches.

I don't think anyone said that bindings patches must be separate from
DTS. The only restriction is DTS cannot go with drivers.

Bindings for boards go pretty often with DTS (subarch). This is exactly
what maintainers do, e.g.:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
Bindings for hardware should go via subsystem maintainer (drivers).

Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/07/2022 18:43, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 21/07/2022 15:37, Doug Anderson wrote:
> >>>
> >>> Not worth sending a new version for, but normally I expect the
> >>> bindings to be patch #1 and the dts change to be patch #2. In any
> >>> case:
> >>>
> >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>
> >> I would say worth v4, because otherwise patches is not bisectable.
> >
> > You're saying because `dtbs_check` will fail between the two?
>
> Yes

OK. Then I assume you agree that reversing the order of the patches
won't help, only combining the two patches into one.


> > How does
> > flipping the order help? If `dtbs_check` needs to be bisectable then
> > these two need to be one patch, but I was always under the impression
> > that we wanted bindings patches separate from dts patches.
>
> I don't think anyone said that bindings patches must be separate from
> DTS. The only restriction is DTS cannot go with drivers.

I have always heard that best practice is to have bindings in a patch
by themselves. If I've misunderstood and/or folks have changed their
minds, that's fine, but historically I've been told to keep them
separate.


> Bindings for boards go pretty often with DTS (subarch). This is exactly
> what maintainers do, e.g.:
> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
> Bindings for hardware should go via subsystem maintainer (drivers).

OK, fair that in this case both the bindings and the yaml will land
through the Qualcomm tree. I guess it's really up to Bjorn and whether
he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer
the bindings and dts change to be in separate patches from each other.

-Doug

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Rob Herring]

On Thu, Jul 21, 2022 at 11:29:13AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 21/07/2022 18:43, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:
> > >>
> > >> On 21/07/2022 15:37, Doug Anderson wrote:
> > >>>
> > >>> Not worth sending a new version for, but normally I expect the
> > >>> bindings to be patch #1 and the dts change to be patch #2. In any
> > >>> case:
> > >>>
> > >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >>
> > >> I would say worth v4, because otherwise patches is not bisectable.
> > >
> > > You're saying because `dtbs_check` will fail between the two?
> >
> > Yes
> 
> OK. Then I assume you agree that reversing the order of the patches
> won't help, only combining the two patches into one.
> 
> 
> > > How does
> > > flipping the order help? If `dtbs_check` needs to be bisectable then
> > > these two need to be one patch, but I was always under the impression
> > > that we wanted bindings patches separate from dts patches.
> >
> > I don't think anyone said that bindings patches must be separate from
> > DTS. The only restriction is DTS cannot go with drivers.
> 
> I have always heard that best practice is to have bindings in a patch
> by themselves. If I've misunderstood and/or folks have changed their
> minds, that's fine, but historically I've been told to keep them
> separate.

Correct.


> > Bindings for boards go pretty often with DTS (subarch). This is exactly
> > what maintainers do, e.g.:
> > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
> > Bindings for hardware should go via subsystem maintainer (drivers).
> 
> OK, fair that in this case both the bindings and the yaml will land
> through the Qualcomm tree. I guess it's really up to Bjorn and whether
> he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer
> the bindings and dts change to be in separate patches from each other.

Bindings go first if applied together because you have to define the 
binding before you use it. But sometimes things go via multiple trees 
and that's fine because it's just easier. In that case, the subsystem 
tree is preferred for bindings (i.e. with the driver). But in this case, 
Bjorn is the subsystem tree.

Rob

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Thu, Jul 21, 2022 at 5:22 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Jul 21, 2022 at 11:29:13AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 21/07/2022 18:43, Doug Anderson wrote:
> > > > Hi,
> > > >
> > > > On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
> > > > <krzysztof.kozlowski@linaro.org> wrote:
> > > >>
> > > >> On 21/07/2022 15:37, Doug Anderson wrote:
> > > >>>
> > > >>> Not worth sending a new version for, but normally I expect the
> > > >>> bindings to be patch #1 and the dts change to be patch #2. In any
> > > >>> case:
> > > >>>
> > > >>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > > >>
> > > >> I would say worth v4, because otherwise patches is not bisectable.
> > > >
> > > > You're saying because `dtbs_check` will fail between the two?
> > >
> > > Yes
> >
> > OK. Then I assume you agree that reversing the order of the patches
> > won't help, only combining the two patches into one.
> >
> >
> > > > How does
> > > > flipping the order help? If `dtbs_check` needs to be bisectable then
> > > > these two need to be one patch, but I was always under the impression
> > > > that we wanted bindings patches separate from dts patches.
> > >
> > > I don't think anyone said that bindings patches must be separate from
> > > DTS. The only restriction is DTS cannot go with drivers.
> >
> > I have always heard that best practice is to have bindings in a patch
> > by themselves. If I've misunderstood and/or folks have changed their
> > minds, that's fine, but historically I've been told to keep them
> > separate.
>
> Correct.
>
>
> > > Bindings for boards go pretty often with DTS (subarch). This is exactly
> > > what maintainers do, e.g.:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
> > > Bindings for hardware should go via subsystem maintainer (drivers).
> >
> > OK, fair that in this case both the bindings and the yaml will land
> > through the Qualcomm tree. I guess it's really up to Bjorn and whether
> > he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer
> > the bindings and dts change to be in separate patches from each other.
>
> Bindings go first if applied together because you have to define the
> binding before you use it. But sometimes things go via multiple trees
> and that's fine because it's just easier. In that case, the subsystem
> tree is preferred for bindings (i.e. with the driver). But in this case,
> Bjorn is the subsystem tree.

Thanks! I'll interpret your response as:

1. Keep this as two patches and it's more important to keep dts and
bindings separate than it is to avoid breaking bisectability of "make
dtbs_check".

2. Bindings should have been patch #1, but it's not a massive deal.

3. I'll assume that Bjorn will yell if he'd like this series re-posted
with the reverse order.

-Doug

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 22/07/2022 17:41, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 21, 2022 at 5:22 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Thu, Jul 21, 2022 at 11:29:13AM -0700, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/07/2022 18:43, Doug Anderson wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> On 21/07/2022 15:37, Doug Anderson wrote:
>>>>>>>
>>>>>>> Not worth sending a new version for, but normally I expect the
>>>>>>> bindings to be patch #1 and the dts change to be patch #2. In any
>>>>>>> case:
>>>>>>>
>>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>>>
>>>>>> I would say worth v4, because otherwise patches is not bisectable.
>>>>>
>>>>> You're saying because `dtbs_check` will fail between the two?
>>>>
>>>> Yes
>>>
>>> OK. Then I assume you agree that reversing the order of the patches
>>> won't help, only combining the two patches into one.
>>>
>>>
>>>>> How does
>>>>> flipping the order help? If `dtbs_check` needs to be bisectable then
>>>>> these two need to be one patch, but I was always under the impression
>>>>> that we wanted bindings patches separate from dts patches.
>>>>
>>>> I don't think anyone said that bindings patches must be separate from
>>>> DTS. The only restriction is DTS cannot go with drivers.
>>>
>>> I have always heard that best practice is to have bindings in a patch
>>> by themselves. If I've misunderstood and/or folks have changed their
>>> minds, that's fine, but historically I've been told to keep them
>>> separate.
>>
>> Correct.
>>
>>
>>>> Bindings for boards go pretty often with DTS (subarch). This is exactly
>>>> what maintainers do, e.g.:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
>>>> Bindings for hardware should go via subsystem maintainer (drivers).
>>>
>>> OK, fair that in this case both the bindings and the yaml will land
>>> through the Qualcomm tree. I guess it's really up to Bjorn and whether
>>> he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer
>>> the bindings and dts change to be in separate patches from each other.
>>
>> Bindings go first if applied together because you have to define the
>> binding before you use it. But sometimes things go via multiple trees
>> and that's fine because it's just easier. In that case, the subsystem
>> tree is preferred for bindings (i.e. with the driver). But in this case,
>> Bjorn is the subsystem tree.
> 
> Thanks! I'll interpret your response as:
> 
> 1. Keep this as two patches and it's more important to keep dts and
> bindings separate than it is to avoid breaking bisectability of "make
> dtbs_check".

No one questioned this here...

> 
> 2. Bindings should have been patch #1, but it's not a massive deal.

This started our discussion and I said it should be a v4 with a proper
order. It's not massive deal, but hopefully the submitter will learn
something.

> 
> 3. I'll assume that Bjorn will yell if he'd like this series re-posted
> with the reverse order.



Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 21/07/2022 20:29, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/07/2022 18:43, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/07/2022 15:37, Doug Anderson wrote:
>>>>>
>>>>> Not worth sending a new version for, but normally I expect the
>>>>> bindings to be patch #1 and the dts change to be patch #2. In any
>>>>> case:
>>>>>
>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>
>>>> I would say worth v4, because otherwise patches is not bisectable.
>>>
>>> You're saying because `dtbs_check` will fail between the two?
>>
>> Yes
> 
> OK. Then I assume you agree that reversing the order of the patches
> won't help, only combining the two patches into one.
> 
> 
>>> How does
>>> flipping the order help? If `dtbs_check` needs to be bisectable then
>>> these two need to be one patch, but I was always under the impression
>>> that we wanted bindings patches separate from dts patches.
>>
>> I don't think anyone said that bindings patches must be separate from
>> DTS. The only restriction is DTS cannot go with drivers.
> 
> I have always heard that best practice is to have bindings in a patch
> by themselves. 

Yes, bindings must be separate patch, no one here objects this. You said
they cannot go together via one maintainer tree or I misunderstood?

> If I've misunderstood and/or folks have changed their
> minds, that's fine, but historically I've been told to keep them
> separate.

Nothing changed. Bindings must be separate. They will be applied by
maintainer and, if correctly ordered, this is bisectable.
> 
> 
>> Bindings for boards go pretty often with DTS (subarch). This is exactly
>> what maintainers do, e.g.:
>> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=arm64-for-5.20
>> Bindings for hardware should go via subsystem maintainer (drivers).
> 
> OK, fair that in this case both the bindings and the yaml will land
> through the Qualcomm tree. I guess it's really up to Bjorn and whether
> he'd prefer "make dtbs_check" to be bisectable or whether he'd prefer
> the bindings and dts change to be in separate patches from each other.

??? The patches must be separate commits and if properly ordered in one
branch they are bisectable.


Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Doug Anderson]

Hi,

On Fri, Jul 22, 2022 at 10:14 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/07/2022 20:29, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 21/07/2022 18:43, Doug Anderson wrote:
> >>> Hi,
> >>>
> >>> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 21/07/2022 15:37, Doug Anderson wrote:
> >>>>>
> >>>>> Not worth sending a new version for, but normally I expect the
> >>>>> bindings to be patch #1 and the dts change to be patch #2. In any
> >>>>> case:
> >>>>>
> >>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >>>>
> >>>> I would say worth v4, because otherwise patches is not bisectable.
> >>>
> >>> You're saying because `dtbs_check` will fail between the two?
> >>
> >> Yes
> >
> > OK. Then I assume you agree that reversing the order of the patches
> > won't help, only combining the two patches into one.
> >
> >
> >>> How does
> >>> flipping the order help? If `dtbs_check` needs to be bisectable then
> >>> these two need to be one patch, but I was always under the impression
> >>> that we wanted bindings patches separate from dts patches.
> >>
> >> I don't think anyone said that bindings patches must be separate from
> >> DTS. The only restriction is DTS cannot go with drivers.
> >
> > I have always heard that best practice is to have bindings in a patch
> > by themselves.
>
> Yes, bindings must be separate patch, no one here objects this. You said
> they cannot go together via one maintainer tree or I misunderstood?
>
> > If I've misunderstood and/or folks have changed their
> > minds, that's fine, but historically I've been told to keep them
> > separate.
>
> Nothing changed. Bindings must be separate. They will be applied by
> maintainer and, if correctly ordered, this is bisectable.

OK, I think this is the disconnect here.

No matter what order Jimmy's patches land in, it won't be bisectable
from the standpoint of "make dtbs_check". This is what I've been
trying to say.

* If the bindings land first then the device tree won't have sku6 and
will fail "make dtbs_check"

* If the dts lands first then the bindings won't have sku6 and will
fail "make dtbs_check".

Am I missing something?

So when you said "I don't think anyone said that bindings patches must
be separate from DTS" and that you cared about "make dtbs_check" being
bisectable that you were saying you wanted these squashed into one
patch. I guess that's not the case.

-Doug

Re: [PATCH v3 2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel

[Krzysztof Kozlowski]

On 22/07/2022 19:23, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jul 22, 2022 at 10:14 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/07/2022 20:29, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Thu, Jul 21, 2022 at 9:52 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/07/2022 18:43, Doug Anderson wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Jul 21, 2022 at 9:33 AM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> On 21/07/2022 15:37, Doug Anderson wrote:
>>>>>>>
>>>>>>> Not worth sending a new version for, but normally I expect the
>>>>>>> bindings to be patch #1 and the dts change to be patch #2. In any
>>>>>>> case:
>>>>>>>
>>>>>>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>>>>>>
>>>>>> I would say worth v4, because otherwise patches is not bisectable.
>>>>>
>>>>> You're saying because `dtbs_check` will fail between the two?
>>>>
>>>> Yes
>>>
>>> OK. Then I assume you agree that reversing the order of the patches
>>> won't help, only combining the two patches into one.
>>>
>>>
>>>>> How does
>>>>> flipping the order help? If `dtbs_check` needs to be bisectable then
>>>>> these two need to be one patch, but I was always under the impression
>>>>> that we wanted bindings patches separate from dts patches.
>>>>
>>>> I don't think anyone said that bindings patches must be separate from
>>>> DTS. The only restriction is DTS cannot go with drivers.
>>>
>>> I have always heard that best practice is to have bindings in a patch
>>> by themselves.
>>
>> Yes, bindings must be separate patch, no one here objects this. You said
>> they cannot go together via one maintainer tree or I misunderstood?
>>
>>> If I've misunderstood and/or folks have changed their
>>> minds, that's fine, but historically I've been told to keep them
>>> separate.
>>
>> Nothing changed. Bindings must be separate. They will be applied by
>> maintainer and, if correctly ordered, this is bisectable.
> 
> OK, I think this is the disconnect here.
> 
> No matter what order Jimmy's patches land in, it won't be bisectable
> from the standpoint of "make dtbs_check". This is what I've been
> trying to say.
> 
> * If the bindings land first then the device tree won't have sku6 and
> will fail "make dtbs_check"
> 
> * If the dts lands first then the bindings won't have sku6 and will
> fail "make dtbs_check".
> 
> Am I missing something?

Ah, you're right... The patch changes the bindings of a board instead of
bringing a new variant. Yeah, this cannot be bisectable if kept
separate, thus order does no matter.

> 
> So when you said "I don't think anyone said that bindings patches must
> be separate from DTS" and that you cared about "make dtbs_check" being
> bisectable that you were saying you wanted these squashed into one
> patch. I guess that's not the case.
>

Best regards,
Krzysztof

Re: [PATCH v3 1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade

[Doug Anderson]

Hi,

On Wed, Jul 20, 2022 at 8:59 PM Yunlong Jia
<yunlong.jia@ecs.corp-partner.google.com> wrote:
>
> SKU6 is LTE(w/o eSIM)+WIFI+Parade
>
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>
> ---
>
> Changes in v3:
> - Bindings and dts in the same series.
>
> Changes in v2:
> - Put sku6 before sku4.
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v3 1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade

[Doug Anderson]

Hi,

On Thu, Jul 21, 2022 at 6:36 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 20, 2022 at 8:59 PM Yunlong Jia
> <yunlong.jia@ecs.corp-partner.google.com> wrote:
> >
> > SKU6 is LTE(w/o eSIM)+WIFI+Parade
> >
> > Signed-off-by: Yunlong Jia <yunlong.jia@ecs.corp-partner.google.com>
> > ---
> >
> > Changes in v3:
> > - Bindings and dts in the same series.
> >
> > Changes in v2:
> > - Put sku6 before sku4.
> >
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dts | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

We're at a point now where this won't be able to land for at least 2.5
weeks. As an experiment, I created a staging tree atop the current
arm64 dts tree and put this there. I'll try to put only things that I
believe are truly ready to land there, but git hashes won't be stable
since it's just a staging tree:

https://github.com/dianders/kernel-staging/commits/qcom/arm64-staging

I reversed the order of patch #1 and patch #2 when applying as per
discussion in patch #2.

-Doug

Re: (subset) [PATCH v3 1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade

[Bjorn Andersson]

On Thu, 21 Jul 2022 03:58:42 +0000, Yunlong Jia wrote:
> SKU6 is LTE(w/o eSIM)+WIFI+Parade
> 
> 

Applied, thanks!

[1/2] arm64: dts: qcom: Add SKU6 for sc7180-trogdor-pazquel-lte-parade
      commit: 030a7bfb365fd19714e25e9547764bff690cb227
[2/2] dt-bindings: arm: qcom: Document additional sku6 for sc7180 pazquel
      commit: 07603a1c17cf9eec5c963b470daba780cd7b9981

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>