mbox series

[0/6] Add clock support for Actions Semi S500 SoC

Message ID 20181231185517.18517-1-manivannan.sadhasivam@linaro.org
Headers show
Series Add clock support for Actions Semi S500 SoC | expand

Message

Manivannan Sadhasivam Dec. 31, 2018, 6:55 p.m. UTC
Hello,

This patchset adds common clock support for Actions Semi S500 SoC of
the Owl family SoCs. This series is based on the initial work done
by Edgar Bernardi Righi. https://patchwork.kernel.org/cover/10587527/

Since there isn't any update from him for long time, I took the liberty
to modify his patches, address review comments and send to list for review.

This series has been tested on Allo Sparky SBC.

Thanks,
Mani

Edgar Bernardi Righi (1):
  dt-bindings: clock: Add DT bindings for Actions Semi S500 CMU

Manivannan Sadhasivam (5):
  clk: actions: Add configurable PLL delay
  ARM: dts: Add CMU support for Actions Semi Owl S500 SoC
  ARM: dts: Remove fake UART clock for S500 based SBCs
  clk: actions: Add clock driver for S500 SoC
  MAINTAINERS: Add linux-actions mailing list for Actions Semi

 .../bindings/clock/actions,owl-cmu.txt        |   7 +-
 MAINTAINERS                                   |   1 +
 arch/arm/boot/dts/owl-s500-cubieboard6.dts    |   7 -
 .../arm/boot/dts/owl-s500-guitar-bb-rev-b.dts |   7 -
 arch/arm/boot/dts/owl-s500-sparky.dts         |   7 -
 arch/arm/boot/dts/owl-s500.dtsi               |  22 +
 drivers/clk/actions/Kconfig                   |   5 +
 drivers/clk/actions/Makefile                  |   1 +
 drivers/clk/actions/owl-pll.c                 |   2 +-
 drivers/clk/actions/owl-pll.h                 |  30 +-
 drivers/clk/actions/owl-s500.c                | 524 ++++++++++++++++++
 include/dt-bindings/clock/actions,s500-cmu.h  |  78 +++
 12 files changed, 660 insertions(+), 31 deletions(-)
 create mode 100644 drivers/clk/actions/owl-s500.c
 create mode 100644 include/dt-bindings/clock/actions,s500-cmu.h

Comments

Rob Herring Jan. 11, 2019, 2:56 p.m. UTC | #1
On Tue, Jan 01, 2019 at 12:25:13AM +0530, Manivannan Sadhasivam wrote:
> From: Edgar Bernardi Righi <edgar.righi@lsitec.org.br>
> 
> Add devicetree bindings for Actions Semi S500 Clock Management Unit.
> 
> Signed-off-by: Edgar Bernardi Righi <edgar.righi@lsitec.org.br>
> [Mani: Documented S500 CMU compatible]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> 
> Rob, I have removed your Reviewed-by tag for this patch since the
> earlier revision contained only bindings constants and lacked the
> compatible documentation, which is added now.
> 
>  .../bindings/clock/actions,owl-cmu.txt        |  7 +-
>  include/dt-bindings/clock/actions,s500-cmu.h  | 78 +++++++++++++++++++
>  2 files changed, 82 insertions(+), 3 deletions(-)
>  create mode 100644 include/dt-bindings/clock/actions,s500-cmu.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
> index 2ef86ae96df8..86183f559022 100644
> --- a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
> +++ b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
> @@ -2,13 +2,14 @@
>  
>  The Actions Semi Owl Clock Management Unit generates and supplies clock
>  to various controllers within the SoC. The clock binding described here is
> -applicable to S900 and S700 SoC's.
> +applicable to S900,S700 and S500 SoC's.

space needed after the ','.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>
Stephen Boyd Jan. 11, 2019, 10:51 p.m. UTC | #2
Quoting Manivannan Sadhasivam (2018-12-31 10:55:11)
> Hello,
> 
> This patchset adds common clock support for Actions Semi S500 SoC of
> the Owl family SoCs. This series is based on the initial work done
> by Edgar Bernardi Righi. https://patchwork.kernel.org/cover/10587527/
> 
> Since there isn't any update from him for long time, I took the liberty
> to modify his patches, address review comments and send to list for review.
> 
> This series has been tested on Allo Sparky SBC.
> 
> Thanks,
> Mani
> 
> Edgar Bernardi Righi (1):
>   dt-bindings: clock: Add DT bindings for Actions Semi S500 CMU
> 
> Manivannan Sadhasivam (5):
>   clk: actions: Add configurable PLL delay
>   ARM: dts: Add CMU support for Actions Semi Owl S500 SoC
>   ARM: dts: Remove fake UART clock for S500 based SBCs
>   clk: actions: Add clock driver for S500 SoC
>   MAINTAINERS: Add linux-actions mailing list for Actions Semi

