[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e6qtpkzzugddgy6dkpruecdty73qjbja5wpvrvio53mq3dfd2g@axgmdfw6oywg>
Date: Wed, 22 Nov 2023 21:25:46 +0100
From: Marijn Suijten <marijn.suijten@...ainline.org>
To: Krishna Kurapati <quic_kriskura@...cinc.com>
Cc: Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, quic_wcheng@...cinc.com,
linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
quic_ppratap@...cinc.com, quic_jackp@...cinc.com
Subject: Re: [PATCH 1/6] dt-bindings: usb: dwc3: Clean up hs_phy_irq in
bindings
Looks like this patch will conflict with:
https://lore.kernel.org/linux-arm-msm/20231111141953.51841-1-krzysztof.kozlowski@linaro.org/
On 2023-11-23 00:43:35, Krishna Kurapati wrote:
> The high speed related interrupts present on QC targets are as follows:
>
> dp/dm Irq's
irqs
> These IRQ's directly reflect changes on the DP/DM pads of the SoC. These
> are used as wakeup interrupts only on SoCs with non-QUSBb2 targets with
drop -b-: QUSB2?
Perhaps a description like this deserves to live in the yaml?
> exception of SDM670/SDM845/SM6350.
> qusb2_phy irq
> SoCs with QUSB2 PHY do not have separate DP/DM IRQs and expose only a
> single IRQ whose behavior can be modified by the QUSB2PHY_INTR_CTRL
> register. The required DPSE/DMSE configuration is done in
> QUSB2PHY_INTR_CTRL register of phy address space.
>
> hs_phy_irq
> This is completely different from the above two and is present on all
> targets with exception of a few IPQ ones. The interrupt is not enabled by
> default and its functionality is mutually exclusive of qusb2_phy on QUSB
> targets and DP/DM on femto phy targets.
>
> The DTs of several QUSB2 PHY based SoCs incorrectly define "hs_phy_irq"
> when they should have been "qusb2_phy_irq". On Femto phy targets, the
> "hs_phy_irq" mentioned is either the actual "hs_phy_irq" or "pwr_event",
> neither of which would never be triggered directly are non-functional
> currently. The implementation tries to clean up this issue by addressing
> the discrepencies involved and fixing the hs_phy_irq's in respective DT's.
>
> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
> ---
> .../devicetree/bindings/usb/qcom,dwc3.yaml | 125 ++++++++++--------
> 1 file changed, 69 insertions(+), 56 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> index e889158ca205..4a46346e2ead 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -17,20 +17,25 @@ properties:
> - qcom,ipq5018-dwc3
> - qcom,ipq5332-dwc3
> - qcom,ipq6018-dwc3
> + - qcom,ipq6018-dwc3-sec
> - qcom,ipq8064-dwc3
> - qcom,ipq8074-dwc3
> - qcom,ipq9574-dwc3
> - qcom,msm8953-dwc3
> - qcom,msm8994-dwc3
> - qcom,msm8996-dwc3
> + - qcom,msm8996-dwc3-sec
> - qcom,msm8998-dwc3
> - qcom,qcm2290-dwc3
> - qcom,qcs404-dwc3
> - qcom,sa8775p-dwc3
> + - qcom,sa8775p-dwc3-ter
> - qcom,sc7180-dwc3
> - qcom,sc7280-dwc3
> + - qcom,sc7280-dwc3-sec
> - qcom,sc8280xp-dwc3
> - qcom,sdm660-dwc3
> + - qcom,sdm660-dwc3-sec
> - qcom,sdm670-dwc3
> - qcom,sdm845-dwc3
> - qcom,sdx55-dwc3
> @@ -98,11 +103,11 @@ properties:
>
> interrupts:
> minItems: 1
> - maxItems: 4
> + maxItems: 5
>
> interrupt-names:
> minItems: 1
> - maxItems: 4
> + maxItems: 5
>
> qcom,select-utmi-as-pipe-clk:
> description:
> @@ -175,10 +180,13 @@ allOf:
> - qcom,ipq9574-dwc3
> - qcom,msm8953-dwc3
> - qcom,msm8996-dwc3
> + - qcom,msm8996-dwc3-sec
> - qcom,msm8998-dwc3
> - qcom,sa8775p-dwc3
> + - qcom,sa8775p-dwc3-ter
> - qcom,sc7180-dwc3
> - qcom,sc7280-dwc3
> + - qcom,sc7280-dwc3-sec
> - qcom,sdm670-dwc3
> - qcom,sdm845-dwc3
> - qcom,sdx55-dwc3
> @@ -203,6 +211,7 @@ allOf:
> contains:
> enum:
> - qcom,ipq6018-dwc3
> + - qcom,ipq6018-dwc3-sec
> then:
> properties:
> clocks:
> @@ -285,6 +294,7 @@ allOf:
> contains:
> enum:
> - qcom,sdm660-dwc3
> + - qcom,sdm660-dwc3-sec
> then:
> properties:
> clocks:
> @@ -357,20 +367,15 @@ allOf:
> compatible:
> contains:
> enum:
> - - qcom,ipq4019-dwc3
> - - qcom,ipq6018-dwc3
> - - qcom,ipq8064-dwc3
> - - qcom,ipq8074-dwc3
> - - qcom,msm8994-dwc3
> - - qcom,qcs404-dwc3
> + - qcom,sc8280xp-dwc3
> + - qcom,sa8775p-dwc3
> - qcom,sc7180-dwc3
> + - qcom,sc7280-dwc3
> - qcom,sdm670-dwc3
> - qcom,sdm845-dwc3
> - qcom,sdx55-dwc3
> - qcom,sdx65-dwc3
> - qcom,sdx75-dwc3
> - - qcom,sm4250-dwc3
> - - qcom,sm6125-dwc3
> - qcom,sm6350-dwc3
> - qcom,sm8150-dwc3
> - qcom,sm8250-dwc3
> @@ -381,16 +386,37 @@ allOf:
> properties:
> interrupts:
> items:
> + - description: Wakeup event on DM line.
> + - description: Wakeup event on DP line.
> - description: The interrupt that is asserted
> - when a wakeup event is received on USB2 bus.
> + based on linestates. Is enabled if qscratch
> + registers are configured appropirately. This
Arr.
In all seriousness: I think you meant to write "appropriately".
> + interrupt functionality is mutually exclusive
> + to that of {dp/d}_hs_phy_irq)
dm?
- Marijn
> + - description: Wakeup based on power events.
> - description: The interrupt that is asserted
> when a wakeup event is received on USB3 bus.
> - - description: Wakeup event on DM line.
> - - description: Wakeup event on DP line.
> interrupt-names:
> items:
> + - const: dm_hs_phy_irq
> + - const: dp_hs_phy_irq
> - const: hs_phy_irq
> + - const: pwr_event
> - const: ss_phy_irq
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,sc7280-dwc3-sec
> + - qcom,sa8775p-ter
> + then:
> + properties:
> + interrupt-names:
> + items:
> + - const: pwr_event
> + - const: hs_phy_irq
> - const: dm_hs_phy_irq
> - const: dp_hs_phy_irq
>
> @@ -399,36 +425,29 @@ allOf:
> compatible:
> contains:
> enum:
> - - qcom,msm8953-dwc3
> - - qcom,msm8996-dwc3
> - - qcom,msm8998-dwc3
> - - qcom,sm6115-dwc3
> + - qcom,ipq6018-dwc3-sec
> then:
> properties:
> - interrupts:
> - maxItems: 2
> interrupt-names:
> items:
> - - const: hs_phy_irq
> - - const: ss_phy_irq
> + - const: pwr_event
> + - const: qusb2_phy
>
> - if:
> properties:
> compatible:
> contains:
> enum:
> - - qcom,ipq5018-dwc3
> - - qcom,ipq5332-dwc3
> - - qcom,sdm660-dwc3
> + - qcom,ipq6018-dwc3
> + - qcom,ipq8074-dwc3
> + - qcom,msm8953-dwc3
> + - qcom,msm8998-dwc3
> then:
> properties:
> - interrupts:
> - minItems: 1
> - maxItems: 2
> interrupt-names:
> - minItems: 1
> items:
> - - const: hs_phy_irq
> + - const: pwr_event
> + - const: qusb2_phy
> - const: ss_phy_irq
>
> - if:
> @@ -436,55 +455,48 @@ allOf:
> compatible:
> contains:
> enum:
> - - qcom,sc7280-dwc3
> + - qcom,msm8996-dwc3
> + - qcom,sdm660-dwc3
> + - qcom,sm4250-dwc3
> + - qcom,sm6115-dwc3
> + - qcom,sm6125-dwc3
> then:
> properties:
> - interrupts:
> - minItems: 3
> - maxItems: 4
> interrupt-names:
> - minItems: 3
> items:
> - const: hs_phy_irq
> - - const: dp_hs_phy_irq
> - - const: dm_hs_phy_irq
> + - const: pwr_event
> + - const: qusb2_phy
> - const: ss_phy_irq
> -
> - if:
> properties:
> compatible:
> contains:
> enum:
> - - qcom,sc8280xp-dwc3
> + - qcom,sdm660-dwc3-sec
> + - qcom,msm8996-dwc3-sec
> + - qcom,qcs404-dwc3
> then:
> properties:
> - interrupts:
> - maxItems: 4
> interrupt-names:
> items:
> + - const: hs_phy_irq
> - const: pwr_event
> - - const: dp_hs_phy_irq
> - - const: dm_hs_phy_irq
> - - const: ss_phy_irq
> + - const: qusb2_phy
>
> - if:
> properties:
> compatible:
> contains:
> enum:
> - - qcom,sa8775p-dwc3
> + - qcom,ipq5332-dwc3
> then:
> properties:
> - interrupts:
> - minItems: 3
> - maxItems: 4
> interrupt-names:
> - minItems: 3
> items:
> - - const: pwr_event
> - const: dp_hs_phy_irq
> - const: dm_hs_phy_irq
> - - const: ss_phy_irq
> + - const: pwr_event
>
> additionalProperties: false
>
> @@ -519,12 +531,13 @@ examples:
> <&gcc GCC_USB30_PRIM_MASTER_CLK>;
> assigned-clock-rates = <19200000>, <150000000>;
>
> - interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "hs_phy_irq", "ss_phy_irq",
> - "dm_hs_phy_irq", "dp_hs_phy_irq";
> + interrupts = <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "dm_hs_phy_irq", "dp_hs_phy_irq",
> + "hs_phy_irq", "pwr_event", "ss_phy_irq";
>
> power-domains = <&gcc USB30_PRIM_GDSC>;
>
> --
> 2.42.0
>
Powered by blists - more mailing lists