On 01/08/2024 00:23, Elson Serrao wrote: > > > On 7/30/2024 10:33 PM, Krzysztof Kozlowski wrote: >> On 31/07/2024 00:24, Elson Roy Serrao wrote: >>> Embedded USB Debugger(EUD) being a High-Speed USB hub needs >>> HS-Phy support for it's operation. Hence document phy bindings >>> to support this. >>> >>> Signed-off-by: Elson Roy Serrao <quic_eserrao@xxxxxxxxxxx> >> >> A nit, subject: drop second/last, redundant "bindings". The >> "dt-bindings" prefix is already stating that these are bindings. >> See also: >> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 >> > Ack >>> --- >>> .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> index f2c5ec7e6437..fca5b608ec63 100644 >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml >>> @@ -29,6 +29,14 @@ properties: >>> description: EUD interrupt >>> maxItems: 1 >>> >>> + phys: >>> + items: >>> + - description: USB2/HS PHY needed for EUD functionality >>> + >>> + phy-names: >>> + items: >>> + - const: usb2-phy >>> + >>> ports: >>> $ref: /schemas/graph.yaml#/properties/ports >>> description: >>> @@ -48,6 +56,8 @@ properties: >>> required: >>> - compatible >>> - reg >>> + - phys >>> + - phy-names >> >> That's an ABI break and nothing in commit msg justified it. >> > > Hi Krzysztof > > Thank you for the review. > I see that the only user for EUD as of now is QC sc7280 SoC where phy property Did you ask all customers and all users of Linux kernel? > is missing and EUD node is disabled. As described in my cover letter, HS phy > support is needed for EUD functionality and this is applicable to all SoCs > where EUD is to be enabled. Hence phy would be a required property. Nothing in commit msg explained that, but I have a bit hard time to believe that this never worked. If that's the case, say it explicitly in commit msg - this was always broken. > Given that the changes in this series are directly applicable to sc7280 as well, > I will re-enable/rectify EUD feature on sc7280 SoC first, by adhering it to this binding > requirement. That would address the ABI break. I don't understand what you are saying here. > Once the base framework is set I shall extend it to sm8450 SoC. Best regards, Krzysztof
