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@xxxxxxxxxxx>
>> ---
>> 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
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
