[PATCH 0/9] Enable USB3 for IPQ5332

[PATCH 0/9] Enable USB3 for IPQ5332

[Praveenkumar I]

IPQ5332 has UNIPHY for USB and PCIe which is similar to the UNIPHY
present in IPQ4019. Few extra settings like clock, reset delay, mux
selection and voltage regulator are required for IPQ5332. Hence
repurposed the IPQ4019 PHY driver for IPQ5332 UNIPHY. Few more Qualcomm
SoCs are also having the UNIPHY which can use the same driver for both
USB and PCIe PHY.

Praveenkumar I (9):
  dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY
  phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver
  phy: qcom: uniphy: Update UNIPHY driver to be a common driver
  dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY
  dt-bindings: usb: dwc3: Update IPQ5332 compatible
  arm64: dts: qcom: ipq5332: Add USB3 related nodes
  arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY
  phy: qcom: uniphy: Add ipq5332 USB UNIPHY support
  arm64: defconfig: Enable UNIPHY driver

 .../devicetree/bindings/phy/qcom,uniphy.yaml  | 168 +++++++
 .../bindings/phy/qcom-usb-ipq4019-phy.yaml    |  52 --
 .../devicetree/bindings/usb/qcom,dwc3.yaml    |  20 +-
 MAINTAINERS                                   |   7 +-
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts   |   7 +-
 arch/arm64/boot/dts/qcom/ipq5332.dtsi         |  39 +-
 arch/arm64/configs/defconfig                  |   1 +
 drivers/phy/qualcomm/Kconfig                  |   7 +-
 drivers/phy/qualcomm/Makefile                 |   2 +-
 drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c   | 145 ------
 drivers/phy/qualcomm/phy-qcom-uniphy.c        | 451 ++++++++++++++++++
 11 files changed, 686 insertions(+), 213 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
 delete mode 100644 drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
 create mode 100644 drivers/phy/qualcomm/phy-qcom-uniphy.c

-- 
2.34.1

[PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Praveenkumar I]

UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
can be used for other qualcomm SoCs which are having similar UNIPHY.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)

diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
similarity index 78%
rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
index 09c614952fea..cbe2cc820009 100644
--- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
@@ -1,13 +1,18 @@
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
+$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Qualcom IPQ40xx Dakota HS/SS USB PHY
+title: Qualcomm UNIPHY
 
 maintainers:
   - Robert Marko <robert.marko@sartura.hr>
+  - Praveenkumar I <quic_ipkumar@quicinc.com>
+
+description:
+  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
+  Qualcomm chipsets.
 
 properties:
   compatible:
-- 
2.34.1

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
> can be used for other qualcomm SoCs which are having similar UNIPHY.
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>  rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> similarity index 78%
> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index 09c614952fea..cbe2cc820009 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -1,13 +1,18 @@
>  # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>  %YAML 1.2
>  ---
> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
> +title: Qualcomm UNIPHY

We know that UNIPHY was a common design / IP block used for APQ8064
SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
the name reused by the Qualcomm for some other PHYs?
Several latest generations have USB QMP PHYs which are called 'uni-phy'.

>
>  maintainers:
>    - Robert Marko <robert.marko@sartura.hr>
> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
> +
> +description:
> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
> +  Qualcomm chipsets.
>
>  properties:
>    compatible:
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Praveenkumar I]

On 8/29/2023 7:49 PM, Dmitry Baryshkov wrote:
> On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
>> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
>> can be used for other qualcomm SoCs which are having similar UNIPHY.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>   rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>> similarity index 78%
>> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>> index 09c614952fea..cbe2cc820009 100644
>> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>> @@ -1,13 +1,18 @@
>>   # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>   %YAML 1.2
>>   ---
>> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
>> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
>> +title: Qualcomm UNIPHY
> We know that UNIPHY was a common design / IP block used for APQ8064
> SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
> the name reused by the Qualcomm for some other PHYs?
> Several latest generations have USB QMP PHYs which are called 'uni-phy'.
This PHY is build on top of QCA Uniphy 22ull. A combo PHY used between 
USB Gen3 / PCIe Gen3 controller.
It is different from USB QMP PHYs.

- Praveenkumar
>>   maintainers:
>>     - Robert Marko <robert.marko@sartura.hr>
>> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
>> +
>> +description:
>> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
>> +  Qualcomm chipsets.
>>
>>   properties:
>>     compatible:
>> --
>> 2.34.1
>>
>

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Dmitry Baryshkov]

On Thu, 31 Aug 2023 at 14:54, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
>
> On 8/29/2023 7:49 PM, Dmitry Baryshkov wrote:
> > On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
> >> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
> >> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
> >> can be used for other qualcomm SoCs which are having similar UNIPHY.
> >>
> >> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >> ---
> >>   .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
> >>   1 file changed, 7 insertions(+), 2 deletions(-)
> >>   rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >> similarity index 78%
> >> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> >> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >> index 09c614952fea..cbe2cc820009 100644
> >> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> >> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >> @@ -1,13 +1,18 @@
> >>   # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>   %YAML 1.2
> >>   ---
> >> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
> >> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
> >>   $schema: http://devicetree.org/meta-schemas/core.yaml#
> >>
> >> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
> >> +title: Qualcomm UNIPHY
> > We know that UNIPHY was a common design / IP block used for APQ8064
> > SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
> > the name reused by the Qualcomm for some other PHYs?
> > Several latest generations have USB QMP PHYs which are called 'uni-phy'.
> This PHY is build on top of QCA Uniphy 22ull. A combo PHY used between
> USB Gen3 / PCIe Gen3 controller.
> It is different from USB QMP PHYs.

So we have now three different items called Qualcomm uniphy. Could you
please add some distinctive name?

>
> - Praveenkumar
> >>   maintainers:
> >>     - Robert Marko <robert.marko@sartura.hr>
> >> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
> >> +
> >> +description:
> >> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
> >> +  Qualcomm chipsets.
> >>
> >>   properties:
> >>     compatible:
> >> --
> >> 2.34.1
> >>
> >



-- 
With best wishes
Dmitry

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Praveenkumar I]

On 8/31/2023 5:47 PM, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 14:54, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>
>> On 8/29/2023 7:49 PM, Dmitry Baryshkov wrote:
>>> On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>>> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
>>>> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
>>>> can be used for other qualcomm SoCs which are having similar UNIPHY.
>>>>
>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>>>> ---
>>>>    .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>>    rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>> similarity index 78%
>>>> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>>>> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>> index 09c614952fea..cbe2cc820009 100644
>>>> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>> @@ -1,13 +1,18 @@
>>>>    # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>    %YAML 1.2
>>>>    ---
>>>> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
>>>> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
>>>>    $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>
>>>> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
>>>> +title: Qualcomm UNIPHY
>>> We know that UNIPHY was a common design / IP block used for APQ8064
>>> SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
>>> the name reused by the Qualcomm for some other PHYs?
>>> Several latest generations have USB QMP PHYs which are called 'uni-phy'.
>> This PHY is build on top of QCA Uniphy 22ull. A combo PHY used between
>> USB Gen3 / PCIe Gen3 controller.
>> It is different from USB QMP PHYs.
> So we have now three different items called Qualcomm uniphy. Could you
> please add some distinctive name?
There is one more target called IPQ5018 which is also having similar USB 
PHY built on top of
Uniphy 28nm LP. That also can leverage this upcoming IPQ5332 USB PHY 
driver. Considering that,
given a common name 'uniphy'.

- Praveenkumar
>
>> - Praveenkumar
>>>>    maintainers:
>>>>      - Robert Marko <robert.marko@sartura.hr>
>>>> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
>>>> +
>>>> +description:
>>>> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
>>>> +  Qualcomm chipsets.
>>>>
>>>>    properties:
>>>>      compatible:
>>>> --
>>>> 2.34.1
>>>>
>
>

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Dmitry Baryshkov]

On Thu, 31 Aug 2023 at 15:30, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
>
> On 8/31/2023 5:47 PM, Dmitry Baryshkov wrote:
> > On Thu, 31 Aug 2023 at 14:54, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
> >>
> >> On 8/29/2023 7:49 PM, Dmitry Baryshkov wrote:
> >>> On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
> >>>> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
> >>>> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
> >>>> can be used for other qualcomm SoCs which are having similar UNIPHY.
> >>>>
> >>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >>>> ---
> >>>>    .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
> >>>>    1 file changed, 7 insertions(+), 2 deletions(-)
> >>>>    rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >>>> similarity index 78%
> >>>> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> >>>> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >>>> index 09c614952fea..cbe2cc820009 100644
> >>>> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> >>>> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> >>>> @@ -1,13 +1,18 @@
> >>>>    # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>>>    %YAML 1.2
> >>>>    ---
> >>>> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
> >>>> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
> >>>>    $schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>
> >>>> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
> >>>> +title: Qualcomm UNIPHY
> >>> We know that UNIPHY was a common design / IP block used for APQ8064
> >>> SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
> >>> the name reused by the Qualcomm for some other PHYs?
> >>> Several latest generations have USB QMP PHYs which are called 'uni-phy'.
> >> This PHY is build on top of QCA Uniphy 22ull. A combo PHY used between
> >> USB Gen3 / PCIe Gen3 controller.
> >> It is different from USB QMP PHYs.
> > So we have now three different items called Qualcomm uniphy. Could you
> > please add some distinctive name?
> There is one more target called IPQ5018 which is also having similar USB
> PHY built on top of
> Uniphy 28nm LP. That also can leverage this upcoming IPQ5332 USB PHY
> driver. Considering that,
> given a common name 'uniphy'.

Just to verify, do we mean the same thing, when speaking about the
28nm LP UNIPHY?
I was referencing the apq8064 SATA and msm8974 HDMI / DSI PHYs. See [1] and [2].

[1] https://patchwork.freedesktop.org/patch/544131/?series=118210&rev=2
[2] https://patchwork.freedesktop.org/patch/544125/?series=118210&rev=2

>
> - Praveenkumar
> >
> >> - Praveenkumar
> >>>>    maintainers:
> >>>>      - Robert Marko <robert.marko@sartura.hr>
> >>>> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
> >>>> +
> >>>> +description:
> >>>> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
> >>>> +  Qualcomm chipsets.
> >>>>
> >>>>    properties:
> >>>>      compatible:
> >>>> --
> >>>> 2.34.1
> >>>>
> >
> >



-- 
With best wishes
Dmitry

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Praveenkumar I]

On 8/31/2023 6:04 PM, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 15:30, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>
>> On 8/31/2023 5:47 PM, Dmitry Baryshkov wrote:
>>> On Thu, 31 Aug 2023 at 14:54, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>>> On 8/29/2023 7:49 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, 29 Aug 2023 at 16:59, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>>>>> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
>>>>>> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
>>>>>> can be used for other qualcomm SoCs which are having similar UNIPHY.
>>>>>>
>>>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>>>>>> ---
>>>>>>     .../phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml}  | 9 +++++++--
>>>>>>     1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>     rename Documentation/devicetree/bindings/phy/{qcom-usb-ipq4019-phy.yaml => qcom,uniphy.yaml} (78%)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>>>> similarity index 78%
>>>>>> rename from Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>>>>>> rename to Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>>>> index 09c614952fea..cbe2cc820009 100644
>>>>>> --- a/Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
>>>>>> @@ -1,13 +1,18 @@
>>>>>>     # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>>     %YAML 1.2
>>>>>>     ---
>>>>>> -$id: http://devicetree.org/schemas/phy/qcom-usb-ipq4019-phy.yaml#
>>>>>> +$id: http://devicetree.org/schemas/phy/qcom,uniphy.yaml#
>>>>>>     $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>
>>>>>> -title: Qualcom IPQ40xx Dakota HS/SS USB PHY
>>>>>> +title: Qualcomm UNIPHY
>>>>> We know that UNIPHY was a common design / IP block used for APQ8064
>>>>> SATA and MSM8974 DSI and HDMI PHYs. Is this the same design, or was
>>>>> the name reused by the Qualcomm for some other PHYs?
>>>>> Several latest generations have USB QMP PHYs which are called 'uni-phy'.
>>>> This PHY is build on top of QCA Uniphy 22ull. A combo PHY used between
>>>> USB Gen3 / PCIe Gen3 controller.
>>>> It is different from USB QMP PHYs.
>>> So we have now three different items called Qualcomm uniphy. Could you
>>> please add some distinctive name?
>> There is one more target called IPQ5018 which is also having similar USB
>> PHY built on top of
>> Uniphy 28nm LP. That also can leverage this upcoming IPQ5332 USB PHY
>> driver. Considering that,
>> given a common name 'uniphy'.
> Just to verify, do we mean the same thing, when speaking about the
> 28nm LP UNIPHY?
> I was referencing the apq8064 SATA and msm8974 HDMI / DSI PHYs. See [1] and [2].
>
> [1] https://patchwork.freedesktop.org/patch/544131/?series=118210&rev=2
> [2] https://patchwork.freedesktop.org/patch/544125/?series=118210&rev=2
No, this seems different from the PHY used on IPQ5018 / IPQ5332. PHY in 
QualcommIPQ
targets requires minimal SW configuration for the bring up.
>> - Praveenkumar
>>>> - Praveenkumar
>>>>>>     maintainers:
>>>>>>       - Robert Marko <robert.marko@sartura.hr>
>>>>>> +  - Praveenkumar I <quic_ipkumar@quicinc.com>
>>>>>> +
>>>>>> +description:
>>>>>> +  UNIPHY / COMBO PHY supports physical layer functionality for USB and PCIe on
>>>>>> +  Qualcomm chipsets.
>>>>>>
>>>>>>     properties:
>>>>>>       compatible:
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>
>
>

Re: [PATCH 1/9] dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
> ipq4019 PHY. Hence renaming this dt-binding to uniphy dt-binding and
> can be used for other qualcomm SoCs which are having similar UNIPHY.

That's not the reason to rename it. Keep the old name.

Best regards,
Krzysztof

[PATCH 2/9] phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver

[Praveenkumar I]

UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
ipq4019 PHY. Hence renaming this driver to uniphy driver and can be
used for other SoC's which are having the similar UNIPHY.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 MAINTAINERS                                                | 7 ++++---
 drivers/phy/qualcomm/Kconfig                               | 7 ++++---
 drivers/phy/qualcomm/Makefile                              | 2 +-
 .../qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} | 0
 4 files changed, 9 insertions(+), 7 deletions(-)
 rename drivers/phy/qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index ff1f273b4f36..7f4553c1a69a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17774,13 +17774,14 @@ F:	Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
 F:	drivers/mailbox/qcom-ipcc.c
 F:	include/dt-bindings/mailbox/qcom-ipcc.h
 
-QUALCOMM IPQ4019 USB PHY DRIVER
+QUALCOMM UNIPHY DRIVER
 M:	Robert Marko <robert.marko@sartura.hr>
 M:	Luka Perkov <luka.perkov@sartura.hr>
+M:	Praveenkumar I <quic_ipkumar@quicinc.com>
 L:	linux-arm-msm@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
-F:	drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
+F:	Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
+F:	drivers/phy/qualcomm/phy-qcom-uniphy.c
 
 QUALCOMM IPQ4019 VQMMC REGULATOR DRIVER
 M:	Robert Marko <robert.marko@sartura.hr>
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index d891058b7c39..e6981bc212b3 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -28,12 +28,13 @@ config PHY_QCOM_EDP
 	  Enable this driver to support the Qualcomm eDP PHY found in various
 	  Qualcomm chipsets.
 
-config PHY_QCOM_IPQ4019_USB
-	tristate "Qualcomm IPQ4019 USB PHY driver"
+config PHY_QCOM_UNIPHY
+	tristate "Qualcomm UNIPHY driver"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
 	select GENERIC_PHY
 	help
-	  Support for the USB PHY-s on Qualcomm IPQ40xx SoC-s.
+	  Enable this driver to support the Qualcomm UNIPHY found in various
+	  Qualcomm chipsets.
 
 config PHY_QCOM_IPQ806X_SATA
 	tristate "Qualcomm IPQ806x SATA SerDes/PHY driver"
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index ffd609ac6233..7460e1a427d2 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -2,7 +2,7 @@
 obj-$(CONFIG_PHY_ATH79_USB)		+= phy-ath79-usb.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
-obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
+obj-$(CONFIG_PHY_QCOM_UNIPHY)		+= phy-qcom-uniphy.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
diff --git a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
similarity index 100%
rename from drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
rename to drivers/phy/qualcomm/phy-qcom-uniphy.c
-- 
2.34.1

Re: [PATCH 2/9] phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
> ipq4019 PHY. Hence renaming this driver to uniphy driver and can be
> used for other SoC's which are having the similar UNIPHY.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  MAINTAINERS                                                | 7 ++++---
>  drivers/phy/qualcomm/Kconfig                               | 7 ++++---
>  drivers/phy/qualcomm/Makefile                              | 2 +-
>  .../qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} | 0
>  4 files changed, 9 insertions(+), 7 deletions(-)
>  rename drivers/phy/qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ff1f273b4f36..7f4553c1a69a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17774,13 +17774,14 @@ F:	Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
>  F:	drivers/mailbox/qcom-ipcc.c
>  F:	include/dt-bindings/mailbox/qcom-ipcc.h
>  
> -QUALCOMM IPQ4019 USB PHY DRIVER
> +QUALCOMM UNIPHY DRIVER
>  M:	Robert Marko <robert.marko@sartura.hr>
>  M:	Luka Perkov <luka.perkov@sartura.hr>
> +M:	Praveenkumar I <quic_ipkumar@quicinc.com>
>  L:	linux-arm-msm@vger.kernel.org
>  S:	Maintained
> -F:	Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
> -F:	drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> +F:	Documentation/devicetree/bindings/phy/qcom,uniphy.yaml

You broke the path in your previous commit, but anyway this will go away.

> +F:	drivers/phy/qualcomm/phy-qcom-uniphy.c
>  
>  QUALCOMM IPQ4019 VQMMC REGULATOR DRIVER
>  M:	Robert Marko <robert.marko@sartura.hr>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index d891058b7c39..e6981bc212b3 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -28,12 +28,13 @@ config PHY_QCOM_EDP
>  	  Enable this driver to support the Qualcomm eDP PHY found in various
>  	  Qualcomm chipsets.
>  
> -config PHY_QCOM_IPQ4019_USB
> -	tristate "Qualcomm IPQ4019 USB PHY driver"
> +config PHY_QCOM_UNIPHY
> +	tristate "Qualcomm UNIPHY driver"
>  	depends on OF && (ARCH_QCOM || COMPILE_TEST)
>  	select GENERIC_PHY
>  	help
> -	  Support for the USB PHY-s on Qualcomm IPQ40xx SoC-s.
> +	  Enable this driver to support the Qualcomm UNIPHY found in various
> +	  Qualcomm chipsets.

I don't quite get why this is renamed, either. Just because you re-use
it? Re-usage is not affected with old name...

Best regards,
Krzysztof

Re: [PATCH 2/9] phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver

[Praveenkumar I]

