[PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Johan Hovold]

Add the TCSR node which is needed for PCIe configuration.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index b30c3b57fd13..79abe0e2c38d 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -984,6 +984,11 @@ tcsr_mutex: hwlock@1f40000 {
 			#hwlock-cells = <1>;
 		};
 
+		tcsr: syscon@1fc0000 {
+			compatible = "qcom,sc8280xp-tcsr", "syscon";
+			reg = <0x0 0x01fc0000 0x0 0x30000>;
+		};
+
 		usb_0_hsphy: phy@88e5000 {
 			compatible = "qcom,sc8280xp-usb-hs-phy",
 				     "qcom,usb-snps-hs-5nm-phy";
-- 
2.37.3

Re: [PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Krzysztof Kozlowski]

On 24/10/2022 08:58, Johan Hovold wrote:
> Add the TCSR node which is needed for PCIe configuration.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++

Please send the patches together with the binding. There is no need to
have this split and it causes additional effort during review - lookup
of the binding.

Also additional effort during binding review - lack of usage of bindings.



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

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Johan Hovold]

On Mon, Oct 24, 2022 at 09:34:22AM -0400, Krzysztof Kozlowski wrote:
> On 24/10/2022 08:58, Johan Hovold wrote:
> > Add the TCSR node which is needed for PCIe configuration.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
> 
> Please send the patches together with the binding. There is no need to
> have this split and it causes additional effort during review - lookup
> of the binding.

I was under the impression that the dts changes should be submitted
separately from the binding as they go through different trees. (And
last time I posted them together the subsystem maintainer ended up
taking also the dts changes by mistake).

The binding has been picked up by Lee now so I posted the dts change.
Could have added a lore link though.

> Also additional effort during binding review - lack of usage of bindings.
> 
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Johan

Re: [PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Krzysztof Kozlowski]

On 24/10/2022 09:42, Johan Hovold wrote:
> On Mon, Oct 24, 2022 at 09:34:22AM -0400, Krzysztof Kozlowski wrote:
>> On 24/10/2022 08:58, Johan Hovold wrote:
>>> Add the TCSR node which is needed for PCIe configuration.
>>>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
>>
>> Please send the patches together with the binding. There is no need to
>> have this split and it causes additional effort during review - lookup
>> of the binding.
> 
> I was under the impression that the dts changes should be submitted
> separately from the binding as they go through different trees. (And
> last time I posted them together the subsystem maintainer ended up
> taking also the dts changes by mistake).

Yes, that's also true. :)

> The binding has been picked up by Lee now so I posted the dts change.
> Could have added a lore link though.

This also would work and help a lot.

It depends in general on the maintainer - for example Greg does not want
to deal with individual patches, especially if DTS is just one patch and
USB would be 10 of them. Our toolset is not good for picking up 10 out
of 11. For all such cases - please provide link to lore.

If however there are just two patches - one DTS and one for maintainer -
then having them in one patchset should not cause additional effort for
the maintainer.

As you can see on the list, majority of patchsets consist of
bindings+DTS. Pretty often entire piece - bindings+driver+DTS.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Johan Hovold]

On Mon, Oct 24, 2022 at 10:09:51AM -0400, Krzysztof Kozlowski wrote:
> On 24/10/2022 09:42, Johan Hovold wrote:
> > On Mon, Oct 24, 2022 at 09:34:22AM -0400, Krzysztof Kozlowski wrote:
> >> On 24/10/2022 08:58, Johan Hovold wrote:
> >>> Add the TCSR node which is needed for PCIe configuration.
> >>>
> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
> >>
> >> Please send the patches together with the binding. There is no need to
> >> have this split and it causes additional effort during review - lookup
> >> of the binding.
> > 
> > I was under the impression that the dts changes should be submitted
> > separately from the binding as they go through different trees. (And
> > last time I posted them together the subsystem maintainer ended up
> > taking also the dts changes by mistake).
> 
> Yes, that's also true. :)
> 
> > The binding has been picked up by Lee now so I posted the dts change.
> > Could have added a lore link though.
> 
> This also would work and help a lot.
> 
> It depends in general on the maintainer - for example Greg does not want
> to deal with individual patches, especially if DTS is just one patch and
> USB would be 10 of them. Our toolset is not good for picking up 10 out
> of 11. For all such cases - please provide link to lore.
> 
> If however there are just two patches - one DTS and one for maintainer -
> then having them in one patchset should not cause additional effort for
> the maintainer.

I'm pretty sure I saw Lee complaining about Bjorn taking also the
binding update through the qcom tree recently when someone did just
that. Apparently it was TCSR related too:

	https://lore.kernel.org/all/Yzbk%2F6SQdpNQTahV@google.com/

Heh. That was you. :)
 
> As you can see on the list, majority of patchsets consist of
> bindings+DTS. Pretty often entire piece - bindings+driver+DTS.

Yeah, and whatever alternative you go with, someone will get it wrong or
complain it seems.

Johan

Re: [PATCH] arm64: dts: qcom: sc8280xp: add TCSR node

[Krzysztof Kozlowski]

On 24/10/2022 10:54, Johan Hovold wrote:
>>
>>> The binding has been picked up by Lee now so I posted the dts change.
>>> Could have added a lore link though.
>>
>> This also would work and help a lot.
>>
>> It depends in general on the maintainer - for example Greg does not want
>> to deal with individual patches, especially if DTS is just one patch and
>> USB would be 10 of them. Our toolset is not good for picking up 10 out
>> of 11. For all such cases - please provide link to lore.
>>
>> If however there are just two patches - one DTS and one for maintainer -
>> then having them in one patchset should not cause additional effort for
>> the maintainer.
> 
> I'm pretty sure I saw Lee complaining about Bjorn taking also the
> binding update through the qcom tree recently when someone did just
> that. Apparently it was TCSR related too:
> 
> 	https://lore.kernel.org/all/Yzbk%2F6SQdpNQTahV@google.com/
> 
> Heh. That was you. :)

Yep. It was a patchset of 16 patches where one should go to MFD and 15
to Qualcomm SoC.

>  
>> As you can see on the list, majority of patchsets consist of
>> bindings+DTS. Pretty often entire piece - bindings+driver+DTS.
> 
> Yeah, and whatever alternative you go with, someone will get it wrong or
> complain it seems.

:)

Best regards,
Krzysztof