Re: [PATCH] ARM: EXYNOS: Use generic pwm driver in Origen board

Re: [PATCH] ARM: EXYNOS: Use generic pwm driver in Origen board

[Tushar Behera]

+ 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.

-- 
Tushar Behera

Re: [PATCH] ARM: EXYNOS: Use generic pwm driver in Origen board

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2053 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]