Re: [PATCH v5 net-next 04/10] dt-bindings: net: dsa: utilize base definitions for standard dsa switches

[Colin Foster <colin.foster@in-advantage.com>]

On Mon, Dec 12, 2022 at 10:55:19AM -0800, Colin Foster wrote:
> Hi Rob, Arınç,
> 
> On Mon, Dec 12, 2022 at 10:51:47AM -0600, Rob Herring wrote:
> > On Mon, Dec 12, 2022 at 12:28:06PM +0300, Arınç ÜNAL wrote:
> > > On 10.12.2022 21:02, Colin Foster wrote:
> > > > Hi Arınç,
> > > > On Sat, Dec 10, 2022 at 07:24:42PM +0300, Arınç ÜNAL wrote:
> > > > > On 10.12.2022 06:30, Colin Foster wrote:
> > > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yam
> > > > @@ -156,8 +156,7 @@ patternProperties:
> > > > 
> > > >       patternProperties:
> > > >         "^(ethernet-)?port@[0-9]+$":
> > > > -        type: object
> > > > -
> > > > +        $ref: dsa-port.yaml#
> > > >           properties:
> > > >             reg:
> > > >               description:
> > > > @@ -165,7 +164,6 @@ patternProperties:
> > > >                 for user ports.
> > > > 
> > > >           allOf:
> > > > -          - $ref: dsa-port.yaml#
> > > >             - if:
> > > >                 required: [ ethernet ]
> > > >               then:
> > > > 
> > > > 
> > > > 
> > > > This one has me [still] scratching my head...
> > > 
> > > Right there with you. In addition to this, having or deleting type object
> > > on/from "^(ethernet-)?ports$" and "^(ethernet-)?port@[0-9]+$" on dsa.yaml
> > > doesn't cause any warnings (checked with make dt_binding_check
> > > DT_SCHEMA_FILES=net/dsa) which makes me question why it's there in the first
> > > place.
> > 
> > 
> > That check probably doesn't consider an ref being under an 'allOf'. 
> > Perhaps what is missing in understanding is every schema at the 
> > top-level has an implicit 'type: object'. But nothing is ever implicit 
> > in json-schema which will silently ignore keywords which don't make 
> > sense for an instance type. Instead of a bunch of boilerplate, the 
> > processed schema has 'type' added in lots of cases such as this one.
> > 
> > Rob
> 
> What do you suggest on this set? I think this is the only outstanding
> issue, and Jakub brought up the possibility of applying end of today
> (maybe 5-6 hours from now in the US).
> 
> It seems like there's an issue with the dt_bindings_check that causes
> the "allOf: $ref" to throw a warning when it shouldn't. So removing the
> "type: object" was supposed to be correct, but throws warnings.
> 
> It seems like keeping this patch as-is and updating it when the check
> gets fixed might be an acceptable path, but I'd understand if you
> disagree and prefer a resubmission.

Heads up on my plan for this. I plan to re-submit this on Monday after
the merge window with the change where I move the $ref: dsa-port.yaml#
to outside the allOf: section, and remove the object type as the above
code suggests. Hopefully that's the right step to take.