Meson8/Meson8b: introduce a HHI syscon node

The clock controller on Meson8/Meson8m2 and Meson8b is part of a register region called "HHI". This register area contains more functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by Martin Blumenstingl <[email protected]>

Acked-by Neil Armstrong <[email protected]>

---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

arch/arm/boot/dts/meson.dtsi +0 -0

index 0d9faf1a51ea..ad903d1a2cb6 100644

Quoted 23 lines · Martin Blumenstingl

The clock controller on Meson8/Meson8m2 and Meson8b is part of a register region called "HHI". This register area contains more functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by Martin Blumenstingl <[email protected]>

Acked-by Neil Armstrong <[email protected]>

---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

arch/arm/boot/dts/meson.dtsi +0 -0

index 0d9faf1a51ea..ad903d1a2cb6 100644

[...]

arch/arm/boot/dts/meson.dtsi

Quoted 13 lines

+&hhi {

arch/arm/boot/dts/meson.dtsi +7 -0

+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;

Kevin,

I didn't apply this one, but it can wait another cycle I think.

Quoted 43 lines · Neil Armstrong

Quoted 23 lines · Martin Blumenstingl

The clock controller on Meson8/Meson8m2 and Meson8b is part of a register region called "HHI". This register area contains more functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by Martin Blumenstingl <[email protected]>

Acked-by Neil Armstrong <[email protected]>

---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

arch/arm/boot/dts/meson.dtsi +0 -0

index 0d9faf1a51ea..ad903d1a2cb6 100644

[...]

arch/arm/boot/dts/meson.dtsi

Quoted 13 lines

+&hhi {

arch/arm/boot/dts/meson.dtsi +7 -0

+	clkc: clock-controller {
+		compatible = "amlogic,meson8-clkc";
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+};
+
 &hwrng {
 	compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
 	clocks = <&clkc CLKID_RNG0>;

Kevin,

Quoted 1 line

I didn't apply this one, but it can wait another cycle I think.

OK, I'll take that one through my dt branch.

I don't see any obvious dependency on the clock patches,

Quoted 45 lines · Kevin Hilman

Quoted 43 lines · Neil Armstrong

Quoted 23 lines · Martin Blumenstingl

The clock controller on Meson8/Meson8m2 and Meson8b is part of a register region called "HHI". This register area contains more functionality than just a clock controller:
- the clock controller
- some reset controller bits
- temperature sensor calibration data (on Meson8b and Meson8m2 only)
- HDMI controller

Allow access to this HHI register area as "system controller". Also migrate the Meson8 and Meson8b clock controllers to this new node.

Signed-off-by Martin Blumenstingl <[email protected]>

Acked-by Neil Armstrong <[email protected]>

---
 arch/arm/boot/dts/meson.dtsi   |  7 +++++++
 arch/arm/boot/dts/meson8.dtsi  | 15 ++++++++-------
 arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
 3 files changed, 23 insertions(+), 14 deletions(-)

arch/arm/boot/dts/meson.dtsi +0 -0

index 0d9faf1a51ea..ad903d1a2cb6 100644

[...]

arch/arm/boot/dts/meson.dtsi

Quoted 13 lines

+&hhi {

arch/arm/boot/dts/meson.dtsi +7 -0

+   clkc: clock-controller {
+           compatible = "amlogic,meson8-clkc";
+           #clock-cells = <1>;
+           #reset-cells = <1>;
+   };
+};
+
 &hwrng {
    compatible = "amlogic,meson8b-rng", "amlogic,meson-rng";
    clocks = <&clkc CLKID_RNG0>;

Kevin,

Quoted 5 lines

Quoted 1 line

I didn't apply this one, but it can wait another cycle I think.

OK, I'll take that one through my dt branch.

I don't see any obvious dependency on the clock patches,

... and a quick boot test shows I was wrong, and should've (re)read Martin's cover letter a couple more times. :)

I think it's best to leave this one until the next cycle when all the deps are merged.

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset lines are not part of the other reset controller). this reset controller is currently implemented in the clock controller driver
- a HDMI controller

- temperature sensor calibration data (by "data" I really mean data, the ADC driver has four bits for the TSC data in it's own register space, however on Meson8b and Meson8m2 there is a fifth TSC bit which is stored in the HHI register area)

The first three could be implemented with a single node (either in one big driver, or using a MFD driver which would register function- specific drivers).
However, the TSC data is a big problem, because the ADC has it's own set of registers but needs to write one bit in the HHI register area.

NOTE: this series has multiple dependencies:
- the clock controller changes depend "meson8b: add the CPU_DIV16 clock for the ARM TWD" as well as "meson8b: register the clock controller early" [2]
- the dts changes depend on "fix clock controller register size on Meson8/Meson8b" [3]

Changes since v1 at [4]:
- added a "amlogic,meson-hhi-sysctrl" compatible to the syscon node (which is the parent of the clock controller) in patch #1 and #3
- added Neil's Acked-by
- rebased on top of clk-meson's meson-clk-4.20-1 tag and my other series "Meson8b: fixes for the cpu_scale_div clock" from [5]

[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006733.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007890.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007900.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2018-July/007897.html
[4] https://patchwork.kernel.org/cover/10539051/
[5] https://patchwork.kernel.org/cover/10617617/

Martin Blumenstingl (3):
dt-bindings: clock: meson8b: use the registers from the HHI syscon
clk: meson: meson8b: use the HHI syscon if available
ARM: dts: meson: switch the clock controller to the HHI register area

 .../bindings/clock/amlogic,meson8b-clkc.txt   | 13 ++++------
 arch/arm/boot/dts/meson.dtsi                  |  7 ++++++
 arch/arm/boot/dts/meson8.dtsi                 | 15 ++++++------
 arch/arm/boot/dts/meson8b.dtsi                | 15 ++++++------
 drivers/clk/meson/meson8b.c                   | 24 ++++++++++++-------
 5 files changed, 43 insertions(+), 31 deletions(-)

Signature · 1 line

2.19.1

Quoted 9 lines · Martin Blumenstingl

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset lines are not part of the other reset controller). this reset controller is currently implemented in the clock controller driver
- a HDMI controller

Does this have it's own clocks, resets or other resources?

Quoted 5 lines

- temperature sensor calibration data (by "data" I really mean data, the ADC driver has four bits for the TSC data in it's own register space, however on Meson8b and Meson8m2 there is a fifth TSC bit which is stored in the HHI register area)

Quoted 5 lines

The first three could be implemented with a single node (either in one big driver, or using a MFD driver which would register function- specific drivers).
However, the TSC data is a big problem, because the ADC has it's own set of registers but needs to write one bit in the HHI register area.

Generally, that would be solved with a phandle to the HHI and maybe an offset cell in the ADC node. I don't see why that's a big problem.

Hi Rob,

Quoted 12 lines · Rob Herring

Quoted 9 lines · Martin Blumenstingl

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset lines are not part of the other reset controller). this reset controller is currently implemented in the clock controller driver
- a HDMI controller

Quoted 1 line

Does this have it's own clocks, resets or other resources?

I'm not sure what's the best way to answer this as the HDMI part is not part of the public S805 datasheet. however, I went ahead and read Amlogic's BSP code (see [1], [2] and [3] - beware: these files will not make checkpatch happy ;)).
first I have to make a correction here: while looking deeper into this the HDMI controller is *not* inside the HHI register area (but instead on the APB bus). sorry for getting this wrong

here's how would make the .dts to look like (assuming we support all peripherals that I know of in the HHI area):

&hhi {
    compatible = "amlogic,meson-hhi-sysctrl",
                           "simple-mfd",
                           "syscon";

    clkc: clock-controller {
        ...
    };

    hhi_power: power-controller {
        compatible = "amlogic,meson8-hhi-power-controller";
        #power-domain-cells = <1>;
    };

    hdmi_phy: phy {
        compatible = "amlogic,meson8-hdmi-phy";
        #phy-cells = <0>;
        /*
         * HHI_HDMI_PHY_CNTL0 needs a magic value based on
         * power on/off status. HHI_HDMI_PHY_CNTL1 has
         * bit[1]: enable clock, bit[0]: soft reset (maybe
         * these two should be provided by the clkc?)
         */
    };
};

&apb {
    hdmi_tx: hdmi-tx at 42000 {
        compatible = "amlogic,meson8-hdmi";
        reg = <0x42000 0xc>;
        interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
        #address-cells = <1>;
        #size-cells = <0>;

        /*
         * internally the actual HDMI controller registers
         * are not accessed directly but through two special
         * (address and data) registers: HDMI_ADDR_PORT,
         * HDMI_DATA_PORT.
         */

        /*
         * there are five internal reset registers:
         * TX_SYS5_TX_SOFT_RESET_1
         * TX_SYS5_TX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_1
         * TX_SYS5_RX_SOFT_RESET_2
         * TX_SYS5_RX_SOFT_RESET_3
         */

        /*
         * TODO: this has to know a few details about the
         * audio stream as well to route the audio correctly
         * (vendor code differentiates between 2ch / 8ch and
         * programs the HDMI controller accordingly)
         */

        pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
        pinctrl-names = "default";

        /* TODO: what is VCLK1_HDMI exactly? */
        clocks = <&clkc CLKID_HDMI_PCLK>,
                 <&clkc CLKID_HDMI_SYS>,
                 <&clkc CLKID_VCLK1_HDMI>;
        clock-names = "pclk", "sys", "vclk1";

        power-domains = <&hhi_power 0>, <&hhi_power 1> ;
        power-domain-names = "edid", "hdcp";

        phys = <&hdmi_phy>;
        phy-names = "hdmi";

        /* VPU VENC Input */
        hdmi_tx_venc_port: port at 0 {
            reg = <0>;

            hdmi_tx_in: endpoint {
                /*
                 * this is provided by the VPU which has
                 * many (>10) clock inputs - all connected
                 * to "clkc" above
                 */
                remote-endpoint = <&hdmi_tx_out>;
            };
        };

        /* TMDS Output */
        hdmi_tx_tmds_port: port at 1 {
            reg = <1>;
        };
    };
};

&saradc {
...
/*
* patches for this are already sent: TSC bit[4] is
* stored in HHI_DPLL_TOP_0 (which is right between the
* SYS_PLL and VID_PLL registers. the PLL clocks from
* these other registers are implemented in the driver
* for the "clkc" node above)
*/
amlogic,hhi-sysctrl = <&hhi>;
};

Quoted 11 lines

Quoted 10 lines

- temperature sensor calibration data (by "data" I really mean data, the ADC driver has four bits for the TSC data in it's own register space, however on Meson8b and Meson8m2 there is a fifth TSC bit which is stored in the HHI register area)

The first three could be implemented with a single node (either in one big driver, or using a MFD driver which would register function- specific drivers).
However, the TSC data is a big problem, because the ADC has it's own set of registers but needs to write one bit in the HHI register area.

Quoted 2 lines

Generally, that would be solved with a phandle to the HHI and maybe an offset cell in the ADC node. I don't see why that's a big problem.

this is what I'm trying to do, see my dt-bindings patch for the SAR ADC (which is used to read the temperature sensor): [0] my understanding was that I should go through syscon to avoid double ioremap() of the same register set (the clock controller and SAR ADC would have to do it).
which APIs (other than syscon) would you suggest for this case?

Regards
Martin

[0] https://patchwork.kernel.org/patch/10658557/
[1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
[2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
[3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

Quoted 17 lines · Martin Blumenstingl

Hi Rob,

Quoted 13 lines · Rob Herring

Quoted 9 lines · Martin Blumenstingl

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset lines are not part of the other reset controller). this reset controller is currently implemented in the clock controller driver
- a HDMI controller

Does this have it's own clocks, resets or other resources?

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

The first three could be implemented with a single node (either in one big driver, or using a MFD driver which would register function- specific drivers).
However, the TSC data is a big problem, because the ADC has it's own set of registers but needs to write one bit in the HHI register area.

Quoted 8 lines

Quoted 2 lines

Generally, that would be solved with a phandle to the HHI and maybe an offset cell in the ADC node. I don't see why that's a big problem.

this is what I'm trying to do, see my dt-bindings patch for the SAR ADC (which is used to read the temperature sensor): [0] my understanding was that I should go through syscon to avoid double ioremap() of the same register set (the clock controller and SAR ADC would have to do it).
which APIs (other than syscon) would you suggest for this case?

I would suggest avoiding the syscon API when you have other drivers going through the same register space. You can do a syscon like approach by having the HHI driver register one or multiple regmaps, with appropriate access controls, which other drivers can fetch using dev_get_regmap() (via a phandle to the HHI device). I don't know much about simple-mfd though. If the registers are neatly grouped together by function it would be easier to deal with. Or you could just have one device node with appropriate cell properties and have a big driver register all the functions. That driver would likely live under drivers/soc/.

ChenYu

Quoted 9 lines

Regards
Martin

[0] https://patchwork.kernel.org/patch/10658557/
[1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
[2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
[3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h

Quoted 19 lines · Chen-Yu Tsai

Quoted 17 lines · Martin Blumenstingl

Hi Rob,

Quoted 13 lines · Rob Herring

Quoted 9 lines · Martin Blumenstingl

The Meson8/Meson8b/Meson8m2 SoCs are suffering from a similar problem as the GXBB/GXL/GXM SoCs (see the GX series from Jerome: [0]):
There is a register area called "HHI" which is used for multiple IP blocks of the SoC:
- the system clock controller
- a few reset lines (there is a separate reset controller, these reset lines are not part of the other reset controller). this reset controller is currently implemented in the clock controller driver
- a HDMI controller

Does this have it's own clocks, resets or other resources?

[...]

Quoted 16 lines

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

The first three could be implemented with a single node (either in one big driver, or using a MFD driver which would register function- specific drivers).
However, the TSC data is a big problem, because the ADC has it's own set of registers but needs to write one bit in the HHI register area.

Quoted 8 lines

Quoted 2 lines

Generally, that would be solved with a phandle to the HHI and maybe an offset cell in the ADC node. I don't see why that's a big problem.

this is what I'm trying to do, see my dt-bindings patch for the SAR ADC (which is used to read the temperature sensor): [0] my understanding was that I should go through syscon to avoid double ioremap() of the same register set (the clock controller and SAR ADC would have to do it).
which APIs (other than syscon) would you suggest for this case?

Quoted 9 lines

I would suggest avoiding the syscon API when you have other drivers going through the same register space. You can do a syscon like approach by having the HHI driver register one or multiple regmaps, with appropriate access controls, which other drivers can fetch using dev_get_regmap() (via a phandle to the HHI device). I don't know much about simple-mfd though. If the registers are neatly grouped together by function it would be easier to deal with. Or you could just have one device node with appropriate cell properties and have a big driver register all the functions. That driver would likely live under drivers/soc/.

Well, it's the role of simple-mfd, when you only have plaform-tied sub drivers, and it's over-engineered to write an MFD driver for this.

At some point, we should move forward with this since it's a "system controller" specific mapping, and "syscon" was designed specifically to solve this kind of mapping in DT.

The other SoCs (GX, AXG and G12A) uses this exact DT architecture to solve this issue, and we should use this same schema for other Amlogic SoCs even if it's not the most perfect way to solve it.

Neil

Quoted 12 lines

ChenYu

Quoted 9 lines

Regards
Martin

[0] https://patchwork.kernel.org/patch/10658557/
[1] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_hw.c
[2] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/hdmi_tx_hw/hdmi_tx_reg.c
[3] https://github.com/endlessm/linux-meson/blob/3a749cb45a381fb977bae0bee36c6c1cbee32ac1/arch/arm/mach-meson8b/include/mach/register.h