diff mbox

pwm: pxa: add device tree support to pwm driver

Message ID 1378236233-15637-1-git-send-email-mikedunn@newsguy.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Dunn Sept. 3, 2013, 7:23 p.m. UTC
This patch adds device tree support to the pxa's pwm driver.  Only an OF match
table is added; nothing needs to be extracted from the device tree node.  The
existing platform_device id table is reused for the match table data.

Tested on a Palm Treo 680 (both platform data and DT cases).

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

I don't have data sheets handy for the newer Marvell pxa's (and I'm confused
about the presence of a pwm unit on pxa25x), so only pxa27x.dtsi was updated.
Thanks for looking!

 arch/arm/boot/dts/pxa27x.dtsi | 12 ++++++++++++
 drivers/pwm/pwm-pxa.c         | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Marek Vasut Sept. 3, 2013, 10:20 p.m. UTC | #1
Dear Mike Dunn,

> This patch adds device tree support to the pxa's pwm driver.  Only an OF
> match table is added; nothing needs to be extracted from the device tree
> node.  The existing platform_device id table is reused for the match table
> data.
> 
> Tested on a Palm Treo 680 (both platform data and DT cases).
> 
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> ---
> 
> I don't have data sheets handy for the newer Marvell pxa's (and I'm
> confused about the presence of a pwm unit on pxa25x), so only pxa27x.dtsi
> was updated. Thanks for looking!
> 
>  arch/arm/boot/dts/pxa27x.dtsi | 12 ++++++++++++
>  drivers/pwm/pwm-pxa.c         | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi
> index d7c5d72..4031dce 100644
> --- a/arch/arm/boot/dts/pxa27x.dtsi
> +++ b/arch/arm/boot/dts/pxa27x.dtsi
> @@ -10,5 +10,17 @@
>  			marvell,intc-priority;
>  			marvell,intc-nr-irqs = <34>;
>  		};
> +
> +		pwm0: pwm@40b00000 {
> +			compatible = "marvell,pxa27x-pwm";
> +			reg = <0x40b00000 0x100000>;
> +			#pwm-cells = <2>;
> +		};
> +
> +		pwm1: pwm@40c00000 {
> +			compatible = "marvell,pxa27x-pwm";
> +			reg = <0x40c00000 0x100000>;
> +			#pwm-cells = <2>;
> +		};
>  	};
>  };
> diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> index a4d2164..be5f914 100644
> --- a/drivers/pwm/pwm-pxa.c
> +++ b/drivers/pwm/pwm-pxa.c
> @@ -19,6 +19,7 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/pwm.h>
> +#include <linux/of_device.h>
> 
>  #include <asm/div64.h>
> 
> @@ -124,6 +125,30 @@ static struct pwm_ops pxa_pwm_ops = {
>  	.owner = THIS_MODULE,
>  };
> 
> +#ifdef CONFIG_OF
> +/* use the platform_device id table for OF match table data */
> +static struct of_device_id pwm_of_match[] = {
> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pwm_of_match);

Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why not just 
stick with pxa25x-pwm only for all of the CPUs (aka. the lowest CPU model). Then 
the table would have but a single entry.

[...]

Best regards,
Marek Vasut
Mike Dunn Sept. 4, 2013, 2:23 p.m. UTC | #2
On 09/03/2013 03:20 PM, Marek Vasut wrote:

[...]

>>
>> +#ifdef CONFIG_OF
>> +/* use the platform_device id table for OF match table data */
>> +static struct of_device_id pwm_of_match[] = {
>> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
>> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
>> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
>> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, pwm_of_match);
> 
> Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why not just 
> stick with pxa25x-pwm only for all of the CPUs (aka. the lowest CPU model). Then 
> the table would have but a single entry.


I'm just echoing the existing platform_device_id table...

static const struct platform_device_id pwm_id_table[] = {
	/*   PWM    has_secondary_pwm? */
	{ "pxa25x-pwm", 0 },
	{ "pxa27x-pwm", HAS_SECONDARY_PWM },
	{ "pxa168-pwm", 0 },
	{ "pxa910-pwm", 0 },
	{ },
};
MODULE_DEVICE_TABLE(platform, pwm_id_table);

