diff mbox

[RESEND,v5,1/2] PWM: PXA: add device tree support to PWM driver

Message ID 1379791174-2369-2-git-send-email-mikedunn@newsguy.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Dunn Sept. 21, 2013, 7:19 p.m. UTC
This patch adds device tree support to the PXA's PWM driver.  Nothing
needs to be extracted from the device tree node by the PWM device.
Client devices need only specify the period; the per-chip index is
implicitly zero because one device node must be present for each PWM
output in use.  This approach is more convenient due to the wide
variability in the number of PWM channels present across the various PXA
variants, and is made possible by the fact that the register sets for
each PWM channel are segregated from each other.  An of_xlate() method
is added to parse this single-cell 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>
---
 Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
 drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt

Comments

Thierry Reding Oct. 8, 2013, 1:12 p.m. UTC | #1
On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
> This patch adds device tree support to the PXA's PWM driver.  Nothing
> needs to be extracted from the device tree node by the PWM device.
> Client devices need only specify the period; the per-chip index is
> implicitly zero because one device node must be present for each PWM
> output in use.  This approach is more convenient due to the wide
> variability in the number of PWM channels present across the various PXA
> variants, and is made possible by the fact that the register sets for
> each PWM channel are segregated from each other.  An of_xlate() method
> is added to parse this single-cell 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>
> ---
>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>  2 files changed, 81 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt

This looks good to me, but I'd like to get an Acked-by: from one of the
device tree bindings maintainers.

Thierry

