mbox series

[v2,0/5] Add USB Support on Qualcomm's SDX75 Platform

Message ID 1693909838-6682-1-git-send-email-quic_rohiagar@quicinc.com
Headers show
Series Add USB Support on Qualcomm's SDX75 Platform | expand

Message

Rohit Agarwal Sept. 5, 2023, 10:30 a.m. UTC 
Hi,

Changes in v2:
 - Dropped the new dt schema introduced in v1 for sdx75 usb3 phy
   and reusing the bindings.
 - Rephrased the commit message of patch 3/5
 - Removed stray lines from the patch 5/5

This series adds support of USB3 PHY support for Qualcomm's SDX75 Platform.

Thanks,
Rohit.

Rohit Agarwal (5):
  dt-bindings: phy: qcom,snps-eusb2-phy: Add compatible for SDX75
  dt-bindings: phy: qcom,qmp-usb: Add SDX75 USB3 PHY
  dt-bindings: usb: qcom,dwc3: Fix SDX65 clocks
  dt-bindings: usb: dwc3: Add SDX75 compatible
  phy: qcom-qmp-usb: Add Qualcomm SDX75 USB3 PHY support

 .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml        |  40 +++++-
 .../bindings/phy/qcom,snps-eusb2-phy.yaml          |   7 +-
 .../devicetree/bindings/usb/qcom,dwc3.yaml         |   4 +
 drivers/phy/qualcomm/phy-qcom-qmp-usb.c            | 158 +++++++++++++++++++++
 4 files changed, 205 insertions(+), 4 deletions(-)

Comments

Rohit Agarwal Sept. 6, 2023, 5:22 a.m. UTC | #1 
On 9/6/2023 2:04 AM, Dmitry Baryshkov wrote:
> On 05/09/2023 13:30, Rohit Agarwal wrote:
>> Add dt-bindings for USB3 PHY found on Qualcomm SDX75.
>>
>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
>> ---
>>   .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml        | 40 
>> ++++++++++++++++++++--
>>   1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml 
>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml 
>>
>> index f99fbbc..5725620 100644
>> --- 
>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
>> +++ 
>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
>> @@ -20,6 +20,7 @@ properties:
>>         - qcom,qcm2290-qmp-usb3-phy
>>         - qcom,sa8775p-qmp-usb3-uni-phy
>>         - qcom,sc8280xp-qmp-usb3-uni-phy
>> +      - qcom,sdx75-qmp-usb3-uni-phy
>
> I think the ident is wrong here.

Same. Somehow, your reply has the issue but original not able to see.

>
>>         - qcom,sm6115-qmp-usb3-phy
>>       reg:
>> @@ -38,9 +39,7 @@ properties:
>>       maxItems: 2
>>       reset-names:
>> -    items:
>> -      - const: phy
>> -      - const: phy_phy
>> +    maxItems: 2
>>       vdda-phy-supply: true
>>   @@ -75,6 +74,7 @@ allOf:
>>             contains:
>>               enum:
>>                 - qcom,ipq9574-qmp-usb3-phy
>> +              - qcom,sdx75-qmp-usb3-uni-phy
>>       then:
>>         properties:
>>           clock-names:
>> @@ -122,6 +122,40 @@ allOf:
>>         required:
>>           - power-domains
>>   +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,ipq9574-qmp-usb3-phy
>> +              - qcom,qcm2290-qmp-usb3-phy
>> +              - qcom,sa8775p-qmp-usb3-uni-phy
>> +              - qcom,sc8280xp-qmp-usb3-uni-phy
>> +              - qcom,sm6115-qmp-usb3-phy
>> +    then:
>> +      properties:
>> +        resets:
>> +          maxItems: 2
>> +        reset-names:
>> +          items:
>> +            - const: phy
>> +            - const: phy_phy
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdx75-qmp-usb3-uni-phy
>> +    then:
>> +      properties:
>> +        resets:
>> +          maxItems: 2
>> +        reset-names:
>> +          items:
>> +            - const: phy
>> +            - const: common
>
> Could you please point us to the actual DTS patch adding this PHY? I'd 
> say, it is highly likely that you are trying to bring in the 
> unnecessary change.
I have not posted the dt patches yet. But sdx75 uses these resets.
GCC_USB3PHY_PHY_BCR, GCC_USB3_PHY_BCR

These are same as sdx65 and sdx55.

Thanks,
Rohit.

>
>> +
>>   additionalProperties: false
>>     examples:
>
Rohit Agarwal Sept. 6, 2023, 5:35 a.m. UTC | #2 
On 9/6/2023 2:08 AM, Dmitry Baryshkov wrote:
> On 05/09/2023 13:30, Rohit Agarwal wrote:
>> Add support for USB3 QMP PHY found in SDX75 platform.
>>
>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 158 
>> ++++++++++++++++++++++++++++++++
>>   1 file changed, 158 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c 
>> b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>> index 0130bb8..57b8b5b 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>> @@ -23,6 +23,7 @@
>>   #include "phy-qcom-qmp-pcs-misc-v3.h"
>>   #include "phy-qcom-qmp-pcs-usb-v4.h"
>>   #include "phy-qcom-qmp-pcs-usb-v5.h"
>> +#include "phy-qcom-qmp-pcs-usb-v6.h"
>>     /* QPHY_SW_RESET bit */
>>   #define SW_RESET                BIT(0)
>> @@ -858,6 +859,134 @@ static const struct qmp_phy_init_tbl 
>> sdx65_usb3_uniphy_rx_tbl[] = {
>
> [skipped the tables]
>
>> @@ -1556,6 +1685,32 @@ static const struct qmp_phy_cfg 
>> sdx65_usb3_uniphy_cfg = {
>>       .has_pwrdn_delay    = true,
>>   };
>>   +static const struct qmp_phy_cfg sdx75_usb3_uniphy_cfg = {
>> +    .lanes            = 1,
>> +    .offsets        = &qmp_usb_offsets_v5,
>
> v6?
Since the offsets for v5 and v6 are same, I did not introduce a new 
struct with the same values.
Please correct me if I have to introduce v6 offsets.
>
>> +
>> +    .serdes_tbl        = sdx75_usb3_uniphy_serdes_tbl,
>> +    .serdes_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_serdes_tbl),
>> +    .tx_tbl            = sdx75_usb3_uniphy_tx_tbl,
>> +    .tx_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_tx_tbl),
>> +    .rx_tbl            = sdx75_usb3_uniphy_rx_tbl,
>> +    .rx_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_rx_tbl),
>> +    .pcs_tbl        = sdx75_usb3_uniphy_pcs_tbl,
>> +    .pcs_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_pcs_tbl),
>> +    .pcs_usb_tbl        = sdx75_usb3_uniphy_pcs_usb_tbl,
>> +    .pcs_usb_tbl_num    = ARRAY_SIZE(sdx75_usb3_uniphy_pcs_usb_tbl),
>> +    .clk_list        = qmp_v4_sdx55_usbphy_clk_l,
>> +    .num_clks        = ARRAY_SIZE(qmp_v4_sdx55_usbphy_clk_l),
>> +    .reset_list        = msm8996_usb3phy_reset_l,
>> +    .num_resets        = ARRAY_SIZE(msm8996_usb3phy_reset_l),
>
> Clocks and resets are gone in 
> https://lore.kernel.org/linux-phy/20230824211952.1397699-1-dmitry.baryshkov@linaro.org/
>
Sure.
>> +    .vreg_list        = qmp_phy_vreg_l,
>> +    .num_vregs        = ARRAY_SIZE(qmp_phy_vreg_l),
>> +    .regs            = qmp_v5_usb3phy_regs_layout,
>
> This must be v6, if the rest of the PHY is using v6 register names.
Same, Shall I introduce v6 struct?

