On Fri, Dec 15, 2023 at 03:22:11PM +0100, Christian Marangi wrote: > > > + properties: > > > + marvell,polarity: > > > + description: | > > > + Electrical polarity and drive type for this LED. In the > > > + active state, hardware may drive the pin either low or > > > + high. In the inactive state, the pin can either be > > > + driven to the opposite logic level, or be tristated. > > > + $ref: /schemas/types.yaml#/definitions/string > > > + enum: > > > + - active-low > > > + - active-high > > > + - active-low-tristate > > > + - active-high-tristate > > > > Christian is working on adding a generic active-low property, which > > any PHY LED could use. The assumption being if the bool property is > > not present, it defaults to active-high. > > > > Hi, it was pointed out this series sorry for not noticing before. > > > So we should consider, how popular are these two tristate values? Is > > this a Marvell only thing, or do other PHYs also have them? Do we want > > to make them part of the generic PHY led binding? Also, is an enum the > > correct representation? Maybe tristate should be another bool > > property? Hi/Low and tristate seem to be orthogonal, so maybe two > > properties would make it cleaner with respect to generic properties? > > For parsing it would make it easier to have the thing split. > > But on DT I feel an enum like it's done here might be more clear. I took a look at a datasheet for a standalone 1G Marvell PHY. It has the same capabilities. So this is something which can be reused by a few devices. So an enum in DT, and an enum for the API to the PHY driver seems like a good idea. I doubt there will be too many more variants, but it does give us an easy way to add more values. Andrew