Messages in this thread | | | Date | Thu, 23 Nov 2023 13:27:20 +0530 | Subject | Re: [PATCH 1/6] dt-bindings: usb: dwc3: Clean up hs_phy_irq in bindings | From | Krishna Kurapati PSSNV <> |
| |
On 11/23/2023 1:20 PM, Krzysztof Kozlowski wrote: > On 23/11/2023 08:44, Krishna Kurapati PSSNV wrote: >> >> >> On 11/23/2023 1:11 PM, Krzysztof Kozlowski wrote: >>> On 22/11/2023 20:13, Krishna Kurapati wrote: >>>> The high speed related interrupts present on QC targets are as follows: >>>> >>>> dp/dm Irq's >>>> 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 >>>> 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@quicinc.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 >>> >>> I could not understand from commit msg why you are adding new compatible >>> and what it is supposed to fix. >>> >>> The entire diff is huge thus difficult to review. Why fixing hs_phy_irq >>> causes three new interrupts being added? >> >> Some targets have two controllers where the second one is only HS >> capable and doesn't have ss_phy_irq. In such cases to make it clear in >> bindings, I added a suffix "-sec" and accordingly changed in DT as well. >> Should've put this in commit text. > > Where did you explain it in the commit msg? Why adding new compatibles > is squashed to this patch? > Apologies. I meant I should've put the explanation in commit text which I missed. Will do it while revising this patch.
> You need separate commit with its own justification. I am not sure if > calling things secondary and tertiary scales. Please describe all the > differences and come with some reason for the naming. > >
Sure, will separate out the new compatible additions and then make different commits.
Regards, Krishna,
| |