netdev - Re: [PATCH net-next 4/4] dt-bindings: net: marvell10g: Document LED polarity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3bc3b802-7575-4f0c-b66b-086a767efdb6@lunn.ch>
Date: Sat, 16 Dec 2023 13:25:56 +0100
From: Andrew Lunn <andrew@...n.ch>
To: 20231214201442.660447-5-tobias@...dekranz.com
Cc: Tobias Waldekranz <tobias@...dekranz.com>, davem@...emloft.net,
	kuba@...nel.org, linux@...linux.org.uk, kabel@...nel.org,
	hkallweit1@...il.com, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	netdev@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH net-next 4/4] dt-bindings: net: marvell10g: Document LED
 polarity

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