linux-kernel - Re: [net-next PATCH 1/2] dt-bindings: Document QCA808x PHYs

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]

Date:   Mon, 11 Dec 2023 18:57:50 +0100
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH 1/2] dt-bindings: Document QCA808x PHYs

On Mon, Dec 11, 2023 at 06:49:02PM +0100, Andrew Lunn wrote:
> On Mon, Dec 11, 2023 at 06:19:46PM +0100, Christian Marangi wrote:
> > On Mon, Dec 11, 2023 at 04:54:12PM +0100, Andrew Lunn wrote:
> > > > Mhhh with a generic property and LED core or phylib handling it... How
> > > > it would work applying that setting on PHY side?
> > > 
> > > Add a .led_set_polarity callback to the PHY driver structure?
> > > 
> > > Take a look at other LED drivers. Does anything similar already exist?
> > > It is unlikely that PHYs are the only sort of LED to have a polarity.
> > >
> > 
> > Interesting topic... With a quick grep on Documentation for polarity of
> > high, I can't find any use of it...
> 
> As i said, active-high is the default. So there is no need to specify
> it. But if you look in Documentation/devicetree/binding/leds for
> 'active-low' you will find a few examples.
>

Yes I was searching more and I just notice active-low and led-active-low
usage for bcm6358.

> > Also main problem is that the thing is controlled globally and not per
> > LED. (can be handled internally to the driver with some priv and check
> > magic)
> 
> Ah, missed that. Marvell PHYs have polarity per LED.
> 
> It would be better to describe this correctly, so one property at a
> higher level. We can then in the future add an 'active-low' property
> per PHY.
> 
> > Is it worth to impemement the additional API to control this? And I
> > guess a egenric binding should be added to ethernet-phy? Or should it be
> > added to LEDs?
> 
> Since it is above individual LEDs, i would not add it to the generic
> LED binding. But it could go inside the leds object of
> ethernet-phy.yaml.
> 
>            leds {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
> 		active-low;
> 
>                 led@0 {
>                     reg = <0>;
>                     color = <LED_COLOR_ID_WHITE>;
>                     function = LED_FUNCTION_LAN;
>                     default-state = "keep";
>                 };
>

Ok! And I guess the additional API will (initially to be later expanded
for other usage?) take this value and call the set polarity based on
this correct? 

bool active_low = of_property_read_bool(leds_node, "active-low");

.led_set_polarity(struct phy_device *phydev, bool active_low);


Maybe a more flexible approach might be scan for both. (either in leds
node or in the led subnode)

.led_set_polarity(struct phy_device *phydev, int index, bool active_low);

Where index is -1 if it's global and the led index if it's in the led
node?

PHY driver will know what to ignore/use as I can't immagine to have a
PHY that have both global and per LED polarity. What do you think?

-- 
	Ansuel