On Mon, Jul 16, 2012 at 18:16:08, Thierry Reding wrote:
> On Mon, Jul 16, 2012 at 12:23:46PM +0000, Philip, Avinash wrote:
> > On Mon, Jul 16, 2012 at 17:09:21, Thierry Reding wrote:
> > > On Mon, Jul 16, 2012 at 11:15:50AM +0000, Philip, Avinash wrote:
> > > > Hi Thierry,
> > > >
> > > > On one of the custom boards we are using, uses PWM to drive the
> > > > backlight. However, for
> > > > this device, PWM signal needs to be inversed.
> > > > So, we need to a platform data to indicate this parameter.
> > > > Current PWM framework doesn't provide .support for setting polarity (or
> > > > inverse polarity).
> > > >
> > > > Have you come across any such requirements? If so, do you have any
> > > > plans to implement it?
> > >
> > > I don't have any plans to implement such a feature.
> >
> > Ok. Thanks for the quick response.
> > >
> > > > I am planning to add support for the same but want to avoid duplication
> > > > of work.
> > > >
> > > > If you have no plans, then I will send a patch to support the same.
> > >
> > > I wonder how you want to implement this. You'll need special hardware
> > > support for it
> >
> > Yes. Our custom hardware (backlight booster) requires the pwm signal to be
> > inverted.
> >
> > > you may be able to implement it in the driver itself
> > > instead of putting it into the framework.
> >
> > This is a client specific data (backlight needs pwm signal inversed)
> > and not the main device feature (not PWM IP). So we cannot send this in
> > pwm platform data. This would come as call from client driver (which in
> > our case is from pwm_bl.c)
>
> Okay, I see.
>
> > > Anyway I'm interested in seeing your patch.
> >
> > I am planning to modify PWM framework as below.
> > 1. Configure PWM polarity from client driver (using platform data provided
> > to pwm backlight driver).
> > 2. PWM device needs to be disabled before calling the set-polarity API.
>
> Okay, that sounds sensible. A couple of comments though.
>
> > This involves
> >
> > 1. PWM framework API addition.
> > PWM frame work API support.
> > /**
> > * pwm_setpolarity() - change a PWM device Polarity
> > * @pwm: PWM device
> > * @polarity: Configure polarity of PWM
> > *
> > * polarity - false -> "on" time defined by duty ns
> > * - true -> "off' time defined by duty ns.
> > */
> > int pwm_setpolarity(struct pwm_device *pwm, bool inversepol);
>
> This should match the pwm_ops name, i.e.: pwm_set_polarity().
Ok.
>
> Making the polarity argument a boolean is slightly confusing. For
> instance I'd say the logical value if I want normal behaviour would be
> to set it to true, which doesn't match your example. So I propose you
> define the polarity parameter as an enumeration to make its meaning more
> explicit:
>
> enum pwm_polarity {
> PWM_POLARITY_NORMAL,
> PWM_POLARITY_INVERSE,
> };
>
> PWM_POLARITY_{HIGH,LOW} and PWM_POLARITY_{POSITIVE,NEGATIVE} would be
> other good name pairs.
Ok. I will do.
>
> > 2. Add "set_polarity" operation support in pwm_ops.
> >
> > 3. Modification in backlight driver (pwm_bl.c) to support polarity
> > configuration.
>
> We also need to think about how this could be represented in the device
> tree. The most obvious choice seems to be a third cell for the specifier
> and use a custom of_xlate callback for controllers that support polarity
> inversion (and later perhaps other flags).
>
Ok I will try to use modified of_xlate callback and hope this can
rescue pwm_bl.c modification.
> Also would you mind sharing the board setup code that you need this for?
> I find it easier to get into the right mindset when looking at code that
> actually uses this.
>
Here is the TI BSP link to support backlight inverse.
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;h=
59e96b24925e64fffd4664d696e41e1090c959b1;hp=
b180dcb341db0ff4ca1adbfac3f5dcd07be9e91d
But here we were Supporting PWM frame work from Bill Gatliff.
Also we were configuring PWM polarity directly using eCAP platform data
not from backlight platform data.
But I think controlling through device/client is more methodical than
direct PWM data handling.
Thanks
Avinash
> Thierry
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
