[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[Paul Parsons]

Recent changes to PXA PWM support changed the PXA27X PWM device
numbering scheme.

The linux-3.5 PXA PWM driver followed the hardware numbering scheme for
the 4 PWMs, while the linux-3.6-rc1 PXA PWM driver has adopted a linear
numbering scheme:

Address		Hardware	3.5 pwm_id	3.6-rc1 pwm_id
0x40b00000	PWM0		0		0
0x40b00010	PWM2		2		1
0x40c00000	PWM1		1		2
0x40c00010	PWM3		3		3

The hx4700 backlight uses PWM1 at 0x40c00000. Consequently the pwm_id
must be changed from 1 to 2.

Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
Cc: Thierry Reding <thierry.reding@avionic-design.de>
---
  arch/arm/mach-pxa/hx4700.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c
index e631198..3720f99 100644
--- a/arch/arm/mach-pxa/hx4700.c
+++ b/arch/arm/mach-pxa/hx4700.c
@@ -556,7 +556,7 @@ static struct platform_device hx4700_lcd = {
   */

  static struct platform_pwm_backlight_data backlight_data = {
-	.pwm_id         = 1,
+	.pwm_id         = 2,	/* PWM1 */
  	.max_brightness = 200,
  	.dft_brightness = 100,
  	.pwm_period_ns  = 30923,
-- 
1.7.8.6

[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[Vasily Khoruzhick]

On Wed, Sep 26, 2012 at 5:17 AM, Paul Parsons <lost.distance@yahoo.com> wrote:
> Recent changes to PXA PWM support changed the PXA27X PWM device
> numbering scheme.
>
> The linux-3.5 PXA PWM driver followed the hardware numbering scheme for
> the 4 PWMs, while the linux-3.6-rc1 PXA PWM driver has adopted a linear
> numbering scheme:
>
> Address         Hardware        3.5 pwm_id      3.6-rc1 pwm_id
> 0x40b00000      PWM0            0               0
> 0x40b00010      PWM2            2               1
> 0x40c00000      PWM1            1               2
> 0x40c00010      PWM3            3               3

Looks very weird to me. Maybe it would be better to fix pwm numbering
scheme to prevent breakage of
other machines?

Regards
Vasily

> The hx4700 backlight uses PWM1 at 0x40c00000. Consequently the pwm_id
> must be changed from 1 to 2.
>
> Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  arch/arm/mach-pxa/hx4700.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c
> index e631198..3720f99 100644
> --- a/arch/arm/mach-pxa/hx4700.c
> +++ b/arch/arm/mach-pxa/hx4700.c
> @@ -556,7 +556,7 @@ static struct platform_device hx4700_lcd = {
>   */
>
>  static struct platform_pwm_backlight_data backlight_data = {
> -       .pwm_id         = 1,
> +       .pwm_id         = 2,    /* PWM1 */
>         .max_brightness = 200,
>         .dft_brightness = 100,
>         .pwm_period_ns  = 30923,
> --
> 1.7.8.6
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[Paul Parsons]

Hello Vasily,

On 26/09/12 07:40, Vasily Khoruzhick wrote:
> On Wed, Sep 26, 2012 at 5:17 AM, Paul Parsons <lost.distance@yahoo.com> wrote:
>> Recent changes to PXA PWM support changed the PXA27X PWM device
>> numbering scheme.
>>
>> The linux-3.5 PXA PWM driver followed the hardware numbering scheme for
>> the 4 PWMs, while the linux-3.6-rc1 PXA PWM driver has adopted a linear
>> numbering scheme:
>>
>> Address         Hardware        3.5 pwm_id      3.6-rc1 pwm_id
>> 0x40b00000      PWM0            0               0
>> 0x40b00010      PWM2            2               1
>> 0x40c00000      PWM1            1               2
>> 0x40c00010      PWM3            3               3
>
> Looks very weird to me. Maybe it would be better to fix pwm numbering
> scheme to prevent breakage of
> other machines?

I thought the same thing. However the code doesn't make it easy:

1. The definition of pwm_id values moved from the pwm_probe() in
arch/arm/plat-pxa/pwm.c to pwmchip_add() in drivers/pwm/core.c.
Changing the latter generic code would impact all of the PWM drivers,
not just the PXA PWM driver.

2. Changing just the PXA PWM driver would require its pwm_ops handlers
(config(), enable(), disable()) to quietly translate between PWM1 and
PWM2 devices. However PWM1 and PWM2 are on different chips (i.e.
pwm_chip structures). So what would the driver do when there is only
one chip? i.e.:
a) Between the first and second calls to pwm_probe().
b) Between the first and second calls to pwm_remove().
And in any case quietly translating between PWM1 and PWM2 just doesn't
seem like a good idea: sooner or later it will quietly break the
generic code.

On balance it seemed preferable to change the platform code.

Regards,
Paul

[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[Paul Parsons]

Hello Thierry,

On 26/09/12 07:03, Thierry Reding wrote:
> On Wed, Sep 26, 2012 at 03:15:06AM +0100, Paul Parsons wrote:
>> Recent changes to PXA PWM support changed the PXA27X PWM device
>> numbering scheme.
>>
>> The linux-3.5 PXA PWM driver followed the hardware numbering scheme for
>> the 4 PWMs, while the linux-3.6-rc1 PXA PWM driver has adopted a linear
>> numbering scheme:
>>
>> Address		Hardware	3.5 pwm_id	3.6-rc1 pwm_id
>> 0x40b00000	PWM0		0		0
>> 0x40b00010	PWM2		2		1
>> 0x40c00000	PWM1		1		2
>> 0x40c00010	PWM3		3		3
>>
>> The hx4700 backlight uses PWM1 at 0x40c00000. Consequently the pwm_id
>> must be changed from 1 to 2.
>>
>> Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>> ---
>>   arch/arm/mach-pxa/hx4700.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> I wasn't aware of this difference. Sorry for breaking your board. While
> at it, would you mind converting this to the new style PWM lookup table?
>
> You can find the information in Documentation/pwm.txt, but it should be
> roughly something like this:
>
> 	static struct pwm_lookup hx4700_pwm_lookup[] = {
> 		PWM_LOOKUP("pxa27x-pwm.1", 0, "pwm-backlight", NULL),
> 	};
>
> You can then call:
>
> 	pwm_add_table(hx4700_pwm_lookup, ARRAY_SIZE(hx4700_pwm_lookup));
>
> from hx4700_init(). The pwm-backlight driver will then be able to use
> the new pwm_get() to obtain the PWM device. Note that pwm_request() will
> go away sometime soon, so this conversion will have to be done anyway.
> If you do it now I won't have to take care of it anymore. =)

OK, will do.

BTW, it is possible that the changed numbering scheme will also have
broken three other PXA27X/PXA3XX boards which use PWM1 or PWM2:

cm-x300
tavorevb
z2

Unfortunately I have no way of testing these.

Regards,
Paul

[PATCH] ARM: pxa: hx4700: Fix backlight PWM device number

[Vasily Khoruzhick]

On Wed, Sep 26, 2012 at 2:01 PM, Paul Parsons <lost.distance@yahoo.com> wrote:
> Hello Thierry,
>
> On 26/09/12 07:03, Thierry Reding wrote:
>>
>> On Wed, Sep 26, 2012 at 03:15:06AM +0100, Paul Parsons wrote:
>>>
>>> Recent changes to PXA PWM support changed the PXA27X PWM device
>>> numbering scheme.
>>>
>>> The linux-3.5 PXA PWM driver followed the hardware numbering scheme for
>>> the 4 PWMs, while the linux-3.6-rc1 PXA PWM driver has adopted a linear
>>> numbering scheme:
>>>
>>> Address         Hardware        3.5 pwm_id      3.6-rc1 pwm_id
>>> 0x40b00000      PWM0            0               0
>>> 0x40b00010      PWM2            2               1
>>> 0x40c00000      PWM1            1               2
>>> 0x40c00010      PWM3            3               3
>>>
>>> The hx4700 backlight uses PWM1 at 0x40c00000. Consequently the pwm_id
>>> must be changed from 1 to 2.
>>>
>>> Signed-off-by: Paul Parsons <lost.distance@yahoo.com>
>>> Cc: Thierry Reding <thierry.reding@avionic-design.de>
>>> ---
>>>   arch/arm/mach-pxa/hx4700.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>>
>> I wasn't aware of this difference. Sorry for breaking your board. While
>> at it, would you mind converting this to the new style PWM lookup table?
>>
>> You can find the information in Documentation/pwm.txt, but it should be
>> roughly something like this:
>>
>>         static struct pwm_lookup hx4700_pwm_lookup[] = {
>>                 PWM_LOOKUP("pxa27x-pwm.1", 0, "pwm-backlight", NULL),
>>         };
>>
>> You can then call:
>>
>>         pwm_add_table(hx4700_pwm_lookup, ARRAY_SIZE(hx4700_pwm_lookup));
>>
>> from hx4700_init(). The pwm-backlight driver will then be able to use
>> the new pwm_get() to obtain the PWM device. Note that pwm_request() will
>> go away sometime soon, so this conversion will have to be done anyway.
>> If you do it now I won't have to take care of it anymore. =)
>
>
> OK, will do.
>
> BTW, it is possible that the changed numbering scheme will also have
> broken three other PXA27X/PXA3XX boards which use PWM1 or PWM2:
>
> cm-x300
> tavorevb
> z2
>
> Unfortunately I have no way of testing these.
>
> Regards,
> Paul

I have Z2, looks like it's time to test 3.6-rcs on it :)

Regards
Vasily