On 8/29/2023 10:25 PM, Krzysztof Kozlowski wrote:
> On 29/08/2023 15:58, Praveenkumar I wrote:
>> UNIPHY / Combo PHY used on various qualcomm SoC's are very similar to
>> ipq4019 PHY. Hence renaming this driver to uniphy driver and can be
>> used for other SoC's which are having the similar UNIPHY.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   MAINTAINERS                                                | 7 ++++---
>>   drivers/phy/qualcomm/Kconfig                               | 7 ++++---
>>   drivers/phy/qualcomm/Makefile                              | 2 +-
>>   .../qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} | 0
>>   4 files changed, 9 insertions(+), 7 deletions(-)
>>   rename drivers/phy/qualcomm/{phy-qcom-ipq4019-usb.c => phy-qcom-uniphy.c} (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ff1f273b4f36..7f4553c1a69a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17774,13 +17774,14 @@ F:	Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
>>   F:	drivers/mailbox/qcom-ipcc.c
>>   F:	include/dt-bindings/mailbox/qcom-ipcc.h
>>   
>> -QUALCOMM IPQ4019 USB PHY DRIVER
>> +QUALCOMM UNIPHY DRIVER
>>   M:	Robert Marko <robert.marko@sartura.hr>
>>   M:	Luka Perkov <luka.perkov@sartura.hr>
>> +M:	Praveenkumar I <quic_ipkumar@quicinc.com>
>>   L:	linux-arm-msm@vger.kernel.org
>>   S:	Maintained
>> -F:	Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
>> -F:	drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
>> +F:	Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> You broke the path in your previous commit, but anyway this will go away.
>
>> +F:	drivers/phy/qualcomm/phy-qcom-uniphy.c
>>   
>>   QUALCOMM IPQ4019 VQMMC REGULATOR DRIVER
>>   M:	Robert Marko <robert.marko@sartura.hr>
>> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
>> index d891058b7c39..e6981bc212b3 100644
>> --- a/drivers/phy/qualcomm/Kconfig
>> +++ b/drivers/phy/qualcomm/Kconfig
>> @@ -28,12 +28,13 @@ config PHY_QCOM_EDP
>>   	  Enable this driver to support the Qualcomm eDP PHY found in various
>>   	  Qualcomm chipsets.
>>   
>> -config PHY_QCOM_IPQ4019_USB
>> -	tristate "Qualcomm IPQ4019 USB PHY driver"
>> +config PHY_QCOM_UNIPHY
>> +	tristate "Qualcomm UNIPHY driver"
>>   	depends on OF && (ARCH_QCOM || COMPILE_TEST)
>>   	select GENERIC_PHY
>>   	help
>> -	  Support for the USB PHY-s on Qualcomm IPQ40xx SoC-s.
>> +	  Enable this driver to support the Qualcomm UNIPHY found in various
>> +	  Qualcomm chipsets.
> I don't quite get why this is renamed, either. Just because you re-use
> it? Re-usage is not affected with old name...
Understood. As suggested by Dmitry Baryshkov, will add new driver for 
Qualcomm IPQ5332 USB Gen3
PHY driver.

- Praveenkumar
>
> Best regards,
> Krzysztof
>

[PATCH 3/9] phy: qcom: uniphy: Update UNIPHY driver to be a common driver

[Praveenkumar I]

This patch updates the UNIPHY driver to be a common driver to
accommodate all UNIPHY / Combo PHY. This driver can be used for
both USB and PCIe UNIPHY. Using phy-mul-sel from DTS MUX selection
for USB / PCIe can be acheived.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-uniphy.c | 401 +++++++++++++++++++++----
 1 file changed, 335 insertions(+), 66 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
index da6f290af722..eb71588f5417 100644
--- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
+++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
@@ -5,141 +5,410 @@
  * Based on code from
  * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
  *
+ * Modified the driver to be common for Qualcomm UNIPHYs
+ * Copyright (c) 2023, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
-struct ipq4019_usb_phy {
+struct uniphy_init_tbl {
+	unsigned int offset;
+	unsigned int val;
+};
+
+#define UNIPHY_INIT_CFG(o, v)		\
+	{				\
+		.offset = o,		\
+		.val = v,		\
+	}
+
+struct uniphy_cfg {
+	const struct uniphy_init_tbl *init_seq;
+	int num_init_seq;
+	const char * const *clk_list;
+	int num_clks;
+	const char * const *reset_list;
+	int num_resets;
+	const char * const *vreg_list;
+	int num_vregs;
+	unsigned int pipe_clk_rate;
+	unsigned int reset_udelay;
+	unsigned int autoload_udelay;
+};
+
+struct qcom_uniphy {
 	struct device		*dev;
+	const struct uniphy_cfg	*cfg;
 	struct phy		*phy;
 	void __iomem		*base;
-	struct reset_control	*por_rst;
-	struct reset_control	*srif_rst;
+	struct clk_bulk_data	*clks;
+	struct reset_control_bulk_data	*resets;
+	struct regulator_bulk_data *vregs;
+	struct clk_fixed_rate pipe_clk_fixed;
+};
+
+static const char * const ipq4019_ssphy_reset_l[] = {
+	"por_rst",
+};
+
+static const struct uniphy_cfg ipq4019_usb_ssphy_cfg = {
+	.reset_list	= ipq4019_ssphy_reset_l,
+	.num_resets	= ARRAY_SIZE(ipq4019_ssphy_reset_l),
+	.reset_udelay	= 10000,
+
 };
 
-static int ipq4019_ss_phy_power_off(struct phy *_phy)
+static const char * const ipq4019_hsphy_reset_l[] = {
+	"por_rst", "srif_rst",
+};
+
+static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
+	.reset_list	= ipq4019_hsphy_reset_l,
+	.num_resets	= ARRAY_SIZE(ipq4019_hsphy_reset_l),
+	.reset_udelay	= 10000,
+};
+
+static int phy_mux_sel(struct phy *phy)
+{
+	struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
+	struct device *dev = uniphy->dev;
+	struct regmap *tcsr;
+	unsigned int args[2];
+	int ret;
+
+	tcsr = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,phy-mux-sel",
+						    ARRAY_SIZE(args), args);
+	if (IS_ERR(tcsr)) {
+		ret = PTR_ERR(tcsr);
+		if (ret == -ENOENT)
+			return 0;
+
+		dev_err(dev, "failed to lookup syscon for phy mux %d\n", ret);
+		return ret;
+	}
+
+	/* PHY MUX registers only have this BIT0 */
+	ret = regmap_write(tcsr, args[0], args[1]);
+	if (ret < 0) {
+		dev_err(dev, "PHY Mux selection failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int uniphy_enable(struct phy *phy)
 {
-	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+	struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	const struct uniphy_init_tbl *tbl;
+	void __iomem *base = uniphy->base;
+	int i, ret;
 
-	reset_control_assert(phy->por_rst);
-	msleep(10);
+	ret = regulator_bulk_enable(cfg->num_vregs, uniphy->vregs);
+	if (ret) {
+		dev_err(uniphy->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	/* Assert all available resets */
+	for (i = 0; i < cfg->num_resets; i++) {
+		ret = reset_control_assert(uniphy->resets[i].rstc);
+		if (ret) {
+			dev_err(uniphy->dev, "reset assert failed: %d\n", ret);
+			goto err_assert_reset;
+		}
+		if (cfg->reset_udelay)
+			usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
+	}
+
+	/* Deassert all available resets */
+	for (i = 0; i < cfg->num_resets; i++) {
+		ret = reset_control_deassert(uniphy->resets[i].rstc);
+		if (ret) {
+			dev_err(uniphy->dev, "reset deassert failed: %d\n", ret);
+			goto err_assert_reset;
+		}
+		if (cfg->reset_udelay)
+			usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
+	}
+
+	ret = phy_mux_sel(phy);
+	if (ret < 0)
+		goto err_assert_reset;
+
+	ret = clk_bulk_prepare_enable(cfg->num_clks, uniphy->clks);
+	if (ret) {
+		dev_err(uniphy->dev, "failed to enable clocks: %d\n", ret);
+		goto err_assert_reset;
+	}
+
+	if (cfg->autoload_udelay)
+		usleep_range(cfg->autoload_udelay, cfg->autoload_udelay + 10);
+
+	if (cfg->num_init_seq) {
+		tbl = cfg->init_seq;
+		for (i = 0; i < cfg->num_init_seq; i++, tbl++)
+			writel(tbl->val, base + tbl->offset);
+	}
 
 	return 0;
+
+err_assert_reset:
+	/* Assert all available resets */
+	for (i = 0; i < cfg->num_resets; i++) {
+		reset_control_assert(uniphy->resets[i].rstc);
+		if (cfg->reset_udelay)
+			usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
+	}
+
+	return ret;
 }
 
-static int ipq4019_ss_phy_power_on(struct phy *_phy)
+static int uniphy_disable(struct phy *phy)
 {
-	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+	struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	int i;
 
-	ipq4019_ss_phy_power_off(_phy);
+	/* Assert all available resets */
+	for (i = 0; i < cfg->num_resets; i++) {
+		reset_control_assert(uniphy->resets[i].rstc);
+		if (cfg->reset_udelay)
+			usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
+	}
 
-	reset_control_deassert(phy->por_rst);
+	clk_bulk_disable_unprepare(cfg->num_clks, uniphy->clks);
+
+	regulator_bulk_disable(cfg->num_vregs, uniphy->vregs);
 
 	return 0;
 }
 
-static const struct phy_ops ipq4019_usb_ss_phy_ops = {
-	.power_on	= ipq4019_ss_phy_power_on,
-	.power_off	= ipq4019_ss_phy_power_off,
+static const struct phy_ops uniphy_phy_ops = {
+	.power_on	= uniphy_enable,
+	.power_off	= uniphy_disable,
+	.owner		= THIS_MODULE,
 };
 
-static int ipq4019_hs_phy_power_off(struct phy *_phy)
+static int qcom_uniphy_vreg_init(struct qcom_uniphy *uniphy)
+{
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	struct device *dev = uniphy->dev;
+	int i, ret;
+
+	uniphy->vregs = devm_kcalloc(dev, cfg->num_vregs,
+				     sizeof(*uniphy->vregs), GFP_KERNEL);
+	if (!uniphy->vregs)
+		return -ENOMEM;
+
+	for (i = 0; i < cfg->num_vregs; i++)
+		uniphy->vregs[i].supply = cfg->vreg_list[i];
+
+	ret = devm_regulator_bulk_get(dev, cfg->num_vregs, uniphy->vregs);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+	return 0;
+}
+
+static int qcom_uniphy_reset_init(struct qcom_uniphy *uniphy)
 {
-	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	struct device *dev = uniphy->dev;
+	int i, ret;
+
+	uniphy->resets = devm_kcalloc(dev, cfg->num_resets,
+				      sizeof(*uniphy->resets), GFP_KERNEL);
+	if (!uniphy->resets)
+		return -ENOMEM;
 
-	reset_control_assert(phy->por_rst);
-	msleep(10);
+	for (i = 0; i < cfg->num_resets; i++)
+		uniphy->resets[i].id = cfg->reset_list[i];
 
-	reset_control_assert(phy->srif_rst);
-	msleep(10);
+	ret = devm_reset_control_bulk_get_exclusive(dev, cfg->num_resets, uniphy->resets);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get resets\n");
 
 	return 0;
 }
 
-static int ipq4019_hs_phy_power_on(struct phy *_phy)
+static int qcom_uniphy_clk_init(struct qcom_uniphy *uniphy)
 {
-	struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	struct device *dev = uniphy->dev;
+	int i, ret;
 
-	ipq4019_hs_phy_power_off(_phy);
 
-	reset_control_deassert(phy->srif_rst);
-	msleep(10);
+	uniphy->clks = devm_kcalloc(dev, cfg->num_clks,
+				    sizeof(*uniphy->clks), GFP_KERNEL);
+	if (!uniphy->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < cfg->num_clks; i++)
+		uniphy->clks[i].id = cfg->clk_list[i];
 
-	reset_control_deassert(phy->por_rst);
+	ret = devm_clk_bulk_get(dev, cfg->num_clks, uniphy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get clocks\n");
 
 	return 0;
 }
 
-static const struct phy_ops ipq4019_usb_hs_phy_ops = {
-	.power_on	= ipq4019_hs_phy_power_on,
-	.power_off	= ipq4019_hs_phy_power_off,
-};
+static void phy_clk_release_provider(void *res)
+{
+	of_clk_del_provider(res);
+}
 
-static const struct of_device_id ipq4019_usb_phy_of_match[] = {
-	{ .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hs_phy_ops},
-	{ .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ss_phy_ops},
-	{ },
-};
-MODULE_DEVICE_TABLE(of, ipq4019_usb_phy_of_match);
+/*
+ * Register a fixed rate pipe clock.
+ *
+ * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
+ * controls it. The <s>_pipe_clk coming out of the GCC is requested
+ * by the PHY driver for its operations.
+ * We register the <s>_pipe_clksrc here. The gcc driver takes care
+ * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
+ * Below picture shows this relationship.
+ *
+ *         +---------------+
+ *         |   PHY block   |<<---------------------------------------+
+ *         |               |                                         |
+ *         |   +-------+   |                   +-----+               |
+ *   I/P---^-->|  PLL  |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
+ *    clk  |   +-------+   |                   +-----+
+ *         +---------------+
+ */
+static int phy_pipe_clk_register(struct qcom_uniphy *uniphy, struct device_node *np)
+{
+	struct clk_fixed_rate *fixed = &uniphy->pipe_clk_fixed;
+	const struct uniphy_cfg *cfg = uniphy->cfg;
+	struct device *dev = uniphy->dev;
+	struct clk_init_data init = { };
+	int ret;
+
+	ret = of_property_read_string(np, "clock-output-names", &init.name);
+	if (ret) {
+		dev_err(dev, "%pOFn: No clock-output-names\n", np);
+		return ret;
+	}
+
+	init.ops = &clk_fixed_rate_ops;
+
+	fixed->fixed_rate = cfg->pipe_clk_rate;
+	fixed->hw.init = &init;
 
-static int ipq4019_usb_phy_probe(struct platform_device *pdev)
+	ret = devm_clk_hw_register(dev, &fixed->hw);
+	if (ret)
+		return ret;
+
+	ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
+	if (ret)
+		return ret;
+
+	/*
+	 * Roll a devm action because the clock provider is the child node, but
+	 * the child node is not actually a device.
+	 */
+	return devm_add_action_or_reset(dev, phy_clk_release_provider, np);
+}
+
+static int qcom_uniphy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct phy_provider *phy_provider;
-	struct ipq4019_usb_phy *phy;
+	struct qcom_uniphy *uniphy;
+	struct device_node *np;
+	int ret;
 
-	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
-	if (!phy)
+	uniphy = devm_kzalloc(dev, sizeof(*uniphy), GFP_KERNEL);
+	if (!uniphy)
 		return -ENOMEM;
 
-	phy->dev = &pdev->dev;
-	phy->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(phy->base)) {
-		dev_err(dev, "failed to remap register memory\n");
-		return PTR_ERR(phy->base);
-	}
+	uniphy->dev = dev;
 
-	phy->por_rst = devm_reset_control_get(phy->dev, "por_rst");
-	if (IS_ERR(phy->por_rst)) {
-		if (PTR_ERR(phy->por_rst) != -EPROBE_DEFER)
-			dev_err(dev, "POR reset is missing\n");
-		return PTR_ERR(phy->por_rst);
+	uniphy->cfg = of_device_get_match_data(dev);
+	if (!uniphy->cfg)
+		return -EINVAL;
+
+	uniphy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(uniphy->base)) {
+		ret = PTR_ERR(uniphy->base);
+		dev_err_probe(dev, ret, "failed to remap register memory\n");
+		return ret;
 	}
 
-	phy->srif_rst = devm_reset_control_get_optional(phy->dev, "srif_rst");
-	if (IS_ERR(phy->srif_rst))
-		return PTR_ERR(phy->srif_rst);
+	ret = qcom_uniphy_clk_init(uniphy);
+	if (ret)
+		return ret;
+
+	ret = qcom_uniphy_reset_init(uniphy);
+	if (ret)
+		return ret;
+
+	ret = qcom_uniphy_vreg_init(uniphy);
+	if (ret < 0)
+		return ret;
+
+	if (uniphy->cfg->pipe_clk_rate) {
+		np = of_node_get(dev->of_node);
+		ret = phy_pipe_clk_register(uniphy, np);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to register pipe clk\n");
+			goto err;
+		}
+	}
 
-	phy->phy = devm_phy_create(dev, NULL, of_device_get_match_data(dev));
-	if (IS_ERR(phy->phy)) {
-		dev_err(dev, "failed to create PHY\n");
-		return PTR_ERR(phy->phy);
+	uniphy->phy = devm_phy_create(dev, NULL, &uniphy_phy_ops);
+	if (IS_ERR(uniphy->phy)) {
+		ret = PTR_ERR(uniphy->phy);
+		dev_err_probe(dev, ret, "failed to create PHY\n");
+		goto err;
 	}
-	phy_set_drvdata(phy->phy, phy);
+
+	phy_set_drvdata(uniphy->phy, uniphy);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
-	return PTR_ERR_OR_ZERO(phy_provider);
+	ret = PTR_ERR_OR_ZERO(phy_provider);
+
+err:
+	if (uniphy->cfg->pipe_clk_rate)
+		of_node_put(np);
+	return ret;
 }
 
-static struct platform_driver ipq4019_usb_phy_driver = {
-	.probe	= ipq4019_usb_phy_probe,
+static const struct of_device_id qcom_uniphy_of_match[] = {
+	{ .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
+	{ .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
+
+static struct platform_driver qcom_uniphy_driver = {
+	.probe	= qcom_uniphy_probe,
 	.driver = {
-		.of_match_table	= ipq4019_usb_phy_of_match,
-		.name  = "ipq4019-usb-phy",
+		.of_match_table	= qcom_uniphy_of_match,
+		.name  = "qcom-uniphy",
 	}
 };
-module_platform_driver(ipq4019_usb_phy_driver);
+module_platform_driver(qcom_uniphy_driver);
 
-MODULE_DESCRIPTION("QCOM/IPQ4019 USB phy driver");
+MODULE_DESCRIPTION("QCOM uniphy driver");
 MODULE_AUTHOR("John Crispin <john@phrozen.org>");
 MODULE_LICENSE("GPL v2");
-- 
2.34.1

Re: [PATCH 3/9] phy: qcom: uniphy: Update UNIPHY driver to be a common driver

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> This patch updates the UNIPHY driver to be a common driver to
> accommodate all UNIPHY / Combo PHY. This driver can be used for
> both USB and PCIe UNIPHY. Using phy-mul-sel from DTS MUX selection
> for USB / PCIe can be acheived.

I'm not sure why you are talking about PCIe here. This patch adds only
SS PHY support.

Also, I'd like to point out that we had this 'USB and PCIe and
everything else' design in the QMP driver. We had to split the driver
into individual pieces to make it manageable again.

>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-uniphy.c | 401 +++++++++++++++++++++----
>  1 file changed, 335 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> index da6f290af722..eb71588f5417 100644
> --- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> @@ -5,141 +5,410 @@
>   * Based on code from
>   * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
>   *
> + * Modified the driver to be common for Qualcomm UNIPHYs
> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.

I'd not call this 'modified', but rather 'rewritten from scratch.

>   */
>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>
> -struct ipq4019_usb_phy {
> +struct uniphy_init_tbl {
> +       unsigned int offset;
> +       unsigned int val;
> +};

unused

> +
> +#define UNIPHY_INIT_CFG(o, v)          \
> +       {                               \
> +               .offset = o,            \
> +               .val = v,               \
> +       }

unused

> +
> +struct uniphy_cfg {
> +       const struct uniphy_init_tbl *init_seq;
> +       int num_init_seq;
> +       const char * const *clk_list;
> +       int num_clks;
> +       const char * const *reset_list;
> +       int num_resets;
> +       const char * const *vreg_list;
> +       int num_vregs;
> +       unsigned int pipe_clk_rate;
> +       unsigned int reset_udelay;
> +       unsigned int autoload_udelay;
> +};
> +
> +struct qcom_uniphy {
>         struct device           *dev;
> +       const struct uniphy_cfg *cfg;
>         struct phy              *phy;
>         void __iomem            *base;
> -       struct reset_control    *por_rst;
> -       struct reset_control    *srif_rst;
> +       struct clk_bulk_data    *clks;
> +       struct reset_control_bulk_data  *resets;
> +       struct regulator_bulk_data *vregs;
> +       struct clk_fixed_rate pipe_clk_fixed;
> +};
> +
> +static const char * const ipq4019_ssphy_reset_l[] = {
> +       "por_rst",
> +};
> +
> +static const struct uniphy_cfg ipq4019_usb_ssphy_cfg = {
> +       .reset_list     = ipq4019_ssphy_reset_l,
> +       .num_resets     = ARRAY_SIZE(ipq4019_ssphy_reset_l),
> +       .reset_udelay   = 10000,
> +
>  };
>
> -static int ipq4019_ss_phy_power_off(struct phy *_phy)
> +static const char * const ipq4019_hsphy_reset_l[] = {
> +       "por_rst", "srif_rst",
> +};
+
> +static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
> +       .reset_list     = ipq4019_hsphy_reset_l,
> +       .num_resets     = ARRAY_SIZE(ipq4019_hsphy_reset_l),
> +       .reset_udelay   = 10000,
> +};
> +
> +static int phy_mux_sel(struct phy *phy)
> +{
> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
> +       struct device *dev = uniphy->dev;
> +       struct regmap *tcsr;
> +       unsigned int args[2];
> +       int ret;
> +
> +       tcsr = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,phy-mux-sel",
> +                                                   ARRAY_SIZE(args), args);

No. mux data should come from match_data rather than polluting DT with it.

> +       if (IS_ERR(tcsr)) {
> +               ret = PTR_ERR(tcsr);
> +               if (ret == -ENOENT)
> +                       return 0;
> +
> +               dev_err(dev, "failed to lookup syscon for phy mux %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* PHY MUX registers only have this BIT0 */

huh?

> +       ret = regmap_write(tcsr, args[0], args[1]);
> +       if (ret < 0) {
> +               dev_err(dev, "PHY Mux selection failed: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int uniphy_enable(struct phy *phy)
>  {
> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       const struct uniphy_init_tbl *tbl;
> +       void __iomem *base = uniphy->base;
> +       int i, ret;
>
> -       reset_control_assert(phy->por_rst);
> -       msleep(10);
> +       ret = regulator_bulk_enable(cfg->num_vregs, uniphy->vregs);
> +       if (ret) {
> +               dev_err(uniphy->dev, "failed to enable regulators: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* Assert all available resets */
> +       for (i = 0; i < cfg->num_resets; i++) {
> +               ret = reset_control_assert(uniphy->resets[i].rstc);
> +               if (ret) {
> +                       dev_err(uniphy->dev, "reset assert failed: %d\n", ret);
> +                       goto err_assert_reset;
> +               }
> +               if (cfg->reset_udelay)
> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
> +       }

There is a whole reset_control_bulk_*() set of API. Please use it
instead of hardcoding reset cycles.

> +
> +       /* Deassert all available resets */
> +       for (i = 0; i < cfg->num_resets; i++) {
> +               ret = reset_control_deassert(uniphy->resets[i].rstc);
> +               if (ret) {
> +                       dev_err(uniphy->dev, "reset deassert failed: %d\n", ret);
> +                       goto err_assert_reset;
> +               }
> +               if (cfg->reset_udelay)
> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
> +       }
> +
> +       ret = phy_mux_sel(phy);
> +       if (ret < 0)
> +               goto err_assert_reset;
> +
> +       ret = clk_bulk_prepare_enable(cfg->num_clks, uniphy->clks);
> +       if (ret) {
> +               dev_err(uniphy->dev, "failed to enable clocks: %d\n", ret);
> +               goto err_assert_reset;
> +       }
> +
> +       if (cfg->autoload_udelay)
> +               usleep_range(cfg->autoload_udelay, cfg->autoload_udelay + 10);
> +
> +       if (cfg->num_init_seq) {
> +               tbl = cfg->init_seq;
> +               for (i = 0; i < cfg->num_init_seq; i++, tbl++)
> +                       writel(tbl->val, base + tbl->offset);
> +       }

unused

>
>         return 0;
> +
> +err_assert_reset:
> +       /* Assert all available resets */
> +       for (i = 0; i < cfg->num_resets; i++) {
> +               reset_control_assert(uniphy->resets[i].rstc);
> +               if (cfg->reset_udelay)
> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
> +       }
> +
> +       return ret;
>  }
>
> -static int ipq4019_ss_phy_power_on(struct phy *_phy)
> +static int uniphy_disable(struct phy *phy)
>  {
> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       int i;
>
> -       ipq4019_ss_phy_power_off(_phy);
> +       /* Assert all available resets */
> +       for (i = 0; i < cfg->num_resets; i++) {
> +               reset_control_assert(uniphy->resets[i].rstc);
> +               if (cfg->reset_udelay)
> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
> +       }
>
> -       reset_control_deassert(phy->por_rst);
> +       clk_bulk_disable_unprepare(cfg->num_clks, uniphy->clks);
> +
> +       regulator_bulk_disable(cfg->num_vregs, uniphy->vregs);
>
>         return 0;
>  }
>
> -static const struct phy_ops ipq4019_usb_ss_phy_ops = {
> -       .power_on       = ipq4019_ss_phy_power_on,
> -       .power_off      = ipq4019_ss_phy_power_off,
> +static const struct phy_ops uniphy_phy_ops = {
> +       .power_on       = uniphy_enable,
> +       .power_off      = uniphy_disable,

Using _enable / _disable for power_on() and power_off() isn't logical.

> +       .owner          = THIS_MODULE,
>  };
>
> -static int ipq4019_hs_phy_power_off(struct phy *_phy)
> +static int qcom_uniphy_vreg_init(struct qcom_uniphy *uniphy)
> +{
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       struct device *dev = uniphy->dev;
> +       int i, ret;
> +
> +       uniphy->vregs = devm_kcalloc(dev, cfg->num_vregs,
> +                                    sizeof(*uniphy->vregs), GFP_KERNEL);

You know the maximum amount of regulators. Can you use an array
instead of allocating data?

> +       if (!uniphy->vregs)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < cfg->num_vregs; i++)
> +               uniphy->vregs[i].supply = cfg->vreg_list[i];
> +
> +       ret = devm_regulator_bulk_get(dev, cfg->num_vregs, uniphy->vregs);
> +

Drop empty lines between ret assignment and check.

> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> +       return 0;
> +}
> +
> +static int qcom_uniphy_reset_init(struct qcom_uniphy *uniphy)
>  {
> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       struct device *dev = uniphy->dev;
> +       int i, ret;
> +
> +       uniphy->resets = devm_kcalloc(dev, cfg->num_resets,
> +                                     sizeof(*uniphy->resets), GFP_KERNEL);

Same here, can you use an array?

> +       if (!uniphy->resets)
> +               return -ENOMEM;
>
> -       reset_control_assert(phy->por_rst);
> -       msleep(10);
> +       for (i = 0; i < cfg->num_resets; i++)
> +               uniphy->resets[i].id = cfg->reset_list[i];

Declare common resets list and use
devm_reset_control_bulk_get_optional_exclusive().

>
> -       reset_control_assert(phy->srif_rst);
> -       msleep(10);
> +       ret = devm_reset_control_bulk_get_exclusive(dev, cfg->num_resets, uniphy->resets);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to get resets\n");
>
>         return 0;
>  }
>
> -static int ipq4019_hs_phy_power_on(struct phy *_phy)
> +static int qcom_uniphy_clk_init(struct qcom_uniphy *uniphy)

I don't see clocks actually being used. Please do not introduce unused features.

>  {
> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       struct device *dev = uniphy->dev;
> +       int i, ret;
>
> -       ipq4019_hs_phy_power_off(_phy);
>
> -       reset_control_deassert(phy->srif_rst);
> -       msleep(10);
> +       uniphy->clks = devm_kcalloc(dev, cfg->num_clks,
> +                                   sizeof(*uniphy->clks), GFP_KERNEL);
> +       if (!uniphy->clks)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < cfg->num_clks; i++)
> +               uniphy->clks[i].id = cfg->clk_list[i];
>
> -       reset_control_deassert(phy->por_rst);
> +       ret = devm_clk_bulk_get(dev, cfg->num_clks, uniphy->clks);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to get clocks\n");
>
>         return 0;
>  }
>
> -static const struct phy_ops ipq4019_usb_hs_phy_ops = {
> -       .power_on       = ipq4019_hs_phy_power_on,
> -       .power_off      = ipq4019_hs_phy_power_off,
> -};
> +static void phy_clk_release_provider(void *res)
> +{
> +       of_clk_del_provider(res);
> +}
>
> -static const struct of_device_id ipq4019_usb_phy_of_match[] = {
> -       { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hs_phy_ops},
> -       { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ss_phy_ops},
> -       { },
> -};
> -MODULE_DEVICE_TABLE(of, ipq4019_usb_phy_of_match);
> +/*
> + * Register a fixed rate pipe clock.
> + *
> + * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
> + * controls it. The <s>_pipe_clk coming out of the GCC is requested
> + * by the PHY driver for its operations.
> + * We register the <s>_pipe_clksrc here. The gcc driver takes care
> + * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
> + * Below picture shows this relationship.
> + *
> + *         +---------------+
> + *         |   PHY block   |<<---------------------------------------+
> + *         |               |                                         |
> + *         |   +-------+   |                   +-----+               |
> + *   I/P---^-->|  PLL  |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
> + *    clk  |   +-------+   |                   +-----+
> + *         +---------------+
> + */
> +static int phy_pipe_clk_register(struct qcom_uniphy *uniphy, struct device_node *np)
> +{
> +       struct clk_fixed_rate *fixed = &uniphy->pipe_clk_fixed;
> +       const struct uniphy_cfg *cfg = uniphy->cfg;
> +       struct device *dev = uniphy->dev;
> +       struct clk_init_data init = { };
> +       int ret;
> +
> +       ret = of_property_read_string(np, "clock-output-names", &init.name);
> +       if (ret) {
> +               dev_err(dev, "%pOFn: No clock-output-names\n", np);
> +               return ret;
> +       }
> +
> +       init.ops = &clk_fixed_rate_ops;
> +
> +       fixed->fixed_rate = cfg->pipe_clk_rate;
> +       fixed->hw.init = &init;
>
> -static int ipq4019_usb_phy_probe(struct platform_device *pdev)
> +       ret = devm_clk_hw_register(dev, &fixed->hw);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> +       if (ret)
> +               return ret;

When you c&p something, please take care to understand the code you are copying.
Unlike QMP drivers you can (and should) use devm_of_clk_add_hw_provider() here.

Not to mention that pipe clocks are not in this patch.

> +
> +       /*
> +        * Roll a devm action because the clock provider is the child node, but
> +        * the child node is not actually a device.
> +        */
> +       return devm_add_action_or_reset(dev, phy_clk_release_provider, np);
> +}
> +
> +static int qcom_uniphy_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct phy_provider *phy_provider;
> -       struct ipq4019_usb_phy *phy;
> +       struct qcom_uniphy *uniphy;
> +       struct device_node *np;
> +       int ret;
>
> -       phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> -       if (!phy)
> +       uniphy = devm_kzalloc(dev, sizeof(*uniphy), GFP_KERNEL);
> +       if (!uniphy)
>                 return -ENOMEM;
>
> -       phy->dev = &pdev->dev;
> -       phy->base = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(phy->base)) {
> -               dev_err(dev, "failed to remap register memory\n");
> -               return PTR_ERR(phy->base);
> -       }
> +       uniphy->dev = dev;
>
> -       phy->por_rst = devm_reset_control_get(phy->dev, "por_rst");
> -       if (IS_ERR(phy->por_rst)) {
> -               if (PTR_ERR(phy->por_rst) != -EPROBE_DEFER)
> -                       dev_err(dev, "POR reset is missing\n");
> -               return PTR_ERR(phy->por_rst);
> +       uniphy->cfg = of_device_get_match_data(dev);
> +       if (!uniphy->cfg)
> +               return -EINVAL;
> +
> +       uniphy->base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(uniphy->base)) {
> +               ret = PTR_ERR(uniphy->base);
> +               dev_err_probe(dev, ret, "failed to remap register memory\n");
> +               return ret;
>         }
>
> -       phy->srif_rst = devm_reset_control_get_optional(phy->dev, "srif_rst");
> -       if (IS_ERR(phy->srif_rst))
> -               return PTR_ERR(phy->srif_rst);
> +       ret = qcom_uniphy_clk_init(uniphy);
> +       if (ret)
> +               return ret;
> +
> +       ret = qcom_uniphy_reset_init(uniphy);
> +       if (ret)
> +               return ret;
> +
> +       ret = qcom_uniphy_vreg_init(uniphy);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (uniphy->cfg->pipe_clk_rate) {
> +               np = of_node_get(dev->of_node);

What for? Do you think that the driver can outlive struct device?

> +               ret = phy_pipe_clk_register(uniphy, np);
> +               if (ret) {
> +                       dev_err_probe(dev, ret, "failed to register pipe clk\n");
> +                       goto err;
> +               }
> +       }
>
> -       phy->phy = devm_phy_create(dev, NULL, of_device_get_match_data(dev));
> -       if (IS_ERR(phy->phy)) {
> -               dev_err(dev, "failed to create PHY\n");
> -               return PTR_ERR(phy->phy);
> +       uniphy->phy = devm_phy_create(dev, NULL, &uniphy_phy_ops);
> +       if (IS_ERR(uniphy->phy)) {
> +               ret = PTR_ERR(uniphy->phy);
> +               dev_err_probe(dev, ret, "failed to create PHY\n");
> +               goto err;
>         }
> -       phy_set_drvdata(phy->phy, phy);
> +
> +       phy_set_drvdata(uniphy->phy, uniphy);
>
>         phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> -       return PTR_ERR_OR_ZERO(phy_provider);
> +       ret = PTR_ERR_OR_ZERO(phy_provider);
> +
> +err:
> +       if (uniphy->cfg->pipe_clk_rate)
> +               of_node_put(np);
> +       return ret;
>  }
>
> -static struct platform_driver ipq4019_usb_phy_driver = {
> -       .probe  = ipq4019_usb_phy_probe,
> +static const struct of_device_id qcom_uniphy_of_match[] = {
> +       { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
> +       { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
> +
> +static struct platform_driver qcom_uniphy_driver = {
> +       .probe  = qcom_uniphy_probe,
>         .driver = {
> -               .of_match_table = ipq4019_usb_phy_of_match,
> -               .name  = "ipq4019-usb-phy",
> +               .of_match_table = qcom_uniphy_of_match,
> +               .name  = "qcom-uniphy",
>         }
>  };
> -module_platform_driver(ipq4019_usb_phy_driver);
> +module_platform_driver(qcom_uniphy_driver);
>
> -MODULE_DESCRIPTION("QCOM/IPQ4019 USB phy driver");
> +MODULE_DESCRIPTION("QCOM uniphy driver");
>  MODULE_AUTHOR("John Crispin <john@phrozen.org>");
>  MODULE_LICENSE("GPL v2");

General comment: please consider dropping this beast and starting from
scratch, adding only really necessary bits to the existing ipq4019 USB
PHY driver.

-- 
With best wishes
Dmitry

Re: [PATCH 3/9] phy: qcom: uniphy: Update UNIPHY driver to be a common driver

[Praveenkumar I]

On 8/29/2023 8:07 PM, Dmitry Baryshkov wrote:
> On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>> This patch updates the UNIPHY driver to be a common driver to
>> accommodate all UNIPHY / Combo PHY. This driver can be used for
>> both USB and PCIe UNIPHY. Using phy-mul-sel from DTS MUX selection
>> for USB / PCIe can be acheived.
> I'm not sure why you are talking about PCIe here. This patch adds only
> SS PHY support.
>
> Also, I'd like to point out that we had this 'USB and PCIe and
> everything else' design in the QMP driver. We had to split the driver
> into individual pieces to make it manageable again.
This Uniphy 22ull is a combo PHY used between USB GEN3 or PCIe GEN3. Via 
mux selection
the one of the controller interface can use it. Hence thought to have a 
same PHY driver for
both USB3 and PCIe. Will drop the plan of adding the PCIe support.
>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-uniphy.c | 401 +++++++++++++++++++++----
>>   1 file changed, 335 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
>> index da6f290af722..eb71588f5417 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
>> @@ -5,141 +5,410 @@
>>    * Based on code from
>>    * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
>>    *
>> + * Modified the driver to be common for Qualcomm UNIPHYs
>> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> I'd not call this 'modified', but rather 'rewritten from scratch.
Sure, will correct.
>
>>    */
>>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>   #include <linux/delay.h>
>>   #include <linux/err.h>
>>   #include <linux/io.h>
>>   #include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>>   #include <linux/of.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>>   #include <linux/reset.h>
>>
>> -struct ipq4019_usb_phy {
>> +struct uniphy_init_tbl {
>> +       unsigned int offset;
>> +       unsigned int val;
>> +};
> unused
>
>> +
>> +#define UNIPHY_INIT_CFG(o, v)          \
>> +       {                               \
>> +               .offset = o,            \
>> +               .val = v,               \
>> +       }
> unused
>
>> +
>> +struct uniphy_cfg {
>> +       const struct uniphy_init_tbl *init_seq;
>> +       int num_init_seq;
>> +       const char * const *clk_list;
>> +       int num_clks;
>> +       const char * const *reset_list;
>> +       int num_resets;
>> +       const char * const *vreg_list;
>> +       int num_vregs;
>> +       unsigned int pipe_clk_rate;
>> +       unsigned int reset_udelay;
>> +       unsigned int autoload_udelay;
>> +};
>> +
>> +struct qcom_uniphy {
>>          struct device           *dev;
>> +       const struct uniphy_cfg *cfg;
>>          struct phy              *phy;
>>          void __iomem            *base;
>> -       struct reset_control    *por_rst;
>> -       struct reset_control    *srif_rst;
>> +       struct clk_bulk_data    *clks;
>> +       struct reset_control_bulk_data  *resets;
>> +       struct regulator_bulk_data *vregs;
>> +       struct clk_fixed_rate pipe_clk_fixed;
>> +};
>> +
>> +static const char * const ipq4019_ssphy_reset_l[] = {
>> +       "por_rst",
>> +};
>> +
>> +static const struct uniphy_cfg ipq4019_usb_ssphy_cfg = {
>> +       .reset_list     = ipq4019_ssphy_reset_l,
>> +       .num_resets     = ARRAY_SIZE(ipq4019_ssphy_reset_l),
>> +       .reset_udelay   = 10000,
>> +
>>   };
>>
>> -static int ipq4019_ss_phy_power_off(struct phy *_phy)
>> +static const char * const ipq4019_hsphy_reset_l[] = {
>> +       "por_rst", "srif_rst",
>> +};
> +
>> +static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
>> +       .reset_list     = ipq4019_hsphy_reset_l,
>> +       .num_resets     = ARRAY_SIZE(ipq4019_hsphy_reset_l),
>> +       .reset_udelay   = 10000,
>> +};
>> +
>> +static int phy_mux_sel(struct phy *phy)
>> +{
>> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
>> +       struct device *dev = uniphy->dev;
>> +       struct regmap *tcsr;
>> +       unsigned int args[2];
>> +       int ret;
>> +
>> +       tcsr = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,phy-mux-sel",
>> +                                                   ARRAY_SIZE(args), args);
> No. mux data should come from match_data rather than polluting DT with it.
>
>> +       if (IS_ERR(tcsr)) {
>> +               ret = PTR_ERR(tcsr);
>> +               if (ret == -ENOENT)
>> +                       return 0;
>> +
>> +               dev_err(dev, "failed to lookup syscon for phy mux %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* PHY MUX registers only have this BIT0 */
> huh?
>
>> +       ret = regmap_write(tcsr, args[0], args[1]);
>> +       if (ret < 0) {
>> +               dev_err(dev, "PHY Mux selection failed: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int uniphy_enable(struct phy *phy)
>>   {
>> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       const struct uniphy_init_tbl *tbl;
>> +       void __iomem *base = uniphy->base;
>> +       int i, ret;
>>
>> -       reset_control_assert(phy->por_rst);
>> -       msleep(10);
>> +       ret = regulator_bulk_enable(cfg->num_vregs, uniphy->vregs);
>> +       if (ret) {
>> +               dev_err(uniphy->dev, "failed to enable regulators: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* Assert all available resets */
>> +       for (i = 0; i < cfg->num_resets; i++) {
>> +               ret = reset_control_assert(uniphy->resets[i].rstc);
>> +               if (ret) {
>> +                       dev_err(uniphy->dev, "reset assert failed: %d\n", ret);
>> +                       goto err_assert_reset;
>> +               }
>> +               if (cfg->reset_udelay)
>> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
>> +       }
> There is a whole reset_control_bulk_*() set of API. Please use it
> instead of hardcoding reset cycles.
>
>> +
>> +       /* Deassert all available resets */
>> +       for (i = 0; i < cfg->num_resets; i++) {
>> +               ret = reset_control_deassert(uniphy->resets[i].rstc);
>> +               if (ret) {
>> +                       dev_err(uniphy->dev, "reset deassert failed: %d\n", ret);
>> +                       goto err_assert_reset;
>> +               }
>> +               if (cfg->reset_udelay)
>> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
>> +       }
>> +
>> +       ret = phy_mux_sel(phy);
>> +       if (ret < 0)
>> +               goto err_assert_reset;
>> +
>> +       ret = clk_bulk_prepare_enable(cfg->num_clks, uniphy->clks);
>> +       if (ret) {
>> +               dev_err(uniphy->dev, "failed to enable clocks: %d\n", ret);
>> +               goto err_assert_reset;
>> +       }
>> +
>> +       if (cfg->autoload_udelay)
>> +               usleep_range(cfg->autoload_udelay, cfg->autoload_udelay + 10);
>> +
>> +       if (cfg->num_init_seq) {
>> +               tbl = cfg->init_seq;
>> +               for (i = 0; i < cfg->num_init_seq; i++, tbl++)
>> +                       writel(tbl->val, base + tbl->offset);
>> +       }
> unused
>
>>          return 0;
>> +
>> +err_assert_reset:
>> +       /* Assert all available resets */
>> +       for (i = 0; i < cfg->num_resets; i++) {
>> +               reset_control_assert(uniphy->resets[i].rstc);
>> +               if (cfg->reset_udelay)
>> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
>> +       }
>> +
>> +       return ret;
>>   }
>>
>> -static int ipq4019_ss_phy_power_on(struct phy *_phy)
>> +static int uniphy_disable(struct phy *phy)
>>   {
>> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>> +       struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       int i;
>>
>> -       ipq4019_ss_phy_power_off(_phy);
>> +       /* Assert all available resets */
>> +       for (i = 0; i < cfg->num_resets; i++) {
>> +               reset_control_assert(uniphy->resets[i].rstc);
>> +               if (cfg->reset_udelay)
>> +                       usleep_range(cfg->reset_udelay, cfg->reset_udelay + 10);
>> +       }
>>
>> -       reset_control_deassert(phy->por_rst);
>> +       clk_bulk_disable_unprepare(cfg->num_clks, uniphy->clks);
>> +
>> +       regulator_bulk_disable(cfg->num_vregs, uniphy->vregs);
>>
>>          return 0;
>>   }
>>
>> -static const struct phy_ops ipq4019_usb_ss_phy_ops = {
>> -       .power_on       = ipq4019_ss_phy_power_on,
>> -       .power_off      = ipq4019_ss_phy_power_off,
>> +static const struct phy_ops uniphy_phy_ops = {
>> +       .power_on       = uniphy_enable,
>> +       .power_off      = uniphy_disable,
> Using _enable / _disable for power_on() and power_off() isn't logical.
>
>> +       .owner          = THIS_MODULE,
>>   };
>>
>> -static int ipq4019_hs_phy_power_off(struct phy *_phy)
>> +static int qcom_uniphy_vreg_init(struct qcom_uniphy *uniphy)
>> +{
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       struct device *dev = uniphy->dev;
>> +       int i, ret;
>> +
>> +       uniphy->vregs = devm_kcalloc(dev, cfg->num_vregs,
>> +                                    sizeof(*uniphy->vregs), GFP_KERNEL);
> You know the maximum amount of regulators. Can you use an array
> instead of allocating data?
>
>> +       if (!uniphy->vregs)
>> +               return -ENOMEM;
>> +
>> +       for (i = 0; i < cfg->num_vregs; i++)
>> +               uniphy->vregs[i].supply = cfg->vreg_list[i];
>> +
>> +       ret = devm_regulator_bulk_get(dev, cfg->num_vregs, uniphy->vregs);
>> +
> Drop empty lines between ret assignment and check.
>
>> +       if (ret)
>> +               return dev_err_probe(dev, ret, "failed to get regulators\n");
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_uniphy_reset_init(struct qcom_uniphy *uniphy)
>>   {
>> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       struct device *dev = uniphy->dev;
>> +       int i, ret;
>> +
>> +       uniphy->resets = devm_kcalloc(dev, cfg->num_resets,
>> +                                     sizeof(*uniphy->resets), GFP_KERNEL);
> Same here, can you use an array?
>
>> +       if (!uniphy->resets)
>> +               return -ENOMEM;
>>
>> -       reset_control_assert(phy->por_rst);
>> -       msleep(10);
>> +       for (i = 0; i < cfg->num_resets; i++)
>> +               uniphy->resets[i].id = cfg->reset_list[i];
> Declare common resets list and use
> devm_reset_control_bulk_get_optional_exclusive().
>
>> -       reset_control_assert(phy->srif_rst);
>> -       msleep(10);
>> +       ret = devm_reset_control_bulk_get_exclusive(dev, cfg->num_resets, uniphy->resets);
>> +       if (ret)
>> +               return dev_err_probe(dev, ret, "failed to get resets\n");
>>
>>          return 0;
>>   }
>>
>> -static int ipq4019_hs_phy_power_on(struct phy *_phy)
>> +static int qcom_uniphy_clk_init(struct qcom_uniphy *uniphy)
> I don't see clocks actually being used. Please do not introduce unused features.
>
>>   {
>> -       struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       struct device *dev = uniphy->dev;
>> +       int i, ret;
>>
>> -       ipq4019_hs_phy_power_off(_phy);
>>
>> -       reset_control_deassert(phy->srif_rst);
>> -       msleep(10);
>> +       uniphy->clks = devm_kcalloc(dev, cfg->num_clks,
>> +                                   sizeof(*uniphy->clks), GFP_KERNEL);
>> +       if (!uniphy->clks)
>> +               return -ENOMEM;
>> +
>> +       for (i = 0; i < cfg->num_clks; i++)
>> +               uniphy->clks[i].id = cfg->clk_list[i];
>>
>> -       reset_control_deassert(phy->por_rst);
>> +       ret = devm_clk_bulk_get(dev, cfg->num_clks, uniphy->clks);
>> +       if (ret)
>> +               return dev_err_probe(dev, ret, "failed to get clocks\n");
>>
>>          return 0;
>>   }
>>
>> -static const struct phy_ops ipq4019_usb_hs_phy_ops = {
>> -       .power_on       = ipq4019_hs_phy_power_on,
>> -       .power_off      = ipq4019_hs_phy_power_off,
>> -};
>> +static void phy_clk_release_provider(void *res)
>> +{
>> +       of_clk_del_provider(res);
>> +}
>>
>> -static const struct of_device_id ipq4019_usb_phy_of_match[] = {
>> -       { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hs_phy_ops},
>> -       { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ss_phy_ops},
>> -       { },
>> -};
>> -MODULE_DEVICE_TABLE(of, ipq4019_usb_phy_of_match);
>> +/*
>> + * Register a fixed rate pipe clock.
>> + *
>> + * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
>> + * controls it. The <s>_pipe_clk coming out of the GCC is requested
>> + * by the PHY driver for its operations.
>> + * We register the <s>_pipe_clksrc here. The gcc driver takes care
>> + * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
>> + * Below picture shows this relationship.
>> + *
>> + *         +---------------+
>> + *         |   PHY block   |<<---------------------------------------+
>> + *         |               |                                         |
>> + *         |   +-------+   |                   +-----+               |
>> + *   I/P---^-->|  PLL  |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
>> + *    clk  |   +-------+   |                   +-----+
>> + *         +---------------+
>> + */
>> +static int phy_pipe_clk_register(struct qcom_uniphy *uniphy, struct device_node *np)
>> +{
>> +       struct clk_fixed_rate *fixed = &uniphy->pipe_clk_fixed;
>> +       const struct uniphy_cfg *cfg = uniphy->cfg;
>> +       struct device *dev = uniphy->dev;
>> +       struct clk_init_data init = { };
>> +       int ret;
>> +
>> +       ret = of_property_read_string(np, "clock-output-names", &init.name);
>> +       if (ret) {
>> +               dev_err(dev, "%pOFn: No clock-output-names\n", np);
>> +               return ret;
>> +       }
>> +
>> +       init.ops = &clk_fixed_rate_ops;
>> +
>> +       fixed->fixed_rate = cfg->pipe_clk_rate;
>> +       fixed->hw.init = &init;
>>
>> -static int ipq4019_usb_phy_probe(struct platform_device *pdev)
>> +       ret = devm_clk_hw_register(dev, &fixed->hw);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
>> +       if (ret)
>> +               return ret;
> When you c&p something, please take care to understand the code you are copying.
> Unlike QMP drivers you can (and should) use devm_of_clk_add_hw_provider() here.
>
> Not to mention that pipe clocks are not in this patch.
>
>> +
>> +       /*
>> +        * Roll a devm action because the clock provider is the child node, but
>> +        * the child node is not actually a device.
>> +        */
>> +       return devm_add_action_or_reset(dev, phy_clk_release_provider, np);
>> +}
>> +
>> +static int qcom_uniphy_probe(struct platform_device *pdev)
>>   {
>>          struct device *dev = &pdev->dev;
>>          struct phy_provider *phy_provider;
>> -       struct ipq4019_usb_phy *phy;
>> +       struct qcom_uniphy *uniphy;
>> +       struct device_node *np;
>> +       int ret;
>>
>> -       phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>> -       if (!phy)
>> +       uniphy = devm_kzalloc(dev, sizeof(*uniphy), GFP_KERNEL);
>> +       if (!uniphy)
>>                  return -ENOMEM;
>>
>> -       phy->dev = &pdev->dev;
>> -       phy->base = devm_platform_ioremap_resource(pdev, 0);
>> -       if (IS_ERR(phy->base)) {
>> -               dev_err(dev, "failed to remap register memory\n");
>> -               return PTR_ERR(phy->base);
>> -       }
>> +       uniphy->dev = dev;
>>
>> -       phy->por_rst = devm_reset_control_get(phy->dev, "por_rst");
>> -       if (IS_ERR(phy->por_rst)) {
>> -               if (PTR_ERR(phy->por_rst) != -EPROBE_DEFER)
>> -                       dev_err(dev, "POR reset is missing\n");
>> -               return PTR_ERR(phy->por_rst);
>> +       uniphy->cfg = of_device_get_match_data(dev);
>> +       if (!uniphy->cfg)
>> +               return -EINVAL;
>> +
>> +       uniphy->base = devm_platform_ioremap_resource(pdev, 0);
>> +       if (IS_ERR(uniphy->base)) {
>> +               ret = PTR_ERR(uniphy->base);
>> +               dev_err_probe(dev, ret, "failed to remap register memory\n");
>> +               return ret;
>>          }
>>
>> -       phy->srif_rst = devm_reset_control_get_optional(phy->dev, "srif_rst");
>> -       if (IS_ERR(phy->srif_rst))
>> -               return PTR_ERR(phy->srif_rst);
>> +       ret = qcom_uniphy_clk_init(uniphy);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = qcom_uniphy_reset_init(uniphy);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = qcom_uniphy_vreg_init(uniphy);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       if (uniphy->cfg->pipe_clk_rate) {
>> +               np = of_node_get(dev->of_node);
> What for? Do you think that the driver can outlive struct device?
>
>> +               ret = phy_pipe_clk_register(uniphy, np);
>> +               if (ret) {
>> +                       dev_err_probe(dev, ret, "failed to register pipe clk\n");
>> +                       goto err;
>> +               }
>> +       }
>>
>> -       phy->phy = devm_phy_create(dev, NULL, of_device_get_match_data(dev));
>> -       if (IS_ERR(phy->phy)) {
>> -               dev_err(dev, "failed to create PHY\n");
>> -               return PTR_ERR(phy->phy);
>> +       uniphy->phy = devm_phy_create(dev, NULL, &uniphy_phy_ops);
>> +       if (IS_ERR(uniphy->phy)) {
>> +               ret = PTR_ERR(uniphy->phy);
>> +               dev_err_probe(dev, ret, "failed to create PHY\n");
>> +               goto err;
>>          }
>> -       phy_set_drvdata(phy->phy, phy);
>> +
>> +       phy_set_drvdata(uniphy->phy, uniphy);
>>
>>          phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>>
>> -       return PTR_ERR_OR_ZERO(phy_provider);
>> +       ret = PTR_ERR_OR_ZERO(phy_provider);
>> +
>> +err:
>> +       if (uniphy->cfg->pipe_clk_rate)
>> +               of_node_put(np);
>> +       return ret;
>>   }
>>
>> -static struct platform_driver ipq4019_usb_phy_driver = {
>> -       .probe  = ipq4019_usb_phy_probe,
>> +static const struct of_device_id qcom_uniphy_of_match[] = {
>> +       { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
>> +       { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
>> +
>> +static struct platform_driver qcom_uniphy_driver = {
>> +       .probe  = qcom_uniphy_probe,
>>          .driver = {
>> -               .of_match_table = ipq4019_usb_phy_of_match,
>> -               .name  = "ipq4019-usb-phy",
>> +               .of_match_table = qcom_uniphy_of_match,
>> +               .name  = "qcom-uniphy",
>>          }
>>   };
>> -module_platform_driver(ipq4019_usb_phy_driver);
>> +module_platform_driver(qcom_uniphy_driver);
>>
>> -MODULE_DESCRIPTION("QCOM/IPQ4019 USB phy driver");
>> +MODULE_DESCRIPTION("QCOM uniphy driver");
>>   MODULE_AUTHOR("John Crispin <john@phrozen.org>");
>>   MODULE_LICENSE("GPL v2");
> General comment: please consider dropping this beast and starting from
> scratch, adding only really necessary bits to the existing ipq4019 USB
> PHY driver.
Rewritten the entire driver considering the IPQ5332 as well and because 
of it
there are unused code which are utilized in patch [8/9]. I should have 
added only the IPQ4019
support alone in this patch and then incrementally update it for 
IPQ5332. Will follow that here
onwards. Will address all the review comments.

- Praveenkumar

Re: [PATCH 3/9] phy: qcom: uniphy: Update UNIPHY driver to be a common driver

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> This patch updates the UNIPHY driver to be a common driver to

Please do not use "This commit/patch", but imperative mood. See longer
explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> accommodate all UNIPHY / Combo PHY. This driver can be used for
> both USB and PCIe UNIPHY. Using phy-mul-sel from DTS MUX selection
> for USB / PCIe can be acheived.

This patch is entirely unreadable. You speak "unify" but change much
more. There is no code removal, so what are you unifying?

...

> -	phy->phy = devm_phy_create(dev, NULL, of_device_get_match_data(dev));
> -	if (IS_ERR(phy->phy)) {
> -		dev_err(dev, "failed to create PHY\n");
> -		return PTR_ERR(phy->phy);
> +	uniphy->phy = devm_phy_create(dev, NULL, &uniphy_phy_ops);

NAK, really, this does not make sense, is not explained and not needed.
If needed, then it would deserve its own patch with own justification.

> +	if (IS_ERR(uniphy->phy)) {
> +		ret = PTR_ERR(uniphy->phy);
> +		dev_err_probe(dev, ret, "failed to create PHY\n");

That's not even the syntax. By "unifying" you introduce different, wrong
code.

> +		goto err;
>  	}
> -	phy_set_drvdata(phy->phy, phy);
> +
> +	phy_set_drvdata(uniphy->phy, uniphy);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>  
> -	return PTR_ERR_OR_ZERO(phy_provider);
> +	ret = PTR_ERR_OR_ZERO(phy_provider);
> +
> +err:
> +	if (uniphy->cfg->pipe_clk_rate)
> +		of_node_put(np);
> +	return ret;
>  }
>  
> -static struct platform_driver ipq4019_usb_phy_driver = {
> -	.probe	= ipq4019_usb_phy_probe,
> +static const struct of_device_id qcom_uniphy_of_match[] = {
> +	{ .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
> +	{ .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);

What happens here?

Best regards,
Krzysztof

[PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Praveenkumar I]

Add ipq5332 USB3 SS UNIPHY support.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
 1 file changed, 114 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
index cbe2cc820009..17ba661b3d9b 100644
--- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
@@ -19,21 +19,53 @@ properties:
     enum:
       - qcom,usb-ss-ipq4019-phy
       - qcom,usb-hs-ipq4019-phy
+      - qcom,ipq5332-usb-ssphy
 
   reg:
     maxItems: 1
 
+  reg-names:
+    items:
+      - const: phy_base
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    maxItems: 3
+
+  "#clock-cells":
+    const: 0
+
   resets:
+    minItems: 1
     maxItems: 2
 
   reset-names:
-    items:
-      - const: por_rst
-      - const: srif_rst
+    minItems: 1
+    maxItems: 2
+
+  clock-output-names:
+    maxItems: 1
 
   "#phy-cells":
     const: 0
 
+  qcom,phy-mux-sel:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      PHY Mux Selection for used to select which interface is going to use the
+      combo PHY.
+    items:
+      - items:
+          - description: phandle to TCSR syscon region
+          - description: offset to the PHY Mux selection register
+          - description: value to write on the PHY Mux selection register
+
+  vdd-supply:
+    description:
+      Phandle to 5V regulator supply to PHY digital circuit.
+
 required:
   - compatible
   - reg
@@ -41,6 +73,68 @@ required:
   - reset-names
   - "#phy-cells"
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5332-usb-ssphy
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pipe
+            - const: phy_cfg_ahb
+            - const: phy_ahb
+
+        "#clock-cells":
+          const: 0
+
+        clock-output-names:
+          maxItems: 1
+
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: por_rst
+
+        vdda-supply:
+          description:
+            Phandle to 5V regulator supply to PHY digital circuit.
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,usb-ss-ipq4019-phy
+    then:
+      properties:
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: por_rst
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,usb-hs-ipq4019-phy
+    then:
+      properties:
+        resets:
+          maxItems: 2
+        reset-names:
+          items:
+            - const: por_rst
+            - const: srif_rst
+
 additionalProperties: false
 
 examples:
@@ -55,3 +149,20 @@ examples:
                <&gcc USB2_HSPHY_S_ARES>;
       reset-names = "por_rst", "srif_rst";
     };
+
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+
+    ssuniphy@4b0000 {
+      #phy-cells = <0>;
+      #clock-cells = <0>;
+      compatible = "qcom,ipq5332-usb-ssphy";
+      reg = <0x4b0000 0x800>;
+      clocks = <&gcc GCC_USB0_PIPE_CLK>,
+               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
+               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
+      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
+
+      resets = <&gcc GCC_USB0_PHY_BCR>;
+      reset-names = "por_rst";
+    };
-- 
2.34.1

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> Add ipq5332 USB3 SS UNIPHY support.
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>
>    reg:
>      maxItems: 1
>
> +  reg-names:
> +    items:
> +      - const: phy_base
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
>    resets:
> +    minItems: 1
>      maxItems: 2
>
>    reset-names:
> -    items:
> -      - const: por_rst
> -      - const: srif_rst
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-output-names:
> +    maxItems: 1
>
>    "#phy-cells":
>      const: 0
>
> +  qcom,phy-mux-sel:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      PHY Mux Selection for used to select which interface is going to use the
> +      combo PHY.
> +    items:
> +      - items:
> +          - description: phandle to TCSR syscon region
> +          - description: offset to the PHY Mux selection register
> +          - description: value to write on the PHY Mux selection register

Generally these values should be a part of the driver, since they are
specific to the particular SoC, rather than being different from
device to device.

> +
> +  vdd-supply:
> +    description:
> +      Phandle to 5V regulator supply to PHY digital circuit.
> +
>  required:
>    - compatible
>    - reg
> @@ -41,6 +73,68 @@ required:
>    - reset-names
>    - "#phy-cells"
>
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,ipq5332-usb-ssphy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pipe
> +            - const: phy_cfg_ahb
> +            - const: phy_ahb
> +
> +        "#clock-cells":
> +          const: 0
> +
> +        clock-output-names:
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +        vdda-supply:
> +          description:
> +            Phandle to 5V regulator supply to PHY digital circuit.
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-ss-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-hs-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: por_rst
> +            - const: srif_rst
> +
>  additionalProperties: false
>
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {
> +      #phy-cells = <0>;
> +      #clock-cells = <0>;
> +      compatible = "qcom,ipq5332-usb-ssphy";
> +      reg = <0x4b0000 0x800>;
> +      clocks = <&gcc GCC_USB0_PIPE_CLK>,
> +               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
> +      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
> +
> +      resets = <&gcc GCC_USB0_PHY_BCR>;
> +      reset-names = "por_rst";
> +    };
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Rob Herring]

On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
   19 | #define GCC_BLSP1_AHB_CLK                               10
      | 
In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:18:
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:40: note: this is the location of the previous definition
   40 | #define GCC_BLSP1_AHB_CLK                               21
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:20: warning: "GCC_BLSP1_QUP1_I2C_APPS_CLK" redefined
   20 | #define GCC_BLSP1_QUP1_I2C_APPS_CLK                     11
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:41: note: this is the location of the previous definition
   41 | #define GCC_BLSP1_QUP1_I2C_APPS_CLK                     22
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:21: warning: "GCC_BLSP1_QUP1_SPI_APPS_CLK" redefined
   21 | #define GCC_BLSP1_QUP1_SPI_APPS_CLK                     12
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:42: note: this is the location of the previous definition
   42 | #define GCC_BLSP1_QUP1_SPI_APPS_CLK                     23
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:23: warning: "GCC_BLSP1_QUP2_I2C_APPS_CLK" redefined
   23 | #define GCC_BLSP1_QUP2_I2C_APPS_CLK                     14
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:43: note: this is the location of the previous definition
   43 | #define GCC_BLSP1_QUP2_I2C_APPS_CLK                     24
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:24: warning: "GCC_BLSP1_QUP2_SPI_APPS_CLK" redefined
   24 | #define GCC_BLSP1_QUP2_SPI_APPS_CLK                     15
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:44: note: this is the location of the previous definition
   44 | #define GCC_BLSP1_QUP2_SPI_APPS_CLK                     25
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:30: warning: "GCC_BLSP1_UART1_APPS_CLK" redefined
   30 | #define GCC_BLSP1_UART1_APPS_CLK                        21
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:45: note: this is the location of the previous definition
   45 | #define GCC_BLSP1_UART1_APPS_CLK                        26
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:32: warning: "GCC_BLSP1_UART2_APPS_CLK" redefined
   32 | #define GCC_BLSP1_UART2_APPS_CLK                        23
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:46: note: this is the location of the previous definition
   46 | #define GCC_BLSP1_UART2_APPS_CLK                        27
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:42: warning: "GCC_GP1_CLK" redefined
   42 | #define GCC_GP1_CLK                                     33
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:48: note: this is the location of the previous definition
   48 | #define GCC_GP1_CLK                                     29
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:44: warning: "GCC_GP2_CLK" redefined
   44 | #define GCC_GP2_CLK                                     35
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:49: note: this is the location of the previous definition
   49 | #define GCC_GP2_CLK                                     30
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:98: warning: "GCC_PRNG_AHB_CLK" redefined
   98 | #define GCC_PRNG_AHB_CLK                                89
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:62: note: this is the location of the previous definition
   62 | #define GCC_PRNG_AHB_CLK                                43
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:117: warning: "GCC_QPIC_AHB_CLK" redefined
  117 | #define GCC_QPIC_AHB_CLK                                108
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:63: note: this is the location of the previous definition
   63 | #define GCC_QPIC_AHB_CLK                                44
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:118: warning: "GCC_QPIC_CLK" redefined
  118 | #define GCC_QPIC_CLK                                    109
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:64: note: this is the location of the previous definition
   64 | #define GCC_QPIC_CLK                                    45
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:122: warning: "GCC_SDCC1_AHB_CLK" redefined
  122 | #define GCC_SDCC1_AHB_CLK                               113
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:65: note: this is the location of the previous definition
   65 | #define GCC_SDCC1_AHB_CLK                               46
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:123: warning: "GCC_SDCC1_APPS_CLK" redefined
  123 | #define GCC_SDCC1_APPS_CLK                              114
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:66: note: this is the location of the previous definition
   66 | #define GCC_SDCC1_APPS_CLK                              47
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:191: warning: "GCC_BLSP1_BCR" redefined
  191 | #define GCC_BLSP1_BCR                                   8
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:126: note: this is the location of the previous definition
  126 | #define GCC_BLSP1_BCR                                   30
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:192: warning: "GCC_BLSP1_QUP1_BCR" redefined
  192 | #define GCC_BLSP1_QUP1_BCR                              9
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:127: note: this is the location of the previous definition
  127 | #define GCC_BLSP1_QUP1_BCR                              31
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:195: warning: "GCC_BLSP1_QUP2_BCR" redefined
  195 | #define GCC_BLSP1_QUP2_BCR                              12
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:129: note: this is the location of the previous definition
  129 | #define GCC_BLSP1_QUP2_BCR                              33
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:203: warning: "GCC_BLSP1_UART1_BCR" redefined
  203 | #define GCC_BLSP1_UART1_BCR                             20
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:128: note: this is the location of the previous definition
  128 | #define GCC_BLSP1_UART1_BCR                             32
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:205: warning: "GCC_BLSP1_UART2_BCR" redefined
  205 | #define GCC_BLSP1_UART2_BCR                             22
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:130: note: this is the location of the previous definition
  130 | #define GCC_BLSP1_UART2_BCR                             34
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:260: warning: "GCC_PCNOC_BCR" redefined
  260 | #define GCC_PCNOC_BCR                                   77
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:148: note: this is the location of the previous definition
  148 | #define GCC_PCNOC_BCR                                   52
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:263: warning: "GCC_PRNG_BCR" redefined
  263 | #define GCC_PRNG_BCR                                    80
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:135: note: this is the location of the previous definition
  135 | #define GCC_PRNG_BCR                                    39
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:274: warning: "GCC_QDSS_BCR" redefined
  274 | #define GCC_QDSS_BCR                                    91
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:165: note: this is the location of the previous definition
  165 | #define GCC_QDSS_BCR                                    69
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:290: warning: "GCC_QPIC_BCR" redefined
  290 | #define GCC_QPIC_BCR                                    107
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:141: note: this is the location of the previous definition
  141 | #define GCC_QPIC_BCR                                    45
      | 

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
MAINTAINERS: Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230829135818.2219438-5-quic_ipkumar@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Krzysztof Kozlowski]

On 29/08/2023 16:35, Rob Herring wrote:
> 
> On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
>> Add ipq5332 USB3 SS UNIPHY support.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>>  1 file changed, 114 insertions(+), 3 deletions(-)
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
> ./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
>    19 | #define GCC_BLSP1_AHB_CLK                               10
>       | 

So the only patch which actually needed dependency information did not
have it. All other patches have something, even defconfig (!). Confusing.

Best regards,
Krzysztof

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 20:09, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/08/2023 16:35, Rob Herring wrote:
> >
> > On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
> >> Add ipq5332 USB3 SS UNIPHY support.
> >>
> >> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >> ---
> >>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
> >>  1 file changed, 114 insertions(+), 3 deletions(-)
> >>
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
> > ./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
> >    19 | #define GCC_BLSP1_AHB_CLK                               10
> >       |
>
> So the only patch which actually needed dependency information did not
> have it. All other patches have something, even defconfig (!). Confusing.

Much simpler. This patch adds a second example to the schema. Both
examples include something-gcc.h. As both examples end up in the same
example.dts file, second include conflicts with the first one.

-- 
With best wishes
Dmitry

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Rob Herring]

On Tue, Aug 29, 2023 at 07:28:13PM +0530, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>  
>    reg:
>      maxItems: 1
>  
> +  reg-names:
> +    items:
> +      - const: phy_base
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
>    resets:
> +    minItems: 1
>      maxItems: 2
>  
>    reset-names:
> -    items:
> -      - const: por_rst
> -      - const: srif_rst

No need to remove this and duplicate the names multiple times. Just add 
'minItems: 1' here and then the if/then schemas only need either 
minItems or maxItems.

> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-output-names:
> +    maxItems: 1
>  
>    "#phy-cells":
>      const: 0
>  
> +  qcom,phy-mux-sel:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      PHY Mux Selection for used to select which interface is going to use the
> +      combo PHY.
> +    items:
> +      - items:
> +          - description: phandle to TCSR syscon region
> +          - description: offset to the PHY Mux selection register
> +          - description: value to write on the PHY Mux selection register
> +
> +  vdd-supply:
> +    description:
> +      Phandle to 5V regulator supply to PHY digital circuit.
> +
>  required:
>    - compatible
>    - reg
> @@ -41,6 +73,68 @@ required:
>    - reset-names
>    - "#phy-cells"
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,ipq5332-usb-ssphy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pipe
> +            - const: phy_cfg_ahb
> +            - const: phy_ahb

How do the other variants work without any clocks? Magic?

Define the names in the top level and then just set the number of items 
or disallow the property in the if/then schemas.

> +
> +        "#clock-cells":
> +          const: 0
> +
> +        clock-output-names:
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +        vdda-supply:
> +          description:
> +            Phandle to 5V regulator supply to PHY digital circuit.
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-ss-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-hs-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: por_rst
> +            - const: srif_rst
> +
>  additionalProperties: false
>  
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {
> +      #phy-cells = <0>;
> +      #clock-cells = <0>;
> +      compatible = "qcom,ipq5332-usb-ssphy";
> +      reg = <0x4b0000 0x800>;
> +      clocks = <&gcc GCC_USB0_PIPE_CLK>,
> +               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
> +      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
> +
> +      resets = <&gcc GCC_USB0_PHY_BCR>;
> +      reset-names = "por_rst";
> +    };
> -- 
> 2.34.1
> 

Re: [PATCH 4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>  
>    reg:
>      maxItems: 1
>  
> +  reg-names:
> +    items:
> +      - const: phy_base

Why do you need it?

> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
...
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



Best regards,
Krzysztof

[PATCH 5/9] dt-bindings: usb: dwc3: Update IPQ5332 compatible

[Praveenkumar I]

IPQ5332 USB supports both USB2 and USB3. Updated the USB clocks
for the same.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 .../devicetree/bindings/usb/qcom,dwc3.yaml    | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 67591057f234..18af2887b984 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -149,6 +149,25 @@ allOf:
             - const: sleep
             - const: mock_utmi
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5332-dwc3
+    then:
+      properties:
+        clocks:
+          maxItems: 6
+        clock-names:
+          items:
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+            - const: aux
+            - const: lfps
+
   - if:
       properties:
         compatible:
@@ -238,7 +257,6 @@ allOf:
         compatible:
           contains:
             enum:
-              - qcom,ipq5332-dwc3
               - qcom,msm8994-dwc3
               - qcom,qcs404-dwc3
     then:
-- 
2.34.1

Re: [PATCH 5/9] dt-bindings: usb: dwc3: Update IPQ5332 compatible

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> IPQ5332 USB supports both USB2 and USB3. Updated the USB clocks
> for the same.

Subject: everything is an update. You are not updating compatible, but
adding clocks. Describe your changes properly.

> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>

I think I repeat this third time to Qualcomm this week. Split unrelated
patches touching other subsystems to separate patchsets. Please share it
with your colleagues, so there won't be a need to repeat it fourth time.

> ---
>  .../devicetree/bindings/usb/qcom,dwc3.yaml    | 20 ++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> '


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

Best regards,
Krzysztof

[PATCH 6/9] arm64: dts: qcom: ipq5332: Add USB3 related nodes

[Praveenkumar I]

Add SS UNIPHY and update controller node for USB3.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
This patch depends on the below series which adds support for USB2 in
IPQ5332
https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/

 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 39 ++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index e6baf694488c..7fbe6c9f4784 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -158,6 +158,27 @@ usbphy0: phy@7b000 {
 			status = "disabled";
 		};
 
+		ssuniphy0: ssuniphy@4b0000 {
+			compatible = "qcom,ipq5332-usb-ssphy";
+			reg = <0x4b0000 0x800>;
+			clocks = <&gcc GCC_USB0_PIPE_CLK>,
+				 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
+				 <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
+
+			#clock-cells = <0>;
+			clock-output-names = "usb_pcie_wrapper_pipe_clk";
+
+			clock-names = "pipe",
+				      "phy_cfg_ahb",
+				      "phy_ahb";
+
+			resets =  <&gcc GCC_USB0_PHY_BCR>;
+			reset-names = "por_rst";
+			#phy-cells = <0>;
+			qcom,phy-mux-sel = <&tcsr 0x10540 0x1>;
+			status = "disabled";
+		};
+
 		qfprom: efuse@a4000 {
 			compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
 			reg = <0x000a4000 0x721>;
@@ -313,30 +334,34 @@ usb: usb@8a00000 {
 			clocks = <&gcc GCC_USB0_MASTER_CLK>,
 				 <&gcc GCC_SNOC_USB_CLK>,
 				 <&gcc GCC_USB0_SLEEP_CLK>,
-				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+				 <&gcc GCC_USB0_MOCK_UTMI_CLK>,
+				 <&gcc GCC_USB0_AUX_CLK>,
+				 <&gcc GCC_USB0_LFPS_CLK>;
+
 			clock-names = "core",
 				      "iface",
 				      "sleep",
-				      "mock_utmi";
+				      "mock_utmi",
+				      "aux",
+				      "lfps";
 
 			resets = <&gcc GCC_USB_BCR>;
 
-			qcom,select-utmi-as-pipe-clk;
-
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
 
 			status = "disabled";
 
-			usb2_0_dwc: usb@8a00000 {
+			usb3_0_dwc: usb@8a00000 {
 				compatible = "snps,dwc3";
 				reg = <0x08a00000 0xe000>;
 				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
 				clock-names = "ref";
 				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
-				phy-names = "usb2-phy";
-				phys = <&usbphy0>;
+				phy-names = "usb2-phy", "usb3-phy";
+				phys = <&usbphy0>, <&ssuniphy0>;
+
 				tx-fifo-resize;
 				snps,is-utmi-l1-suspend;
 				snps,hird-threshold = /bits/ 8 <0x0>;
-- 
2.34.1

Re: [PATCH 6/9] arm64: dts: qcom: ipq5332: Add USB3 related nodes

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> Add SS UNIPHY and update controller node for USB3.
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
>
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 39 ++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index e6baf694488c..7fbe6c9f4784 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -158,6 +158,27 @@ usbphy0: phy@7b000 {
>                         status = "disabled";
>                 };
>
> +               ssuniphy0: ssuniphy@4b0000 {
> +                       compatible = "qcom,ipq5332-usb-ssphy";
> +                       reg = <0x4b0000 0x800>;
> +                       clocks = <&gcc GCC_USB0_PIPE_CLK>,
> +                                <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +                                <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
> +
> +                       #clock-cells = <0>;
> +                       clock-output-names = "usb_pcie_wrapper_pipe_clk";
> +
> +                       clock-names = "pipe",
> +                                     "phy_cfg_ahb",
> +                                     "phy_ahb";
> +
> +                       resets =  <&gcc GCC_USB0_PHY_BCR>;
> +                       reset-names = "por_rst";
> +                       #phy-cells = <0>;
> +                       qcom,phy-mux-sel = <&tcsr 0x10540 0x1>;
> +                       status = "disabled";
> +               };
> +
>                 qfprom: efuse@a4000 {
>                         compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
>                         reg = <0x000a4000 0x721>;
> @@ -313,30 +334,34 @@ usb: usb@8a00000 {
>                         clocks = <&gcc GCC_USB0_MASTER_CLK>,
>                                  <&gcc GCC_SNOC_USB_CLK>,
>                                  <&gcc GCC_USB0_SLEEP_CLK>,
> -                                <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> +                                <&gcc GCC_USB0_MOCK_UTMI_CLK>,
> +                                <&gcc GCC_USB0_AUX_CLK>,
> +                                <&gcc GCC_USB0_LFPS_CLK>;
> +
>                         clock-names = "core",
>                                       "iface",
>                                       "sleep",
> -                                     "mock_utmi";
> +                                     "mock_utmi",
> +                                     "aux",
> +                                     "lfps";
>
>                         resets = <&gcc GCC_USB_BCR>;
>
> -                       qcom,select-utmi-as-pipe-clk;
> -
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
>
>                         status = "disabled";
>
> -                       usb2_0_dwc: usb@8a00000 {
> +                       usb3_0_dwc: usb@8a00000 {

At this point you have broken compilation of all ipq5332 DT files.
Don't. The kernel should be bisectable. At each point, after each
commit it should compile and work.

>                                 compatible = "snps,dwc3";
>                                 reg = <0x08a00000 0xe000>;
>                                 clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
>                                 clock-names = "ref";
>                                 interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> -                               phy-names = "usb2-phy";
> -                               phys = <&usbphy0>;
> +                               phy-names = "usb2-phy", "usb3-phy";
> +                               phys = <&usbphy0>, <&ssuniphy0>;
> +
>                                 tx-fifo-resize;
>                                 snps,is-utmi-l1-suspend;
>                                 snps,hird-threshold = /bits/ 8 <0x0>;
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 6/9] arm64: dts: qcom: ipq5332: Add USB3 related nodes

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> Add SS UNIPHY and update controller node for USB3.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
> 
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 39 ++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index e6baf694488c..7fbe6c9f4784 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -158,6 +158,27 @@ usbphy0: phy@7b000 {
>  			status = "disabled";
>  		};
>  
> +		ssuniphy0: ssuniphy@4b0000 {

From where did you get such pattern of node naming? Downstream, right?
Please, do not work on downstream but upstream.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



Best regards,
Krzysztof

[PATCH 7/9] arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY

[Praveenkumar I]

Enable USB3 SS UNIPHY and update USB node name.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
This patch depends on the below series which adds support for USB2 in
IPQ5332
https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/

 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
index 53696f4b46fc..c450153cfaac 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
@@ -95,10 +95,15 @@ &usbphy0 {
 	status = "okay";
 };
 
+&ssuniphy0 {
+	vdd-supply = <&regulator_fixed_5p0>;
+	status = "okay";
+};
+
 &usb {
 	status = "okay";
 };
 
-&usb2_0_dwc {
+&usb3_0_dwc {
 	dr_mode = "host";
 };
-- 
2.34.1

Re: [PATCH 7/9] arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> Enable USB3 SS UNIPHY and update USB node name.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
> 
>  arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> index 53696f4b46fc..c450153cfaac 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> @@ -95,10 +95,15 @@ &usbphy0 {
>  	status = "okay";
>  };
>  
> +&ssuniphy0 {
> +	vdd-supply = <&regulator_fixed_5p0>;
> +	status = "okay";
> +};
> +
>  &usb {
>  	status = "okay";
>  };
>  
> -&usb2_0_dwc {
> +&usb3_0_dwc {

This means previous patch was not even built. Sorry, that's bad. Please
test your commits before sending.

Best regards,
Krzysztof

[PATCH 8/9] phy: qcom: uniphy: Add ipq5332 USB UNIPHY support

[Praveenkumar I]

This patch adds ipq5332 USB SS UNIPHY support.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
This patch depends on the below series which adds support for USB2 in
IPQ5332
https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/

 drivers/phy/qualcomm/phy-qcom-uniphy.c | 37 ++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
index eb71588f5417..91487e68bb6e 100644
--- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
+++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
@@ -26,6 +26,10 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
+#define PCIE_USB_COMBO_PHY_CFG_MISC1		0x214
+#define PCIE_USB_COMBO_PHY_CFG_RX_AFE_2		0x7C4
+#define PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2	0x7E8
+
 struct uniphy_init_tbl {
 	unsigned int offset;
 	unsigned int val;
@@ -37,6 +41,12 @@ struct uniphy_init_tbl {
 		.val = v,		\
 	}
 
+static const struct uniphy_init_tbl ipq5332_usb_ssphy_init_tbl[] = {
+	UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_AFE_2, 0x1076),
+	UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2, 0x3142),
+	UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_MISC1, 0x3),
+};
+
 struct uniphy_cfg {
 	const struct uniphy_init_tbl *init_seq;
 	int num_init_seq;
@@ -83,6 +93,32 @@ static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
 	.reset_udelay	= 10000,
 };
 
+static const char * const ipq5332_usb_ssphy_clk_l[] = {
+	"phy_ahb", "phy_cfg_ahb", "pipe",
+};
+
+static const char * const ipq5332_usb_ssphy_reset_l[] = {
+	"por_rst",
+};
+
+static const char * const ipq5332_usb_ssphy_vreg_l[] = {
+	"vdda-phy",
+};
+
+static const struct uniphy_cfg ipq5332_usb_ssphy_cfg = {
+	.init_seq	= ipq5332_usb_ssphy_init_tbl,
+	.num_init_seq	= ARRAY_SIZE(ipq5332_usb_ssphy_init_tbl),
+	.clk_list	= ipq5332_usb_ssphy_clk_l,
+	.num_clks	= ARRAY_SIZE(ipq5332_usb_ssphy_clk_l),
+	.reset_list	= ipq5332_usb_ssphy_reset_l,
+	.num_resets	= ARRAY_SIZE(ipq5332_usb_ssphy_reset_l),
+	.vreg_list	= ipq5332_usb_ssphy_vreg_l,
+	.num_vregs	= ARRAY_SIZE(ipq5332_usb_ssphy_vreg_l),
+	.pipe_clk_rate	= 250000000,
+	.reset_udelay	= 1,
+	.autoload_udelay = 35,
+};
+
 static int phy_mux_sel(struct phy *phy)
 {
 	struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
@@ -396,6 +432,7 @@ static int qcom_uniphy_probe(struct platform_device *pdev)
 static const struct of_device_id qcom_uniphy_of_match[] = {
 	{ .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
 	{ .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
+	{ .compatible = "qcom,ipq5332-usb-ssphy", .data = &ipq5332_usb_ssphy_cfg},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
-- 
2.34.1

Re: [PATCH 8/9] phy: qcom: uniphy: Add ipq5332 USB UNIPHY support

[Dmitry Baryshkov]

On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> This patch adds ipq5332 USB SS UNIPHY support.

First, please read to Documentation/process/submitting-patches.rst,
then rewrite the commit message.

Next, I tend to say that this driver doesn't have a lot in common with
the ipq4019 driver you have modified. Please consider adding new
driver for ipq5332, then we can see whether it makes sense to fold
ipq4019 to use new infrastructure.

>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
>
>  drivers/phy/qualcomm/phy-qcom-uniphy.c | 37 ++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> index eb71588f5417..91487e68bb6e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> @@ -26,6 +26,10 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>
> +#define PCIE_USB_COMBO_PHY_CFG_MISC1           0x214
> +#define PCIE_USB_COMBO_PHY_CFG_RX_AFE_2                0x7C4
> +#define PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2  0x7E8
> +
>  struct uniphy_init_tbl {
>         unsigned int offset;
>         unsigned int val;
> @@ -37,6 +41,12 @@ struct uniphy_init_tbl {
>                 .val = v,               \
>         }
>
> +static const struct uniphy_init_tbl ipq5332_usb_ssphy_init_tbl[] = {
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_AFE_2, 0x1076),
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2, 0x3142),
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_MISC1, 0x3),
> +};
> +
>  struct uniphy_cfg {
>         const struct uniphy_init_tbl *init_seq;
>         int num_init_seq;
> @@ -83,6 +93,32 @@ static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
>         .reset_udelay   = 10000,
>  };
>
> +static const char * const ipq5332_usb_ssphy_clk_l[] = {
> +       "phy_ahb", "phy_cfg_ahb", "pipe",
> +};
> +
> +static const char * const ipq5332_usb_ssphy_reset_l[] = {
> +       "por_rst",
> +};
> +
> +static const char * const ipq5332_usb_ssphy_vreg_l[] = {
> +       "vdda-phy",
> +};
> +
> +static const struct uniphy_cfg ipq5332_usb_ssphy_cfg = {
> +       .init_seq       = ipq5332_usb_ssphy_init_tbl,
> +       .num_init_seq   = ARRAY_SIZE(ipq5332_usb_ssphy_init_tbl),
> +       .clk_list       = ipq5332_usb_ssphy_clk_l,
> +       .num_clks       = ARRAY_SIZE(ipq5332_usb_ssphy_clk_l),
> +       .reset_list     = ipq5332_usb_ssphy_reset_l,
> +       .num_resets     = ARRAY_SIZE(ipq5332_usb_ssphy_reset_l),
> +       .vreg_list      = ipq5332_usb_ssphy_vreg_l,
> +       .num_vregs      = ARRAY_SIZE(ipq5332_usb_ssphy_vreg_l),
> +       .pipe_clk_rate  = 250000000,
> +       .reset_udelay   = 1,
> +       .autoload_udelay = 35,
> +};
> +
>  static int phy_mux_sel(struct phy *phy)
>  {
>         struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
> @@ -396,6 +432,7 @@ static int qcom_uniphy_probe(struct platform_device *pdev)
>  static const struct of_device_id qcom_uniphy_of_match[] = {
>         { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
>         { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
> +       { .compatible = "qcom,ipq5332-usb-ssphy", .data = &ipq5332_usb_ssphy_cfg},
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
> --
> 2.34.1


-- 
With best wishes
Dmitry

Re: [PATCH 8/9] phy: qcom: uniphy: Add ipq5332 USB UNIPHY support

[Dmitry Baryshkov]

On Thu, 31 Aug 2023 at 15:13, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
>
> On 8/29/2023 8:15 PM, Dmitry Baryshkov wrote:
>
> On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> This patch adds ipq5332 USB SS UNIPHY support.
>
> First, please read to Documentation/process/submitting-patches.rst,
> then rewrite the commit message.
>
> Next, I tend to say that this driver doesn't have a lot in common with
> the ipq4019 driver you have modified. Please consider adding new
> driver for ipq5332, then we can see whether it makes sense to fold
> ipq4019 to use new infrastructure.
>
> Sure, will add new driver for IPQ5332 USB3 PHY. Thanks a lot for the review.

No HTML mail please. Ever. And use proper quotation. Thank you.

>
> --
> Thanks,
> Praveenkumar
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
>
>  drivers/phy/qualcomm/phy-qcom-uniphy.c | 37 ++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.c b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> index eb71588f5417..91487e68bb6e 100644
> --- a/drivers/phy/qualcomm/phy-qcom-uniphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.c
> @@ -26,6 +26,10 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>
> +#define PCIE_USB_COMBO_PHY_CFG_MISC1           0x214
> +#define PCIE_USB_COMBO_PHY_CFG_RX_AFE_2                0x7C4
> +#define PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2  0x7E8
> +
>  struct uniphy_init_tbl {
>         unsigned int offset;
>         unsigned int val;
> @@ -37,6 +41,12 @@ struct uniphy_init_tbl {
>                 .val = v,               \
>         }
>
> +static const struct uniphy_init_tbl ipq5332_usb_ssphy_init_tbl[] = {
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_AFE_2, 0x1076),
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2, 0x3142),
> +       UNIPHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_MISC1, 0x3),
> +};
> +
>  struct uniphy_cfg {
>         const struct uniphy_init_tbl *init_seq;
>         int num_init_seq;
> @@ -83,6 +93,32 @@ static const struct uniphy_cfg ipq4019_usb_hsphy_cfg = {
>         .reset_udelay   = 10000,
>  };
>
> +static const char * const ipq5332_usb_ssphy_clk_l[] = {
> +       "phy_ahb", "phy_cfg_ahb", "pipe",
> +};
> +
> +static const char * const ipq5332_usb_ssphy_reset_l[] = {
> +       "por_rst",
> +};
> +
> +static const char * const ipq5332_usb_ssphy_vreg_l[] = {
> +       "vdda-phy",
> +};
> +
> +static const struct uniphy_cfg ipq5332_usb_ssphy_cfg = {
> +       .init_seq       = ipq5332_usb_ssphy_init_tbl,
> +       .num_init_seq   = ARRAY_SIZE(ipq5332_usb_ssphy_init_tbl),
> +       .clk_list       = ipq5332_usb_ssphy_clk_l,
> +       .num_clks       = ARRAY_SIZE(ipq5332_usb_ssphy_clk_l),
> +       .reset_list     = ipq5332_usb_ssphy_reset_l,
> +       .num_resets     = ARRAY_SIZE(ipq5332_usb_ssphy_reset_l),
> +       .vreg_list      = ipq5332_usb_ssphy_vreg_l,
> +       .num_vregs      = ARRAY_SIZE(ipq5332_usb_ssphy_vreg_l),
> +       .pipe_clk_rate  = 250000000,
> +       .reset_udelay   = 1,
> +       .autoload_udelay = 35,
> +};
> +
>  static int phy_mux_sel(struct phy *phy)
>  {
>         struct qcom_uniphy *uniphy = phy_get_drvdata(phy);
> @@ -396,6 +432,7 @@ static int qcom_uniphy_probe(struct platform_device *pdev)
>  static const struct of_device_id qcom_uniphy_of_match[] = {
>         { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hsphy_cfg},
>         { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ssphy_cfg},
> +       { .compatible = "qcom,ipq5332-usb-ssphy", .data = &ipq5332_usb_ssphy_cfg},
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, qcom_uniphy_of_match);
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

[PATCH 9/9] arm64: defconfig: Enable UNIPHY driver

[Praveenkumar I]

Enable UNIPHY driver for IPQ5322.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
This patch depends on the below series which adds support for USB2 in
IPQ5332
https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c1e2372e6319..47c7f3d242fe 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1417,6 +1417,7 @@ CONFIG_PHY_QCOM_PCIE2=m
 CONFIG_PHY_QCOM_QMP=m
 CONFIG_PHY_QCOM_QUSB2=m
 CONFIG_PHY_QCOM_SNPS_EUSB2=m
+CONFIG_PHY_QCOM_UNIPHY=m
 CONFIG_PHY_QCOM_USB_HS=m
 CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=m
 CONFIG_PHY_QCOM_USB_HS_28NM=m
-- 
2.34.1

Re: [PATCH 9/9] arm64: defconfig: Enable UNIPHY driver

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> Enable UNIPHY driver for IPQ5322.

This we see from the diff. You *must* say *why*, not *what*.

Samsung IPQ5322 or NXP IPQ5322? Which boards need it?

> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> This patch depends on the below series which adds support for USB2 in
> IPQ5332
> https://lore.kernel.org/all/cover.1692699472.git.quic_varada@quicinc.com/
> 

How does it depend? Obviously it is part of the series, but what is
depending here?

Best regards,
Krzysztof

Re: [PATCH 0/9] Enable USB3 for IPQ5332

[Krzysztof Kozlowski]

On 29/08/2023 15:58, Praveenkumar I wrote:
> IPQ5332 has UNIPHY for USB and PCIe which is similar to the UNIPHY
> present in IPQ4019. Few extra settings like clock, reset delay, mux
> selection and voltage regulator are required for IPQ5332. Hence
> repurposed the IPQ4019 PHY driver for IPQ5332 UNIPHY. Few more Qualcomm
> SoCs are also having the UNIPHY which can use the same driver for both
> USB and PCIe PHY.
> 
> Praveenkumar I (9):
>   dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY
>   phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver
>   phy: qcom: uniphy: Update UNIPHY driver to be a common driver
>   dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY
>   dt-bindings: usb: dwc3: Update IPQ5332 compatible
>   arm64: dts: qcom: ipq5332: Add USB3 related nodes
>   arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY

DTS does not go before drivers. DTS should be sent separately or as the
last patches. If you stuff it in the middle, means your patchset has
dependencies which it cannot have. Thus it is broken.

Best regards,
Krzysztof

Re: [PATCH 0/9] Enable USB3 for IPQ5332

[Praveenkumar I]

On 8/29/2023 10:37 PM, Krzysztof Kozlowski wrote:
> On 29/08/2023 15:58, Praveenkumar I wrote:
>> IPQ5332 has UNIPHY for USB and PCIe which is similar to the UNIPHY
>> present in IPQ4019. Few extra settings like clock, reset delay, mux
>> selection and voltage regulator are required for IPQ5332. Hence
>> repurposed the IPQ4019 PHY driver for IPQ5332 UNIPHY. Few more Qualcomm
>> SoCs are also having the UNIPHY which can use the same driver for both
>> USB and PCIe PHY.
>>
>> Praveenkumar I (9):
>>    dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY
>>    phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver
>>    phy: qcom: uniphy: Update UNIPHY driver to be a common driver
>>    dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY
>>    dt-bindings: usb: dwc3: Update IPQ5332 compatible
>>    arm64: dts: qcom: ipq5332: Add USB3 related nodes
>>    arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY
> DTS does not go before drivers. DTS should be sent separately or as the
> last patches. If you stuff it in the middle, means your patchset has
> dependencies which it cannot have. Thus it is broken.

Sorry, I ordered it wrongly. Will correct in the next patches.

-  Praveenkumar

>
> Best regards,
> Krzysztof
>

Re: [PATCH 0/9] Enable USB3 for IPQ5332

[Praveenkumar I]

On 8/29/2023 10:37 PM, Krzysztof Kozlowski wrote:
> On 29/08/2023 15:58, Praveenkumar I wrote:
>> IPQ5332 has UNIPHY for USB and PCIe which is similar to the UNIPHY
>> present in IPQ4019. Few extra settings like clock, reset delay, mux
>> selection and voltage regulator are required for IPQ5332. Hence
>> repurposed the IPQ4019 PHY driver for IPQ5332 UNIPHY. Few more Qualcomm
>> SoCs are also having the UNIPHY which can use the same driver for both
>> USB and PCIe PHY.
>>
>> Praveenkumar I (9):
>>    dt-bindings: phy: qcom,uniphy: Rename ipq4019 usb PHY to UNIPHY
>>    phy: qcom: uniphy: Rename ipq4019 USB phy driver to UNIPHY driver
>>    phy: qcom: uniphy: Update UNIPHY driver to be a common driver
>>    dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY
>>    dt-bindings: usb: dwc3: Update IPQ5332 compatible
>>    arm64: dts: qcom: ipq5332: Add USB3 related nodes
>>    arm64: dts: qcom: ipq5332: Enable USB SS UNIPHY
> DTS does not go before drivers. DTS should be sent separately or as the
> last patches. If you stuff it in the middle, means your patchset has
> dependencies which it cannot have. Thus it is broken.
Sorry, I ordered it wrongly. Will correct in the next patches.

- Praveenkumar
>
> Best regards,
> Krzysztof
>