Re: [PATCH v6 06/12] clk: actions: Add common clock driver

On 18/03/2020 18:25, Amit Singh Tomar wrote:

This patch converts S900 clock driver to something common that can be used for other SoCs, for instance S700(few of clk registers are same).

Signed-off-by: Amit Singh Tomar amittomer25@gmail.com

Compared clk_s900.h vs clk_owl.h, and checked the actual clock driver, see the one bug below. Didn't check the S700 registers.

With that bug fixed:

Reviewed-by: Andre Przywara andre.przywara@arm.com

Cheers, Andre

....

---

Changes since v5:

• Removed the white space errors from "clk_owl.h".

Changes since v4: * Removed parentheses from "regs_s700.h" * Moved this patch, from 03/11 to 05/11. * Merged owl_uart_clk_enable() into owl_clk_enable. Changes since v3: * Fixed register spelling. * Removed the ethernet related clocks. * Returned -EINVAL instead 0. Changes since v2: * Fixed the commit message. * Checked for the clk->id. * Added a .data member with SoC type. * Removed #ifdefs from few places. Changes since v1: * Moved CLK and CLK_OWL symbols from defconfig to arch/arm/Kconfig.

---

arch/arm/Kconfig | 2 + arch/arm/include/asm/arch-owl/clk_s900.h | 57 ----------- arch/arm/include/asm/arch-owl/regs_s700.h | 56 +++++++++++ configs/bubblegum_96_defconfig | 3 - drivers/clk/owl/Kconfig | 8 +- drivers/clk/owl/Makefile | 2 +- drivers/clk/owl/clk_owl.c | 152 ++++++++++++++++++++++++++++++ drivers/clk/owl/clk_owl.h | 65 +++++++++++++ drivers/clk/owl/clk_s900.c | 137 --------------------------- 9 files changed, 278 insertions(+), 204 deletions(-) delete mode 100644 arch/arm/include/asm/arch-owl/clk_s900.h create mode 100644 arch/arm/include/asm/arch-owl/regs_s700.h create mode 100644 drivers/clk/owl/clk_owl.c create mode 100644 drivers/clk/owl/clk_owl.h delete mode 100644 drivers/clk/owl/clk_s900.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 1cfdcb6..dba4d8c 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -876,6 +876,8 @@ config ARCH_OWL select DM select DM_SERIAL select OWL_SERIAL

• select CLK
• select CLK_OWL select OF_CONTROL imply CMD_DM

diff --git a/arch/arm/include/asm/arch-owl/clk_s900.h b/arch/arm/include/asm/arch-owl/clk_s900.h deleted file mode 100644 index 88e88f7..0000000 --- a/arch/arm/include/asm/arch-owl/clk_s900.h +++ /dev/null @@ -1,57 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/*

• • Actions Semi S900 Clock Definitions
• • Copyright (C) 2015 Actions Semi Co., Ltd.
• • Copyright (C) 2018 Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
• */

-#ifndef _OWL_CLK_S900_H_ -#define _OWL_CLK_S900_H_

-#include <clk-uclass.h>

-struct owl_clk_priv {

• phys_addr_t base;

-};

-/* BUSCLK register definitions */ -#define CMU_PDBGDIV_8 7 -#define CMU_PDBGDIV_SHIFT 26 -#define CMU_PDBGDIV_DIV (CMU_PDBGDIV_8 << CMU_PDBGDIV_SHIFT) -#define CMU_PERDIV_8 7 -#define CMU_PERDIV_SHIFT 20 -#define CMU_PERDIV_DIV (CMU_PERDIV_8 << CMU_PERDIV_SHIFT) -#define CMU_NOCDIV_2 1 -#define CMU_NOCDIV_SHIFT 19 -#define CMU_NOCDIV_DIV (CMU_NOCDIV_2 << CMU_NOCDIV_SHIFT) -#define CMU_DMMCLK_SRC_APLL 2 -#define CMU_DMMCLK_SRC_SHIFT 10 -#define CMU_DMMCLK_SRC (CMU_DMMCLK_SRC_APLL << CMU_DMMCLK_SRC_SHIFT) -#define CMU_APBCLK_DIV BIT(8) -#define CMU_NOCCLK_SRC BIT(7) -#define CMU_AHBCLK_DIV BIT(4) -#define CMU_CORECLK_MASK 3 -#define CMU_CORECLK_CPLL BIT(1) -#define CMU_CORECLK_HOSC BIT(0)