What's the merge strategy for these patches? Some are for clk, others
are for arm-soc, etc. I see a sandwich of patches too so it sounds like
I can't even take just the clk ones for fear of breaking something that
the dts bits in the middle clear out of the way.
Stephen Boyd Jan. 11, 2019, 11:56 p.m. UTC | #3
Quoting Manivannan Sadhasivam (2018-12-31 10:55:16)
> diff --git a/drivers/clk/actions/owl-s500.c b/drivers/clk/actions/owl-s500.c
> new file mode 100644
> index 000000000000..93feea8d71e2
> --- /dev/null
> +++ b/drivers/clk/actions/owl-s500.c
> @@ -0,0 +1,524 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//

Please make the reset of these the typical /* type of comments.

> +// Actions Semi Owl S500 SoC clock driver
> +//
> +// Copyright (c) 2014 Actions Semi Inc.
> +// Author: David Liu <liuwei@actions-semi.com>
> +//
> +// Copyright (c) 2018 Linaro Ltd.
> +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +//
> +// Copyright (c) 2018 LSI-TEC - Caninos Loucos
> +// Author: Edgar Bernardi Righi <edgar.righi@lsitec.org.br>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>

I'm amazed nothing else is required to be included.

[..]
> +#define CMU_NANDPLLDEBUG               (0x00E4)
> +#define CMU_DISPLAYPLLDEBUG            (0x00E8)
> +#define CMU_TVOUTPLLDEBUG              (0x00EC)
> +#define CMU_DEEPCOLORPLLDEBUG          (0x00F4)
> +#define CMU_AUDIOPLL_ETHPLLDEBUG       (0x00F8)
> +#define CMU_CVBSPLLDEBUG               (0x00FC)
> +
> +#define OWL_S500_COREPLL_DELAY         (150)
> +#define OWL_S500_DDRPLL_DELAY          (63)
> +#define OWL_S500_DEVPLL_DELAY          (28)
> +#define OWL_S500_NANDPLL_DELAY         (44)
> +#define OWL_S500_DISPLAYPLL_DELAY      (57)
> +#define OWL_S500_ETHERNETPLL_DELAY     (25)
> +#define OWL_S500_AUDIOPLL_DELAY                (100)
> +
> +static const struct clk_pll_table clk_audio_pll_table[] = {
> +       { 0, 45158400 }, { 1, 49152000 },
> +       { 0, 0 },
> +};
> +
> +/* pll clocks */
> +static OWL_PLL_NO_PARENT_DELAY(ethernet_pll_clk, "ethernet_pll_clk", CMU_ETHERNETPLL, 500000000, 0, 0, 0, 0, 0, OWL_S500_ETHERNETPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(core_pll_clk, "core_pll_clk", CMU_COREPLL, 12000000, 9, 0, 8, 4, 134, OWL_S500_COREPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(ddr_pll_clk, "ddr_pll_clk", CMU_DDRPLL, 12000000, 8, 0, 8, 1, 67, OWL_S500_DDRPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(nand_pll_clk, "nand_pll_clk", CMU_NANDPLL, 6000000, 8, 0, 7, 2, 86, OWL_S500_NANDPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(display_pll_clk, "display_pll_clk", CMU_DISPLAYPLL, 6000000, 8, 0, 8, 2, 126, OWL_S500_DISPLAYPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(dev_pll_clk, "dev_pll_clk", CMU_DEVPLL, 6000000, 8, 0, 7, 8, 126, OWL_S500_DEVPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> +static OWL_PLL_NO_PARENT_DELAY(audio_pll_clk, "audio_pll_clk", CMU_AUDIOPLL, 0, 4, 0, 1, 0, 0, OWL_S500_AUDIOPLL_DELAY, clk_audio_pll_table, CLK_IGNORE_UNUSED);
> +
> +static const char *dev_clk_mux_p[] = { "hosc", "dev_pll_clk" };

These can't be const char * const ?

> +static const char *bisp_clk_mux_p[] = { "display_pll_clk", "dev_clk" };
> +static const char *sensor_clk_mux_p[] = { "hosc", "bisp_clk" };
> +static const char *sd_clk_mux_p[] = { "dev_clk", "nand_pll_clk" };
> +static const char *pwm_clk_mux_p[] = { "losc", "hosc" };
> +static const char *ahbprediv_clk_mux_p[] = { "dev_clk", "display_pll_clk", "nand_pll_clk", "ddr_pll_clk" };
> +static const char *uart_clk_mux_p[] = { "hosc", "dev_pll_clk" };
> +static const char *de_clk_mux_p[] = { "display_pll_clk", "dev_clk" };
> +static const char *i2s_clk_mux_p[] = { "audio_pll_clk" };
> +static const char *hde_clk_mux_p[] = { "dev_clk", "display_pll_clk", "nand_pll_clk", "ddr_pll_clk" };
> +static const char *nand_clk_mux_p[] = { "nand_pll_clk", "display_pll_clk", "dev_clk", "ddr_pll_clk" };
> +
> +static struct clk_factor_table sd_factor_table[] = {

Can these tables be const?

> +       /* bit0 ~ 4 */
> +       { 0, 1, 1 }, { 1, 1, 2 }, { 2, 1, 3 }, { 3, 1, 4 },
> +       { 4, 1, 5 }, { 5, 1, 6 }, { 6, 1, 7 }, { 7, 1, 8 },
> +       { 8, 1, 9 }, { 9, 1, 10 }, { 10, 1, 11 }, { 11, 1, 12 },
> +       { 12, 1, 13 }, { 13, 1, 14 }, { 14, 1, 15 }, { 15, 1, 16 },
> +       { 16, 1, 17 }, { 17, 1, 18 }, { 18, 1, 19 }, { 19, 1, 20 },
> +       { 20, 1, 21 }, { 21, 1, 22 }, { 22, 1, 23 }, { 23, 1, 24 },
> +       { 24, 1, 25 }, { 25, 1, 26 }, { 26, 1, 27 }, { 27, 1, 28 },
> +       { 28, 1, 29 }, { 29, 1, 30 }, { 30, 1, 31 }, { 31, 1, 32 },
> +
> +       /* bit8: /128 */
> +       { 256, 1, 1 * 128 }, { 257, 1, 2 * 128 }, { 258, 1, 3 * 128 }, { 259, 1, 4 * 128 },
> +       { 260, 1, 5 * 128 }, { 261, 1, 6 * 128 }, { 262, 1, 7 * 128 }, { 263, 1, 8 * 128 },
> +       { 264, 1, 9 * 128 }, { 265, 1, 10 * 128 }, { 266, 1, 11 * 128 }, { 267, 1, 12 * 128 },
> +       { 268, 1, 13 * 128 }, { 269, 1, 14 * 128 }, { 270, 1, 15 * 128 }, { 271, 1, 16 * 128 },
> +       { 272, 1, 17 * 128 }, { 273, 1, 18 * 128 }, { 274, 1, 19 * 128 }, { 275, 1, 20 * 128 },
> +       { 276, 1, 21 * 128 }, { 277, 1, 22 * 128 }, { 278, 1, 23 * 128 }, { 279, 1, 24 * 128 },
> +       { 280, 1, 25 * 128 }, { 281, 1, 26 * 128 }, { 282, 1, 27 * 128 }, { 283, 1, 28 * 128 },
> +       { 284, 1, 29 * 128 }, { 285, 1, 30 * 128 }, { 286, 1, 31 * 128 }, { 287, 1, 32 * 128 },
> +       { 0, 0, 0 },
> +};
> +
> +static struct clk_factor_table bisp_factor_table[] = {
> +       { 0, 1, 1 }, { 1, 1, 2 }, { 2, 1, 3 }, { 3, 1, 4 },
> +       { 4, 1, 5 }, { 5, 1, 6 }, { 6, 1, 7 }, { 7, 1, 8 },
> +       { 0, 0, 0 },
> +};
> +
> +static struct clk_factor_table ahb_factor_table[] = {
> +       { 1, 1, 2 }, { 2, 1, 3 },
> +       { 0, 0, 0 },
> +};
> +
> +static struct clk_div_table rmii_ref_div_table[] = {
> +       { 0, 4 }, { 1, 10 },
> +       { 0, 0 },
> +};
> +
> +static struct clk_div_table i2s_div_table[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 6 }, { 5, 8 }, { 6, 12 }, { 7, 16 },
> +       { 8, 24 },
> +       { 0, 0 },
> +};
> +
> +static struct clk_div_table nand_div_table[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 4 }, { 3, 6 },
> +       { 4, 8 }, { 5, 10 }, { 6, 12 }, { 7, 14 },
> +       { 8, 16 }, { 9, 18 }, { 10, 20 }, { 11, 22 },
> +       { 0, 0 },
> +};
> +
> +/* mux clock */
> +static OWL_MUX(dev_clk, "dev_clk", dev_clk_mux_p, CMU_DEVPLL, 12, 1, CLK_SET_RATE_PARENT);
> +static OWL_MUX(ahbprediv_clk, "ahbprediv_clk", ahbprediv_clk_mux_p, CMU_BUSCLK1, 8, 3, CLK_SET_RATE_PARENT);
> +
> +/* gate clocks */
> +static OWL_GATE(spi0_clk, "spi0_clk", "ahb_clk", CMU_DEVCLKEN1, 10, 0, CLK_IGNORE_UNUSED);
> +static OWL_GATE(spi1_clk, "spi1_clk", "ahb_clk", CMU_DEVCLKEN1, 11, 0, CLK_IGNORE_UNUSED);
[...]
> +               [CLK_HDMI]              = &hdmi_clk.common.hw,
> +               [CLK_VDE]               = &vde_clk.common.hw,
> +               [CLK_VCE]               = &vce_clk.common.hw,
> +               [CLK_SPDIF]             = &spdif_clk.common.hw,
> +               [CLK_NAND]              = &nand_clk.common.hw,
> +               [CLK_ECC]               = &ecc_clk.common.hw,
> +       },
> +       .num = CLK_NR_CLKS,
> +};
> +
> +static struct owl_clk_desc s500_clk_desc = {

This can't be const?

> +       .clks       = s500_clks,
> +       .num_clks   = ARRAY_SIZE(s500_clks),
> +
> +       .hw_clks    = &s500_hw_clks,
> +};
> +
Stephen Boyd Jan. 11, 2019, 11:57 p.m. UTC | #4
Quoting Manivannan Sadhasivam (2018-12-31 10:55:12)
> S500 SoC requires configurable delay for different PLLs. Hence, add
> a separate macro for declaring a PLL with configurable delay and also
> modify the existing OWL_PLL_NO_PARENT macro to use default delay so
> that no need to modify the existing S700/S900 drivers.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

This one looks good to me.
Manivannan Sadhasivam Jan. 14, 2019, 9:18 a.m. UTC | #5
Hi Stephen,

On Fri, Jan 11, 2019 at 02:51:04PM -0800, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2018-12-31 10:55:11)
> > Hello,
> > 
> > This patchset adds common clock support for Actions Semi S500 SoC of
> > the Owl family SoCs. This series is based on the initial work done
> > by Edgar Bernardi Righi. https://patchwork.kernel.org/cover/10587527/
> > 
> > Since there isn't any update from him for long time, I took the liberty
> > to modify his patches, address review comments and send to list for review.
> > 
> > This series has been tested on Allo Sparky SBC.
> > 
> > Thanks,
> > Mani
> > 
> > Edgar Bernardi Righi (1):
> >   dt-bindings: clock: Add DT bindings for Actions Semi S500 CMU
> > 
> > Manivannan Sadhasivam (5):
> >   clk: actions: Add configurable PLL delay
> >   ARM: dts: Add CMU support for Actions Semi Owl S500 SoC
> >   ARM: dts: Remove fake UART clock for S500 based SBCs
> >   clk: actions: Add clock driver for S500 SoC
> >   MAINTAINERS: Add linux-actions mailing list for Actions Semi
> 
> What's the merge strategy for these patches? Some are for clk, others
> are for arm-soc, etc. I see a sandwich of patches too so it sounds like
> I can't even take just the clk ones for fear of breaking something that
> the dts bits in the middle clear out of the way.
> 

As we did with previous patchsets, clk patches will go through your tree
and I'll take the ARM/MAINTAINERS patches through Actions sub-tree. Let's
target these for 5.1. Even if your patches reach first, there won't be
any regression with existing DTS.

Thanks,
Mani
Stephen Boyd Jan. 14, 2019, 9:55 p.m. UTC | #6
Quoting Manivannan Sadhasivam (2019-01-14 01:18:50)
> Hi Stephen,
> 
> On Fri, Jan 11, 2019 at 02:51:04PM -0800, Stephen Boyd wrote:
> > Quoting Manivannan Sadhasivam (2018-12-31 10:55:11)
> > > Hello,
> > > 
> > > This patchset adds common clock support for Actions Semi S500 SoC of
> > > the Owl family SoCs. This series is based on the initial work done
> > > by Edgar Bernardi Righi. https://patchwork.kernel.org/cover/10587527/
> > > 
> > > Since there isn't any update from him for long time, I took the liberty
> > > to modify his patches, address review comments and send to list for review.
> > > 
> > > This series has been tested on Allo Sparky SBC.
> > > 
> > > Thanks,
> > > Mani
> > > 
> > > Edgar Bernardi Righi (1):
> > >   dt-bindings: clock: Add DT bindings for Actions Semi S500 CMU
> > > 
> > > Manivannan Sadhasivam (5):
> > >   clk: actions: Add configurable PLL delay
> > >   ARM: dts: Add CMU support for Actions Semi Owl S500 SoC
> > >   ARM: dts: Remove fake UART clock for S500 based SBCs
> > >   clk: actions: Add clock driver for S500 SoC
> > >   MAINTAINERS: Add linux-actions mailing list for Actions Semi
> > 
> > What's the merge strategy for these patches? Some are for clk, others
> > are for arm-soc, etc. I see a sandwich of patches too so it sounds like
> > I can't even take just the clk ones for fear of breaking something that
> > the dts bits in the middle clear out of the way.
> > 
> 
> As we did with previous patchsets, clk patches will go through your tree
> and I'll take the ARM/MAINTAINERS patches through Actions sub-tree. Let's
> target these for 5.1. Even if your patches reach first, there won't be
> any regression with existing DTS.
> 

Ok. I have just minor nits. Please resend and I will merge the clk
patches to clk tree.
Manivannan Sadhasivam Jan. 15, 2019, 3:15 a.m. UTC | #7
Hi Stephen,

On Fri, Jan 11, 2019 at 03:56:11PM -0800, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2018-12-31 10:55:16)
> > diff --git a/drivers/clk/actions/owl-s500.c b/drivers/clk/actions/owl-s500.c
> > new file mode 100644
> > index 000000000000..93feea8d71e2
> > --- /dev/null
> > +++ b/drivers/clk/actions/owl-s500.c
> > @@ -0,0 +1,524 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +//
> 
> Please make the reset of these the typical /* type of comments.
> 

Okay. I will do separate patchset for converting S700 and S900.

> > +// Actions Semi Owl S500 SoC clock driver
> > +//
> > +// Copyright (c) 2014 Actions Semi Inc.
> > +// Author: David Liu <liuwei@actions-semi.com>
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > +//
> > +// Copyright (c) 2018 LSI-TEC - Caninos Loucos
> > +// Author: Edgar Bernardi Righi <edgar.righi@lsitec.org.br>
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> 
> I'm amazed nothing else is required to be included.
> 

Most of the generic includes are added in the local header files. But
yeah, we need to declare those here also. Will do it as an incremental
patchset.

> [..]
> > +#define CMU_NANDPLLDEBUG               (0x00E4)
> > +#define CMU_DISPLAYPLLDEBUG            (0x00E8)
> > +#define CMU_TVOUTPLLDEBUG              (0x00EC)
> > +#define CMU_DEEPCOLORPLLDEBUG          (0x00F4)
> > +#define CMU_AUDIOPLL_ETHPLLDEBUG       (0x00F8)
> > +#define CMU_CVBSPLLDEBUG               (0x00FC)
> > +
> > +#define OWL_S500_COREPLL_DELAY         (150)
> > +#define OWL_S500_DDRPLL_DELAY          (63)
> > +#define OWL_S500_DEVPLL_DELAY          (28)
> > +#define OWL_S500_NANDPLL_DELAY         (44)
> > +#define OWL_S500_DISPLAYPLL_DELAY      (57)
> > +#define OWL_S500_ETHERNETPLL_DELAY     (25)
> > +#define OWL_S500_AUDIOPLL_DELAY                (100)
> > +
> > +static const struct clk_pll_table clk_audio_pll_table[] = {
> > +       { 0, 45158400 }, { 1, 49152000 },
> > +       { 0, 0 },
> > +};
> > +
> > +/* pll clocks */
> > +static OWL_PLL_NO_PARENT_DELAY(ethernet_pll_clk, "ethernet_pll_clk", CMU_ETHERNETPLL, 500000000, 0, 0, 0, 0, 0, OWL_S500_ETHERNETPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(core_pll_clk, "core_pll_clk", CMU_COREPLL, 12000000, 9, 0, 8, 4, 134, OWL_S500_COREPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(ddr_pll_clk, "ddr_pll_clk", CMU_DDRPLL, 12000000, 8, 0, 8, 1, 67, OWL_S500_DDRPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(nand_pll_clk, "nand_pll_clk", CMU_NANDPLL, 6000000, 8, 0, 7, 2, 86, OWL_S500_NANDPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(display_pll_clk, "display_pll_clk", CMU_DISPLAYPLL, 6000000, 8, 0, 8, 2, 126, OWL_S500_DISPLAYPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(dev_pll_clk, "dev_pll_clk", CMU_DEVPLL, 6000000, 8, 0, 7, 8, 126, OWL_S500_DEVPLL_DELAY, NULL, CLK_IGNORE_UNUSED);
> > +static OWL_PLL_NO_PARENT_DELAY(audio_pll_clk, "audio_pll_clk", CMU_AUDIOPLL, 0, 4, 0, 1, 0, 0, OWL_S500_AUDIOPLL_DELAY, clk_audio_pll_table, CLK_IGNORE_UNUSED);
> > +
> > +static const char *dev_clk_mux_p[] = { "hosc", "dev_pll_clk" };
> 
> These can't be const char * const ?
> 

Ack.

> > +static const char *bisp_clk_mux_p[] = { "display_pll_clk", "dev_clk" };
> > +static const char *sensor_clk_mux_p[] = { "hosc", "bisp_clk" };
> > +static const char *sd_clk_mux_p[] = { "dev_clk", "nand_pll_clk" };
> > +static const char *pwm_clk_mux_p[] = { "losc", "hosc" };
> > +static const char *ahbprediv_clk_mux_p[] = { "dev_clk", "display_pll_clk", "nand_pll_clk", "ddr_pll_clk" };
> > +static const char *uart_clk_mux_p[] = { "hosc", "dev_pll_clk" };
> > +static const char *de_clk_mux_p[] = { "display_pll_clk", "dev_clk" };
> > +static const char *i2s_clk_mux_p[] = { "audio_pll_clk" };
> > +static const char *hde_clk_mux_p[] = { "dev_clk", "display_pll_clk", "nand_pll_clk", "ddr_pll_clk" };
> > +static const char *nand_clk_mux_p[] = { "nand_pll_clk", "display_pll_clk", "dev_clk", "ddr_pll_clk" };
> > +
> > +static struct clk_factor_table sd_factor_table[] = {
> 
> Can these tables be const?
> 

They can but sadly const will get discarded during assignments. I will
fix that in incremental patchset.

> > +       /* bit0 ~ 4 */
> > +       { 0, 1, 1 }, { 1, 1, 2 }, { 2, 1, 3 }, { 3, 1, 4 },
> > +       { 4, 1, 5 }, { 5, 1, 6 }, { 6, 1, 7 }, { 7, 1, 8 },
> > +       { 8, 1, 9 }, { 9, 1, 10 }, { 10, 1, 11 }, { 11, 1, 12 },
> > +       { 12, 1, 13 }, { 13, 1, 14 }, { 14, 1, 15 }, { 15, 1, 16 },
> > +       { 16, 1, 17 }, { 17, 1, 18 }, { 18, 1, 19 }, { 19, 1, 20 },
> > +       { 20, 1, 21 }, { 21, 1, 22 }, { 22, 1, 23 }, { 23, 1, 24 },
> > +       { 24, 1, 25 }, { 25, 1, 26 }, { 26, 1, 27 }, { 27, 1, 28 },
> > +       { 28, 1, 29 }, { 29, 1, 30 }, { 30, 1, 31 }, { 31, 1, 32 },
> > +
> > +       /* bit8: /128 */
> > +       { 256, 1, 1 * 128 }, { 257, 1, 2 * 128 }, { 258, 1, 3 * 128 }, { 259, 1, 4 * 128 },
> > +       { 260, 1, 5 * 128 }, { 261, 1, 6 * 128 }, { 262, 1, 7 * 128 }, { 263, 1, 8 * 128 },
> > +       { 264, 1, 9 * 128 }, { 265, 1, 10 * 128 }, { 266, 1, 11 * 128 }, { 267, 1, 12 * 128 },
> > +       { 268, 1, 13 * 128 }, { 269, 1, 14 * 128 }, { 270, 1, 15 * 128 }, { 271, 1, 16 * 128 },
> > +       { 272, 1, 17 * 128 }, { 273, 1, 18 * 128 }, { 274, 1, 19 * 128 }, { 275, 1, 20 * 128 },
> > +       { 276, 1, 21 * 128 }, { 277, 1, 22 * 128 }, { 278, 1, 23 * 128 }, { 279, 1, 24 * 128 },
> > +       { 280, 1, 25 * 128 }, { 281, 1, 26 * 128 }, { 282, 1, 27 * 128 }, { 283, 1, 28 * 128 },
> > +       { 284, 1, 29 * 128 }, { 285, 1, 30 * 128 }, { 286, 1, 31 * 128 }, { 287, 1, 32 * 128 },
> > +       { 0, 0, 0 },
> > +};
> > +
> > +static struct clk_factor_table bisp_factor_table[] = {
> > +       { 0, 1, 1 }, { 1, 1, 2 }, { 2, 1, 3 }, { 3, 1, 4 },
> > +       { 4, 1, 5 }, { 5, 1, 6 }, { 6, 1, 7 }, { 7, 1, 8 },
> > +       { 0, 0, 0 },
> > +};
> > +
> > +static struct clk_factor_table ahb_factor_table[] = {
> > +       { 1, 1, 2 }, { 2, 1, 3 },
> > +       { 0, 0, 0 },
> > +};
> > +
> > +static struct clk_div_table rmii_ref_div_table[] = {
> > +       { 0, 4 }, { 1, 10 },
> > +       { 0, 0 },
> > +};
> > +
> > +static struct clk_div_table i2s_div_table[] = {
> > +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > +       { 4, 6 }, { 5, 8 }, { 6, 12 }, { 7, 16 },
> > +       { 8, 24 },
> > +       { 0, 0 },
> > +};
> > +
> > +static struct clk_div_table nand_div_table[] = {
> > +       { 0, 1 }, { 1, 2 }, { 2, 4 }, { 3, 6 },
> > +       { 4, 8 }, { 5, 10 }, { 6, 12 }, { 7, 14 },
> > +       { 8, 16 }, { 9, 18 }, { 10, 20 }, { 11, 22 },
> > +       { 0, 0 },
> > +};
> > +
> > +/* mux clock */
> > +static OWL_MUX(dev_clk, "dev_clk", dev_clk_mux_p, CMU_DEVPLL, 12, 1, CLK_SET_RATE_PARENT);
> > +static OWL_MUX(ahbprediv_clk, "ahbprediv_clk", ahbprediv_clk_mux_p, CMU_BUSCLK1, 8, 3, CLK_SET_RATE_PARENT);
> > +
> > +/* gate clocks */
> > +static OWL_GATE(spi0_clk, "spi0_clk", "ahb_clk", CMU_DEVCLKEN1, 10, 0, CLK_IGNORE_UNUSED);
> > +static OWL_GATE(spi1_clk, "spi1_clk", "ahb_clk", CMU_DEVCLKEN1, 11, 0, CLK_IGNORE_UNUSED);
> [...]
> > +               [CLK_HDMI]              = &hdmi_clk.common.hw,
> > +               [CLK_VDE]               = &vde_clk.common.hw,
> > +               [CLK_VCE]               = &vce_clk.common.hw,
> > +               [CLK_SPDIF]             = &spdif_clk.common.hw,
> > +               [CLK_NAND]              = &nand_clk.common.hw,
> > +               [CLK_ECC]               = &ecc_clk.common.hw,
> > +       },
> > +       .num = CLK_NR_CLKS,
> > +};
> > +
> > +static struct owl_clk_desc s500_clk_desc = {
> 
> This can't be const?
> 

Same as above.

Thanks,
Mani

> > +       .clks       = s500_clks,
> > +       .num_clks   = ARRAY_SIZE(s500_clks),
> > +
> > +       .hw_clks    = &s500_hw_clks,
> > +};
> > +