* Re: [PATCH v2 1/4] dt-bindings: pwm: binding allwinner sun8i.
2018-02-25 13:50 [PATCH v2 1/4] dt-bindings: pwm: binding allwinner sun8i hao_zhang
@ 2018-02-26 8:44 ` Maxime Ripard
2018-02-28 1:51 ` [linux-sunxi] " André Przywara
1 sibling, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2018-02-26 8:44 UTC (permalink / raw)
To: hao_zhang
Cc: thierry.reding, robh+dt, mark.rutland, linux, wens,
Claudiu.Beznea, linux-gpio, linux-kernel, devicetree,
linux-arm-kernel, linux-pwm, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
Hi,
On Sun, Feb 25, 2018 at 09:50:45PM +0800, hao_zhang wrote:
> This patch adds allwinner sun8i pwm binding documents.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> Documentation/devicetree/bindings/pwm/pwm-sun8i.txt | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> new file mode 100644
> index 0000000..e8c48be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> @@ -0,0 +1,18 @@
> +Allwinner sun8i R40/V40/T3 SoC PWM controller
> +
> +Required properties:
> + - compatible: should be one of:
> + - "allwinner,sun8i-r40-pwm"
> + - reg: physical base address and length of the controller's registers
> + - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
> + the cells format.
> + - clocks: From common clock binding, handle to the parent clock.
> +
> +Example:
> +
> +pwm: pwm@1c23400 {
> + compatible = "allwinner,sun8i-pwm";
The compatible you have here isn't the one documented.
> + reg = <0x01c23400 0x154>;
And the size isn't correct, even though that doesn't really make any
difference here.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-sunxi] [PATCH v2 1/4] dt-bindings: pwm: binding allwinner sun8i.
2018-02-25 13:50 [PATCH v2 1/4] dt-bindings: pwm: binding allwinner sun8i hao_zhang
2018-02-26 8:44 ` Maxime Ripard
@ 2018-02-28 1:51 ` André Przywara
2018-03-01 9:26 ` Maxime Ripard
1 sibling, 1 reply; 4+ messages in thread
From: André Przywara @ 2018-02-28 1:51 UTC (permalink / raw)
To: hao5781286, thierry.reding, robh+dt, mark.rutland, maxime.ripard
Cc: linux, wens, Claudiu.Beznea, linux-gpio, linux-kernel,
devicetree, linux-arm-kernel, linux-pwm, linux-sunxi
Hi,
On 25/02/18 13:50, hao_zhang wrote:
> This patch adds allwinner sun8i pwm binding documents.
>
> Signed-off-by: hao_zhang <hao5781286@gmail.com>
> ---
> Documentation/devicetree/bindings/pwm/pwm-sun8i.txt | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> new file mode 100644
> index 0000000..e8c48be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
> @@ -0,0 +1,18 @@
> +Allwinner sun8i R40/V40/T3 SoC PWM controller
> +
> +Required properties:
> + - compatible: should be one of:
> + - "allwinner,sun8i-r40-pwm"
> + - reg: physical base address and length of the controller's registers
> + - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
> + the cells format.
> + - clocks: From common clock binding, handle to the parent clock.
The manual tells me that there are two possible clock sources (24 MHz
OSC and APB1), with actually two bits for encoding the mux source,
allowing for two more potential clock sources.
So can we extend this description to provide up to four clocks, with a
clock-names property telling the driver how this maps to the mux value?
Either we use clock names matching the clocks mentioned in the manual:
clocks = <&osc24M>, <&ccu CLK_APB1>;
clock-names = "osc", "apb1";
or we encode the mux values in the clock-names:
clock-names = "mux-0", "mux-1";
Don't know what's more widely used in those cases, the latter seems to
be more future-proof.
Each channel pair can be configured to use one of the two (or four)
clocks, so we can be more flexible with multiple clocks.
Also, can you please add an "interrupts" property here? You don't need
to use it in the driver right now, but since there are multiple
registers dealing with the interrupts we should have it in the binding.
If in need, we can then add support later.
> +
> +Example:
> +
> +pwm: pwm@1c23400 {
> + compatible = "allwinner,sun8i-pwm";
That should match what you wrote above: allwinner,sun8i-r40-pwm.
And I believe this is the right compatible name, since there are
numerous other sun8i SoCs with the sun4i PWM IP, so just sun8i-pwm would
be misleading and confusing.
> + reg = <0x01c23400 0x154>;
> + clocks = <&osc24M>;
This should then be amended to what I sketched above.
> + #pwm-cells = <3>;
And then here please add:
interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
Cheers,
Andre
^ permalink raw reply [flat|nested] 4+ messages in thread