RE: Adding support for configuring polarity in PWM framework.
On Mon, Jul 16, 2012 at 18:16:13, Lars-Peter Clausen wrote: > On 07/16/2012 02:23 PM, Philip, Avinash wrote: > > > > > 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. > > */ > > Isn't this more about whether we start with a low or a high signal? No, it is more about the average on time. On time determines the on duration of the panel power and there by brightness. On one custom board, backlight power booster gives power output on the OFF time of PWM (hardware connection). > If it is > just about the duty time you can easily achieve the same effect by setting > it to (period_ns - duty_ns). Yes this is possible. But the PWM hardware we uses supports configuration of polarity. So I prefer PWM polarity configuration. Also with polarity configuration support, we can achieve (period_ns - duty_ns), for PWM devices don't have hardware support. Polarity configuration support can be used for setting flags, duty cycle can be configured from config depending on flag. Thanks Avinash > > - Lars > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: Adding support for configuring polarity in PWM framework.
On Mon, Jul 16, 2012 at 18:16:08, Thierry Reding wrote: > On Mon, Jul 16, 2012 at 12:23:46PM +, Philip, Avinash wrote: > > On Mon, Jul 16, 2012 at 17:09:21, Thierry Reding wrote: > > > On Mon, Jul 16, 2012 at 11:15:50AM +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Adding support for configuring polarity in PWM framework.
On Mon, Jul 16, 2012 at 12:23:46PM +, Philip, Avinash wrote: > On Mon, Jul 16, 2012 at 17:09:21, Thierry Reding wrote: > > On Mon, Jul 16, 2012 at 11:15:50AM +, 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(). 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. > 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). 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. Thierry pgpvfd9kSArVg.pgp Description: PGP signature
Re: Adding support for configuring polarity in PWM framework.
On 07/16/2012 02:23 PM, Philip, Avinash wrote: > On Mon, Jul 16, 2012 at 17:09:21, Thierry Reding wrote: >> On Mon, Jul 16, 2012 at 11:15:50AM +, 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. > I think this is a common feature amongst PWM chips that they are able to invert the PWM signal. And some applications of PWM require that you are able to specify the polarity, so I think it makes sense to put this into the common framework. >> 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. > > 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. > */ Isn't this more about whether we start with a low or a high signal? If it is just about the duty time you can easily achieve the same effect by setting it to (period_ns - duty_ns). - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: Adding support for configuring polarity in PWM framework.
On Mon, Jul 16, 2012 at 17:09:21, Thierry Reding wrote: > On Mon, Jul 16, 2012 at 11:15:50AM +, 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) > 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. 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); 2. Add "set_polarity" operation support in pwm_ops. 3. Modification in backlight driver (pwm_bl.c) to support polarity configuration. Thanks Avinash > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Adding support for configuring polarity in PWM framework.
On Mon, Jul 16, 2012 at 11:15:50AM +, 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. > 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 and you may be able to implement it in the driver itself instead of putting it into the framework. Anyway I'm interested in seeing your patch. Thierry pgpzsz2NCGUaq.pgp Description: PGP signature