Re: [PATCH v2 2/2] backlight: add new lp8788 backlight driver

[Thierry Reding]

On Thu, Jan 03, 2013 at 08:54:27AM +0000, Kim, Milo wrote:
> Hi Thierry,
> 
> > > (Platform data)
> > > Configurable data can be defined in the platform side.
> > >  name                  : backlight driver name. (default: "lcd-
> > backlight")
> > >  initial_brightness    : initial value of backlight brightness
> > >  bl_mode               : brightness control by PWM or lp8788 register
> > >  dim_mode              : dimming mode selection
> > >  full_scale            : full scale current setting
> > >  rise_time             : brightness ramp up step time
> > >  fall_time             : brightness ramp down step time
> > >  pwm_pol               : PWM polarity setting when bl_mode is PWM
> > based
> > 
> > You might want to consider using enum pwm_polarity from linux/pwm.h
> > instead and convert to the driver representation internally.
> > 
> > I'm saying this because I'm thinking about extending the PWM framework
> > to allow PWM polarity and period to be specified in the PWM lookup
> > table
> > so that they can be treated transparently, independent of whether they
> > are obtained from DT or the lookup table.
> > 
> > That would allow the polarity and period to be retrieved with accessors
> > like pwm_get_polarity() and pwm_get_period().
> 
> OK, pwm_pol will be replaced with pwm_get_polarity() in the next patch.

No, you can't do that yet because it will only work if you get the PWM
from DT. I meant that once the pieces in the PWM framework are there we
could encode the polarity in the PWM lookup table and *then* use
pwm_get_polarity() to obtain it instead of via platform data.

For now, I think you should only convert it to enum pwm_polarity so that
at least the data type is the same as the one used in the PWM framework.

Thierry

Attachment: pgp00000.pgp
Description: PGP signature