-/* COREPLL register definitions */ -#define CMU_COREPLL_EN BIT(9) -#define CMU_COREPLL_HOSC_EN BIT(8) -#define CMU_COREPLL_OUT (1104 / 24)

-/* DEVPLL register definitions */ -#define CMU_DEVPLL_CLK BIT(12) -#define CMU_DEVPLL_EN BIT(8) -#define CMU_DEVPLL_OUT (660 / 6)

-/* UARTCLK register definitions */ -#define CMU_UARTCLK_SRC_DEVPLL BIT(16)

-/* DEVCLKEN1 register definitions */ -#define CMU_DEVCLKEN1_UART5 BIT(21)

-#define PLL_STABILITY_WAIT_US 50

-#endif diff --git a/arch/arm/include/asm/arch-owl/regs_s700.h b/arch/arm/include/asm/arch-owl/regs_s700.h new file mode 100644 index 0000000..2f21c15 --- /dev/null +++ b/arch/arm/include/asm/arch-owl/regs_s700.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Actions Semi S700 Register Definitions
• */

+#ifndef _OWL_REGS_S700_H_ +#define _OWL_REGS_S700_H_

+#define CMU_COREPLL 0x0000 +#define CMU_DEVPLL 0x0004 +#define CMU_DDRPLL 0x0008 +#define CMU_NANDPLL 0x000C +#define CMU_DISPLAYPLL 0x0010 +#define CMU_AUDIOPLL 0x0014 +#define CMU_TVOUTPLL 0x0018 +#define CMU_BUSCLK 0x001C +#define CMU_SENSORCLK 0x0020 +#define CMU_LCDCLK 0x0024 +#define CMU_DSIPLLCLK 0x0028 +#define CMU_CSICLK 0x002C +#define CMU_DECLK 0x0030 +#define CMU_SICLK 0x0034 +#define CMU_BUSCLK1 0x0038 +#define CMU_HDECLK 0x003C +#define CMU_VDECLK 0x0040 +#define CMU_VCECLK 0x0044 +#define CMU_NANDCCLK 0x004C +#define CMU_SD0CLK 0x0050 +#define CMU_SD1CLK 0x0054 +#define CMU_SD2CLK 0x0058 +#define CMU_UART0CLK 0x005C +#define CMU_UART1CLK 0x0060 +#define CMU_UART2CLK 0x0064 +#define CMU_UART3CLK 0x0068 +#define CMU_UART4CLK 0x006C +#define CMU_UART5CLK 0x0070 +#define CMU_UART6CLK 0x0074 +#define CMU_PWM0CLK 0x0078 +#define CMU_PWM1CLK 0x007C +#define CMU_PWM2CLK 0x0080 +#define CMU_PWM3CLK 0x0084 +#define CMU_PWM4CLK 0x0088 +#define CMU_PWM5CLK 0x008C +#define CMU_GPU3DCLK 0x0090 +#define CMU_CORECTL 0x009C +#define CMU_DEVCLKEN0 0x00A0 +#define CMU_DEVCLKEN1 0x00A4 +#define CMU_DEVRST0 0x00A8 +#define CMU_DEVRST1 0x00AC +#define CMU_USBPLL 0x00B0 +#define CMU_ETHERNETPLL 0x00B4 +#define CMU_CVBSPLL 0x00B8 +#define CMU_SSTSCLK 0x00C0

+#endif diff --git a/configs/bubblegum_96_defconfig b/configs/bubblegum_96_defconfig index 8c94def..e76e9a2 100644 --- a/configs/bubblegum_96_defconfig +++ b/configs/bubblegum_96_defconfig @@ -17,6 +17,3 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_TIMER=y CONFIG_DEFAULT_DEVICE_TREE="bubblegum_96" CONFIG_SYS_RELOC_GD_ENV_ADDR=y -CONFIG_CLK=y -CONFIG_CLK_OWL=y -CONFIG_CLK_S900=y diff --git a/drivers/clk/owl/Kconfig b/drivers/clk/owl/Kconfig index 661f198..c6afef9 100644 --- a/drivers/clk/owl/Kconfig +++ b/drivers/clk/owl/Kconfig @@ -3,10 +3,6 @@ config CLK_OWL depends on CLK && ARCH_OWL help Enable support for clock managemet unit present in Actions Semi

•  OWL SoCs.
  

•  Owl series S900/S700 SoCs.
  

-config CLK_S900

•    bool "Actions Semi S900 clock driver"
  

•    depends on CLK_OWL && ARM64
  

•    help
  

•      Enable support for the clocks in Actions Semi S900 SoC.
  