> 
> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> new file mode 100644
> index 0000000..5ae9f1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> @@ -0,0 +1,30 @@
> +Marvell PWM controller
> +
> +Required properties:
> +- compatible: should be one or more 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
> +  Note that 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.
> +
> +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/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> index a4d2164..e928cc8 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,45 @@ 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);
> +#else
> +static struct of_device_id *pwm_of_match;
> +#endif
> +
> +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);
> +	return id ? id->data : NULL;
> +}
> +
> +static 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;
> +}
> +
>  static int pwm_probe(struct platform_device *pdev)
>  {
>  	const struct platform_device_id *id = platform_get_device_id(pdev);
> @@ -131,6 +171,12 @@ static int pwm_probe(struct platform_device *pdev)
>  	struct resource *r;
>  	int ret = 0;
>  
> +	if (IS_ENABLED(CONFIG_OF) && id == NULL)
> +		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,7 +191,10 @@ 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;
> -
> +	if (IS_ENABLED(CONFIG_OF)) {
> +		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);
>  	if (IS_ERR(pwm->mmio_base))
> @@ -176,6 +225,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,
> -- 
> 1.8.1.5
>
Mike Dunn Oct. 10, 2013, 4:58 p.m. UTC | #2
On 10/08/2013 06:12 AM, Thierry Reding wrote:
> On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
>> This patch adds device tree support to the PXA's PWM driver.  Nothing
>> needs to be extracted from the device tree node by the PWM device.
>> Client devices need only specify the period; the per-chip index is
>> implicitly zero because one device node must be present for each PWM
>> output in use.  This approach is more convenient due to the wide
>> variability in the number of PWM channels present across the various PXA
>> variants, and is made possible by the fact that the register sets for
>> each PWM channel are segregated from each other.  An of_xlate() method
>> is added to parse this single-cell 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>
>> ---
>>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>>  2 files changed, 81 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> 
> This looks good to me, but I'd like to get an Acked-by: from one of the
> device tree bindings maintainers.
> 
> Thierry


Thanks much Thierry and Stephen for the reviews and advice.  Stephen, are there
any remaining issues with the bindings below?

Thanks again,
Mike


> 
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>> new file mode 100644
>> index 0000000..5ae9f1e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>> @@ -0,0 +1,30 @@
>> +Marvell PWM controller
>> +
>> +Required properties:
>> +- compatible: should be one or more 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
>> +  Note that 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.
>> +
>> +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/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
>> index a4d2164..e928cc8 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,45 @@ 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);
>> +#else
>> +static struct of_device_id *pwm_of_match;
>> +#endif
>> +
>> +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);
>> +	return id ? id->data : NULL;
>> +}
>> +
>> +static 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;
>> +}
>> +
>>  static int pwm_probe(struct platform_device *pdev)
>>  {
>>  	const struct platform_device_id *id = platform_get_device_id(pdev);
>> @@ -131,6 +171,12 @@ static int pwm_probe(struct platform_device *pdev)
>>  	struct resource *r;
>>  	int ret = 0;
>>  
>> +	if (IS_ENABLED(CONFIG_OF) && id == NULL)
>> +		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,7 +191,10 @@ 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;
>> -
>> +	if (IS_ENABLED(CONFIG_OF)) {
>> +		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);
>>  	if (IS_ERR(pwm->mmio_base))
>> @@ -176,6 +225,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,
>> -- 
>> 1.8.1.5
>>
Thierry Reding Dec. 3, 2013, 10:17 a.m. UTC | #3
On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
> This patch adds device tree support to the PXA's PWM driver.  Nothing
> needs to be extracted from the device tree node by the PWM device.
> Client devices need only specify the period; the per-chip index is
> implicitly zero because one device node must be present for each PWM
> output in use.  This approach is more convenient due to the wide
> variability in the number of PWM channels present across the various PXA
> variants, and is made possible by the fact that the register sets for
> each PWM channel are segregated from each other.  An of_xlate() method
> is added to parse this single-cell 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>
> ---
>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>  2 files changed, 81 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt

Hi Mike,

It looks like this fell through the cracks. Is this patch still the
latest one you have? Should it still be applied?

Thierry
Mike Dunn Dec. 3, 2013, 6:45 p.m. UTC | #4
On 12/03/2013 02:17 AM, Thierry Reding wrote:
> On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
>> This patch adds device tree support to the PXA's PWM driver.  Nothing
>> needs to be extracted from the device tree node by the PWM device.
>> Client devices need only specify the period; the per-chip index is
>> implicitly zero because one device node must be present for each PWM
>> output in use.  This approach is more convenient due to the wide
>> variability in the number of PWM channels present across the various PXA
>> variants, and is made possible by the fact that the register sets for
>> each PWM channel are segregated from each other.  An of_xlate() method
>> is added to parse this single-cell 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>
>> ---
>>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>>  2 files changed, 81 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> 
> Hi Mike,
> 
> It looks like this fell through the cracks. Is this patch still the
> latest one you have? Should it still be applied?
> 
> Thierry
> 

Hi Thierry,

Funny I should hear from you about this today.... I just turned my attention
back to this today and noticed that it never made it into your for-next branch.
 Yes, it is the latest.  If the patch still applies cleanly, please feel free.
Otherwise, I'd be glad to rework it against something more recent.

Also, we never got any ACKs for patch 2/2, which just adds the nodes to
arch/arm/boot/dts/pxa27x.dtsi
Any advice on whom to nudge?

Thanks much!
Mike
Haojian Zhuang Dec. 4, 2013, 1:12 a.m. UTC | #5
On 12/04/2013 02:45 AM, Mike Dunn wrote:
> On 12/03/2013 02:17 AM, Thierry Reding wrote:
>> On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
>>> This patch adds device tree support to the PXA's PWM driver.  Nothing
>>> needs to be extracted from the device tree node by the PWM device.
>>> Client devices need only specify the period; the per-chip index is
>>> implicitly zero because one device node must be present for each PWM
>>> output in use.  This approach is more convenient due to the wide
>>> variability in the number of PWM channels present across the various PXA
>>> variants, and is made possible by the fact that the register sets for
>>> each PWM channel are segregated from each other.  An of_xlate() method
>>> is added to parse this single-cell 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>
>>> ---
>>>   Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>>>   drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>>>   2 files changed, 81 insertions(+), 1 deletion(-)
>>>   create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>> Hi Mike,
>>
>> It looks like this fell through the cracks. Is this patch still the
>> latest one you have? Should it still be applied?
>>
>> Thierry
>>
> Hi Thierry,
>
> Funny I should hear from you about this today.... I just turned my attention
> back to this today and noticed that it never made it into your for-next branch.
>   Yes, it is the latest.  If the patch still applies cleanly, please feel free.
> Otherwise, I'd be glad to rework it against something more recent.
>
> Also, we never got any ACKs for patch 2/2, which just adds the nodes to
> arch/arm/boot/dts/pxa27x.dtsi
> Any advice on whom to nudge?
>
> Thanks much!
> Mike
>

It's fine to me.

Acked-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Regards
Haojian
Thierry Reding Dec. 4, 2013, 9:03 a.m. UTC | #6
On Wed, Dec 04, 2013 at 09:12:51AM +0800, Haojian Zhuang wrote:
> 
> On 12/04/2013 02:45 AM, Mike Dunn wrote:
> >On 12/03/2013 02:17 AM, Thierry Reding wrote:
> >>On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
> >>>This patch adds device tree support to the PXA's PWM driver.  Nothing
> >>>needs to be extracted from the device tree node by the PWM device.
> >>>Client devices need only specify the period; the per-chip index is
> >>>implicitly zero because one device node must be present for each PWM
> >>>output in use.  This approach is more convenient due to the wide
> >>>variability in the number of PWM channels present across the various PXA
> >>>variants, and is made possible by the fact that the register sets for
> >>>each PWM channel are segregated from each other.  An of_xlate() method
> >>>is added to parse this single-cell 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>
> >>>---
> >>>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
> >>>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
> >>>  2 files changed, 81 insertions(+), 1 deletion(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> >>Hi Mike,
> >>
> >>It looks like this fell through the cracks. Is this patch still the
> >>latest one you have? Should it still be applied?
> >>
> >>Thierry
> >>
> >Hi Thierry,
> >
> >Funny I should hear from you about this today.... I just turned my attention
> >back to this today and noticed that it never made it into your for-next branch.
> >  Yes, it is the latest.  If the patch still applies cleanly, please feel free.
> >Otherwise, I'd be glad to rework it against something more recent.
> >
> >Also, we never got any ACKs for patch 2/2, which just adds the nodes to
> >arch/arm/boot/dts/pxa27x.dtsi
> >Any advice on whom to nudge?
> >
> >Thanks much!
> >Mike
> >
> 
> It's fine to me.
> 
> Acked-by: Haojian Zhuang <haojian.zhuang@linaro.org>

I think it's customary to take DTS file updates through the architecture
trees, so I think it'd be better if you took patch 2/2. There aren't any
dependencies between both patches either.

Thierry
Haojian Zhuang Dec. 4, 2013, 9:17 a.m. UTC | #7
On 12/04/2013 05:03 PM, Thierry Reding wrote:
> On Wed, Dec 04, 2013 at 09:12:51AM +0800, Haojian Zhuang wrote:
>> On 12/04/2013 02:45 AM, Mike Dunn wrote:
>>> On 12/03/2013 02:17 AM, Thierry Reding wrote:
>>>> On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
>>>>> This patch adds device tree support to the PXA's PWM driver.  Nothing
>>>>> needs to be extracted from the device tree node by the PWM device.
>>>>> Client devices need only specify the period; the per-chip index is
>>>>> implicitly zero because one device node must be present for each PWM
>>>>> output in use.  This approach is more convenient due to the wide
>>>>> variability in the number of PWM channels present across the various PXA
>>>>> variants, and is made possible by the fact that the register sets for
>>>>> each PWM channel are segregated from each other.  An of_xlate() method
>>>>> is added to parse this single-cell 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>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>>>>>   drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>>>>>   2 files changed, 81 insertions(+), 1 deletion(-)
>>>>>   create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>>>> Hi Mike,
>>>>
>>>> It looks like this fell through the cracks. Is this patch still the
>>>> latest one you have? Should it still be applied?
>>>>
>>>> Thierry
>>>>
>>> Hi Thierry,
>>>
>>> Funny I should hear from you about this today.... I just turned my attention
>>> back to this today and noticed that it never made it into your for-next branch.
>>>   Yes, it is the latest.  If the patch still applies cleanly, please feel free.
>>> Otherwise, I'd be glad to rework it against something more recent.
>>>
>>> Also, we never got any ACKs for patch 2/2, which just adds the nodes to
>>> arch/arm/boot/dts/pxa27x.dtsi
>>> Any advice on whom to nudge?
>>>
>>> Thanks much!
>>> Mike
>>>
>> It's fine to me.
>>
>> Acked-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> I think it's customary to take DTS file updates through the architecture
> trees, so I think it'd be better if you took patch 2/2. There aren't any
> dependencies between both patches either.
>
> Thierry
OK. Let me handle this.

Regards
Haojian
Thierry Reding Dec. 4, 2013, 9:21 a.m. UTC | #8
On Tue, Dec 03, 2013 at 10:45:29AM -0800, Mike Dunn wrote:
> On 12/03/2013 02:17 AM, Thierry Reding wrote:
> > On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
> >> This patch adds device tree support to the PXA's PWM driver.  Nothing
> >> needs to be extracted from the device tree node by the PWM device.
> >> Client devices need only specify the period; the per-chip index is
> >> implicitly zero because one device node must be present for each PWM
> >> output in use.  This approach is more convenient due to the wide
> >> variability in the number of PWM channels present across the various PXA
> >> variants, and is made possible by the fact that the register sets for
> >> each PWM channel are segregated from each other.  An of_xlate() method
> >> is added to parse this single-cell 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>
> >> ---
> >>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
> >>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
> >>  2 files changed, 81 insertions(+), 1 deletion(-)
> >>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
> > 
> > Hi Mike,
> > 
> > It looks like this fell through the cracks. Is this patch still the
> > latest one you have? Should it still be applied?
> > 
> > Thierry
> > 
> 
> Hi Thierry,
> 
> Funny I should hear from you about this today.... I just turned my attention
> back to this today and noticed that it never made it into your for-next branch.
>  Yes, it is the latest.  If the patch still applies cleanly, please feel free.
> Otherwise, I'd be glad to rework it against something more recent.

I've applied it to my for-next branch (with some minor whitespace fixups
and some tuning to how the OF match table is defined). I don't consider
any of the changes risky, but it'd be great if you could still test the
version that I pushed.

Thierry
Mike Dunn Dec. 4, 2013, 6:21 p.m. UTC | #9
On 12/04/2013 01:21 AM, Thierry Reding wrote:
> On Tue, Dec 03, 2013 at 10:45:29AM -0800, Mike Dunn wrote:
>> On 12/03/2013 02:17 AM, Thierry Reding wrote:
>>> On Sat, Sep 21, 2013 at 12:19:33PM -0700, Mike Dunn wrote:
>>>> This patch adds device tree support to the PXA's PWM driver.  Nothing
>>>> needs to be extracted from the device tree node by the PWM device.
>>>> Client devices need only specify the period; the per-chip index is
>>>> implicitly zero because one device node must be present for each PWM
>>>> output in use.  This approach is more convenient due to the wide
>>>> variability in the number of PWM channels present across the various PXA
>>>> variants, and is made possible by the fact that the register sets for
>>>> each PWM channel are segregated from each other.  An of_xlate() method
>>>> is added to parse this single-cell 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>
>>>> ---
>>>>  Documentation/devicetree/bindings/pwm/pxa-pwm.txt | 30 +++++++++++++
>>>>  drivers/pwm/pwm-pxa.c                             | 52 ++++++++++++++++++++++-
>>>>  2 files changed, 81 insertions(+), 1 deletion(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>>>
>>> Hi Mike,
>>>
>>> It looks like this fell through the cracks. Is this patch still the
>>> latest one you have? Should it still be applied?
>>>
>>> Thierry
>>>
>>
>> Hi Thierry,
>>
>> Funny I should hear from you about this today.... I just turned my attention
>> back to this today and noticed that it never made it into your for-next branch.
>>  Yes, it is the latest.  If the patch still applies cleanly, please feel free.
>> Otherwise, I'd be glad to rework it against something more recent.
> 
> I've applied it to my for-next branch (with some minor whitespace fixups
> and some tuning to how the OF match table is defined). I don't consider
> any of the changes risky, but it'd be great if you could still test the
> version that I pushed.


I just tested it... looks good.  Thanks again Thierry.  Thanks also Haojian.

Mike
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
new file mode 100644
index 0000000..5ae9f1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
@@ -0,0 +1,30 @@ 
+Marvell PWM controller
+
+Required properties:
+- compatible: should be one or more 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
+  Note that 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.
+
+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/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index a4d2164..e928cc8 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,45 @@  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);
+#else
+static struct of_device_id *pwm_of_match;
+#endif
+
+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);
+	return id ? id->data : NULL;
+}
+
+static 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;
+}
+
 static int pwm_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -131,6 +171,12 @@  static int pwm_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret = 0;
 
+	if (IS_ENABLED(CONFIG_OF) && id == NULL)
+		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,7 +191,10 @@  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;
-
+	if (IS_ENABLED(CONFIG_OF)) {
+		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);
 	if (IS_ERR(pwm->mmio_base))
@@ -176,6 +225,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,