Re: [PATCH] platform/chrome: Add Wilco EC keyboard backlight LEDs support

[Dmitry Torokhov]

On Thu, Apr 4, 2019 at 1:11 PM Pavel Machek <pavel@xxxxxx> wrote:
>
> On Thu 2019-04-04 13:07:39, Dmitry Torokhov wrote:
> > On Thu, Apr 4, 2019 at 12:23 PM Pavel Machek <pavel@xxxxxx> wrote:
> > >
> > > Hi!
> > >
> > > > > > > Yeah, well, we not let the cros_kbd_led_backlight.c use chromeos:: in
> > > > > > > the first place. But it happened. We want all backlights for the
> > > > > > > system keyboard to use common name, and "chromeos" is not really
> > > > > > > suitable for that. "platform" is.
> > > > > >
> > > > > > Pavel, who exactly wants this and why? Looking at today's -next I see:
> > > > > >
> > > > > > dtor@dtor-ws:~/kernel/linux-next ((next-20190404))$ git grep
> > > > > > "::kbd_backlight" | wc -l
> > > > > > 18
> > > > > > dtor@dtor-ws:~/kernel/linux-next ((next-20190404))$ git grep
> > > > > > "platform::kbd_backlight" | wc -l
> > > > > > 0
> > > > > >
> > > > > > so there isn't a single instance of "platform::kbd_backlight" and we
> > > > > > definitely not changing existing names.
> > > > >
> > > > > Yeah, we made mistakes in the past. We _don't_ want userspace to have
> > > > > ever growing list of names for userspace to follow.
> > > > >
> > > > > Backlight of internal keyboard is pretty common concept and there
> > > > > should be one name for it.
> > > >
> > > > It is the *function* that is interesting to userspace, not full name,
> > > > and we have proper standardization there.
> > >
> > > Well, if full name is not interesting, as you argue, why do we have
> > > this discussion?
> >
> > Because I need to understand why you believe that device name for
> > kbd_backlight matters, and having wilco::kbd_backlight is a bad idea,
> > but, for example, having max77650::kbd_backlight is perfectly fine if
> > somebody decided to wire it in this way.
>
> max77650::kbd_backlight is not fine and we'll try to prevent that in
> future.

You do not control DTS for systems though.

>
> We want one name for internal keyboard backlight. What exactly that
> name is is not _that_ important, but platform::kbd_backlight seems
> like reasonable choice.

And I am trying to show that depending on device and product (as in
entire computing device) the same driver could be used in multitude of
ways and expecting that all devices that can be internal will always
have "platform::" prefix is not realistic. It will also fail if you
have multiples of devices, as you need unique names, and that is what
<device> component in name provides you with.

You need smarter userspace to implement policy that is best suited for
your product. Maybe you can help it by adding additional properties to
LED devices, like we have connection_type in USB ports, to tell
whether device is internal or not, but I'd leave the naming alone.


Thanks.

-- 
Dmitry