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

[Daniel Golle]

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@xxxxxxxxxxxxxx/
> > > 
> > > 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@xxxxxxxxxxxxxx/
> 

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.