diff --git a/drivers/clk/owl/Makefile b/drivers/clk/owl/Makefile index 63ab573..5218b6b 100644 --- a/drivers/clk/owl/Makefile +++ b/drivers/clk/owl/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0+

-obj-$(CONFIG_CLK_S900) += clk_s900.o +obj-$(CONFIG_CLK_OWL) += clk_owl.o diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c new file mode 100644 index 0000000..ba53b43 --- /dev/null +++ b/drivers/clk/owl/clk_owl.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Common clock driver for Actions Semi SoCs.
• • Copyright (C) 2015 Actions Semi Co., Ltd.
• • Copyright (C) 2018 Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
• */

+#include <common.h> +#include <dm.h> +#include "clk_owl.h" +#include <asm/io.h> +#if defined(CONFIG_MACH_S900) +#include <asm/arch-owl/regs_s900.h> +#include <dt-bindings/clock/actions,s900-cmu.h> +#elif defined(CONFIG_MACH_S700) +#include <asm/arch-owl/regs_s700.h> +#include <dt-bindings/clock/actions,s700-cmu.h> +#endif

+void owl_clk_init(struct owl_clk_priv *priv) +{

• u32 bus_clk = 0, core_pll, dev_pll;

+#if defined(CONFIG_MACH_S900)

• /* Enable ASSIST_PLL */
• setbits_le32(priv->base + CMU_ASSISTPLL, BIT(0));
• udelay(PLL_STABILITY_WAIT_US);

+#endif

• /* Source HOSC to DEV_CLK */
• clrbits_le32(priv->base + CMU_DEVPLL, CMU_DEVPLL_CLK);
• /* Configure BUS_CLK */
• bus_clk |= (CMU_PDBGDIV_DIV | CMU_PERDIV_DIV | CMU_NOCDIV_DIV |

• 	CMU_DMMCLK_SRC | CMU_APBCLK_DIV | CMU_AHBCLK_DIV |
  

• 	CMU_NOCCLK_SRC | CMU_CORECLK_HOSC);
  

• writel(bus_clk, priv->base + CMU_BUSCLK);
• udelay(PLL_STABILITY_WAIT_US);
• /* Configure CORE_PLL */
• core_pll = readl(priv->base + CMU_COREPLL);
• core_pll |= (CMU_COREPLL_EN | CMU_COREPLL_HOSC_EN | CMU_COREPLL_OUT);
• writel(core_pll, priv->base + CMU_COREPLL);
• udelay(PLL_STABILITY_WAIT_US);
• /* Configure DEV_PLL */
• dev_pll = readl(priv->base + CMU_DEVPLL);
• dev_pll |= (CMU_DEVPLL_EN | CMU_DEVPLL_OUT);
• writel(dev_pll, priv->base + CMU_DEVPLL);
• udelay(PLL_STABILITY_WAIT_US);
• /* Source CORE_PLL for CORE_CLK */
• clrsetbits_le32(priv->base + CMU_BUSCLK, CMU_CORECLK_MASK,

• 	CMU_CORECLK_CPLL);
  

• /* Source DEV_PLL for DEV_CLK */
• setbits_le32(priv->base + CMU_DEVPLL, CMU_DEVPLL_CLK);
• udelay(PLL_STABILITY_WAIT_US);

+}

+int owl_clk_enable(struct clk *clk) +{

• struct owl_clk_priv *priv = dev_get_priv(clk->dev);
• switch (clk->id) {
• case CLK_UART5:

• if (priv->soc == S700)
  

• 	return -EINVAL;
  

• /* Source HOSC for UART5 interface */
  

• clrbits_le32(priv->base + CMU_UART5CLK, CMU_UARTCLK_SRC_DEVPLL);
  

• /* Enable UART5 interface clock */
  

• setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);
  

This is missing a break here.

Cheers, Andre.

• case CLK_UART3:

• if (priv->soc == S900)
  

• 	return -EINVAL;
  

• /* Source HOSC for UART3 interface */
  

• clrbits_le32(priv->base + CMU_UART3CLK, CMU_UARTCLK_SRC_DEVPLL);
  

• /* Enable UART3 interface clock */
  

• setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART3);
  

• break;
  

• default:

• return -EINVAL;
  

• }
• return 0;

+}

+int owl_clk_disable(struct clk *clk) +{

• struct owl_clk_priv *priv = dev_get_priv(clk->dev);
• switch (clk->id) {
• case CLK_UART5:

• if (priv->soc == S700)
  

• 	return -EINVAL;
  

• /* Disable UART5 interface clock */
  

• clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);
  

• break;
  

• case CLK_UART3:

• if (priv->soc == S900)
  

• 	return -EINVAL;
  

• /* Disable UART3 interface clock */
  

• clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART3);
  

