Re: [PATCH v3 3/3] pwm: New driver to support PWM driven LEDs onTWL4030/6030 series of PMICs

[Peter Ujfalusi]

On 11/23/2012 04:04 PM, Thierry Reding wrote:
> On Tue, Nov 20, 2012 at 10:56:22AM +0100, Peter Ujfalusi wrote:
>> The driver supports the following LED outputs as generic PWM driver:
>> TWL4030 LEDA and LEDB (PWMA and PWMB)
>> TWL6030 Charging indicator LED (PWM LED)
>>
>> On TWL6030 when the PWM requested LED is configured to be controlled by SW.
>> In this case the user can enable/disable and set the duty period freely.
>> When the PWM has been freed, the LED driver is put back to HW control.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
>> ---
>>  drivers/pwm/Kconfig       |  10 ++
>>  drivers/pwm/Makefile      |   1 +
>>  drivers/pwm/pwm-twl-led.c | 303 ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 314 insertions(+)
>>  create mode 100644 drivers/pwm/pwm-twl-led.c
> 
> Doesn't this belong in the drivers/leds subsystem? Besides that, the
> same comments as for the previous patch apply. One additional note
> below.

The PINs itself are called as LED but they are PWMs at the end. If we
represent them as PWMs they can be used for different purposes which is going
to be needed for example in BeagleBoard, where the LEDA (PWMA) is used as a
GPO to enable/disable the USB host power.
Also the removed 'twl6030-pwm' driver was only controlled the LED part of twl6030.
With this series I enable the use of the PWMs and the PWMs behind of the LED
functions to give us flexibility on how we are using them.

> 
>> +static struct platform_driver twl_pwmled_driver = {
>> +	.driver = {
>> +		.name = "twl-pwmled",
>> +		.of_match_table = of_match_ptr(twl_pwmled_of_match),
>> +	},
>> +	.probe = twl_pwmled_probe,
>> +	.remove = __devexit_p(twl_pwmled_remove),
> 
> You didn't annotate twl_pwmled_remove() with __devexit, so __devexit_p
> isn't needed here either.

Oh yes, I have also received patches from a series which removes the
_devexit_p() from the kernel.
But should the __devexit need to be added to the remove function?

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/