| Message ID | 1379091281-23662-1-git-send-email-mikedunn@newsguy.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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> > --- > Changle log: > v3: > - remove support for the polarity flag > - remove per-chip pwm index cell; define custom of_xlate() > (now #pwm-cells = <1>) > - "compatible" strings for all devices added to OF match table > - various stylistic changes recommended by reviewers > > v2: > - of_match_table contains only the "pxa250-pwm" compatible string; require > one device instance per pwm > - add Documentation/devicetree/bindings/pwm/pxa-pwm.txt > - add support for polarity flag in DT and implement set_polarity() method > (the treo 680 inverts the signal between pwm out and backlight) > - return -EINVAL instead of -ENODEV if platform data or DT node not found > - output dev_info string if platform data missing > - expanded CC list of patch > > Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 31 ++++++++++++ > arch/arm/boot/dts/pxa27x.dtsi | 24 +++++++++ > drivers/pwm/pwm-pxa.c | 62 > +++++++++++++++++++++++ 3 files changed, 117 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt > > diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt new file mode 100644 > index 0000000..6fcf90c > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > @@ -0,0 +1,31 @@ > +Marvell PWM controller > + > +Required properties: > +- compatible: should be one of: > + - "marvell,pxa250-pwm" > + - "marvell,pxa270-pwm" > + - "marvell,pxa168-pwm" > + - "marvell,pxa910-pwm" This really is something I dont quite understand. Why should the driver list _every_ _single_ existing CPU that contains such PWM block? Is there any agreement about that? For me, it'd make much more sense to list only the CPUs where the IP block actually changed in some way, so that the differences can be discerned that way. Best regards, Marek Vasut
On 09/15/2013 07:07 AM, Marek Vasut wrote: > 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 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> >> --- >> Changle log: >> v3: >> - remove support for the polarity flag >> - remove per-chip pwm index cell; define custom of_xlate() >> (now #pwm-cells = <1>) >> - "compatible" strings for all devices added to OF match table >> - various stylistic changes recommended by reviewers >> >> v2: >> - of_match_table contains only the "pxa250-pwm" compatible string; require >> one device instance per pwm >> - add Documentation/devicetree/bindings/pwm/pxa-pwm.txt >> - add support for polarity flag in DT and implement set_polarity() method >> (the treo 680 inverts the signal between pwm out and backlight) >> - return -EINVAL instead of -ENODEV if platform data or DT node not found >> - output dev_info string if platform data missing >> - expanded CC list of patch >> >> Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 31 ++++++++++++ >> arch/arm/boot/dts/pxa27x.dtsi | 24 +++++++++ >> drivers/pwm/pwm-pxa.c | 62 >> +++++++++++++++++++++++ 3 files changed, 117 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt >> >> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >> b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt new file mode 100644 >> index 0000000..6fcf90c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >> @@ -0,0 +1,31 @@ >> +Marvell PWM controller >> + >> +Required properties: >> +- compatible: should be one of: >> + - "marvell,pxa250-pwm" >> + - "marvell,pxa270-pwm" >> + - "marvell,pxa168-pwm" >> + - "marvell,pxa910-pwm" > > This really is something I dont quite understand. Why should the driver list > _every_ _single_ existing CPU that contains such PWM block? Is there any > agreement about that? For me, it'd make much more sense to list only the CPUs > where the IP block actually changed in some way, so that the differences can be > discerned that way. I believe that this was Stephen's suggestion. I actually don't object myself. For the price of a few strings, it - clearly shows which SoCs the driver supports - ensures that any future differences are handled cleanly (e.g., if a hw bug in one is discovered and a work-around is implemented) - keeps thngs clean if support for another processor which does have pwm hw differences is added Especially the third point... otherwise, you have the case of a somewhat confusing many-to-one mapping of processors to compatible strings. Thanks, Mike
Dear Mike Dunn, > On 09/15/2013 07:07 AM, Marek Vasut wrote: > > 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 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> > >> --- > >> Changle log: > >> v3: > >> - remove support for the polarity flag > >> - remove per-chip pwm index cell; define custom of_xlate() > >> > >> (now #pwm-cells = <1>) > >> > >> - "compatible" strings for all devices added to OF match table > >> - various stylistic changes recommended by reviewers > >> > >> v2: > >> - of_match_table contains only the "pxa250-pwm" compatible string; > >> require one device instance per pwm > >> - add Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >> - add support for polarity flag in DT and implement set_polarity() > >> method > >> > >> (the treo 680 inverts the signal between pwm out and backlight) > >> > >> - return -EINVAL instead of -ENODEV if platform data or DT node not > >> found - output dev_info string if platform data missing > >> - expanded CC list of patch > >> > >> Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 31 ++++++++++++ > >> arch/arm/boot/dts/pxa27x.dtsi | 24 +++++++++ > >> drivers/pwm/pwm-pxa.c | 62 > >> > >> +++++++++++++++++++++++ 3 files changed, 117 insertions(+) > >> > >> create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >> > >> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >> b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt new file mode 100644 > >> index 0000000..6fcf90c > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >> @@ -0,0 +1,31 @@ > >> +Marvell PWM controller > >> + > >> +Required properties: > >> +- compatible: should be one of: > >> + - "marvell,pxa250-pwm" > >> + - "marvell,pxa270-pwm" > >> + - "marvell,pxa168-pwm" > >> + - "marvell,pxa910-pwm" > > > > This really is something I dont quite understand. Why should the driver > > list _every_ _single_ existing CPU that contains such PWM block? Is > > there any agreement about that? For me, it'd make much more sense to > > list only the CPUs where the IP block actually changed in some way, so > > that the differences can be discerned that way. > > I believe that this was Stephen's suggestion. > > I actually don't object myself. For the price of a few strings, it > - clearly shows which SoCs the driver supports > - ensures that any future differences are handled cleanly (e.g., if a hw > bug in one is discovered and a work-around is implemented) > - keeps thngs clean if support for another processor which does have pwm hw > differences is added > > Especially the third point... otherwise, you have the case of a somewhat > confusing many-to-one mapping of processors to compatible strings. OK, to push it ad-absurdum, shall we not start listing every single revision of the CPU as well ? Like pxa250a0 250a1 etc ? Yes , this many-to-one mapping is something I don't quite understand, that's why I'd love to see a good reasoning from Stephen. Best regards, Marek Vasut
On 09/13/2013 10:54 AM, Mike Dunn wrote: > 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 ID table is reused for the match table data. > > Tested on a Palm Treo 680 (both platform data and DT cases). > diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > +- compatible: should be one of: > + - "marvell,pxa250-pwm" > + - "marvell,pxa270-pwm" > + - "marvell,pxa168-pwm" > + - "marvell,pxa910-pwm" Not just one of, but possible more than one... > diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi > + pwm0: pwm@40b00000 { > + compatible = "marvell,pxa270-pwm"; I thought the assertion was that pax270 and pxa250 were both compatible? If so, that should be: compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm";
On 09/16/2013 09:10 AM, Marek Vasut wrote: ... > OK, to push it ad-absurdum, shall we not start listing every single revision of > the CPU as well? Like pxa250a0 250a1 etc ? Yes , this many-to-one mapping is It's not about CPU revisions, it's about revisions of the SoC that contain the IP block. > something I don't quite understand, that's why I'd love to see a good reasoning > from Stephen. Please see the earlier discussions.
Dear Stephen Warren, > On 09/16/2013 09:10 AM, Marek Vasut wrote: > ... > > > OK, to push it ad-absurdum, shall we not start listing every single > > revision of the CPU as well? Like pxa250a0 250a1 etc ? Yes , this > > many-to-one mapping is > > It's not about CPU revisions, it's about revisions of the SoC that > contain the IP block. But that doesn't matter if the IP block is the same in all of them. > > something I don't quite understand, that's why I'd love to see a good > > reasoning from Stephen. > > Please see the earlier discussions. I did, so I must really be slow or something. Best regards, Marek Vasut
Dear Stephen Warren, > On 09/13/2013 10:54 AM, Mike Dunn wrote: > > 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 ID table is reused for the match table data. > > > > Tested on a Palm Treo 680 (both platform data and DT cases). > > > > diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > > b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > > > > +- compatible: should be one of: > > + - "marvell,pxa250-pwm" > > + - "marvell,pxa270-pwm" > > + - "marvell,pxa168-pwm" > > + - "marvell,pxa910-pwm" > > Not just one of, but possible more than one... > > > diff --git a/arch/arm/boot/dts/pxa27x.dtsi > > b/arch/arm/boot/dts/pxa27x.dtsi > > > > + pwm0: pwm@40b00000 { > > + compatible = "marvell,pxa270-pwm"; > > I thought the assertion was that pax270 and pxa250 were both compatible? > If so, that should be: > > compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; I see what you mean with the compatible strings ... but if we have this "fallback" compatiblity string, do we also need the table of all chips sporting the IP block in the driver itself? Best regards, Marek Vasut
On 09/16/2013 05:01 PM, Marek Vasut wrote: > Dear Stephen Warren, > >> On 09/13/2013 10:54 AM, Mike Dunn wrote: >>> 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 ID table is reused for the match table data. >>> >>> Tested on a Palm Treo 680 (both platform data and DT cases). >>> >>> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >>> b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >>> >>> +- compatible: should be one of: >>> + - "marvell,pxa250-pwm" >>> + - "marvell,pxa270-pwm" >>> + - "marvell,pxa168-pwm" >>> + - "marvell,pxa910-pwm" >> >> Not just one of, but possible more than one... >> >>> diff --git a/arch/arm/boot/dts/pxa27x.dtsi >>> b/arch/arm/boot/dts/pxa27x.dtsi >>> >>> + pwm0: pwm@40b00000 { >>> + compatible = "marvell,pxa270-pwm"; >> >> I thought the assertion was that pax270 and pxa250 were both compatible? >> If so, that should be: >> >> compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; > > I see what you mean with the compatible strings ... but if we have this > "fallback" compatiblity string, do we also need the table of all chips sporting > the IP block in the driver itself? I would assume the driver can just support "marvell,pxa250-pwm" for now.
On 09/16/2013 12:44 PM, Stephen Warren wrote: > On 09/13/2013 10:54 AM, Mike Dunn wrote: >> 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 ID table is reused for the match table data. >> >> Tested on a Palm Treo 680 (both platform data and DT cases). > >> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >> +- compatible: should be one of: >> + - "marvell,pxa250-pwm" >> + - "marvell,pxa270-pwm" >> + - "marvell,pxa168-pwm" >> + - "marvell,pxa910-pwm" > > Not just one of, but possible more than one... So the phrasing should be "compatible: should be among:" ? > >> diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi > >> + pwm0: pwm@40b00000 { >> + compatible = "marvell,pxa270-pwm"; > > I thought the assertion was that pax270 and pxa250 were both compatible? > If so, that should be: > > compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; Indeed. Currently, they are all the same. By that logic, shouldn't it be compatible = "marvell,pxa250-pwm", "marvell,pxa270-pwm", "marvell,pxa168-pwm", "marvell,pxa910-pwm"; I am admittedly fuzzy on device tree, but this is starting to seem strange. Anyway, I'll defer to your judgement and resubmit if you can give me precise guidance on the above. Thanks again for the review. Thanks, Mike
On 09/17/2013 08:07 AM, Mike Dunn wrote: > On 09/16/2013 12:44 PM, Stephen Warren wrote: >> On 09/13/2013 10:54 AM, Mike Dunn wrote: >>> 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 ID table is reused for the match table data. >>> >>> Tested on a Palm Treo 680 (both platform data and DT cases). >> >>> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >> >>> +- compatible: should be one of: >>> + - "marvell,pxa250-pwm" >>> + - "marvell,pxa270-pwm" >>> + - "marvell,pxa168-pwm" >>> + - "marvell,pxa910-pwm" >> >> Not just one of, but possible more than one... > > So the phrasing should be "compatible: should be among:" ? I think I've seen "One or more of". >>> diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi >> >>> + pwm0: pwm@40b00000 { >>> + compatible = "marvell,pxa270-pwm"; >> >> I thought the assertion was that pax270 and pxa250 were both compatible? >> If so, that should be: >> >> compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; > > > Indeed. Currently, they are all the same. By that logic, shouldn't it be > > compatible = "marvell,pxa250-pwm", "marvell,pxa270-pwm", "marvell,pxa168-pwm", > "marvell,pxa910-pwm"; compatible should include: * The exact HW model (so the driver knows exactly which HW is present in order to enable any bug quirks). * The "first" HW model this HW is compatible with, since this is what the driver will bind to (ignoring the possibility of bug quirks). So, you don't need to include all the values, just two in this case.
On 09/17/2013 09:17 AM, Stephen Warren wrote: > On 09/17/2013 08:07 AM, Mike Dunn wrote: >> On 09/16/2013 12:44 PM, Stephen Warren wrote: >>> On 09/13/2013 10:54 AM, Mike Dunn wrote: >>>> 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 ID table is reused for the match table data. >>>> >>>> Tested on a Palm Treo 680 (both platform data and DT cases). >>> >>>> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt >>> >>>> +- compatible: should be one of: >>>> + - "marvell,pxa250-pwm" >>>> + - "marvell,pxa270-pwm" >>>> + - "marvell,pxa168-pwm" >>>> + - "marvell,pxa910-pwm" >>> >>> Not just one of, but possible more than one... >> >> So the phrasing should be "compatible: should be among:" ? > > I think I've seen "One or more of". OK. > > >>>> diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi >>> >>>> + pwm0: pwm@40b00000 { >>>> + compatible = "marvell,pxa270-pwm"; >>> >>> I thought the assertion was that pax270 and pxa250 were both compatible? >>> If so, that should be: >>> >>> compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; >> >> >> Indeed. Currently, they are all the same. By that logic, shouldn't it be >> >> compatible = "marvell,pxa250-pwm", "marvell,pxa270-pwm", "marvell,pxa168-pwm", >> "marvell,pxa910-pwm"; > > compatible should include: > > * The exact HW model (so the driver knows exactly which HW is present in > order to enable any bug quirks). > * The "first" HW model this HW is compatible with, since this is what > the driver will bind to (ignoring the possibility of bug quirks). > > So, you don't need to include all the values, just two in this case. I think I see... and hypothetically, if the the driver must know that it is a pxa270 in order for it to function correctly due to (hypothetical) hardware differences, then only the first compatible string should appear? Anyway, thanks again! I'll resubmit tomorrow. Mike
On 09/17/2013 12:56 PM, Mike Dunn wrote: > On 09/17/2013 09:17 AM, Stephen Warren wrote: >> On 09/17/2013 08:07 AM, Mike Dunn wrote: >>> On 09/16/2013 12:44 PM, Stephen Warren wrote: >>>> On 09/13/2013 10:54 AM, Mike Dunn wrote: >>>>> 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 ID table is reused for the match table data. >>>>> >>>>> Tested on a Palm Treo 680 (both platform data and DT cases). >>>>> diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi >>>> >>>>> + pwm0: pwm@40b00000 { >>>>> + compatible = "marvell,pxa270-pwm"; >>>> >>>> I thought the assertion was that pax270 and pxa250 were both compatible? >>>> If so, that should be: >>>> >>>> compatible = "marvell,pxa270-pwm", "marvell,pxa250-pwm"; >>> >>> >>> Indeed. Currently, they are all the same. By that logic, shouldn't it be >>> >>> compatible = "marvell,pxa250-pwm", "marvell,pxa270-pwm", "marvell,pxa168-pwm", >>> "marvell,pxa910-pwm"; >> >> compatible should include: >> >> * The exact HW model (so the driver knows exactly which HW is present in >> order to enable any bug quirks). >> * The "first" HW model this HW is compatible with, since this is what >> the driver will bind to (ignoring the possibility of bug quirks). >> >> So, you don't need to include all the values, just two in this case. > > I think I see... and hypothetically, if the the driver must know that it is a > pxa270 in order for it to function correctly due to (hypothetical) hardware > differences, then only the first compatible string should appear? Yes, if the 250 and 270 HW were actually different, then you'd only include one or the other compatible values in the property.
diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt new file mode 100644 index 0000000..6fcf90c --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt @@ -0,0 +1,31 @@ +Marvell PWM controller + +Required properties: +- compatible: should be one of: + - "marvell,pxa250-pwm" + - "marvell,pxa270-pwm" + - "marvell,pxa168-pwm" + - "marvell,pxa910-pwm" +- reg: physical base address and length of the registers used by the PWM channel + NB: One device instance must be created for each PWM that is used, so the + length covers only the register window for one PWM output, not that of the + entire PWM controller. Currently length is 0x10 for all supported devices. +- #pwm-cells: should be 1. This cell is used to specify the period in + nanoseconds. (Because one device instance is created for each PWM output, + the per-chip index is superflous and not used.) + +Example PWM device node: + +pwm0: pwm@40b00000 { + compatible = "marvell,pxa250-pwm"; + reg = <0x40b00000 0x10>; + #pwm-cells = <1>; +}; + +Example PWM client node: + +backlight { + compatible = "pwm-backlight"; + pwms = <&pwm0 5000000>; + ... +} diff --git a/arch/arm/boot/dts/pxa27x.dtsi b/arch/arm/boot/dts/pxa27x.dtsi index d7c5d72..5057c41 100644 --- a/arch/arm/boot/dts/pxa27x.dtsi +++ b/arch/arm/boot/dts/pxa27x.dtsi @@ -10,5 +10,29 @@ marvell,intc-priority; marvell,intc-nr-irqs = <34>; }; + + pwm0: pwm@40b00000 { + compatible = "marvell,pxa270-pwm"; + reg = <0x40b00000 0x10>; + #pwm-cells = <1>; + }; + + pwm1: pwm@40b00010 { + compatible = "marvell,pxa270-pwm"; + reg = <0x40b00010 0x10>; + #pwm-cells = <1>; + }; + + pwm2: pwm@40c00000 { + compatible = "marvell,pxa270-pwm"; + reg = <0x40c00000 0x10>; + #pwm-cells = <1>; + }; + + pwm3: pwm@40c00010 { + compatible = "marvell,pxa270-pwm"; + reg = <0x40c00010 0x10>; + #pwm-cells = <1>; + }; }; }; diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c index a4d2164..91b1cff 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,59 @@ static struct pwm_ops pxa_pwm_ops = { .owner = THIS_MODULE, }; +#ifdef CONFIG_OF +/* + * Device tree users must create one device instance for each pwm channel. + * Hence we dispense with the HAS_SECONDARY_PWM and "tell" the original driver + * code that this is a single channel pxa25x-pwm. Currently all devices are + * supported identically. + */ +static struct of_device_id pwm_of_match[] = { + { .compatible = "marvell,pxa250-pwm", .data = &pwm_id_table[0]}, + { .compatible = "marvell,pxa270-pwm", .data = &pwm_id_table[0]}, + { .compatible = "marvell,pxa168-pwm", .data = &pwm_id_table[0]}, + { .compatible = "marvell,pxa910-pwm", .data = &pwm_id_table[0]}, + { } +}; +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 *id = of_match_device(pwm_of_match, dev); + if (id) + return (const struct platform_device_id *)id->data; + else + return NULL; +} + +struct pwm_device * +pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) +{ + struct pwm_device *pwm; + + pwm = pwm_request_from_chip(pc, 0, NULL); + if (IS_ERR(pwm)) + return pwm; + + pwm_set_period(pwm, args->args[0]); + + return pwm; +} + +#else /* !CONFIG_OF */ +static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev) +{ + dev_err(dev, "missing platform data\n"); + return NULL; +} + +struct pwm_device * +pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args) +{ + return NULL; +} +#endif + static int pwm_probe(struct platform_device *pdev) { const struct platform_device_id *id = platform_get_device_id(pdev); @@ -131,6 +185,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 -EINVAL; + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL); if (pwm == NULL) { dev_err(&pdev->dev, "failed to allocate memory\n"); @@ -145,6 +204,8 @@ static int pwm_probe(struct platform_device *pdev) pwm->chip.ops = &pxa_pwm_ops; pwm->chip.base = -1; pwm->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1; + pwm->chip.of_xlate = pxa_pwm_of_xlate; + pwm->chip.of_pwm_n_cells = 1; r = platform_get_resource(pdev, IORESOURCE_MEM, 0); pwm->mmio_base = devm_ioremap_resource(&pdev->dev, r); @@ -176,6 +237,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,
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 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> --- Changle log: v3: - remove support for the polarity flag - remove per-chip pwm index cell; define custom of_xlate() (now #pwm-cells = <1>) - "compatible" strings for all devices added to OF match table - various stylistic changes recommended by reviewers v2: - of_match_table contains only the "pxa250-pwm" compatible string; require one device instance per pwm - add Documentation/devicetree/bindings/pwm/pxa-pwm.txt - add support for polarity flag in DT and implement set_polarity() method (the treo 680 inverts the signal between pwm out and backlight) - return -EINVAL instead of -ENODEV if platform data or DT node not found - output dev_info string if platform data missing - expanded CC list of patch Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 31 ++++++++++++ arch/arm/boot/dts/pxa27x.dtsi | 24 +++++++++ drivers/pwm/pwm-pxa.c | 62 +++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt