diff mbox series

[v3,03/21] clk: actions: Add common clock driver

Message ID 1579954983-11329-4-git-send-email-amittomer25@gmail.com
State Superseded
Headers show
Series Actions S700 SoC support | expand

Commit Message

Amit Tomer Jan. 25, 2020, 12:22 p.m. UTC
This patch converts S900 clock driver to something common that can
be used for other SoCs, for instance S700(few of clk registres are same).

Signed-off-by: Amit Singh Tomar <amittomer25 at gmail.com>
---
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                 | 159 ++++++++++++++++++++++++++++++
 drivers/clk/owl/clk_owl.h                 |  65 ++++++++++++
 drivers/clk/owl/clk_s900.c                | 137 -------------------------
 9 files changed, 285 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

Comments

Amit Tomer March 3, 2020, 8:23 a.m. UTC | #1
Hi,

Thanks for having a look.

> Either use `priv->soc` or the guard throughout the driver. Please don't mix
> both.

But have used #ifdef guard only where it is really necessary and to
keep implementation
clean used priv->soc.

> > +
> > +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
>
> Guard is not necessary here.

But this is something suggested by Andre in previous review round

"just protect the compatible strings below with MACH_S[79]00 ifdefs,
so that both SoCs share the
file, but compile to different drivers supporting only one compatible
(due to the different reg_sx00.h and the #ifdef protected parts)."

Thanks
Amit
Andre Przywara March 3, 2020, 10:20 a.m. UTC | #2
On 23/02/2020 17:25, Manivannan Sadhasivam wrote:

Hi,

> On Sat, Jan 25, 2020 at 05:52:45PM +0530, 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 registres are same).
>>
> 
> registers
> 
>> Signed-off-by: Amit Singh Tomar <amittomer25 at gmail.com>
>> ---
>> 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                 | 159 ++++++++++++++++++++++++++++++
>>  drivers/clk/owl/clk_owl.h                 |  65 ++++++++++++
>>  drivers/clk/owl/clk_s900.c                | 137 -------------------------
>>  9 files changed, 285 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 905118b..e6a9d32 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -886,6 +886,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 at 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..a0bd737
>> --- /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..b2582d3 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.
>> +	  S900/S700 SoCs.
>> +
> 
> It'd be good to keep the family name:
> 
> Enable support for clock managemet unit present in Actions Semi 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..050dd45
>> --- /dev/null
>> +++ b/drivers/clk/owl/clk_owl.c
>> @@ -0,0 +1,159 @@
>> +// 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 at 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
> 
> It'd be interesting to see how we can add support for S500 which is somewhat
> different with these configurations... Let's do that later.
> 
>> +
>> +	/* 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)
>> +{
>> +	if (priv->soc == S900) {
>> +		/* 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);
>> +	} else if (priv->soc == S700) {
>> +		/* 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);
>> +	}
>> +}
>> +
>> +void owl_ether_clk_enable(struct owl_clk_priv *priv)
>> +{
>> +#if defined(CONFIG_MACH_S700)
> 
> Either use `priv->soc` or the guard throughout the driver. Please don't mix
> both.

I think the idea is to use priv->soc wherever possible, but revert to the #ifdef guards where this affects SoC specific register offsets. This prevents us from dummy-defining CMU_DEVCLKEN1_ETH_S700 for the S900, for instance.

> 
>> +	setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH_S700);
>> +	setbits_le32(priv->base + CMU_ETHERNETPLL, 5);
>> +#endif
>> +}
>> +
>> +int owl_clk_enable(struct clk *clk)
>> +{
>> +	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>> +
>> +	switch (clk->id) {
>> +	case CLK_UART5:
>> +	case CLK_UART3:
>> +		owl_uart_clk_enable(priv);
>> +		break;
>> +	case CLK_RMII_REF:
>> +	case CLK_ETHERNET:
>> +		owl_ether_clk_enable(priv);
>> +		break;
>> +	default:
>> +		return 0;
> 
> Why not return -EINVAL for default case?
> 
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int owl_clk_disable(struct clk *clk)
>> +{
>> +	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>> +
>> +	if (priv->soc == S900) {
>> +		/* Disable UART5 interface clock */
>> +		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);
>> +	} else if (priv->soc == S700) {
>> +		/* Disable UART3 interface clock */
>> +		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART3);
>> +	}
>> +
>> +	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
> 
> Guard is not necessary here.

