1. Patchew
  2. linux
  3. [RFT PATCH 00/14] ARM/arm64: dts: qcom: correct TLMM gpio-ranges
  4. View patch

[RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges

Krzysztof Kozlowski posted 14 patches 2 years, 7 months ago
[RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Krzysztof Kozlowski 2 years, 7 months ago 
Correct the number of GPIOs in TLMM pin controller.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index fa2d0d7d1367..17e8c26a9ae6 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
-			gpio-ranges = <&tlmm 0 0 230>;
+			gpio-ranges = <&tlmm 0 0 228>;
 		};
 
 		apps_smmu: iommu@15000000 {
-- 
2.34.1
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Brian Masney 2 years, 7 months ago 
On Wed, Feb 01, 2023 at 04:50:56PM +0100, Krzysztof Kozlowski wrote:
> Correct the number of GPIOs in TLMM pin controller.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index fa2d0d7d1367..17e8c26a9ae6 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> -			gpio-ranges = <&tlmm 0 0 230>;
> +			gpio-ranges = <&tlmm 0 0 228>;
>  		};

I verified that this count matches what's in downstream.

Reviewed-by: Brian Masney <bmasney@redhat.com>


However, I noticed in upstream that we're using this reg property:

   reg = <0 0x0f100000 0 0x300000>;

Downstream has a different base address and a wider size. Note: I added
spaces for easy comparison.

   reg = <  0x0F000000   0x1000000>;

I don't have access to the appropriate documents to see which is
correct. I assume the base address in upstream is at least correct since
pinctrl is working on this platform.

Brian
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Konrad Dybcio 2 years, 7 months ago 

On 2.02.2023 23:58, Brian Masney wrote:
> On Wed, Feb 01, 2023 at 04:50:56PM +0100, Krzysztof Kozlowski wrote:
>> Correct the number of GPIOs in TLMM pin controller.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> index fa2d0d7d1367..17e8c26a9ae6 100644
>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>> @@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
>>  			#gpio-cells = <2>;
>>  			interrupt-controller;
>>  			#interrupt-cells = <2>;
>> -			gpio-ranges = <&tlmm 0 0 230>;
>> +			gpio-ranges = <&tlmm 0 0 228>;
Won't that kill the UFS pins?


>>  		};
> 
> I verified that this count matches what's in downstream.
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> 
> 
> However, I noticed in upstream that we're using this reg property:
> 
>    reg = <0 0x0f100000 0 0x300000>;
> 
> Downstream has a different base address and a wider size. Note: I added
> spaces for easy comparison.
> 
>    reg = <  0x0F000000   0x1000000>;
> 
> I don't have access to the appropriate documents to see which is
> correct. I assume the base address in upstream is at least correct since
> pinctrl is working on this platform.
Downstream offsets things in the driver

https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blob/LV.AU.1.2.3.r1-03600-gen3meta.0/drivers/pinctrl/qcom/pinctrl-direwolf.c#L20

Notice how UFS/QDSD pins addresses differ by 0x1000... up- and downstream too.
I'd imagine Bjorn/Johan/whoever did that used magic PDFs instead of not-very-
tested downstream sources.

Another note, the downstream driver may be incomplete/wrong, as Linux was
not exactly the main usecase of 8280xp so the testing there was most likely
only basic.

Konrad
> 
> Brian
>
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Krzysztof Kozlowski 2 years, 7 months ago 
On 03/02/2023 00:45, Konrad Dybcio wrote:
> 
> 
> On 2.02.2023 23:58, Brian Masney wrote:
>> On Wed, Feb 01, 2023 at 04:50:56PM +0100, Krzysztof Kozlowski wrote:
>>> Correct the number of GPIOs in TLMM pin controller.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> index fa2d0d7d1367..17e8c26a9ae6 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> @@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
>>>  			#gpio-cells = <2>;
>>>  			interrupt-controller;
>>>  			#interrupt-cells = <2>;
>>> -			gpio-ranges = <&tlmm 0 0 230>;
>>> +			gpio-ranges = <&tlmm 0 0 228>;
> Won't that kill the UFS pins?

This patchset is obsolete and replaced with v2. I alerady replied here
that this was not good approach...

