[RFC 02/11] dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings

[RFC 02/11] dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings

[Neil Armstrong]

Add bindings for the Amlogic SM1 Power control:
- the VPU power control compatible
- the general-purpose power controller, controlling the USB, PCIe, NNA and
  GE2D power domains.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/power/amlogic,meson-gx-pwrc.txt  | 35 +++++++++++++++++++
 include/dt-bindings/power/meson-sm1-power.h   | 15 ++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 include/dt-bindings/power/meson-sm1-power.h

diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-gx-pwrc.txt b/Documentation/devicetree/bindings/power/amlogic,meson-gx-pwrc.txt
index 0fdc3dd1125e..f0a1e20555bf 100644
--- a/Documentation/devicetree/bindings/power/amlogic,meson-gx-pwrc.txt
+++ b/Documentation/devicetree/bindings/power/amlogic,meson-gx-pwrc.txt
@@ -19,6 +19,7 @@ Required properties:
 - compatible: should be one of the following :
 	- "amlogic,meson-gx-pwrc-vpu" for the Meson GX SoCs
 	- "amlogic,meson-g12a-pwrc-vpu" for the Meson G12A SoCs
+	- "amlogic,meson-sm1-pwrc-vpu" for the Meson SM1 SoCs
 - #power-domain-cells: should be 0
 - amlogic,hhi-sysctrl: phandle to the HHI sysctrl node
 - resets: phandles to the reset lines needed for this power demain sequence
@@ -60,4 +61,38 @@ ao_sysctrl: sys-ctrl@0 {
 	};
 };
 
+General Purpose Power Controller
+--------------------------------
 
+The Amlogic SM1 SoCs embeds a General Purpose Power Controller used
+to control the power domain for, at least, the USB PHYs and PCIe
+peripherals.
+
+
+Device Tree Bindings:
+---------------------
+
+Required properties:
+- compatible: should be one of the following :
+	- "amlogic,meson-sm1-pwrc" for the Meson SM1 SoCs
+- #power-domain-cells: should be 0
+- amlogic,hhi-sysctrl: phandle to the HHI sysctrl node
+
+Parent node should have the following properties :
+- compatible: "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd"
+- reg: base address and size of the AO system control register space.
+
+
+Example:
+-------
+
+ao_sysctrl: sys-ctrl@0 {
+	compatible = "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd";
+	reg =  <0x0 0x0 0x0 0x100>;
+
+	pwrc: power-controller {
+		compatible = "amlogic,meson-sm1-pwrc";
+		#power-domain-cells = <1>;
+		amlogic,hhi-sysctrl = <&hhi>;
+	};
+};
diff --git a/include/dt-bindings/power/meson-sm1-power.h b/include/dt-bindings/power/meson-sm1-power.h
new file mode 100644
index 000000000000..30e17e4a478e
--- /dev/null
+++ b/include/dt-bindings/power/meson-sm1-power.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#ifndef _DT_BINDINGS_MESON_SM1_POWER_H
+#define _DT_BINDINGS_MESON_SM1_POWER_H
+
+#define PWRC_SM1_NNA_ID		0
+#define PWRC_SM1_USB_ID		1
+#define PWRC_SM1_PCIE_ID	2
+#define PWRC_SM1_GE2D_ID	3
+
+#endif
-- 
2.21.0

[RFC 05/11] dt-bindings: soc: amlogic: clk-measure: Add SM1 compatible

[Neil Armstrong]

Add the Amlogic SM1 Compatible for the clk-measurer IP.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
index 6bf6b43f8dd8..3dd563cec794 100644
--- a/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
+++ b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
@@ -11,6 +11,7 @@ Required properties:
 			"amlogic,meson8b-clk-measure" for Meson8b SoCs
 			"amlogic,meson-axg-clk-measure" for AXG SoCs
 			"amlogic,meson-g12a-clk-measure" for G12a SoCs
+			"amlogic,meson-sm1-clk-measure" for SM1 SoCs
 - reg: base address and size of the Clock Measurer register space.
 
 Example:
-- 
2.21.0

[RFC 07/11] dt-bindings: media: meson-ao-cec: add SM1 compatible

[Neil Armstrong]

Add AO-CEC compatible string for the Amlogic SM1 SoC family,
a derivate of the G12A AO-CECB controller.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/media/meson-ao-cec.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/meson-ao-cec.txt b/Documentation/devicetree/bindings/media/meson-ao-cec.txt
index c67fc41d4aa2..ad92ee41c0dd 100644
--- a/Documentation/devicetree/bindings/media/meson-ao-cec.txt
+++ b/Documentation/devicetree/bindings/media/meson-ao-cec.txt
@@ -5,10 +5,12 @@ to handle communication between HDMI connected devices over the CEC bus.
 
 Required properties:
   - compatible : value should be following depending on the SoC :