... so that my changes to the driver are minimal.  Yes, apparently the only
difference is the existance of a "secondary" pwm for pxa27x.

BTW, the pxa27x actually has four pwms, which is why the addition I made to
pxa27x.dtsi has two nodes (the driver handles two pwms for each device instance
in the pxa27x case).

Thanks Marek.

Mike
Marek Vasut Sept. 4, 2013, 2:35 p.m. UTC | #3
Dear Mike Dunn,

> On 09/03/2013 03:20 PM, Marek Vasut wrote:
> 
> [...]
> 
> >> +#ifdef CONFIG_OF
> >> +/* use the platform_device id table for OF match table data */
> >> +static struct of_device_id pwm_of_match[] = {
> >> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
> >> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
> >> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
> >> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
> >> +	{ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, pwm_of_match);
> > 
> > Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why
> > not just stick with pxa25x-pwm only for all of the CPUs (aka. the lowest
> > CPU model). Then the table would have but a single entry.
> 
> I'm just echoing the existing platform_device_id table...
> 
> static const struct platform_device_id pwm_id_table[] = {
> 	/*   PWM    has_secondary_pwm? */
> 	{ "pxa25x-pwm", 0 },
> 	{ "pxa27x-pwm", HAS_SECONDARY_PWM },
> 	{ "pxa168-pwm", 0 },
> 	{ "pxa910-pwm", 0 },
> 	{ },
> };
> MODULE_DEVICE_TABLE(platform, pwm_id_table);
> 
> ... so that my changes to the driver are minimal.  Yes, apparently the only
> difference is the existance of a "secondary" pwm for pxa27x.
> 
> BTW, the pxa27x actually has four pwms, which is why the addition I made to
> pxa27x.dtsi has two nodes (the driver handles two pwms for each device
> instance in the pxa27x case).
> 

What's that "secondary PWM" there? I no longer remember, sorry. The question 
remains still, we can have two entries there (pxa25x and pxa27x) ORR have one 
entry (pxa25x) + mrvl,has-secondary-pwm entry.

Best regards,
Marek Vasut
Sergei Shtylyov Sept. 4, 2013, 2:38 p.m. UTC | #4
Hello.

On 04-09-2013 18:23, Mike Dunn wrote:

>>> +#ifdef CONFIG_OF
>>> +/* use the platform_device id table for OF match table data */
>>> +static struct of_device_id pwm_of_match[] = {
>>> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
>>> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
>>> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
>>> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, pwm_of_match);

>> Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why not just
>> stick with pxa25x-pwm only for all of the CPUs (aka. the lowest CPU model). Then
>> the table would have but a single entry.

> I'm just echoing the existing platform_device_id table...

> static const struct platform_device_id pwm_id_table[] = {
> 	/*   PWM    has_secondary_pwm? */
> 	{ "pxa25x-pwm", 0 },
> 	{ "pxa27x-pwm", HAS_SECONDARY_PWM },
> 	{ "pxa168-pwm", 0 },
> 	{ "pxa910-pwm", 0 },
> 	{ },
> };
> MODULE_DEVICE_TABLE(platform, pwm_id_table);

    Unlike 'struct platform_device_id', the "compatible" property can't have 
wildcards such as "pxa25x-pwm". You should choose one model (which e.g. was 
produced first in the PXA25x series) and use that.

> Mike

WBR, Sergei
Mike Dunn Sept. 4, 2013, 3:41 p.m. UTC | #5
On 09/04/2013 07:35 AM, Marek Vasut wrote:
> Dear Mike Dunn,
> 
>> On 09/03/2013 03:20 PM, Marek Vasut wrote:
>>
>> [...]
>>
>>>> +#ifdef CONFIG_OF
>>>> +/* use the platform_device id table for OF match table data */
>>>> +static struct of_device_id pwm_of_match[] = {
>>>> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
>>>> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
>>>> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
>>>> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
>>>> +	{ }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, pwm_of_match);
>>>
>>> Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why
>>> not just stick with pxa25x-pwm only for all of the CPUs (aka. the lowest
>>> CPU model). Then the table would have but a single entry.
>>
>> I'm just echoing the existing platform_device_id table...
>>
>> static const struct platform_device_id pwm_id_table[] = {
>> 	/*   PWM    has_secondary_pwm? */
>> 	{ "pxa25x-pwm", 0 },
>> 	{ "pxa27x-pwm", HAS_SECONDARY_PWM },
>> 	{ "pxa168-pwm", 0 },
>> 	{ "pxa910-pwm", 0 },
>> 	{ },
>> };
>> MODULE_DEVICE_TABLE(platform, pwm_id_table);
>>
>> ... so that my changes to the driver are minimal.  Yes, apparently the only
>> difference is the existance of a "secondary" pwm for pxa27x.
>>
>> BTW, the pxa27x actually has four pwms, which is why the addition I made to
>> pxa27x.dtsi has two nodes (the driver handles two pwms for each device
>> instance in the pxa27x case).
>>
> 
> What's that "secondary PWM" there? I no longer remember, sorry. 


If pdev->id_entry->driver_data == HAS_SECONDARY_PWM, then pwm_chip->npwm=2 when
pwmchip_add() is called.  Otherwise pwm_chip->npwm=1.  The driver knows that the
second pwm's registers are at a fixed offset from the first.  For compatibility,
the pxa27x maps the registers for the third pwm at a distant offset, and makes
the offset between 3 and 4 the same as between 1 and 2.  Yes, the driver mkes
this unnecessarily complicated.  There should just be one device instance per
pwm, and dispense with the whole driver_data thing.  I guess there's some
history there.


> The question 
> remains still, we can have two entries there (pxa25x and pxa27x) ORR have one 
> entry (pxa25x) + mrvl,has-secondary-pwm entry.


It looks like defining "compatible" properties that mirror the old
platform_device_id names won't fly... wildcards are verboten (see Sergei's
comment).  So your inclination to use one value for the "compatible" property is
correct.  I think the way to go is to forget the whole HAS_SECONDARY_PWM in the
DT case, have one device instance per pwm, and use "compatible=marvell,pwm".
Other suggestions welcome.