Best regards,
Krzysztof
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Brian Masney 2 years, 7 months ago 
On Fri, Feb 03, 2023 at 12:45:49AM +0100, Konrad Dybcio wrote:
> On 2.02.2023 23:58, Brian Masney wrote:
> > On Wed, Feb 01, 2023 at 04:50:56PM +0100, Krzysztof Kozlowski wrote:
> >> Correct the number of GPIOs in TLMM pin controller.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> index fa2d0d7d1367..17e8c26a9ae6 100644
> >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >> @@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
> >>  			#gpio-cells = <2>;
> >>  			interrupt-controller;
> >>  			#interrupt-cells = <2>;
> >> -			gpio-ranges = <&tlmm 0 0 230>;
> >> +			gpio-ranges = <&tlmm 0 0 228>;
> Won't that kill the UFS pins?

For others quick reference, Konrad is talking about this line from
sa8540p-ride.dts:

	reset-gpios = <&tlmm 228 GPIO_ACTIVE_LOW>;

I noticed that earlier but assumed this was one based. However, looking
at pinctrl-sc8280xp.c I see gpio0..gpio227 defined.

Brian
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Konrad Dybcio 2 years, 7 months ago 

On 3.02.2023 00:59, Brian Masney wrote:
> On Fri, Feb 03, 2023 at 12:45:49AM +0100, Konrad Dybcio wrote:
>> On 2.02.2023 23:58, Brian Masney wrote:
>>> On Wed, Feb 01, 2023 at 04:50:56PM +0100, Krzysztof Kozlowski wrote:
>>>> Correct the number of GPIOs in TLMM pin controller.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>>> index fa2d0d7d1367..17e8c26a9ae6 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>>> @@ -3533,7 +3533,7 @@ tlmm: pinctrl@f100000 {
>>>>  			#gpio-cells = <2>;
>>>>  			interrupt-controller;
>>>>  			#interrupt-cells = <2>;
>>>> -			gpio-ranges = <&tlmm 0 0 230>;
>>>> +			gpio-ranges = <&tlmm 0 0 228>;
>> Won't that kill the UFS pins?
> 
> For others quick reference, Konrad is talking about this line from
> sa8540p-ride.dts:
> 
> 	reset-gpios = <&tlmm 228 GPIO_ACTIVE_LOW>;
> 
> I noticed that earlier but assumed this was one based. However, looking
> at pinctrl-sc8280xp.c I see gpio0..gpio227 defined.
+ gpio229 is the reset pin for the UFS card slot

Konrad
> 
> Brian
>
Re: [RFT PATCH 05/14] arm64: dts: qcom: sc8280xp: correct TLMM gpio-ranges
Posted by Brian Masney 2 years, 7 months ago 
On Fri, Feb 03, 2023 at 01:05:35AM +0100, Konrad Dybcio wrote:
> On 3.02.2023 00:59, Brian Masney wrote:
> > For others quick reference, Konrad is talking about this line from
> > sa8540p-ride.dts:
> > 
> > 	reset-gpios = <&tlmm 228 GPIO_ACTIVE_LOW>;
> > 
> > I noticed that earlier but assumed this was one based. However, looking
> > at pinctrl-sc8280xp.c I see gpio0..gpio227 defined.
>
> + gpio229 is the reset pin for the UFS card slot

We don't have the UFS card slot on the sa8540p exposed. However, it is
available on the sa8295p.

The original DTS in upstream listed 230 pins, however pinctrl-sc8280xp.c
lists 233 pins and the two UFS pins match what we have in DTS.

static const struct pinctrl_pin_desc sc8280xp_pins[] = {
        PINCTRL_PIN(0, "GPIO_0"),
	...
	PINCTRL_PIN(227, "GPIO_227"),
	PINCTRL_PIN(228, "UFS_RESET"),
	PINCTRL_PIN(229, "UFS1_RESET"),
	PINCTRL_PIN(230, "SDC2_CLK"),
	PINCTRL_PIN(231, "SDC2_CMD"),
	PINCTRL_PIN(232, "SDC2_DATA"),

Rescind-Reviewed-by: Brian Masney <bmasney@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.