-	For GXBB, GXL, GXM and G12A (AO_CEC_A module) :
+	For GXBB, GXL, GXM, G12A and SM1 (AO_CEC_A module) :
 	"amlogic,meson-gx-ao-cec"
 	For G12A (AO_CEC_B module) :
 	"amlogic,meson-g12a-ao-cec"
+	For SM1 (AO_CEC_B module) :
+	"amlogic,meson-sm1-ao-cec"
 
   - reg : Physical base address of the IP registers and length of memory
 	  mapped region.
@@ -16,9 +18,9 @@ Required properties:
   - interrupts : AO-CEC interrupt number to the CPU.
   - clocks : from common clock binding: handle to AO-CEC clock.
   - clock-names : from common clock binding, must contain :
-		For GXBB, GXL, GXM and G12A (AO_CEC_A module) :
+		For GXBB, GXL, GXM, G12A and SM1 (AO_CEC_A module) :
 		- "core"
-		For G12A (AO_CEC_B module) :
+		For G12A, SM1 (AO_CEC_B module) :
 		- "oscin"
 		corresponding to entry in the clocks property.
   - hdmi-phandle: phandle to the HDMI controller
-- 
2.21.0

Re: [RFC 02/11] dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings

[Martin Blumenstingl]

Hi Neil,

On Mon, Jul 1, 2019 at 12:48 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> +General Purpose Power Controller
> +--------------------------------
>
> +The Amlogic SM1 SoCs embeds a General Purpose Power Controller used
> +to control the power domain for, at least, the USB PHYs and PCIe
> +peripherals.
AFAIK each binding document should only describe one IP block.
this one seems to be new / different

should it get it's own file?
also should it be a .yaml binding?

> +
> +Device Tree Bindings:
> +---------------------
> +
> +Required properties:
> +- compatible: should be one of the following :
> +       - "amlogic,meson-sm1-pwrc" for the Meson SM1 SoCs
> +- #power-domain-cells: should be 0
> +- amlogic,hhi-sysctrl: phandle to the HHI sysctrl node
> +
> +Parent node should have the following properties :
> +- compatible: "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd"
> +- reg: base address and size of the AO system control register space.
> +
> +
> +Example:
> +-------
> +
> +ao_sysctrl: sys-ctrl@0 {
> +       compatible = "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd";
> +       reg =  <0x0 0x0 0x0 0x100>;
> +
> +       pwrc: power-controller {
> +               compatible = "amlogic,meson-sm1-pwrc";
> +               #power-domain-cells = <1>;
> +               amlogic,hhi-sysctrl = <&hhi>;
> +       };
> +};
I'm not sure that we want to mix HHI and AO power domains in one driver again
back in March I asked a few questions about modelling the power
domains and Kevin explained that we can implement them hierarchical:
[0]
unfortunately I didn't have the time to work on this - however, now
that we implement a new driver: should we follow this hierarchical
approach?


Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2019-March/010512.html

Re: [RFC 05/11] dt-bindings: soc: amlogic: clk-measure: Add SM1 compatible

[Martin Blumenstingl]

On Mon, Jul 1, 2019 at 12:49 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Add the Amlogic SM1 Compatible for the clk-measurer IP.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [RFC 05/11] dt-bindings: soc: amlogic: clk-measure: Add SM1 compatible

[Rob Herring]

On Mon,  1 Jul 2019 12:46:59 +0200, Neil Armstrong wrote:
> Add the Amlogic SM1 Compatible for the clk-measurer IP.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFC 07/11] dt-bindings: media: meson-ao-cec: add SM1 compatible

[Rob Herring]

On Mon,  1 Jul 2019 12:47:01 +0200, Neil Armstrong wrote:
> Add AO-CEC compatible string for the Amlogic SM1 SoC family,
> a derivate of the G12A AO-CECB controller.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/media/meson-ao-cec.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFC 02/11] dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings

[Kevin Hilman]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Neil,
>
> On Mon, Jul 1, 2019 at 12:48 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> [...]
>> +General Purpose Power Controller
>> +--------------------------------
>>
>> +The Amlogic SM1 SoCs embeds a General Purpose Power Controller used
>> +to control the power domain for, at least, the USB PHYs and PCIe
>> +peripherals.
> AFAIK each binding document should only describe one IP block.
> this one seems to be new / different
>
> should it get it's own file?
> also should it be a .yaml binding?

I don't think this is a new IP block.  Comparing across the various
(64-bit) SoCs, it seems to be very similar across all SoCs.

