Re: [RFC 7/7] pwm-backlight: Add rudimentary device-tree support

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

[-- Attachment #1.1: Type: text/plain, Size: 2573 bytes --]

* Stephen Warren wrote:
> Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM:
[...]
> > +  - default-brightness: the default brightness setting
> > +  - max-brightness: the maximum brightness setting
> 
> What are the units of those two properties? Percentage seems like a
> reasonable choice, although that's not what the patch implements.

That's just the way the pwm-backlight driver works. Typically the maximum
brightness is set to 255. I think you can set these values to pretty much
anything, and the driver will convert the brightness set via sysfs to the
range from 0 to max-brightness.

> > +Example:
> > +
> > +	backlight {
> > +		compatible = "pwm-backlight";
> > +		default-brightness = <224>;
> > +		max-brightness = <255>;
> > +		pwm = <&pwm 0 5000000>;
> > +	};
> 
> This may be fine, but I'm not sure that representing the backlight as a
> standalone object is correct;

I don't think there really is any other way because we need the device node in
order to have the corresponding platform device instantiated.

> I wonder if you want to represent a complete
> LCD display complex, including backlight, various GPIOs, and other display
> properties, all in the one node? That said, I suppose you could easily
> layer this as follows:
> 
> reg: regulator {
>     // GPIO regulator
> };
> 
> bl: backlight {
>     compatible = "pwm-backlight";
>     default-brightness = <224>;
>     max-brightness = <255>;
>     pwm = <&pwm 0 5000000>;
>     power-supply = <&reg>;
> };
> 
> lcd@x {
>     backlight = <&bl>;
>     ...
> };
> 
> so this probably is OK.

This looks pretty reasonable. I actually like it. I don't think there's any
code to resolve the &bl reference from the lcd driver yet, but that should be
rather easy to do.

> > +	/*
> > +	 * TODO: Most users of this driver use a number of GPIOs to control
> > +	 *       backlight power. Support for specifying these needs to be
> > +	 *       added.
> > +	 */
> 
> At least for the power GPIO, this should probably modeled as a GPIO-based
> fixed voltage regulator. Are there other GPIOs that are directly related
> to a backlight rather than an LCD complex?

Currently some platforms seem to use more than a single GPIO for the power.
PXA/Magician has two, depending on the brightness. Viper for example takes a
shortcut and controls both the backlight power and LCD enable from the
pwm-backlight callbacks. I guess if/when those machines are converted, they
can use a complete LCD complex as you described.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss