[PATCH 5/7] arm64: dts: qcom: sm8450-hdk: add pmic glink node

Neil Armstrong posted 7 patches 2 years, 7 months ago
There is a newer version of this series
Re: [PATCH 5/7] arm64: dts: qcom: sm8450-hdk: add pmic glink node
Posted by Konrad Dybcio 2 years, 7 months ago

On 30.01.2023 10:54, Neil Armstrong wrote:
> Add the pmic glink node linked with the DWC3 USB controller
> switched to OTG mode and tagged with usb-role-switch.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Missing commit message

> ---
>  arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 34 ++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
> index 5bdc2c1159ae..5ab12c911bfe 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
> @@ -87,6 +87,31 @@ lt9611_3v3: lt9611-3v3-regulator {
>  		enable-active-high;
>  	};
>  
> +	pmic-glink {
> +		compatible = "qcom,sm8450-pmic-glink", "qcom,pmic-glink";
> +
You could remove this newline
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		connector@0 {
> +			compatible = "usb-c-connector";
> +			reg = <0>;
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
And add one here

> +				port@0 {
> +					reg = <0>;
And here

> +					pmic_glink_dwc3_in: endpoint {
> +						remote-endpoint = <&usb_1_dwc3_out>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
>  	vph_pwr: vph-pwr-regulator {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vph_pwr";
> @@ -724,7 +749,14 @@ &usb_1 {
>  };
>  
>  &usb_1_dwc3 {
> -	dr_mode = "peripheral";
> +	dr_mode = "otg";
> +	usb-role-switch;
> +
> +	port {
Hm, maybe this could be moved to 8450 dtsi?

Konrad
> +		usb_1_dwc3_out: endpoint {
> +		      remote-endpoint = <&pmic_glink_dwc3_in>;
> +	      };
> +	};
>  };
>  
>  &usb_1_hsphy {
>
Re: [PATCH 5/7] arm64: dts: qcom: sm8450-hdk: add pmic glink node
Posted by Neil Armstrong 2 years, 7 months ago
On 30/01/2023 11:40, Konrad Dybcio wrote:
> 
> 
> On 30.01.2023 10:54, Neil Armstrong wrote:
>> Add the pmic glink node linked with the DWC3 USB controller
>> switched to OTG mode and tagged with usb-role-switch.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> Missing commit message

??

> 
>> ---
>>   arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 34 ++++++++++++++++++++++++++++++++-
>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> index 5bdc2c1159ae..5ab12c911bfe 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>> @@ -87,6 +87,31 @@ lt9611_3v3: lt9611-3v3-regulator {
>>   		enable-active-high;
>>   	};
>>   
>> +	pmic-glink {
>> +		compatible = "qcom,sm8450-pmic-glink", "qcom,pmic-glink";
>> +
> You could remove this newline
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		connector@0 {
>> +			compatible = "usb-c-connector";
>> +			reg = <0>;
>> +			power-role = "dual";
>> +			data-role = "dual";
>> +
>> +			ports {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
> And add one here
> 
>> +				port@0 {
>> +					reg = <0>;
> And here
> 

Ack

>> +					pmic_glink_dwc3_in: endpoint {
>> +						remote-endpoint = <&usb_1_dwc3_out>;
>> +					};
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>>   	vph_pwr: vph-pwr-regulator {
>>   		compatible = "regulator-fixed";
>>   		regulator-name = "vph_pwr";
>> @@ -724,7 +749,14 @@ &usb_1 {
>>   };
>>   
>>   &usb_1_dwc3 {
>> -	dr_mode = "peripheral";
>> +	dr_mode = "otg";
>> +	usb-role-switch;
>> +
>> +	port {
> Hm, maybe this could be moved to 8450 dtsi?

Nop because it depends on the board layout, I think dr_mode
and eventual connector description should really stay in
the board dts.

Thanks,
Neil

> 
> Konrad
>> +		usb_1_dwc3_out: endpoint {
>> +		      remote-endpoint = <&pmic_glink_dwc3_in>;
>> +	      };
>> +	};
>>   };
>>   
>>   &usb_1_hsphy {
>>
Re: [PATCH 5/7] arm64: dts: qcom: sm8450-hdk: add pmic glink node
Posted by Konrad Dybcio 2 years, 7 months ago

On 30.01.2023 11:58, Neil Armstrong wrote:
> On 30/01/2023 11:40, Konrad Dybcio wrote:
>>
>>
>> On 30.01.2023 10:54, Neil Armstrong wrote:
>>> Add the pmic glink node linked with the DWC3 USB controller
>>> switched to OTG mode and tagged with usb-role-switch.
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> Missing commit message
> 
> ??
> 
>>
>>> ---
>>>   arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 34 ++++++++++++++++++++++++++++++++-
>>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>> index 5bdc2c1159ae..5ab12c911bfe 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>> @@ -87,6 +87,31 @@ lt9611_3v3: lt9611-3v3-regulator {
>>>           enable-active-high;
>>>       };
>>>   +    pmic-glink {
>>> +        compatible = "qcom,sm8450-pmic-glink", "qcom,pmic-glink";
>>> +
>> You could remove this newline
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        connector@0 {
>>> +            compatible = "usb-c-connector";
>>> +            reg = <0>;
>>> +            power-role = "dual";
>>> +            data-role = "dual";
>>> +
>>> +            ports {
>>> +                #address-cells = <1>;
>>> +                #size-cells = <0>;
>> And add one here
>>
>>> +                port@0 {
>>> +                    reg = <0>;
>> And here
>>
> 
> Ack
> 
>>> +                    pmic_glink_dwc3_in: endpoint {
>>> +                        remote-endpoint = <&usb_1_dwc3_out>;
>>> +                    };
>>> +                };
>>> +            };
>>> +        };
>>> +    };
>>> +
>>>       vph_pwr: vph-pwr-regulator {
>>>           compatible = "regulator-fixed";
>>>           regulator-name = "vph_pwr";
>>> @@ -724,7 +749,14 @@ &usb_1 {
>>>   };
>>>     &usb_1_dwc3 {
>>> -    dr_mode = "peripheral";
>>> +    dr_mode = "otg";
>>> +    usb-role-switch;
>>> +
>>> +    port {
>> Hm, maybe this could be moved to 8450 dtsi?
> 
> Nop because it depends on the board layout, I think dr_mode
> and eventual connector description should really stay in
> the board dts.
I just meant the port definition, would it cause any side
effects to have it there?

Konrad
> 
> Thanks,
> Neil
> 
>>
>> Konrad
>>> +        usb_1_dwc3_out: endpoint {
>>> +              remote-endpoint = <&pmic_glink_dwc3_in>;
>>> +          };
>>> +    };
>>>   };
>>>     &usb_1_hsphy {
>>>
> 
Re: [PATCH 5/7] arm64: dts: qcom: sm8450-hdk: add pmic glink node
Posted by neil.armstrong@linaro.org 2 years, 7 months ago
On 30/01/2023 11:59, Konrad Dybcio wrote:
> 
> 
> On 30.01.2023 11:58, Neil Armstrong wrote:
>> On 30/01/2023 11:40, Konrad Dybcio wrote:
>>>
>>>
>>> On 30.01.2023 10:54, Neil Armstrong wrote:
>>>> Add the pmic glink node linked with the DWC3 USB controller
>>>> switched to OTG mode and tagged with usb-role-switch.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> Missing commit message
>>
>> ??
>>
>>>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 34 ++++++++++++++++++++++++++++++++-
>>>>    1 file changed, 33 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>>> index 5bdc2c1159ae..5ab12c911bfe 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
>>>> @@ -87,6 +87,31 @@ lt9611_3v3: lt9611-3v3-regulator {
>>>>            enable-active-high;
>>>>        };
>>>>    +    pmic-glink {
>>>> +        compatible = "qcom,sm8450-pmic-glink", "qcom,pmic-glink";
>>>> +
>>> You could remove this newline
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <0>;
>>>> +
>>>> +        connector@0 {
>>>> +            compatible = "usb-c-connector";
>>>> +            reg = <0>;
>>>> +            power-role = "dual";
>>>> +            data-role = "dual";
>>>> +
>>>> +            ports {
>>>> +                #address-cells = <1>;
>>>> +                #size-cells = <0>;
>>> And add one here
>>>
>>>> +                port@0 {
>>>> +                    reg = <0>;
>>> And here
>>>
>>
>> Ack
>>
>>>> +                    pmic_glink_dwc3_in: endpoint {
>>>> +                        remote-endpoint = <&usb_1_dwc3_out>;
>>>> +                    };
>>>> +                };
>>>> +            };
>>>> +        };
>>>> +    };
>>>> +
>>>>        vph_pwr: vph-pwr-regulator {
>>>>            compatible = "regulator-fixed";
>>>>            regulator-name = "vph_pwr";
>>>> @@ -724,7 +749,14 @@ &usb_1 {
>>>>    };
>>>>      &usb_1_dwc3 {
>>>> -    dr_mode = "peripheral";
>>>> +    dr_mode = "otg";
>>>> +    usb-role-switch;
>>>> +
>>>> +    port {
>>> Hm, maybe this could be moved to 8450 dtsi?
>>
>> Nop because it depends on the board layout, I think dr_mode
>> and eventual connector description should really stay in
>> the board dts.
> I just meant the port definition, would it cause any side
> effects to have it there?

Right, I don't think so, I don't have an opinion on that so whatever

Neil

> 
> Konrad
>>
>> Thanks,
>> Neil
>>
>>>
>>> Konrad
>>>> +        usb_1_dwc3_out: endpoint {
>>>> +              remote-endpoint = <&pmic_glink_dwc3_in>;
>>>> +          };
>>>> +    };
>>>>    };
>>>>      &usb_1_hsphy {
>>>>
>>