Thanks,
Mike
Mike Dunn Sept. 4, 2013, 3:44 p.m. UTC | #6
On 09/04/2013 07:38 AM, Sergei Shtylyov wrote:
> Hello.
> 
> On 04-09-2013 18:23, Mike Dunn wrote:
> 
>>>> +#ifdef CONFIG_OF
>>>> +/* use the platform_device id table for OF match table data */
>>>> +static struct of_device_id pwm_of_match[] = {
>>>> +    { .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
>>>> +    { .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
>>>> +    { .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
>>>> +    { .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
>>>> +    { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, pwm_of_match);
> 
>>> Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why not just
>>> stick with pxa25x-pwm only for all of the CPUs (aka. the lowest CPU model). Then
>>> the table would have but a single entry.
> 
>> I'm just echoing the existing platform_device_id table...
> 
>> static const struct platform_device_id pwm_id_table[] = {
>>     /*   PWM    has_secondary_pwm? */
>>     { "pxa25x-pwm", 0 },
>>     { "pxa27x-pwm", HAS_SECONDARY_PWM },
>>     { "pxa168-pwm", 0 },
>>     { "pxa910-pwm", 0 },
>>     { },
>> };
>> MODULE_DEVICE_TABLE(platform, pwm_id_table);
> 
>    Unlike 'struct platform_device_id', the "compatible" property can't have
> wildcards such as "pxa25x-pwm". You should choose one model (which e.g. was
> produced first in the PXA25x series) and use that.


Thanks Sergei, I forgot about that.  I also forgot to add a file to
Documentation/devicetree/bindings/pwm/  Looks like I have more work.

Mike
Marek Vasut Sept. 4, 2013, 10:11 p.m. UTC | #7
Dear Mike Dunn,

> On 09/04/2013 07:35 AM, Marek Vasut wrote:
> > Dear Mike Dunn,
> > 
> >> On 09/03/2013 03:20 PM, Marek Vasut wrote:
> >> 
> >> [...]
> >> 
> >>>> +#ifdef CONFIG_OF
> >>>> +/* use the platform_device id table for OF match table data */
> >>>> +static struct of_device_id pwm_of_match[] = {
> >>>> +	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] 
},
> >>>> +	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] 
},
> >>>> +	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] 
},
> >>>> +	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] 
},
> >>>> +	{ }
> >>>> +};
> >>>> +MODULE_DEVICE_TABLE(of, pwm_of_match);
> >>> 
> >>> Are PXA2xx and PXA3xx PWM impleemntations not all the same ? If so, why
> >>> not just stick with pxa25x-pwm only for all of the CPUs (aka. the
> >>> lowest CPU model). Then the table would have but a single entry.
> >> 
> >> I'm just echoing the existing platform_device_id table...
> >> 
> >> static const struct platform_device_id pwm_id_table[] = {
> >> 
> >> 	/*   PWM    has_secondary_pwm? */
> >> 	{ "pxa25x-pwm", 0 },
> >> 	{ "pxa27x-pwm", HAS_SECONDARY_PWM },
> >> 	{ "pxa168-pwm", 0 },
> >> 	{ "pxa910-pwm", 0 },
> >> 	{ },
> >> 
> >> };
> >> MODULE_DEVICE_TABLE(platform, pwm_id_table);
> >> 
> >> ... so that my changes to the driver are minimal.  Yes, apparently the
> >> only difference is the existance of a "secondary" pwm for pxa27x.
> >> 
> >> BTW, the pxa27x actually has four pwms, which is why the addition I made
> >> to pxa27x.dtsi has two nodes (the driver handles two pwms for each
> >> device instance in the pxa27x case).
> > 
> > What's that "secondary PWM" there? I no longer remember, sorry.
> 
> If pdev->id_entry->driver_data == HAS_SECONDARY_PWM, then pwm_chip->npwm=2
> when pwmchip_add() is called.  Otherwise pwm_chip->npwm=1.  The driver
> knows that the second pwm's registers are at a fixed offset from the
> first.  For compatibility, the pxa27x maps the registers for the third pwm
> at a distant offset, and makes the offset between 3 and 4 the same as
> between 1 and 2.  Yes, the driver mkes this unnecessarily complicated. 
> There should just be one device instance per pwm, and dispense with the
> whole driver_data thing.  I guess there's some history there.

