On 14.12.2022 06:20, Bhupesh Sharma wrote:
> On Wed, 14 Dec 2022 at 00:29, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>
>> On 13/12/2022 19:52, Bhupesh Sharma wrote:
>>> Hi Krzysztof,
>>>
>>> On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>>>
>>>> On 13/12/2022 13:38, Bhupesh Sharma wrote:
>>>>> Add USB superspeed qmp phy node to dtsi.
>>>>>
>>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>
>>>>> ---
>>>>> arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>>>>> 1 file changed, 36 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> index e4ce135264f3d..9c5c024919f92 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>>>>> status = "disabled";
>>>>> };
>>>>>
>>>>> + usb_qmpphy: phy@1615000 {
>>>>> + compatible = "qcom,sm6115-qmp-usb3-phy";
>>>>> + reg = <0x01615000 0x200>;
>>>>> + #clock-cells = <1>;
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <1>;
>>>>> + ranges;
>>>>> + clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>>>> + <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
>>>>> + <&gcc GCC_AHB2PHY_USB_CLK>;
>>>>> + clock-names = "com_aux",
>>>>> + "ref",
>>>>> + "cfg_ahb";
>>>>> + resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
>>>>> + <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
>>>>> + reset-names = "phy", "phy_phy";
>>>>> + status = "disabled";
>>>>
>>>> Hm, you add a disabled PHY which is used by existing controller. The
>>>> controller is enabled in board DTS, but new PHY node isn't. Aren't you
>>>> now breaking it?
>>>
>>> The USB controller is connected to two PHYs - one is HS PHY and the other is SS
>>> QMP Phy. So while the exiting board dts describes and uses only the HS
>>> PHY, newer
>>> board dts files (which will soon be sent out as a separate patch),
>>
>> Then I miss how do you narrow the existing DTS to use only one PHY. I
>> don't see anything in this patchset.
>>
>>> will use both the HS and SS
>>> USB PHYs.
>>>
>>> So, this will not break the existing board dts files.
>>
>> I still think it will be. The board boots with USB with one phy enabled
>> and one disabled. The driver gets phys unconditionally and one of them
>> is disabled.
>>
>> Even if Linux implementation will work (devm_usb_get_phy_by_phandle will
>> return -ENXIO or -ENODEV for disabled node), it is still a bit confusing
>> and I wonder how other users of such DTS should behave. Although it will
>> affect only one board, so maybe there are no other users?
>
> Ah, now I get your point. So how does the following fix in
> sm4250-oneplus-billie2.dts look like. It allows the base dtsi to carry
> the usb nodes as exposed by the SoC and allows other board dts files
> to use both the USB2 and UBS3 PHYs.
>
> Please let me know.
>
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -232,6 +232,9 @@ &usb {
> &usb_dwc3 {
> maximum-speed = "high-speed";
> dr_mode = "peripheral";
> +
> + phys = <&usb_hsphy>;
> + phy-names = "usb2-phy";
> };
Looks good now!
Konrad
>
> Thanks.
