[v1 04/17] drivers: clk: Add clock driver for Intel N5X device

[Lim, Elly Siew Chin]

> -----Original Message-----
> From: Tan, Ley Foon <ley.foon.tan at intel.com>
> Sent: Monday, April 5, 2021 3:37 PM
> To: Lim, Elly Siew Chin <elly.siew.chin.lim at intel.com>; u-boot at lists.denx.de
> Cc: Marek Vasut <marex at denx.de>; See, Chin Liang
> <chin.liang.see at intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> <tien.fong.chee at intel.com>; Westergreen, Dalon
> <dalon.westergreen at intel.com>; Simon Glass <sjg at chromium.org>; Gan,
> Yau Wai <yau.wai.gan at intel.com>
> Subject: RE: [v1 04/17] drivers: clk: Add clock driver for Intel N5X device
> 
> 
> 
> > -----Original Message-----
> > From: Lim, Elly Siew Chin <elly.siew.chin.lim at intel.com>
> > Sent: Wednesday, March 31, 2021 10:39 PM
> > To: u-boot at lists.denx.de
> > Cc: Marek Vasut <marex at denx.de>; Tan, Ley Foon
> > <ley.foon.tan at intel.com>; See, Chin Liang <chin.liang.see at intel.com>;
> > Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> > <tien.fong.chee at intel.com>; Westergreen, Dalon
> > <dalon.westergreen at intel.com>; Simon Glass <sjg at chromium.org>; Gan,
> > Yau Wai <yau.wai.gan at intel.com>; Lim, Elly Siew Chin
> > <elly.siew.chin.lim at intel.com>
> > Subject: [v1 04/17] drivers: clk: Add clock driver for Intel N5X
> > device
> >
> > Add clock manager driver for N5X. Provides clock initialization and
> > get_rate functions.
> >
> > Signed-off-by: Siew Chin Lim <elly.siew.chin.lim at intel.com>
> > ---
> >  drivers/clk/altera/Makefile           |   3 +-
> >  drivers/clk/altera/clk-n5x.c          | 489
> > ++++++++++++++++++++++++++++++++++
> >  drivers/clk/altera/clk-n5x.h          | 217 +++++++++++++++
> >  include/dt-bindings/clock/n5x-clock.h |  71 +++++
> >  4 files changed, 779 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/clk/altera/clk-n5x.c  create mode 100644
> > drivers/clk/altera/clk-n5x.h create mode 100644
> > include/dt-bindings/clock/n5x-clock.h
> >
> > diff --git a/drivers/clk/altera/Makefile b/drivers/clk/altera/Makefile
> > index
> > 96215ad5c4..38cd730685 100644
> > --- a/drivers/clk/altera/Makefile
> > +++ b/drivers/clk/altera/Makefile
> > @@ -1,7 +1,8 @@
> >  # SPDX-License-Identifier: GPL-2.0+
> >  #
> > -# Copyright (C) 2018 Marek Vasut <marex at denx.de>
> > +# Copyright (C) 2018-2021 Marek Vasut <marex at denx.de>
> >  #
> >
> >  obj-$(CONFIG_TARGET_SOCFPGA_AGILEX) += clk-agilex.o
> >  obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += clk-arria10.o
> > +obj-$(CONFIG_TARGET_SOCFPGA_N5X) += clk-n5x.o
> > diff --git a/drivers/clk/altera/clk-n5x.c
> > b/drivers/clk/altera/clk-n5x.c new file mode 100644 index
> > 0000000000..12e6aa9ac2
> > --- /dev/null
> > +++ b/drivers/clk/altera/clk-n5x.c
> > @@ -0,0 +1,489 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020-2021 Intel Corporation <www.intel.com>  */
> > +
> > +#include <common.h>
> 
> Sorting the include name.
> 
> > +#include <asm/arch/clock_manager.h>
> > +#include <asm/global_data.h>
> > +#include <asm/io.h>
> > +#include <clk-uclass.h>
> > +#include <dm.h>
> > +#include <dm/lists.h>
> > +#include <dm/util.h>
> > +#include <dt-bindings/clock/n5x-clock.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> 
> [...]
> 
> 
> > +
> > +	/* Take all PLLs out of bypass */
> > +	clk_write_bypass_mainpll(plat, 0);
> > +	clk_write_bypass_perpll(plat, 0);
> > +
> > +	/* Clear the loss of lock bits (write 1 to clear) */
> > +	CM_REG_CLRBITS(plat, CLKMGR_INTRCLR,
> > +		       CLKMGR_INTER_PERPLLLOST_MASK |
> > +		       CLKMGR_INTER_MAINPLLLOST_MASK);
> Comment "write 1 to clear" is valid?
> 
> > +
> > +	/* Take all ping pong counters out of reset */
> > +	CM_REG_CLRBITS(plat, CLKMGR_ALTR_EXTCNTRST,
> > +		       CLKMGR_ALT_EXTCNTRST_ALLCNTRST_MASK);
> > +
> > +	/* Out of boot mode */
> > +	clk_write_ctrl(plat,
> > +		       CM_REG_READL(plat, CLKMGR_CTRL) &
> > ~CLKMGR_CTRL_BOOTMODE); }
> > +
> > +static u32 clk_get_5_1_clk_src(struct socfpga_clk_plat *plat, u32
> > +reg) {
> > +	u32 clksrc = CM_REG_READL(plat, reg);
> > +
> > +	return (clksrc & CLKMGR_CLKSRC_MASK) >>
> > CLKMGR_CLKSRC_OFFSET; }
> > +
> > +static u64 clk_get_pll_output_hz(struct socfpga_clk_plat *plat,
> > +				 u32 pllglob_reg, u32 plldiv_reg) {
> > +	u64 clock = 0;
> > +	u32 clklsrc, divf, divr, divq, power = 1;
> > +
> > +	/* Get input clock frequency */
> > +	clklsrc = (CM_REG_READL(plat, pllglob_reg) &
> Redundant () can be removed. Same for other code in this source file.
> 
> > +		   CLKMGR_PLLGLOB_VCO_PSRC_MASK) >>
> > +		   CLKMGR_PLLGLOB_VCO_PSRC_OFFSET;
> > +
> > +	switch (clklsrc) {
> > +	case CLKMGR_VCO_PSRC_EOSC1:
> > +		clock = cm_get_osc_clk_hz();
> > +		break;
> > +	case CLKMGR_VCO_PSRC_INTOSC:
> > +		clock = cm_get_intosc_clk_hz();
> > +		break;
> > +	case CLKMGR_VCO_PSRC_F2S:
> > +		clock = cm_get_fpga_clk_hz();
> > +		break;
> > +	}
> > +
> 
> Regards
> Ley Foon

Noted, I will update in next review.