On 09/08/2013 05:49 PM, 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 platform_device id table is reused for the match table
>> data. Support for inverted polarity is also added.
>>
>> Tested on a Palm Treo 680 (both platform data and DT cases).
>>
>> Signed-off-by: Mike Dunn <mikedunn@xxxxxxxxxxx>
>> ---
>> Changle log:
>> 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 | 33 +++++++++++++
>> arch/arm/boot/dts/pxa27x.dtsi | 24 ++++++++++
>> drivers/pwm/pwm-pxa.c | 57
>> +++++++++++++++++++++++ 3 files changed, 114 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..7b09bfa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt
>> @@ -0,0 +1,33 @@
>> +Marvell pwm controller
>> +
>> +Required properties:
>> +- compatible:
>> + for pxa25x, pxa27x, pxa168, pxa910: must be compatible =
>> "marvell,pxa250-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 3.
>> + cell 1: the per-chip index of the PWM to use,
>> + cell 2: the period in nanoseconds,
>
> Is there no generic OF prop for this?
Yes, all the pwm drivers with DT support have the same definitions for cell 1
and 2. Many have #pwm-cells=2, but for those with #pwm-cells=3, all define cell
3 the same way also.
>
>> + cell 3: flags; set bit 0 to specify inverse polarity.
>
> Do we not have some generic flag to indicate inverted PWM polarity?
Yes, enum pwm_polarity, defined in include/linux/pwm.h. The pwm drivers with
polarity support all define cell 3 the same way. The wording in the bindings
documentation is a little different in each case, but all essentially say the
same thing. In Documentation/devicetree/bindings/pwm/ see for example
atmel-tcb-pwm.txt and pwm-tiecap.txt.
I'm new to device tree, so maybe I am not understanding correctly this and your
previous question... All DT properties are described as integers lacking any
type (aside from their bit width), no?
>
> [...]
>
>> +#ifdef CONFIG_OF
>> +/*
>> + * Device tree users should 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.
>> + */
>
> Above ... pxa250-pwm , no ?
>
>> +static struct of_device_id pwm_of_match[] = {
>> + { .compatible = "marvell,pxa250-pwm", .data = &pwm_id_table[0]},
>
> Surely, data can be NULL, no ?
It could, in which case pxa_pwm_get_id_dt() would explicitly return
&pwm_id_table[0] instead of the .data element of the of_device_id. Not sure
which way is better and why. That dumb platform_device_id table is causing all
kinds of nuisance :)
>
> [...]
>
>> @@ -145,6 +199,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 = of_pwm_xlate_with_flags;
>> + pwm->chip.of_pwm_n_cells = 3;
>
> Are these two settings needed ?
Yes. See drivers/pwm/core.c:of_pwmchip_add(), where they are set to default
values of of_pwm_simple_xlate and 2 if left uninitialized.
Thanks again Marek,
Mike
--
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
