Hi Conor, > > The lan969x switch device supports two RGMII port interfaces that can be > > configured for MAC level rx and tx delays. > > > > Document two new properties {rx,tx}-internal-delay-ps. Make them > > required properties, if the phy-mode is one of: rgmii, rgmii_id, > > rgmii-rxid or rgmii-txid. Also specify accepted values. > > > > Signed-off-by: Daniel Machon <daniel.machon@xxxxxxxxxxxxx> > > --- > > .../bindings/net/microchip,sparx5-switch.yaml | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml b/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml > > index dedfad526666..a3f2b70c5c77 100644 > > --- a/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml > > +++ b/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml > > @@ -129,6 +129,26 @@ properties: > > minimum: 0 > > maximum: 383 > > > > + allOf: > > + - if: > > + properties: > > + phy-mode: > > + contains: > > + enum: > > + - rgmii > > + - rgmii-rxid > > + - rgmii-txid > > + - rgmii-id > > + then: > > + properties: > > + rx-internal-delay-ps: > > + enum: [0, 1000, 1700, 2000, 2500, 3000, 3300] > > + tx-internal-delay-ps: > > + enum: [0, 1000, 1700, 2000, 2500, 3000, 3300] > > Properties should be define at the top level and constrained in the > if/then parts. Please move the property definitions out, and just leave > the required: bit here. > > > + required: > > + - rx-internal-delay-ps > > + - tx-internal-delay-ps > > You've got no else, so these properties are valid even for !rgmii? > > > + > > required: > > - reg > > - phys > > Additionally, please move the conditional bits below the required > property list. > > Cheers, > Conor. I will be getting rid of the 'required' constraints in v3. What I hear you say, is that the two {rx,tx}-internal-delay-ps properties (incl. their enum values) should be moved out of the if/else and to the top-level - can you confirm this? Is specifying the values a property can take not considered a constraint? Thanks, Daniel