netdev - Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high' property

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: <ZwPGbUWImzlbEqb-@makrotopia.org>
Date: Mon, 7 Oct 2024 12:30:53 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Pavel Machek <pavel@....cz>, Lee Jones <lee@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Xu Liang <lxu@...linear.com>,
	Christian Marangi <ansuelsmth@...il.com>,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
	Robert Marko <robimarko@...il.com>,
	Russell King <rmk+kernel@...linux.org.uk>,
	Abhishek Chauhan <quic_abchauha@...cinc.com>,
	Jacek Anaszewski <jacek.anaszewski@...il.com>,
	linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/4] dt-bindings: leds: add 'active-high'
 property

On Mon, Oct 07, 2024 at 08:38:27AM +0200, Krzysztof Kozlowski wrote:
> On Sun, Oct 06, 2024 at 02:04:35PM +0100, Daniel Golle wrote:
> > On Sun, Oct 06, 2024 at 02:44:44PM +0200, Krzysztof Kozlowski wrote:
> > > I think this should be just string enum, see marvell,marvell10g.yaml
> > 
> > I found the vendor-specific 'marvell,polarity' property in
> > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231214201442.660447-5-tobias@waldekranz.com/
> > 
> > However, I can't find that file in any Linux tree.
> > 
> > Looking at the suggested patch on patchwork, I got a few questions on
> > how to deal with the situation as of today:
> > 
> > So should the existing support for the 'active-low' and
> > 'inactive-high-impedance' properties be replaced by that string enum?
> > Or should the string property be interpreted in addition to the
> > bools defined in leds/common.yaml?
> > 
> > Should the string property be defined for each PHY or should we move
> > it into a common file?
> > 
> > If so, should that common file also be leds/common.yaml or should we
> > create a new file only for PHY LEDs instead?
> > 
> > Sorry for being confused, I don't mind going down what ever path to have
> > LED polarity configurable properly in DT.
> 
> Let's ignore my idea.
> 
> However I still wonder whether your choice for lack of properties is
> appropriate. Lack of properties as "bootloader default" means it can
> change. Why would anyone prefer to keep bootloader default? The wiring
> is fixed - it's never "we design PCB based on bootloader, so with new
> bootloader we will change PCB"?
> 
> And if you meant bootstrapping through some hardwired configuration,
> then again it is known and defined.

I agree, and my original intention was to just always apply polarity
settings and force people to correctly declare them in DT.
However, that would break DT compatibility on devices not making use
of those properties and relying only on strapping or bootloader
defaults. See also RFC discussed here:

https://patchwork.kernel.org/project/netdevbpf/patch/473d62f268f2a317fd81d0f38f15d2f2f98e2451.1728056697.git.daniel@makrotopia.org/