OK, I checked the datasheet. The register block for PWM<n + 2> is at offset of 
0x10 from PWM<n> , for n in {0, 1} .

Why can we not just register four PWM blocks, each with 0x10 register window 
size then? I know there's history (maybe), but then, with DT, this might go 
away.

> > The question
> > remains still, we can have two entries there (pxa25x and pxa27x) ORR have
> > one entry (pxa25x) + mrvl,has-secondary-pwm entry.
> 
> It looks like defining "compatible" properties that mirror the old
> platform_device_id names won't fly...

Yes of course, this won't work. I didn't know the layout exactly.

> wildcards are verboten (see Sergei's
> comment).  So your inclination to use one value for the "compatible"
> property is correct.  I think the way to go is to forget the whole
> HAS_SECONDARY_PWM in the DT case, have one device instance per pwm, and
> use "compatible=marvell,pwm". Other suggestions welcome.

compatbile=marvell,pxa25x-pwm , no ? The lowest CPU with the block.

Best regards,
Marek Vasut
Mike Dunn Sept. 5, 2013, 3:24 p.m. UTC | #8
On 09/04/2013 03:11 PM, Marek Vasut wrote:
>>>

[...]

>>> What's that "secondary PWM" there? I no longer remember, sorry.
>>
>> If pdev->id_entry->driver_data == HAS_SECONDARY_PWM, then pwm_chip->npwm=2
>> when pwmchip_add() is called.  Otherwise pwm_chip->npwm=1.  The driver
>> knows that the second pwm's registers are at a fixed offset from the
>> first.  For compatibility, the pxa27x maps the registers for the third pwm
>> at a distant offset, and makes the offset between 3 and 4 the same as
>> between 1 and 2.  Yes, the driver mkes this unnecessarily complicated. 
>> There should just be one device instance per pwm, and dispense with the
>> whole driver_data thing.  I guess there's some history there.
> 
> OK, I checked the datasheet. The register block for PWM<n + 2> is at offset of 
> 0x10 from PWM<n> , for n in {0, 1} .
> 
> Why can we not just register four PWM blocks, each with 0x10 register window 
> size then? I know there's history (maybe), but then, with DT, this might go 
> away.


