.../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Both bindings describe a different layout of the ports properties,
leading to errors when validating DT using this PHY bindings as
reported by Rob Herring.
Reported-by: Rob Herring <robh@kernel.org>
Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/
Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -73,8 +73,11 @@ properties:
description:
See include/dt-bindings/phy/phy-qcom-qmp.h
- mode-switch: true
- orientation-switch: true
+ mode-switch:
+ $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch
+
+ orientation-switch:
+ $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -104,7 +107,6 @@ required:
- "#phy-cells"
allOf:
- - $ref: /schemas/usb/usb-switch.yaml#
- if:
properties:
compatible:
---
base-commit: 3db46a82d467bd23d9ebc473d872a865785299d8
change-id: 20250902-topic-sm8x50-fix-qmp-usb43dp-usb-switch-5df6d494ba5c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: > Both bindings describe a different layout of the ports properties, > leading to errors when validating DT using this PHY bindings as > reported by Rob Herring. > > Reported-by: Rob Herring <robh@kernel.org> > Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ > Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > @@ -73,8 +73,11 @@ properties: > description: > See include/dt-bindings/phy/phy-qcom-qmp.h > > - mode-switch: true > - orientation-switch: true > + mode-switch: > + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch > + > + orientation-switch: > + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch This is a pattern we try to avoid with references at a property level. I prefer you make port and ports not required in usb-switch.yaml. Rob
On Fri, Sep 05, 2025 at 12:55:33PM -0500, Rob Herring wrote: > On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: > > Both bindings describe a different layout of the ports properties, > > leading to errors when validating DT using this PHY bindings as > > reported by Rob Herring. > > > > Reported-by: Rob Herring <robh@kernel.org> > > Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ > > Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > > --- > > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > @@ -73,8 +73,11 @@ properties: > > description: > > See include/dt-bindings/phy/phy-qcom-qmp.h > > > > - mode-switch: true > > - orientation-switch: true > > + mode-switch: > > + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch > > + > > + orientation-switch: > > + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch > > This is a pattern we try to avoid with references at a property level. I > prefer you make port and ports not required in usb-switch.yaml. But this solution is also not perfect. E.g. Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml should only allow the orienttion-switch property, while using allOf:$ref:usb-switch.yaml allows all three (orientation-switch, mode-switch, retimer-switch). -- With best wishes Dmitry
On Tue, Sep 30, 2025 at 10:10:25PM +0300, Dmitry Baryshkov wrote: > On Fri, Sep 05, 2025 at 12:55:33PM -0500, Rob Herring wrote: > > On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: > > > Both bindings describe a different layout of the ports properties, > > > leading to errors when validating DT using this PHY bindings as > > > reported by Rob Herring. > > > > > > Reported-by: Rob Herring <robh@kernel.org> > > > Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ > > > Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > > > --- > > > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > @@ -73,8 +73,11 @@ properties: > > > description: > > > See include/dt-bindings/phy/phy-qcom-qmp.h > > > > > > - mode-switch: true > > > - orientation-switch: true > > > + mode-switch: > > > + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch > > > + > > > + orientation-switch: > > > + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch Looking at this again, this isn't even correct and I don't think it works. It's missing a '/' and should be ...#/properties/... to be a valid json pointer. I thought we checked this... > > > > This is a pattern we try to avoid with references at a property level. I > > prefer you make port and ports not required in usb-switch.yaml. > > But this solution is also not perfect. E.g. > Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml should > only allow the orienttion-switch property, while using > allOf:$ref:usb-switch.yaml allows all three (orientation-switch, > mode-switch, retimer-switch). That can be handled like this: $ref: usb-switch.yaml properties: orientation-switch: true additionalProperties: false Though if you need unevaluatedProperties for some other reason, then that won't enforce it and it's just documentation. In that case, then perhaps usb-switch.yaml is not the right granularity and should be split up. I put this into the category of "this is the least of our problems". I'm not that interested in enforcing what common properties a device uses or not. It's undocumented properties I'm worried about or lack of constraints (on reg, clocks, interrupts, etc.). Rob
On 10/1/25 18:31, Rob Herring wrote: > On Tue, Sep 30, 2025 at 10:10:25PM +0300, Dmitry Baryshkov wrote: >> On Fri, Sep 05, 2025 at 12:55:33PM -0500, Rob Herring wrote: >>> On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: >>>> Both bindings describe a different layout of the ports properties, >>>> leading to errors when validating DT using this PHY bindings as >>>> reported by Rob Herring. >>>> >>>> Reported-by: Rob Herring <robh@kernel.org> >>>> Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ >>>> Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") >>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>> --- >>>> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>>> index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 >>>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>>> @@ -73,8 +73,11 @@ properties: >>>> description: >>>> See include/dt-bindings/phy/phy-qcom-qmp.h >>>> >>>> - mode-switch: true >>>> - orientation-switch: true >>>> + mode-switch: >>>> + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch >>>> + >>>> + orientation-switch: >>>> + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch > > Looking at this again, this isn't even correct and I don't think it > works. It's missing a '/' and should be ...#/properties/... to be a > valid json pointer. > > I thought we checked this... > >>> >>> This is a pattern we try to avoid with references at a property level. I >>> prefer you make port and ports not required in usb-switch.yaml. >> >> But this solution is also not perfect. E.g. >> Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml should >> only allow the orienttion-switch property, while using >> allOf:$ref:usb-switch.yaml allows all three (orientation-switch, >> mode-switch, retimer-switch). > > That can be handled like this: > > $ref: usb-switch.yaml > properties: > orientation-switch: true > additionalProperties: false > > Though if you need unevaluatedProperties for some other reason, then > that won't enforce it and it's just documentation. In that case, then > perhaps usb-switch.yaml is not the right granularity and should be split > up. I did a split up at: https://lore.kernel.org/all/20250930-topic-sm8x50-fix-qmp-usb43dp-usb-switch-v1-1-060568de9538@linaro.org/ > > I put this into the category of "this is the least of our problems". I'm > not that interested in enforcing what common properties a device uses or > not. It's undocumented properties I'm worried about or lack of > constraints (on reg, clocks, interrupts, etc.). Thanks for the comment, I'll have a look if this would work. Thanks, Neil > > Rob
On 9/30/25 21:10, Dmitry Baryshkov wrote: > On Fri, Sep 05, 2025 at 12:55:33PM -0500, Rob Herring wrote: >> On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: >>> Both bindings describe a different layout of the ports properties, >>> leading to errors when validating DT using this PHY bindings as >>> reported by Rob Herring. >>> >>> Reported-by: Rob Herring <robh@kernel.org> >>> Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ >>> Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 >>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> @@ -73,8 +73,11 @@ properties: >>> description: >>> See include/dt-bindings/phy/phy-qcom-qmp.h >>> >>> - mode-switch: true >>> - orientation-switch: true >>> + mode-switch: >>> + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch >>> + >>> + orientation-switch: >>> + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch >> >> This is a pattern we try to avoid with references at a property level. I >> prefer you make port and ports not required in usb-switch.yaml. > > But this solution is also not perfect. E.g. > Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml should > only allow the orienttion-switch property, while using > allOf:$ref:usb-switch.yaml allows all three (orientation-switch, > mode-switch, retimer-switch). > I agree, but I'm not the original author of this change. Neil
On 05/09/2025 19:55, Rob Herring wrote: > On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: >> Both bindings describe a different layout of the ports properties, >> leading to errors when validating DT using this PHY bindings as >> reported by Rob Herring. >> >> Reported-by: Rob Herring <robh@kernel.org> >> Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ >> Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >> index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 >> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >> @@ -73,8 +73,11 @@ properties: >> description: >> See include/dt-bindings/phy/phy-qcom-qmp.h >> >> - mode-switch: true >> - orientation-switch: true >> + mode-switch: >> + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch >> + >> + orientation-switch: >> + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch > > This is a pattern we try to avoid with references at a property level. I > prefer you make port and ports not required in usb-switch.yaml. Sure but the ports definitions will still collide in this case. The other problem here is the difference in ports definition, port@0 and port@1 are not the same. qcom,sc8280xp-qmp-usb43dp-phy.yaml: ports: $ref: /schemas/graph.yaml#/properties/ports properties: port@0: $ref: /schemas/graph.yaml#/properties/port description: Output endpoint of the PHY port@1: $ref: /schemas/graph.yaml#/properties/port description: Incoming endpoint from the USB controller port@2: $ref: /schemas/graph.yaml#/properties/port description: Incoming endpoint from the DisplayPort controller usb-switch.yaml: ports: $ref: /schemas/graph.yaml#/properties/ports properties: port@0: $ref: /schemas/graph.yaml#/properties/port description: Super Speed (SS) Output endpoint to the Type-C connector port@1: $ref: /schemas/graph.yaml#/$defs/port-base description: Super Speed (SS) Input endpoint from the Super-Speed PHY Neil > > Rob
Hi, On 08/09/2025 09:33, Neil Armstrong wrote: > On 05/09/2025 19:55, Rob Herring wrote: >> On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: >>> Both bindings describe a different layout of the ports properties, >>> leading to errors when validating DT using this PHY bindings as >>> reported by Rob Herring. >>> >>> Reported-by: Rob Herring <robh@kernel.org> >>> Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ >>> Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> index c8bc512df08b5694c8599f475de78679a4438449..5005514d7c3a1e4a8893883497fd204bc04e12be 100644 >>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml >>> @@ -73,8 +73,11 @@ properties: >>> description: >>> See include/dt-bindings/phy/phy-qcom-qmp.h >>> - mode-switch: true >>> - orientation-switch: true >>> + mode-switch: >>> + $ref: /schemas/usb/usb-switch.yaml#properties/mode-switch >>> + >>> + orientation-switch: >>> + $ref: /schemas/usb/usb-switch.yaml#properties/orientation-switch >> >> This is a pattern we try to avoid with references at a property level. I >> prefer you make port and ports not required in usb-switch.yaml. > > Sure but the ports definitions will still collide in this case. > > The other problem here is the difference in ports definition, > port@0 and port@1 are not the same. > > qcom,sc8280xp-qmp-usb43dp-phy.yaml: > ports: > $ref: /schemas/graph.yaml#/properties/ports > properties: > port@0: > $ref: /schemas/graph.yaml#/properties/port > description: Output endpoint of the PHY > > port@1: > $ref: /schemas/graph.yaml#/properties/port > description: Incoming endpoint from the USB controller > > port@2: > $ref: /schemas/graph.yaml#/properties/port > description: Incoming endpoint from the DisplayPort controller > > usb-switch.yaml: > ports: > $ref: /schemas/graph.yaml#/properties/ports > properties: > port@0: > $ref: /schemas/graph.yaml#/properties/port > description: > Super Speed (SS) Output endpoint to the Type-C connector > > port@1: > $ref: /schemas/graph.yaml#/$defs/port-base > description: > Super Speed (SS) Input endpoint from the Super-Speed PHY Do you have any advice how to solve this ? Requires is not the problem, the problem is to have the port definitions in usb-switch.yaml Neil > > Neil > >> >> Rob >
On Tue, Sep 02, 2025 at 06:10:05PM +0200, Neil Armstrong wrote: > Both bindings describe a different layout of the ports properties, > leading to errors when validating DT using this PHY bindings as > reported by Rob Herring. > > Reported-by: Rob Herring <robh@kernel.org> > Closes: https://lore.kernel.org/all/175462129176.394940.16810637795278334342.robh@kernel.org/ > Fixes: 3bad7fe22796 ("dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch") > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.