diff mbox

ARM: EXYNOS: Use generic pwm driver in Origen board

Message ID 1344503035-18127-1-git-send-email-tushar.behera@linaro.org
State Superseded
Headers show

Commit Message

Tushar Behera Aug. 9, 2012, 9:03 a.m. UTC
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 arch/arm/mach-exynos/mach-origen.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

Comments

Thierry Reding Aug. 9, 2012, 9:15 a.m. UTC | #1
On Thu, Aug 09, 2012 at 02:33:55PM +0530, Tushar Behera wrote:
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  arch/arm/mach-exynos/mach-origen.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
> index 5ca8030..4c4048d 100644
> --- a/arch/arm/mach-exynos/mach-origen.c
> +++ b/arch/arm/mach-exynos/mach-origen.c
[...]
> @@ -613,6 +614,10 @@ static struct platform_device origen_lcd_hv070wsa = {
>  	.dev.platform_data	= &origen_lcd_hv070wsa_data,
>  };
>  
> +static struct pwm_lookup origen_pwm_lookup[] = {
> +	PWM_LOOKUP("s3c24xx-pwm.0", 0, "pwm-backlight.0", NULL),
> +};
> +

This might conflict with some other patches that Jingoo (Cc'ed) is
working on. His patches were going to rework the Samsung PWM driver to
register multiple PWM devices per chip. In that case the s3c24xx-pwm
device should probably be modified to use .id = -1 and then the .0 can
be dropped from the provider name above.

Otherwise this patch looks good to me.

Thierry
Tushar Behera Aug. 9, 2012, 11:24 a.m. UTC | #2
+ linux-kernel@vger.kernel.org
- linux-kernel@lists.infradead.org

On 08/09/2012 02:45 PM, Thierry Reding wrote:
> On Thu, Aug 09, 2012 at 02:33:55PM +0530, Tushar Behera wrote:
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  arch/arm/mach-exynos/mach-origen.c |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
>> index 5ca8030..4c4048d 100644
>> --- a/arch/arm/mach-exynos/mach-origen.c
>> +++ b/arch/arm/mach-exynos/mach-origen.c
> [...]
>> @@ -613,6 +614,10 @@ static struct platform_device origen_lcd_hv070wsa = {
>>  	.dev.platform_data	= &origen_lcd_hv070wsa_data,
>>  };
>>  
>> +static struct pwm_lookup origen_pwm_lookup[] = {
>> +	PWM_LOOKUP("s3c24xx-pwm.0", 0, "pwm-backlight.0", NULL),
>> +};
>> +
> 
> This might conflict with some other patches that Jingoo (Cc'ed) is
> working on. His patches were going to rework the Samsung PWM driver to
> register multiple PWM devices per chip. In that case the s3c24xx-pwm
> device should probably be modified to use .id = -1 and then the .0 can
> be dropped from the provider name above.
> 
> Otherwise this patch looks good to me.
> 
> Thierry
> 
Thanks for your review.

It was wrong for me to create s3c24xx-pwm platform device as it is
already created in samsung_bl_set(). Hence I would be dropping that.

However, dropping .0 from provider name results in following. (as the
device name is s3c24xx-pwm.N).

[    0.240000] pwm-backlight pwm-backlight.0: unable to request PWM,
trying legacy API

I will resubmit the patch with these changes.
Thierry Reding Aug. 9, 2012, 11:32 a.m. UTC | #3
On Thu, Aug 09, 2012 at 04:54:27PM +0530, Tushar Behera wrote:
> + linux-kernel@vger.kernel.org
> - linux-kernel@lists.infradead.org
> 
> On 08/09/2012 02:45 PM, Thierry Reding wrote:
> > On Thu, Aug 09, 2012 at 02:33:55PM +0530, Tushar Behera wrote:
> >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> >> ---
> >>  arch/arm/mach-exynos/mach-origen.c |   11 +++++++++++
> >>  1 files changed, 11 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
> >> index 5ca8030..4c4048d 100644
> >> --- a/arch/arm/mach-exynos/mach-origen.c
> >> +++ b/arch/arm/mach-exynos/mach-origen.c
> > [...]
> >> @@ -613,6 +614,10 @@ static struct platform_device origen_lcd_hv070wsa = {
> >>  	.dev.platform_data	= &origen_lcd_hv070wsa_data,
> >>  };
> >>  
> >> +static struct pwm_lookup origen_pwm_lookup[] = {
> >> +	PWM_LOOKUP("s3c24xx-pwm.0", 0, "pwm-backlight.0", NULL),
> >> +};
> >> +
> > 
> > This might conflict with some other patches that Jingoo (Cc'ed) is
> > working on. His patches were going to rework the Samsung PWM driver to
> > register multiple PWM devices per chip. In that case the s3c24xx-pwm
> > device should probably be modified to use .id = -1 and then the .0 can
> > be dropped from the provider name above.
> > 
> > Otherwise this patch looks good to me.
> > 
> > Thierry
> > 
> Thanks for your review.
> 
> It was wrong for me to create s3c24xx-pwm platform device as it is
> already created in samsung_bl_set(). Hence I would be dropping that.
> 
> However, dropping .0 from provider name results in following. (as the
> device name is s3c24xx-pwm.N).
> 
> [    0.240000] pwm-backlight pwm-backlight.0: unable to request PWM,
> trying legacy API

That's to be expected. Dropping the .0 requires further changes to the
driver and in arch/arm/plat-samsung/devs.c to register only a single
instance of the s3c24xx-pwm device which can handle all 5 PWM devices.
This should be done separately, though.

Thierry
diff mbox

Patch

diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
index 5ca8030..4c4048d 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -15,6 +15,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/input.h>
+#include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/gpio_keys.h>
 #include <linux/i2c.h>
@@ -613,6 +614,10 @@  static struct platform_device origen_lcd_hv070wsa = {
 	.dev.platform_data	= &origen_lcd_hv070wsa_data,
 };
 
+static struct pwm_lookup origen_pwm_lookup[] = {
+	PWM_LOOKUP("s3c24xx-pwm.0", 0, "pwm-backlight.0", NULL),
+};
+
 #ifdef CONFIG_DRM_EXYNOS
 static struct exynos_drm_fimd_pdata drm_fimd_pdata = {
 	.panel	= {
@@ -681,6 +686,10 @@  static struct platform_device origen_device_bluetooth = {
 	},
 };
 
+static struct platform_device origen_device_pwm = {
+	.name = "s3c24xx-pwm.0",
+};
+
 static struct platform_device *origen_devices[] __initdata = {
 	&s3c_device_hsmmc2,
 	&s3c_device_hsmmc0,
@@ -711,6 +720,7 @@  static struct platform_device *origen_devices[] __initdata = {
 	&origen_lcd_hv070wsa,
 	&origen_leds_gpio,
 	&origen_device_bluetooth,
+	&origen_device_pwm,
 };
 
 /* LCD Backlight data */
@@ -791,6 +801,7 @@  static void __init origen_machine_init(void)
 
 	platform_add_devices(origen_devices, ARRAY_SIZE(origen_devices));
 
+	pwm_add_table(origen_pwm_lookup, ARRAY_SIZE(origen_pwm_lookup));
 	samsung_bl_set(&origen_bl_gpio_info, &origen_bl_data);
 
 	origen_bt_setup();