Messages in this thread | | | From | Martin Blumenstingl <> | Date | Tue, 19 Mar 2019 22:22:13 +0100 | Subject | Re: [PATCH v2 1/8] arm64: dts: meson: g12a: Add AO Clock + Reset Controller support |
| |
Hi Neil,
On Tue, Mar 19, 2019 at 9:47 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > > On 18/03/2019 21:02, Martin Blumenstingl wrote: > > Hi Neil, > > > > On Mon, Mar 18, 2019 at 10:59 AM Neil Armstrong <narmstrong@baylibre.com> wrote: > >> > >> Add nodes and properties for the AO Clocks and Resets. > >> > >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > >> --- > >> arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 18 ++++++++++++++++++ > >> 1 file changed, 18 insertions(+) > >> > >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > >> index 31ddf9444b3e..5c0983edf837 100644 > >> --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > >> @@ -4,6 +4,7 @@ > >> */ > >> > >> #include <dt-bindings/gpio/gpio.h> > >> +#include <dt-bindings/clock/g12a-clkc.h> > >> #include <dt-bindings/interrupt-controller/irq.h> > >> #include <dt-bindings/interrupt-controller/arm-gic.h> > >> > >> @@ -122,6 +123,23 @@ > >> #size-cells = <2>; > >> ranges = <0x0 0x0 0x0 0xff800000 0x0 0x100000>; > >> > >> + rti: sys-ctrl@0 { > >> + compatible = "amlogic,meson-gx-ao-sysctrl", > >> + "simple-mfd", "syscon"; > >> + reg = <0x0 0x0 0x0 0x100>; > >> + #address-cells = <2>; > >> + #size-cells = <2>; > >> + ranges = <0x0 0x0 0x0 0x0 0x0 0x100>; > > sorry for noticing this only very late: I missed the #address-cells, > > #size-cells and ranges property in my last review > > do you have any change queued which requires this? > > my understanding is that the drivers for all RTI children should use > > the register offsets relative to the RTI start address. In that case > > the child nodes neither have a unit-address nor a reg property, making > > the last three properties unnecessary. > > We need the address-cells/size-cells and `ranges;` to satisfy the need > for the gpio subnode of the pinctrl node. > > For GX, we didn't add the pinctrl in the sysctrl_AO subnode, but we should > overwise we have overlapping. ah, I see - thank you for the explanation.
setting #address-cells and #size-cells will probably yield dtc warnings for the children without unit-address / reg property. also the pinctrl driver and the syscon driver would still ioremap overlapping memory regions (syscon: the full 0x100 range, the pinctrl driver various 4 and 8 byte registers) because both are ioremap'ing their register space. but indeed, the overlapping dt nodes are solved with this approach.
Rob acked a binding for the Lantiq XWAY SoC's with a simple-mfd / syscon as parent and children with a reg property. The parent node defines a reg and ranges property, but neither #size-cells nor #address-cells. the binding is documented here: [0] The requirements seem similar to our pinctrl needs. We could end up with only a single syscon and no overlapping ioremap for the pinctrl driver and we may not even have to change the binding.
with this mail I want to start a discussion about the bindings of the AO pin controller (similar to how we re-considered the binding of the AO clock controller in the past). at the same time I'm not saying that we immediately have to change it.
Regards Martin
[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/mips/lantiq/rcu.txt
| |