>> +
>> +Device Tree Bindings:
>> +---------------------
>> +
>> +Required properties:
>> +- compatible: should be one of the following :
>> +       - "amlogic,meson-sm1-pwrc" for the Meson SM1 SoCs
>> +- #power-domain-cells: should be 0
>> +- amlogic,hhi-sysctrl: phandle to the HHI sysctrl node
>> +
>> +Parent node should have the following properties :
>> +- compatible: "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd"
>> +- reg: base address and size of the AO system control register space.
>> +
>> +
>> +Example:
>> +-------
>> +
>> +ao_sysctrl: sys-ctrl@0 {
>> +       compatible = "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd";
>> +       reg =  <0x0 0x0 0x0 0x100>;
>> +
>> +       pwrc: power-controller {
>> +               compatible = "amlogic,meson-sm1-pwrc";
>> +               #power-domain-cells = <1>;
>> +               amlogic,hhi-sysctrl = <&hhi>;
>> +       };
>> +};
>
> I'm not sure that we want to mix HHI and AO power domains in one driver again

We're not mixing here. These are all EE domains.  They just have some
control registers in the AO memory region.

> back in March I asked a few questions about modelling the power
> domains and Kevin explained that we can implement them hierarchical:
> [0]
> unfortunately I didn't have the time to work on this - however, now
> that we implement a new driver: should we follow this hierarchical
> approach?

The more I look at this, I don't think we have a commpelling need to
model them hierarchically.  The main reason being is that of the 3
top-level domains I listed[0], we can only managing the EE domains in the
kernel.  It doesn't make sense to model/manage AO domains because, well,
they are always-on (AO).  The CPU domains are managed my the PSCI
firmware, and we don't/won't have any control over that.

For that reason, I think it makes the most sense to have a generic
driver that handles all the EE domains.

IMO, the SM1 driver that Neil wrote in patch 4 of this series is 80%
there.  If we generalize that little more, it can be quite easily used
for all the EE domains.

Kevin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2019-March/010512.html

Re: [RFC 02/11] dt-bindings: power: amlogic, meson-gx-pwrc: Add SM1 bindings

[Martin Blumenstingl]

Hi Kevin,

On Tue, Aug 20, 2019 at 2:06 AM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> >> +ao_sysctrl: sys-ctrl@0 {
> >> +       compatible = "amlogic,meson-gx-ao-sysctrl", "syscon", "simple-mfd";
> >> +       reg =  <0x0 0x0 0x0 0x100>;
> >> +
> >> +       pwrc: power-controller {
> >> +               compatible = "amlogic,meson-sm1-pwrc";
> >> +               #power-domain-cells = <1>;
> >> +               amlogic,hhi-sysctrl = <&hhi>;
> >> +       };
> >> +};
> >
> > I'm not sure that we want to mix HHI and AO power domains in one driver again
>
> We're not mixing here. These are all EE domains.  They just have some
> control registers in the AO memory region.
looking at patch 04/11 I see what you mean
(I'm describing it in my own words to make sure I got it right)
we are controlling the EE power domains with this binding.
each power domain has 1 bit in the HHI registers and 2 more bits
("sleep" and "isolation") in the AO region

then it makes sense to describe this together

> > back in March I asked a few questions about modelling the power
> > domains and Kevin explained that we can implement them hierarchical:
> > [0]
> > unfortunately I didn't have the time to work on this - however, now
> > that we implement a new driver: should we follow this hierarchical
> > approach?
>
> The more I look at this, I don't think we have a commpelling need to
> model them hierarchically.  The main reason being is that of the 3
> top-level domains I listed[0], we can only managing the EE domains in the
> kernel.  It doesn't make sense to model/manage AO domains because, well,
> they are always-on (AO).  The CPU domains are managed my the PSCI
> firmware, and we don't/won't have any control over that.
agreed, this is the same for the 32-bit SoCs except that we manage the
CPU domains in arch/arm/mach-meson/platsmp.c instead of PSCI firmware
(no problem here, I'm just mentioning it to get a complete picture)

> For that reason, I think it makes the most sense to have a generic
> driver that handles all the EE domains.
>
> IMO, the SM1 driver that Neil wrote in patch 4 of this series is 80%
> there.  If we generalize that little more, it can be quite easily used
> for all the EE domains.
with your description above I agree.

for the 32-bit SoCs it would be beneficial if the register layout in
the bindings was swapped:
- have the power controller as child of HHI
- pass the AO syscon

my main points for this are:
- it seems that for some power domains there are no AO register bits,
for example the Ethernet Memory PD (GXBB datasheet [0] section 18.3 on
page 48 and Meson8b datasheet [1] section 5.4 on page 17)
- less confusion: if it's a power domain controller for the EE region
then it should be located there, even if it has additional bits
elsewhere
- (weakest argument though) on the 32-bit SoCs we currently don't have
a "big AO syscon" (and I don't see that we actually need it), but we
do have a "amlogic,meson8b-pmu", "syscon" binding which covers
AO_RTI_GEN_PWR_SLEEP0 (I should probably extend it so it covers
AO_RTI_GEN_PWR_ISO0 as well, that just extra 4 bytes)

What do you think?


Martin


[0] https://dn.odroid.com/S905/DataSheet/S905_Public_Datasheet_V1.1.4.pdf
[1] https://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%2020150126.pdf