Thanks,
Rohit.
>
>> +    .pcs_usb_offset        = 0x1000,
>> +
>> +    .has_pwrdn_delay    = true,
>> +};
>> +
>>   static const struct qmp_phy_cfg sm8350_usb3_uniphy_cfg = {
>>       .lanes            = 1,
>>   @@ -2256,6 +2411,9 @@ static const struct of_device_id 
>> qmp_usb_of_match_table[] = {
>>           .compatible = "qcom,sdx65-qmp-usb3-uni-phy",
>>           .data = &sdx65_usb3_uniphy_cfg,
>>       }, {
>> +        .compatible = "qcom,sdx75-qmp-usb3-uni-phy",
>> +        .data = &sdx75_usb3_uniphy_cfg,
>> +    }, {
>>           .compatible = "qcom,sm6115-qmp-usb3-phy",
>>           .data = &qcm2290_usb3phy_cfg,
>>       }, {
>
Dmitry Baryshkov Sept. 6, 2023, 5:39 a.m. UTC | #3 
On Wed, 6 Sept 2023 at 08:26, Rohit Agarwal <quic_rohiagar@quicinc.com> wrote:
>
>
> On 9/6/2023 10:52 AM, Rohit Agarwal wrote:
> >
> > On 9/6/2023 2:04 AM, Dmitry Baryshkov wrote:
> >> On 05/09/2023 13:30, Rohit Agarwal wrote:
> >>> Add dt-bindings for USB3 PHY found on Qualcomm SDX75.
> >>>
> >>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
> >>> ---
> >>>   .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml        | 40
> >>> ++++++++++++++++++++--
> >>>   1 file changed, 37 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> >>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> >>>
> >>> index f99fbbc..5725620 100644
> >>> ---
> >>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> >>> +++
> >>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> >>> @@ -20,6 +20,7 @@ properties:
> >>>         - qcom,qcm2290-qmp-usb3-phy
> >>>         - qcom,sa8775p-qmp-usb3-uni-phy
> >>>         - qcom,sc8280xp-qmp-usb3-uni-phy
> >>> +      - qcom,sdx75-qmp-usb3-uni-phy
> >>
> >> I think the ident is wrong here.
> >
> > Same. Somehow, your reply has the issue but original not able to see.
> >
> >>
> >>>         - qcom,sm6115-qmp-usb3-phy
> >>>       reg:
> >>> @@ -38,9 +39,7 @@ properties:
> >>>       maxItems: 2
> >>>       reset-names:
> >>> -    items:
> >>> -      - const: phy
> >>> -      - const: phy_phy
> >>> +    maxItems: 2
> >>>       vdda-phy-supply: true
> >>>   @@ -75,6 +74,7 @@ allOf:
> >>>             contains:
> >>>               enum:
> >>>                 - qcom,ipq9574-qmp-usb3-phy
> >>> +              - qcom,sdx75-qmp-usb3-uni-phy
> >>>       then:
> >>>         properties:
> >>>           clock-names:
> >>> @@ -122,6 +122,40 @@ allOf:
> >>>         required:
> >>>           - power-domains
> >>>   +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - qcom,ipq9574-qmp-usb3-phy
> >>> +              - qcom,qcm2290-qmp-usb3-phy
> >>> +              - qcom,sa8775p-qmp-usb3-uni-phy
> >>> +              - qcom,sc8280xp-qmp-usb3-uni-phy
> >>> +              - qcom,sm6115-qmp-usb3-phy
> >>> +    then:
> >>> +      properties:
> >>> +        resets:
> >>> +          maxItems: 2
> >>> +        reset-names:
> >>> +          items:
> >>> +            - const: phy
> >>> +            - const: phy_phy
> >>> +
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - qcom,sdx75-qmp-usb3-uni-phy
> >>> +    then:
> >>> +      properties:
> >>> +        resets:
> >>> +          maxItems: 2
> >>> +        reset-names:
> >>> +          items:
> >>> +            - const: phy
> >>> +            - const: common
> >>
> >> Could you please point us to the actual DTS patch adding this PHY?
> >> I'd say, it is highly likely that you are trying to bring in the
> >> unnecessary change.
> > I have not posted the dt patches yet. But sdx75 uses these resets.
> > GCC_USB3PHY_PHY_BCR, GCC_USB3_PHY_BCR
> >
> > These are same as sdx65 and sdx55.
> Ok I see in your patch
> https://lore.kernel.org/linux-phy/20230824211952.1397699-17-dmitry.baryshkov@linaro.org/
> you are updating the resets name. Fine, this change becomes unnecessary.
> Will rebase my change on your patches.

Well, even without my changes, GCC_USB3PHY_PHY_BCR is "phy_phy", just
judging by the name.

>
> Thanks,
> Rohit.
> >
> > Thanks,
> > Rohit.
> >
> >>
> >>> +
> >>>   additionalProperties: false
> >>>     examples:
> >>
Rohit Agarwal Sept. 6, 2023, 5:44 a.m. UTC | #4 
On 9/6/2023 11:11 AM, Dmitry Baryshkov wrote:
> On Wed, 6 Sept 2023 at 08:35, Rohit Agarwal <quic_rohiagar@quicinc.com> wrote:
>>
>> On 9/6/2023 2:08 AM, Dmitry Baryshkov wrote:
>>> On 05/09/2023 13:30, Rohit Agarwal wrote:
>>>> Add support for USB3 QMP PHY found in SDX75 platform.
>>>>
>>>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
>>>> ---
>>>>    drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 158
>>>> ++++++++++++++++++++++++++++++++
>>>>    1 file changed, 158 insertions(+)
>>>>
>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>>>> b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>>>> index 0130bb8..57b8b5b 100644
>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
>>>> @@ -23,6 +23,7 @@
>>>>    #include "phy-qcom-qmp-pcs-misc-v3.h"
>>>>    #include "phy-qcom-qmp-pcs-usb-v4.h"
>>>>    #include "phy-qcom-qmp-pcs-usb-v5.h"
>>>> +#include "phy-qcom-qmp-pcs-usb-v6.h"
>>>>      /* QPHY_SW_RESET bit */
>>>>    #define SW_RESET                BIT(0)
>>>> @@ -858,6 +859,134 @@ static const struct qmp_phy_init_tbl
>>>> sdx65_usb3_uniphy_rx_tbl[] = {
>>> [skipped the tables]
>>>
>>>> @@ -1556,6 +1685,32 @@ static const struct qmp_phy_cfg
>>>> sdx65_usb3_uniphy_cfg = {
>>>>        .has_pwrdn_delay    = true,
>>>>    };
>>>>    +static const struct qmp_phy_cfg sdx75_usb3_uniphy_cfg = {
>>>> +    .lanes            = 1,
>>>> +    .offsets        = &qmp_usb_offsets_v5,
>>> v6?
>> Since the offsets for v5 and v6 are same, I did not introduce a new
>> struct with the same values.
>> Please correct me if I have to introduce v6 offsets.
> If the offsets are the same, it's fine to leave at v5.
Ack.
>>>> +
>>>> +    .serdes_tbl        = sdx75_usb3_uniphy_serdes_tbl,
>>>> +    .serdes_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_serdes_tbl),
>>>> +    .tx_tbl            = sdx75_usb3_uniphy_tx_tbl,
>>>> +    .tx_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_tx_tbl),
>>>> +    .rx_tbl            = sdx75_usb3_uniphy_rx_tbl,
>>>> +    .rx_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_rx_tbl),
>>>> +    .pcs_tbl        = sdx75_usb3_uniphy_pcs_tbl,
>>>> +    .pcs_tbl_num        = ARRAY_SIZE(sdx75_usb3_uniphy_pcs_tbl),
>>>> +    .pcs_usb_tbl        = sdx75_usb3_uniphy_pcs_usb_tbl,
>>>> +    .pcs_usb_tbl_num    = ARRAY_SIZE(sdx75_usb3_uniphy_pcs_usb_tbl),
>>>> +    .clk_list        = qmp_v4_sdx55_usbphy_clk_l,
>>>> +    .num_clks        = ARRAY_SIZE(qmp_v4_sdx55_usbphy_clk_l),
>>>> +    .reset_list        = msm8996_usb3phy_reset_l,
>>>> +    .num_resets        = ARRAY_SIZE(msm8996_usb3phy_reset_l),
>>> Clocks and resets are gone in
>>> https://lore.kernel.org/linux-phy/20230824211952.1397699-1-dmitry.baryshkov@linaro.org/
>>>
>> Sure.
>>>> +    .vreg_list        = qmp_phy_vreg_l,
>>>> +    .num_vregs        = ARRAY_SIZE(qmp_phy_vreg_l),
>>>> +    .regs            = qmp_v5_usb3phy_regs_layout,
>>> This must be v6, if the rest of the PHY is using v6 register names.
>> Same, Shall I introduce v6 struct?
> Yes. Otherwise it becomes hard to add offsets for different versions.
> Generic rule: the name of the struct should match the Vn found in the
> register names inside.
Understood. Thanks for the info.
And will also wait for your patches that removes _USB_.

Thanks,
Rohit.
>> Thanks,
>> Rohit.
>>>> +    .pcs_usb_offset        = 0x1000,
>>>> +
>>>> +    .has_pwrdn_delay    = true,
>>>> +};
>>>> +
>>>>    static const struct qmp_phy_cfg sm8350_usb3_uniphy_cfg = {
>>>>        .lanes            = 1,
>>>>    @@ -2256,6 +2411,9 @@ static const struct of_device_id
>>>> qmp_usb_of_match_table[] = {
>>>>            .compatible = "qcom,sdx65-qmp-usb3-uni-phy",
>>>>            .data = &sdx65_usb3_uniphy_cfg,
>>>>        }, {
>>>> +        .compatible = "qcom,sdx75-qmp-usb3-uni-phy",
>>>> +        .data = &sdx75_usb3_uniphy_cfg,
>>>> +    }, {
>>>>            .compatible = "qcom,sm6115-qmp-usb3-phy",
>>>>            .data = &qcm2290_usb3phy_cfg,
>>>>        }, {
>
>