[RFC PATCH] ath79: remove model name from LED labels

[Martin Blumenstingl]

Hi Adrian,

On Wed, Sep 30, 2020 at 9:11 PM Adrian Schmutzler
<mail at adrianschmutzler.de> wrote:
>
> Hi Martin,
>
> > -----Original Message-----
> > From: Martin Blumenstingl [mailto:martin.blumenstingl at googlemail.com]
> > Sent: Mittwoch, 30. September 2020 20:55
> > To: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> > Cc: openwrt-devel at lists.openwrt.org
> > Subject: Re: [RFC PATCH] ath79: remove model name from LED labels
> >
> > Hi Adrian,
> >
> > On Sat, Sep 26, 2020 at 6:33 PM Adrian Schmutzler
> > <freifunk at adrianschmutzler.de> wrote:
> > [...]
> > > Apart from our needs, upstream has deprecated the label property
> > > entirely and introduced new properties to specify color and function
> > > properties separately. However, the implementation does not appear to
> > > be ready and probably won't become ready and/or match our
> > requirements
> > > in the foreseeable future.
> > I recently did an experiment with the color and function properties and it's
> > working well for me with my F9K1115V2 to make it work I had to come up
> > with a (simple) patch that parses the "color" property: [0]
> >
> > there's no feedback in this mail - my goal is to understand what the concerns
> > are so I don't "just" start porting some of my boards to the new way that's
> > recommended upstream only to see this patch then being rejected in
> > OpenWrt.
>
> My initial idea was to go the same way; have a look at these two comment where I summarized the response from upstream and why I consider that not helpful:
>
> https://github.com/openwrt/openwrt/pull/3437#issuecomment-696377807
> https://github.com/openwrt/openwrt/pull/3437#issuecomment-699492366
Thanks for the background info!

> Apart from that, I'm aware that we obviously have the option to _partially_ implement the upstream solution, i.e. use the color ids and just put the last part of the former label into function. However, as discussed in the referenced comments, this would essentially be an abuse of "function" as far as upstream sees it: We should create the corresponding LED_FUNCTION entries instead of putting strings into "function" directly, and my proposals for adding some of them were rejected.
> Of course, we could just ignore that, and put into function what we like. That would work perfectly, it's just a little more complicated to implement in user-space as your patch shows. Nevertheless, I just hesitate to implement a system against its intention (color/function) when we have an alternate solution that we can use as intended (label).
I see

> Apart from that, as stated in the referenced comments, your solution will break anyway when kernel starts to use the devicename strings again for internal "devices" like phy0/wlan0/etc. as proposed in the discussion at linux-leds (e.g. wlan0:green:activity instead of green:wlan2g).
it will break again for anything that requires setting the default
trigger from userspace
my hope is that most LEDs will not need userspace handling for their
default behavior anymore, for example all LAN or wifi activity LEDs.
I'm not sure about corner-cases though (for example: 2GHz wifi
specific WPS LED- if that's used anywhere)

> As I said in the PR, unfortunately all of this is essentially very unsatisfying at the moment. So, what I plan to do with the labels is as much improvement as I can get for now.
my personal opinion: I would skip this intermediate step and work with
upstream to implement the LED-triggers where needed. Then backport
these patches and switch to that ABI. Rafał did this with
ledtrig-usbport
that is because I'm not sure how hard the migration for this
intermediate step is. if much effort is required then I'd rather spend
time on something that will be usable in the future rather than
spending time on some temporary solution


Best regards,
Martin