netdev - Re: [PATCH RFC net-next] net: phy: always set 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: <5c821b2d-17eb-4078-942f-3c1317b025ff@lunn.ch>
Date: Fri, 4 Oct 2024 22:59:45 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Daniel Golle <daniel@...rotopia.org>
Cc: 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>,
	Christian Marangi <ansuelsmth@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next] net: phy: always set polarity_modes if op
 is supported

On Fri, Oct 04, 2024 at 04:46:33PM +0100, Daniel Golle wrote:
> Some PHYs drive LEDs active-low by default and polarity needs to be
> configured in case the 'active-low' property is NOT set.
> One way to achieve this without introducing an additional 'active-high'
> property would be to always call the led_polarity_set operation if it
> is supported by the phy driver.

It is a good idea to state why it is RFC. What comments do you want?

So this potentially causes regressions/change in behaviour. Strapping
or the bootloader could set the polarity, and Linux leaves it alone up
until this patch.

The device tree binding says:

  active-low:
    type: boolean
    description:
      Makes LED active low. To turn the LED ON, line needs to be
      set to low voltage instead of high.

There is no mention that the absence of this property means it is
active-high.

In effect, i think we have a tristate, active-low, active-high, don't
touch.

I think adding an active-high property is probably the safest bet,
even if it is more work.

	Andrew