• break;
  

• default:

• return -EINVAL;
  

• }
• return 0;

+}

+static int owl_clk_probe(struct udevice *dev) +{

• struct owl_clk_priv *priv = dev_get_priv(dev);
• priv->base = dev_read_addr(dev);
• if (priv->base == FDT_ADDR_T_NONE)

• return -EINVAL;
  

• /* setup necessary clocks */
• owl_clk_init(priv);
• return 0;

+}

+static const struct clk_ops owl_clk_ops = {

• .enable = owl_clk_enable,
• .disable = owl_clk_disable,

+};

+static const struct udevice_id owl_clk_ids[] = { +#if defined(CONFIG_MACH_S900)

• { .compatible = "actions,s900-cmu", .data = S900 },

+#elif defined(CONFIG_MACH_S700)

• { .compatible = "actions,s700-cmu", .data = S700 },

+#endif

• { }

+};

+U_BOOT_DRIVER(clk_owl) = {

• .name = "clk_owl",
• .id = UCLASS_CLK,
• .of_match = owl_clk_ids,
• .ops = &owl_clk_ops,
• .priv_auto_alloc_size = sizeof(struct owl_clk_priv),
• .probe = owl_clk_probe,

+}; diff --git a/drivers/clk/owl/clk_owl.h b/drivers/clk/owl/clk_owl.h new file mode 100644 index 0000000..8ebf012 --- /dev/null +++ b/drivers/clk/owl/clk_owl.h @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Actions Semi SoCs Clock Definitions
• • Copyright (C) 2015 Actions Semi Co., Ltd.
• • Copyright (C) 2018 Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
• */

+#ifndef _OWL_CLK_H_ +#define _OWL_CLK_H_

+#include <clk-uclass.h>

+enum owl_soc {

• S700,
• S900,

+};

+struct owl_clk_priv {

• phys_addr_t base;
• enum owl_soc soc;

+};

+/* BUSCLK register definitions */ +#define CMU_PDBGDIV_8 7 +#define CMU_PDBGDIV_SHIFT 26 +#define CMU_PDBGDIV_DIV (CMU_PDBGDIV_8 << CMU_PDBGDIV_SHIFT) +#define CMU_PERDIV_8 7 +#define CMU_PERDIV_SHIFT 20 +#define CMU_PERDIV_DIV (CMU_PERDIV_8 << CMU_PERDIV_SHIFT) +#define CMU_NOCDIV_2 1 +#define CMU_NOCDIV_SHIFT 19 +#define CMU_NOCDIV_DIV (CMU_NOCDIV_2 << CMU_NOCDIV_SHIFT) +#define CMU_DMMCLK_SRC_APLL 2 +#define CMU_DMMCLK_SRC_SHIFT 10 +#define CMU_DMMCLK_SRC (CMU_DMMCLK_SRC_APLL << CMU_DMMCLK_SRC_SHIFT) +#define CMU_APBCLK_DIV BIT(8) +#define CMU_NOCCLK_SRC BIT(7) +#define CMU_AHBCLK_DIV BIT(4) +#define CMU_CORECLK_MASK 3 +#define CMU_CORECLK_CPLL BIT(1) +#define CMU_CORECLK_HOSC BIT(0)

+/* COREPLL register definitions */ +#define CMU_COREPLL_EN BIT(9) +#define CMU_COREPLL_HOSC_EN BIT(8) +#define CMU_COREPLL_OUT (1104 / 24)

+/* DEVPLL register definitions */ +#define CMU_DEVPLL_CLK BIT(12) +#define CMU_DEVPLL_EN BIT(8) +#define CMU_DEVPLL_OUT (660 / 6)

+/* UARTCLK register definitions */ +#define CMU_UARTCLK_SRC_DEVPLL BIT(16)

+#define PLL_STABILITY_WAIT_US 50

+#define CMU_DEVCLKEN1_UART5 BIT(21) +#define CMU_DEVCLKEN1_UART3 BIT(11)

+#define CMU_DEVCLKEN1_ETH_S700 BIT(23)

+#endif diff --git a/drivers/clk/owl/clk_s900.c b/drivers/clk/owl/clk_s900.c deleted file mode 100644 index d60f199..0000000 --- a/drivers/clk/owl/clk_s900.c +++ /dev/null @@ -1,137 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/*