Indeed.  That is what I am also thinking.


> 
>>> The question
>>> remains still, we can have two entries there (pxa25x and pxa27x) ORR have
>>> one entry (pxa25x) + mrvl,has-secondary-pwm entry.
>>
>> It looks like defining "compatible" properties that mirror the old
>> platform_device_id names won't fly...
> 
> Yes of course, this won't work. I didn't know the layout exactly.
> 
>> wildcards are verboten (see Sergei's
>> comment).  So your inclination to use one value for the "compatible"
>> property is correct.  I think the way to go is to forget the whole
>> HAS_SECONDARY_PWM in the DT case, have one device instance per pwm, and
>> use "compatible=marvell,pwm". Other suggestions welcome.
> 
> compatbile=marvell,pxa25x-pwm , no ? The lowest CPU with the block.


Unless I am missing something, the compatible string does not need to replicate
any of the existing platform_device_id names, so wouldn't "marvell,pxa" be
better?  Except for register mapping and the number of units present on a
particular pxa variant, the peripheral is software compatible across all pxa
processors.  Plus there is the problem of the 'x' wildcard in "pxa25x-pwm".

Thanks,
Mike
Marek Vasut Sept. 5, 2013, 3:34 p.m. UTC | #9
Dear Mike Dunn,

> On 09/04/2013 03:11 PM, Marek Vasut wrote:
> 
> 
> [...]
> 
> >>> What's that "secondary PWM" there? I no longer remember, sorry.
> >> 
> >> If pdev->id_entry->driver_data == HAS_SECONDARY_PWM, then
> >> pwm_chip->npwm=2 when pwmchip_add() is called.  Otherwise
> >> pwm_chip->npwm=1.  The driver knows that the second pwm's registers are
> >> at a fixed offset from the first.  For compatibility, the pxa27x maps
> >> the registers for the third pwm at a distant offset, and makes the
> >> offset between 3 and 4 the same as between 1 and 2.  Yes, the driver
> >> mkes this unnecessarily complicated. There should just be one device
> >> instance per pwm, and dispense with the whole driver_data thing.  I
> >> guess there's some history there.
> > 
> > OK, I checked the datasheet. The register block for PWM<n + 2> is at
> > offset of 0x10 from PWM<n> , for n in {0, 1} .
> > 
> > Why can we not just register four PWM blocks, each with 0x10 register
> > window size then? I know there's history (maybe), but then, with DT,
> > this might go away.
> 
> Indeed.  That is what I am also thinking.
> 
> >>> The question
> >>> remains still, we can have two entries there (pxa25x and pxa27x) ORR
> >>> have one entry (pxa25x) + mrvl,has-secondary-pwm entry.
> >> 
> >> It looks like defining "compatible" properties that mirror the old
> >> platform_device_id names won't fly...
> > 
> > Yes of course, this won't work. I didn't know the layout exactly.
> > 
> >> wildcards are verboten (see Sergei's
> >> comment).  So your inclination to use one value for the "compatible"
> >> property is correct.  I think the way to go is to forget the whole
> >> HAS_SECONDARY_PWM in the DT case, have one device instance per pwm, and
> >> use "compatible=marvell,pwm". Other suggestions welcome.
> > 
> > compatbile=marvell,pxa25x-pwm , no ? The lowest CPU with the block.
> 
> Unless I am missing something, the compatible string does not need to
> replicate any of the existing platform_device_id names, so wouldn't
> "marvell,pxa" be better?  Except for register mapping and the number of
> units present on a particular pxa variant, the peripheral is software
> compatible across all pxa processors.  Plus there is the problem of the
> 'x' wildcard in "pxa25x-pwm".

