> > + 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. Assuming the property define the LED polarity, it would make sense to have a single one instead of a sum of boolean. The boolean idea might be problematic in the future for device that devisates from what we expect. Example: A device set the LED to active-high by default and we want a way in DT to define active-low. With the boolean idea of having "active-high" and assume active-low if not defined we would have to put active-high in every PHY node (to reflect the default settings) Having a property instead permits us to support more case. Ideally on code side we would have an enum that map the string to the different modes and we would pass to a .led_set_polarity the enum. (or if we really want a bitmask) If we feel tristate is special enough we can consider leaving that specific to marvell (something like marvell,led-tristate) But if we notice it's more generic then we will have to keep compatibility for both. > > Please work with Christian on this. Think since the current idea is to support this in the LED api with set polarity either the 2 series needs to be merged or the polarity part needs to be detached and submitted later until we sort the generic way to set it? -- Ansuel
