Hi Lukasz,

On 12/8/19 4:05 PM, Lukasz Majewski wrote:
On Wed,  4 Dec 2019 18:44:28 +0100
Giulio Benetti <giulio.bene...@benettiengineering.com> wrote:

Add support for PLLV3 AV type.

Signed-off-by: Giulio Benetti <giulio.bene...@benettiengineering.com>
---
  drivers/clk/imx/clk-pllv3.c | 76
+++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index d5087a104e..fc16416d5f 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -6,6 +6,7 @@
#include <common.h>
  #include <asm/io.h>
+#include <div64.h>
  #include <malloc.h>
  #include <clk-uclass.h>
  #include <dm/device.h>
@@ -16,6 +17,10 @@
  #define UBOOT_DM_CLK_IMX_PLLV3_GENERIC        "imx_clk_pllv3_generic"
  #define UBOOT_DM_CLK_IMX_PLLV3_SYS    "imx_clk_pllv3_sys"
  #define UBOOT_DM_CLK_IMX_PLLV3_USB    "imx_clk_pllv3_usb"
+#define UBOOT_DM_CLK_IMX_PLLV3_AV      "imx_clk_pllv3_av"
+
+#define PLL_NUM_OFFSET         0x10
+#define PLL_DENOM_OFFSET       0x20
#define BM_PLL_POWER (0x1 << 12)
  #define BM_PLL_LOCK           (0x1 << 31)
@@ -143,6 +148,65 @@ static const struct clk_ops clk_pllv3_sys_ops = {
        .set_rate       = clk_pllv3_sys_set_rate,
  };
+static ulong clk_pllv3_av_get_rate(struct clk *clk)
+{
+       struct clk_pllv3 *pll = to_clk_pllv3(clk);
+       unsigned long parent_rate = clk_get_parent_rate(clk);
+       u32 mfn = readl(pll->base + PLL_NUM_OFFSET);
+       u32 mfd = readl(pll->base + PLL_DENOM_OFFSET);
+       u32 div = readl(pll->base) & pll->div_mask;
+       u64 temp64 = (u64)parent_rate;
+
+       temp64 *= mfn;
+       do_div(temp64, mfd);
+
+       return parent_rate * div + (unsigned long)temp64;
+}
+
+static ulong clk_pllv3_av_set_rate(struct clk *clk, ulong rate)
+{
+       struct clk_pllv3 *pll = to_clk_pllv3(clk);
+       unsigned long parent_rate = clk_get_parent_rate(clk);
+       unsigned long min_rate = parent_rate * 27;
+       unsigned long max_rate = parent_rate * 54;
+       u32 val, div;
+       u32 mfn, mfd = 1000000;
+       u32 max_mfd = 0x3FFFFFFF;
+       u64 temp64;
+
+       if (rate < min_rate || rate > max_rate)
+               return -EINVAL;
+
+       if (parent_rate <= max_mfd)
+               mfd = parent_rate;
+
+       div = rate / parent_rate;
+       temp64 = (u64)(rate - div * parent_rate);
+       temp64 *= mfd;
+       do_div(temp64, parent_rate);
+       mfn = temp64;
+
+       val = readl(pll->base);
+       val &= ~pll->div_mask;
+       val |= div;
+       writel(val, pll->base);
+       writel(mfn, pll->base + PLL_NUM_OFFSET);
+       writel(mfd, pll->base + PLL_DENOM_OFFSET);
+
+       /* Wait for PLL to lock */
+       while (!(readl(pll->base) & BM_PLL_LOCK))
+               ;
+
+       return 0;
+}
+
+static const struct clk_ops clk_pllv3_av_ops = {
+       .enable         = clk_pllv3_generic_enable,
+       .disable        = clk_pllv3_generic_disable,
+       .get_rate       = clk_pllv3_av_get_rate,
+       .set_rate       = clk_pllv3_av_set_rate,
+};
+
  struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
                          const char *parent_name, void __iomem
*base, u32 div_mask)
@@ -174,6 +238,11 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type
type, const char *name, pll->div_shift = 1;
                pll->powerup_set = true;
                break;
+       case IMX_PLLV3_AV:
+               drv_name = UBOOT_DM_CLK_IMX_PLLV3_AV;
+               pll->div_shift = 0;
+               pll->powerup_set = false;
+               break;
        default:
                kfree(pll);
                return ERR_PTR(-ENOTSUPP);
@@ -212,3 +281,10 @@ U_BOOT_DRIVER(clk_pllv3_usb) = {
        .ops    = &clk_pllv3_generic_ops,
        .flags = DM_FLAG_PRE_RELOC,
  };
+
+U_BOOT_DRIVER(clk_pllv3_av) = {
+       .name   = UBOOT_DM_CLK_IMX_PLLV3_AV,
+       .id     = UCLASS_CLK,
+       .ops    = &clk_pllv3_av_ops,
+       .flags = DM_FLAG_PRE_RELOC,
+};

I don't mind about adding this new functionality, but I'm a bit
concerned about increase if the size of SPL binary (as it sets the
DM_FLAG_PRE_RELOC).

Do you have any data about increase of the final binary size?

Yes, following what you've pointed me below(using board kp_imx6q_tpc with buildman) these are the results(from patch 1 to 7):
01: clk: imx: pllv3: register PLLV3 GENERIC and USB as 2 different clocks
07: clk: imx: pllv3: add support for PLLV3_AV type
arm: (for 1/1 boards) all +963.0 data +136.0 rodata +99.0 spl/u-boot-spl:all +831.0 spl/u-boot-spl:data +136.0 spl/u-boot-spl:rodata +99.0 spl/u-boot-spl:text +596.0 text +728.0

So for SPL, as I understand, latest text increment of +728 should be taken because it's the sum of text(596)+data(136) then padded, right?
So SPL increased not few.

The buildman script has options to check the difference of the final
binary (i.e. SPL) size (as provided by Tom Rini):

./tools/buildman/$ export SOURCE_DATE_EPOCH=`date +%s`
$ ./tools/buildman/buildman -o  /tmp/test --step 0 -b origin/master.. 
--force-build -CveE
$ ./tools/buildman/buildman -o  /tmp/test --step 0 -b origin/master.. -Ssdel

to get size changes between point A and point Z in a branch, and omit
--step 0 when I need to see which patch in between them caused the size
change.


If the SPL growth is too big - maybe we shall introduce some Kconfig
options to add a separate support for those PLLv3 options ?

If you want I can slice drivers down with different Kconfig entries and corresponding clk-pllv3-generic.c, clk-pllv3-usb.c etc.
Or maybe using #ifdef inside the same file?

Anyway the only board using this except imxrt1050-evk is kp_imx6q_tpc, so this should be easy to test.

Best regards
--
Giulio Benetti
Benetti Engineering sas

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de


Reply via email to