| Message ID | 20190318095851.4062-2-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: g12a: Add peripherals | expand |
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. Regards Martin
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. Neil > > > Regards > Martin >
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
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>; + + clkc_AO: clock-controller { + compatible = "amlogic,meson-g12a-aoclkc"; + #clock-cells = <1>; + #reset-cells = <1>; + clocks = <&xtal>, <&clkc CLKID_CLK81>; + clock-names = "xtal", "mpeg-clk"; + }; + }; + sec_AO: ao-secure@140 { compatible = "amlogic,meson-gx-ao-secure", "syscon"; reg = <0x0 0x140 0x0 0x140>;