Indeed, but it's here to not give false impressions: Despite the joint code, the clock driver only works for one SoC at a time. Having only that one respective compatible string in here makes this obvious.

The main reason for this approach is that the register offsets are quite different, dissecting this at runtime does not sound worthwhile for just those few clocks that we need.
So the idea was to go with one source file, to make use of the commonalities of the clocks, but use #ifdef guards where needed.

This admittedly starts to look quite messy. And with the Ethernet clocks in it is probably already at a point where proper separation would be better.

So to make this easier: Amit, can you remove the Ethernet clocks from this patch, and add them later in your Ethernet series? Maybe this buys us some time to refactor this meanwhile?

Cheers,
Andre
 
>> +};
>> +
>> +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..9a65091
>> --- /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 at 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 a7c15d2..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 at 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/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 CLOCK_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 CLOCK_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,
>> -};
>> -- 
>> 2.7.4
>>
Amit Tomer March 3, 2020, 10:38 a.m. UTC | #3
Hi,

> Indeed, but it's here to not give false impressions: Despite the joint code, the clock driver only works for one SoC at a time. Having only that one respective compatible string in here makes this obvious.
>
> The main reason for this approach is that the register offsets are quite different, dissecting this at runtime does not sound worthwhile for just those few clocks that we need.
> So the idea was to go with one source file, to make use of the commonalities of the clocks, but use #ifdef guards where needed.
>
> This admittedly starts to look quite messy. And with the Ethernet clocks in it is probably already at a point where proper separation would be better.
>
> So to make this easier: Amit, can you remove the Ethernet clocks from this patch, and add them later in your Ethernet series? Maybe this buys us some time to refactor this meanwhile?

Already have taken care of it here:
https://github.com/Atomar25/u-boot/commit/7721ce67d601945b5883fcfa3157207d67c95b1b#diff-0bbc60147217c87a2656365e222cb7b3R81

Thanks
-Amit
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 905118b..e6a9d32 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -886,6 +886,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 at 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..a0bd737
--- /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..b2582d3 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.
+	  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..050dd45
--- /dev/null
+++ b/drivers/clk/owl/clk_owl.c
@@ -0,0 +1,159 @@ 
+// 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 at 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);
+}
+
+void owl_uart_clk_enable(struct owl_clk_priv *priv)
+{
+	if (priv->soc == S900) {
+		/* 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);
+	} else if (priv->soc == S700) {
+		/* 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);
+	}
+}
+
+void owl_ether_clk_enable(struct owl_clk_priv *priv)
+{
+#if defined(CONFIG_MACH_S700)
+	setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH_S700);
+	setbits_le32(priv->base + CMU_ETHERNETPLL, 5);
+#endif
+}
+
+int owl_clk_enable(struct clk *clk)
+{
+	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
+
+	switch (clk->id) {
+	case CLK_UART5:
+	case CLK_UART3:
+		owl_uart_clk_enable(priv);
+		break;
+	case CLK_RMII_REF:
+	case CLK_ETHERNET:
+		owl_ether_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);
+
+	if (priv->soc == S900) {
+		/* Disable UART5 interface clock */
+		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART5);
+	} else if (priv->soc == S700) {
+		/* Disable UART3 interface clock */
+		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_UART3);
+	}
+
+	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..9a65091
--- /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 at 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 a7c15d2..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 at 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/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 CLOCK_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 CLOCK_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,
-};