On 12/7/20 10:12 AM, Maxime Ripard wrote: > Hi, > > On Mon, Dec 07, 2020 at 05:05:03PM +0100, Wilken Gottwalt wrote: >> Adds documentation on how to use the sun8i_hwspinlock driver for sun8i >> compatible SoCs. >> >> Signed-off-by: Wilken Gottwalt <wilken.gottw...@posteo.net> >> --- >> .../bindings/hwlock/sun8i-hwspinlock.yaml | 63 +++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml >> b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml >> new file mode 100644 >> index 000000000000..2954ee0b36a7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml >> @@ -0,0 +1,63 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs >> + >> +maintainers: >> + - Wilken Gottwalt <wilken.gottw...@posteo.net> >> + >> +properties: >> + compatible: >> + enum: >> + - allwinner,sun8i-hwspinlock > > This can be a const instead of an enum, and since it was introduced with > the A33 it should be sun8i-a33-hwspinlock. There's a lot of SoCs in that > family, some without that IP, and we could even see new SoCs in that > family with a different IP at some point.
I just looked at the A31 ARISC blob, and it uses the hwspinlock hardware as well, with the same MMIO address and gate/reset bits as A33-H3. So the first compatible would actually be sun6i-a31-hwspinlock. Cheers, Samuel >> + >> + reg: # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000 >> + maxItems: 1 > > There's no need for those comments > >> + >> + clocks: # phandle to the reference clock > > This should be the description, and it's fairly obvious so you don't > really need that comment. > >> + maxItems: 1 >> + >> + clock-names: # name of the bus ("ahb") >> + maxItems: 1 > > You don't need clock-names if there's a single clock > >> + >> + resets: # phandle to the reset control >> + maxItems: 1 > > Same thing than for the clocks > >> + >> + reset-names: # name of the bus ("ahb") >> + maxItems: 1 >> + > > Ditto > >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - resets >> + - reset-names >> + >> +additionalProperties: false >> + >> +examples: >> + >> + - | >> + /* H2+ based OrangePi Zero */ >> + hwspinlock: hwspinlock@1C18000 { > > Unit-address's are lowercase > >> + compatible = "allwinner,sun8i-hwspinlock"; >> + reg = <0x01c18000 0x1000>; >> + clocks = <&ccu CLK_BUS_SPINLOCK>; >> + clock-names = "ahb"; >> + resets = <&ccu RST_BUS_SPINLOCK>; >> + reset-names = "ahb"; >> + }; >> + >> + /* H6 based OrangePi 3 */ >> + hwspinlock: hwspinlock@3004000 { >> + compatible = "allwinner,sun8i-hwspinlock"; >> + reg = <0x03004000 0x1000>; >> + clocks = <&ccu CLK_BUS_SPINLOCK>; >> + clock-names = "ahb"; >> + resets = <&ccu RST_BUS_SPINLOCK>; >> + reset-names = "ahb"; >> + }; > > Different examples should be different items on that list, but both are > essentially the same binding so you can drop one. > > Maxime >