• • Actions Semi S900 clock driver
• • Copyright (C) 2015 Actions Semi Co., Ltd.
• • Copyright (C) 2018 Manivannan Sadhasivam manivannan.sadhasivam@linaro.org
• */

-#include <common.h> -#include <dm.h> -#include <asm/arch-owl/clk_s900.h> -#include <asm/arch-owl/regs_s900.h> -#include <asm/io.h>

-#include <dt-bindings/clock/actions,s900-cmu.h>

-void owl_clk_init(struct owl_clk_priv *priv) -{

• u32 bus_clk = 0, core_pll, dev_pll;
• /* Enable ASSIST_PLL */
• setbits_le32(priv->base + CMU_ASSISTPLL, BIT(0));
• udelay(PLL_STABILITY_WAIT_US);
• /* Source HOSC to DEV_CLK */
• clrbits_le32(priv->base + CMU_DEVPLL, CMU_DEVPLL_CLK);
• /* Configure BUS_CLK */
• bus_clk |= (CMU_PDBGDIV_DIV | CMU_PERDIV_DIV | CMU_NOCDIV_DIV |

• 	CMU_DMMCLK_SRC | CMU_APBCLK_DIV | CMU_AHBCLK_DIV |
  

• 	CMU_NOCCLK_SRC | CMU_CORECLK_HOSC);
  

• writel(bus_clk, priv->base + CMU_BUSCLK);
• udelay(PLL_STABILITY_WAIT_US);
• /* Configure CORE_PLL */
• core_pll = readl(priv->base + CMU_COREPLL);
• core_pll |= (CMU_COREPLL_EN | CMU_COREPLL_HOSC_EN | CMU_COREPLL_OUT);
• writel(core_pll, priv->base + CMU_COREPLL);
• udelay(PLL_STABILITY_WAIT_US);
• /* Configure DEV_PLL */
• dev_pll = readl(priv->base + CMU_DEVPLL);
• dev_pll |= (CMU_DEVPLL_EN | CMU_DEVPLL_OUT);
• writel(dev_pll, priv->base + CMU_DEVPLL);
• udelay(PLL_STABILITY_WAIT_US);
• /* Source CORE_PLL for CORE_CLK */
• clrsetbits_le32(priv->base + CMU_BUSCLK, CMU_CORECLK_MASK,

• 	CMU_CORECLK_CPLL);
  

• /* Source DEV_PLL for DEV_CLK */
• setbits_le32(priv->base + CMU_DEVPLL, CMU_DEVPLL_CLK);
• udelay(PLL_STABILITY_WAIT_US);

-}

-void owl_uart_clk_enable(struct owl_clk_priv *priv) -{

• /* Source HOSC for UART5 interface */
• clrbits_le32(priv->base + CMU_UART5CLK, CMU_UARTCLK_SRC_DEVPLL);
• /* Enable UART5 interface clock */
• setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);

-}

-void owl_uart_clk_disable(struct owl_clk_priv *priv) -{

• /* Disable UART5 interface clock */
• clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);

-}

-int owl_clk_enable(struct clk *clk) -{

• struct owl_clk_priv *priv = dev_get_priv(clk->dev);
• switch (clk->id) {
• case CLK_UART5:

• owl_uart_clk_enable(priv);
  

• break;
  

• default:

• return 0;
  

• }
• return 0;

-}

-int owl_clk_disable(struct clk *clk) -{

• struct owl_clk_priv *priv = dev_get_priv(clk->dev);
• switch (clk->id) {
• case CLK_UART5:

• owl_uart_clk_disable(priv);
  

• break;
  

• default:

• return 0;
  

• }
• return 0;

-}

-static int owl_clk_probe(struct udevice *dev) -{

• struct owl_clk_priv *priv = dev_get_priv(dev);
• priv->base = dev_read_addr(dev);
• if (priv->base == FDT_ADDR_T_NONE)

• return -EINVAL;
  

• /* setup necessary clocks */
• owl_clk_init(priv);
• return 0;

-}

-static struct clk_ops owl_clk_ops = {

• .enable = owl_clk_enable,
• .disable = owl_clk_disable,

-};

-static const struct udevice_id owl_clk_ids[] = {

• { .compatible = "actions,s900-cmu" },
• { }

-};

-U_BOOT_DRIVER(clk_owl) = {

• .name = "clk_s900",
• .id = UCLASS_CLK,
• .of_match = owl_clk_ids,
• .ops = &owl_clk_ops,
• .priv_auto_alloc_size = sizeof(struct owl_clk_priv),
• .probe = owl_clk_probe,

-};