So use pxa250 ?

My concern is once marvell comes up with PXA1048576 which will have a different 
PWM unit, then what will be the name for this new one?

Best regards,
Marek Vasut
Mike Dunn Sept. 5, 2013, 4:07 p.m. UTC | #10
On 09/05/2013 08:34 AM, Marek Vasut wrote:
>>>

[...]


>>> compatbile=marvell,pxa25x-pwm , no ? The lowest CPU with the block.
>>
>> Unless I am missing something, the compatible string does not need to
>> replicate any of the existing platform_device_id names, so wouldn't
>> "marvell,pxa" be better?  Except for register mapping and the number of
>> units present on a particular pxa variant, the peripheral is software
>> compatible across all pxa processors.  Plus there is the problem of the
>> 'x' wildcard in "pxa25x-pwm".
> 
> So use pxa250 ?
> 
> My concern is once marvell comes up with PXA1048576 which will have a different 
> PWM unit, then what will be the name for this new one?


I see.  OK then, pxa250 it is.

Thanks Marek,
Mike
diff mbox

Patch

diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi
index d7c5d72..4031dce 100644
--- a/arch/arm/boot/dts/pxa27x.dtsi
+++ b/arch/arm/boot/dts/pxa27x.dtsi
@@ -10,5 +10,17 @@ 
 			marvell,intc-priority;
 			marvell,intc-nr-irqs = <34>;
 		};
+
+		pwm0: pwm@40b00000 {
+			compatible = "marvell,pxa27x-pwm";
+			reg = <0x40b00000 0x100000>;
+			#pwm-cells = <2>;
+		};
+
+		pwm1: pwm@40c00000 {
+			compatible = "marvell,pxa27x-pwm";
+			reg = <0x40c00000 0x100000>;
+			#pwm-cells = <2>;
+		};
 	};
 };
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index a4d2164..be5f914 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -19,6 +19,7 @@ 
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/pwm.h>
+#include <linux/of_device.h>
 
 #include <asm/div64.h>
 
@@ -124,6 +125,30 @@  static struct pwm_ops pxa_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+#ifdef CONFIG_OF
+/* use the platform_device id table for OF match table data */
+static struct of_device_id pwm_of_match[] = {
+	{ .compatible = "marvell,pxa25x-pwm", .data = &pwm_id_table[0] },
+	{ .compatible = "marvell,pxa27x-pwm", .data = &pwm_id_table[1] },
+	{ .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[2] },
+	{ .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[3] },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pwm_of_match);
+
+static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev)
+{
+	const struct of_device_id *pwm_pxa_devid =
+		of_match_device(pwm_of_match, dev);
+	if (pwm_pxa_devid)
+		return (const struct platform_device_id *)pwm_pxa_devid->data;
+	else
+		return NULL;
+}
+#else  /* !CONFIG_OF */
+#define pxa_pwm_get_id_dt(dev) ((const struct platform_device_id *)NULL)
+#endif
+
 static int pwm_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -131,6 +156,11 @@  static int pwm_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret = 0;
 
+	if (id == NULL)		/* using device tree */
+		id = pxa_pwm_get_id_dt(&pdev->dev);
+	if (id == NULL)
+		return -ENODEV;
+
 	pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
 	if (pwm == NULL) {
 		dev_err(&pdev->dev, "failed to allocate memory\n");
@@ -176,6 +206,7 @@  static struct platform_driver pwm_driver = {
 	.driver		= {
 		.name	= "pxa25x-pwm",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(pwm_of_match),
 	},
 	.probe		= pwm_probe,
 	.remove		= pwm_remove,