Re: Strange location and name for platform devices when device-tree is used.

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4698 bytes --]

On Sat, Nov 02, 2013 at 10:45:05AM +1100, NeilBrown wrote:
> On Sat, 02 Nov 2013 10:10:25 +1100 Benjamin Herrenschmidt
> <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote:
> 
> > On Fri, 2013-11-01 at 13:47 -0700, Greg Kroah-Hartman wrote:
> > 
> > > > > On my device I seem to have some platform devices registered through
> > > > > device-tree, and some registered through platform_device_add (e.g.
> > > > > 'alarmtimer').  Guaranteeing they remain disjoint sets if the kernel is
> > > > > allowed to evolve independently of the devicetree might be tricky....
> > > > > Maybe we need "/sys/devices/platform" and "/sys/devices/dt_platform" ??
> > > > 
> > > > No, I think device-tree created platform devices should go
> > > > to /sys/devices/platform like the "classic" ones.
> > > > 
> > > > The problem is really how to deal with potential name duplication. We
> > > > could try to register, if we get -EEXIST (assuming sysfs returns the
> > > > right stuff), try again with ".1" etc...
> > > 
> > > How can there be device name collisions?  All platform devices _should_
> > > be named uniquely, if not, you have bigger problems...
> > 
> > The problem is how to create a unique name for a platform device created
> > from a device-tree node.
> > 
> > Device tree nodes aren't necessarily uniquely named. They are unique
> > under a given parent but that hierarchy isn't preserved when creating
> > corresponding platform devices (and it would be very tricky to do so).
> > 
> > Currently, we simply append a number to the name when creating them,
> > which is obtained from a global counter.
> > 
> > Neil is unhappy about that because on his specific hardware, the device
> > has a unique name and thus we introduce a naming difference between
> > device-tree usage and old-style "hard coded" board file usage.
> 
> It occurs to me that a different approach could solve my problem.
> 
> My problem stems from the fact that the name of the device on the
> platform-bus is used as the name of the device in the "backlight" class.
> 
> As Greg writes elsewhere, depending on names with /sys/devices is not
> supported - we need to accept that bus-names might change.
> However names in class devices tend to be a lot more stable.
> Several devices allow these to be explicitly set.
>  leds have 'label'
>  regulators has "regulator-name"
>  gpio-keys has 'label'.
> 
> I could just teach pwm_bl to allow a 'label' property which would be used in
> place of the platform-bus device name when creating the class/backlight
> device.
> 
> The maxim "you cannot trust names to remain stable in /sys/devices" can
> justify both the movement of platform devices into /sys/devices/platform, and
> the use of "label" rather than the device-name for creating the class device.
> 
> Does that sound convincing?

I see how this is a general problem, but since you've brought up the
topic of backlights, let me reply to that.

The correct thing, in my opinion, to do here would be to associate the
backlight with whatever device it is attached to. Typically this would
be a display. fbdev has some support for that already, and if I read the
code correctly then it automatically associates any backlight device
with any fbdev device (which obviously breaks once you start having more
of each, but that's probably not a typical use-case).

In my experience associating a backlight with its display and letting
the display driver control the backlight is the only sane solution. On
one hand it makes the association explicit, so userspace doesn't have to
hardcode any sysfs filenames to access the backlight. Furthermore only
the display driver knows when it's safe to turn on and off the backlight
since it controls the flow of pixels to the display.

Currently the Linux backlight framework turns on the backlight right
when it is registered (or more precisely that's what each individual
driver does). That means depending on the probe order the backlight will
light up before there's any meaningful content on the display. In many
cases that may turn out to be just black, but in other cases it may be
random content.

However, I'm not aware of any way to control the backlight from an fbdev
device in userspace. fbdev is also mostly considered obsolete these days
and I've been working on integrating backlight and panels with DRM for
quite a while now. That's also still missing a way to control the
backlight brightness via DRM IOCTLs, but it's on my TODO.

That probably doesn't help with the overall situation, but at least I
think it's what we should be doing for backlight devices.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]