On Fri, Oct 13, 2023 at 03:04:10PM +0200, Linus Walleij wrote: > Hi Andrew, > > thanks for reviewing! > > On Fri, Oct 13, 2023 at 2:43 PM Andrew Lunn <andrew@xxxxxxx> wrote: > > > > +properties: > > > + compatible: > > > + oneOf: > > > + - enum: > > > + - marvell,mv88e6060 > > > > The 6060 is a separate driver. Its not part of mv88e6xxx. So it should > > have a binding document of its own. > > It really doesn't matter to the DT bindings. > It is not the job of DT to reflect the state of Linux. > > In another operating system they might all be the same driver. > Or all four variants have their own driver. > > If the hardware is distinctly different so a lot of the properties > are unique then it may be warranted with a separate DT > binding, for the sake of keeping bindings simpler and > coherent. Exactly. > > > > + '#interrupt-cells': > > > + description: The internal interrupt controller only supports triggering > > > + on IRQ_TYPE_LEVEL_HIGH > > > + # FIXME: what is this? this should be one cell should it not? > > > + # the Linux mv88e6xxx driver does not implement .irq_set_type in its irq_chip > > > + # so at least in that implementation the type is flat out ignored. > > > + const: 2 > > > > This interrupt controller is for the embedded PHYs. Its is hard wired > > active high. > > Hmm.... I need feedback from the DT people here. It does have a > polarity, but the polarity cannot be changed. So shall we encode this > always the same polarity in the flags cell or skip it altogether? > > I'm uncertain. The currens scheme does reflect a reality. Either way is fine. If users are already doing 2 cells, then I'd probably just keep that and state that the flags cell is ignored/unused. Rob