On 26/12/2023 06:37, Krishna Kurapati PSSNV wrote: > > > On 12/25/2023 6:35 PM, Krzysztof Kozlowski wrote: >> On 22/12/2023 07:36, Krishna Kurapati wrote: >>> The high speed related interrupts present on QC targets are as follows: >>> >> >> >>> >>> interrupt-names: >>> - minItems: 1 >>> - maxItems: 4 >>> + minItems: 2 >>> + maxItems: 5 >>> >>> qcom,select-utmi-as-pipe-clk: >>> description: >>> @@ -361,60 +378,21 @@ allOf: >>> compatible: >>> contains: >>> enum: >>> - - qcom,ipq4019-dwc3 >> >> Why do you remove it, without adding it somewhere else. Nothing in the >> commit msg explains it. >> > > Apologies, Will check and add it back. Please check your patch for other entries. I just took first compatible which turns out to be gone. I did not check the reset and I don't want to keep checking. ... >>> - then: >>> - properties: >>> - interrupts: >>> - minItems: 1 >>> - maxItems: 2 >>> - interrupt-names: >>> - minItems: 1 >>> - items: >>> - - const: hs_phy_irq >>> - - const: ss_phy_irq >>> - >>> - - if: >>> - properties: >>> - compatible: >>> - contains: >>> - enum: >>> - - qcom,sc7280-dwc3 >>> + - qcom,sm6115-dwc3 >>> + - qcom,sm6125-dwc3 >>> then: >>> properties: >>> interrupts: >>> minItems: 3 >>> maxItems: 4 >>> interrupt-names: >>> - minItems: 3 >>> items: >>> + - const: pwr_event >>> - const: hs_phy_irq >>> - - const: dp_hs_phy_irq >>> - - const: dm_hs_phy_irq >>> + - const: qusb2_phy >> >> Why qusb2_phy is after hs_phy_irq? In the earlier if:then: it is the >> second one. >> > > In v3 as well, the hs_phy_irq is before qusb2_phy interrupt: > https://lore.kernel.org/all/20231211121124.4194-2-quic_kriskura@xxxxxxxxxxx/ ? How v3 matters? > >> >>> - const: ss_phy_irq >>> >>> - if: >>> @@ -460,11 +422,13 @@ allOf: >>> compatible: >>> contains: >>> enum: >>> + - qcom,ipq5332-dwc3 >>> - qcom,sc8280xp-dwc3 >>> - qcom,x1e80100-dwc3 >>> then: >>> properties: >>> interrupts: >>> + minItems: 3 >> >> Hm, why? This commit is unmanageable. Your commit msg is already huge >> but still does not explain this. Are you sure you are fixing only one >> logical thing per patch? Does not look like. >> > > This is reordering the targets based on interrupts they have. I put it > in one commit because splitting this into multiple patches breaks one > thing or other. Also once I am defining permutations, I have to group > targets into these combinations in the same patch. I know this is a big > commit but it solves the interrupt cleanup and defines a way for future > targets. This does not answer why, you sc8280xp and x1e80100 not get one optional interrupt. I asked "why" you are doing this change. Why do you need it? What is the rationale? Then I grunted about unmanageable commit, because all my troubles to review it are the effect of it: it is very difficult to read. It is also difficult for you, because you keep making here mistakes. So if you cannot write this commit properly and I cannot review it, then it is way over-complicated, don't you think? But this is still second problem here, don't ignore the fist - "why?" Best regards, Krzysztof
