linux-kernel - 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: <ZwaF_VNG3TE33rfb@makrotopia.org>
Date: Wed, 9 Oct 2024 14:32:45 +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 12:30:53PM +0100, Daniel Golle wrote:
> 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/
> 

I see that the series was marked as "Not Applicable" in patchwork.
What is the reason for that? To me it looks like it can be applied on
today's net-next cleanly:

[daniel@box linux.git]$ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
>From https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
 * branch                      HEAD       -> FETCH_HEAD
[daniel@box linux.git]$ git checkout FETCH_HEAD
HEAD is now at 6607c17c6c5e net: mana: Enable debugfs files for MANA device
[daniel@box linux.git]$ wget -q -O - https://patchwork.kernel.org/series/895863/mbox/ | git am
Applying: dt-bindings: leds: add 'active-high' property
Applying: net: phy: support 'active-high' property for PHY LEDs
Applying: net: phy: aquantia: correctly describe LED polarity override
Applying: net: phy: mxl-gpy: correctly describe LED polarity
[daniel@box linux.git]$ 

Or did I misunderstand the meaning of "Not Applicable"? If so, please
clarify.