[PATCH v3 7/7] ipq806x: Initial TP-Link and ASUS OnHub support

[Brian Norris]

On Thu, Jan 12, 2023 at 9:48 AM Christian Marangi <ansuelsmth at gmail.com> wrote:
> On Thu, Jan 12, 2023 at 09:35:03AM -0800, Brian Norris wrote:
> > On Thu, Jan 12, 2023 at 6:34 AM Christian Marangi <ansuelsmth at gmail.com> wrote:
> > Downside: IIUC, this will no longer reflect in the UCI configuration,
> > and so it's less obvious how to override things. Especially when, like
> > the TP-Link version, you have 18 (!!) LEDs (or, 6 x 3-color LEDs), and
> > you have to guess as to which ones to override. But I can live with
> > that if that's deemed better.
>
> It's not needed to have an UCI conf reflecting the state... If someone
> needs to configure the led the current configuration is ignored anyway
> as it will be changed to what the user wants.

Right, but with the ucidef approach, the default state will be in the
UCI configuration already, and so the user only has to delete (or
rewrite) entries to turn off (or change) LEDs. With defaults in the
DTS, there's no default UCI entry, and the user has to learn which of
the 18 LEDs they want to override. For the TP-Link ring especially,
this could be non-obvious. Maybe that'd get better if (L)UCI did a
better job with multi-color LEDs, so there'd only be 6 things to
configure instead of 18.

> >
> > > Also I think we should use a more
> > > descriptive name.
> >
> > Sure. What do you think would be better? For ASUS, it's only a single
> > (set of) LEDs, so maybe I could go with "{red,green,blue}:status"
> > naming like I see on some others.
>
> Yes the pattern is color:function... Ideally everything should be
> handled with standard linux binding instead of declaring a label...
>
> So using function-enumerator, color and function.
> If something is not here [1] then label are required
>
> [1] https://elixir.bootlin.com/linux/latest/source/include/dt-bindings/leds/common.h

I'll play around with this. Last I checked, the lp5523 driver didn't
end up with very nice names without specifying a "chan-name" and/or
"label" (again, individual drivers seem to have a lot of leeway
despite the "common" bindings), but I'll check again later today.

Brian