On 12/10/19 1:07 AM, Lukasz Majewski wrote:
On Mon, 9 Dec 2019 18:13:04 +0100
Giulio Benetti <giulio.bene...@benettiengineering.com> wrote:
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.
This board's SPL size is ~35KiB. The increase is 2%.
Seems to be not much, but ...
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?
I do guess that iMXRT is some kind of resource constrained SoC?
Wikipedia [1] says that it is for low-latency with real-time (memory
protection - MPU - instead of MMU).
That is true, it provides 32K of sram by default, but with DCD on
imximage you can set OCRAM to be 512K on imxrt1050 and 256K on
imxrt1020(the shortest Mcu of the family with EBI). So you have 256K for
SPL at least(including text, rodata and stack).
So i.MXRT has not so much size costraint on SPL.
Anyway, let me know how to proceed, if keeping a big pllv3.c or making
it possible to select which kind of pll to compile.
If this is the case, then there are some (better?) options for SPL:
- One can use OF_PLATDATA framework to reduce the SPL size - in short -
it removes from SPL libfdt, which parses device tree (DT) and
replaces DT with compiled in C structures. For SPL it brings good
results in reducing binary size.
Yes, I've seen it, very cool.
One good example of OF_PLATDATA conversion is the xea (i.MX28 [*])
board support (now it waits for upstreaming) [2]. It can be easily
applied on top of current -master and you will see how this works in
practice.
I'll check it out
- If OF_PLATDATA is used - then it is possible to NOT compile libfdt and
friends, which brings the substantial footprint reduction.
Yes
Anyway the only board using this except imxrt1050-evk is
kp_imx6q_tpc, so this should be easy to test.
You would not believe, but by chance I do have one kp_imx6q_tpc board
available for testing :-).
That's perfect then! :-)
Thank you
Best regards
--
Giulio Benetti
Benetti Engineering sas
Best regards
Links:
[1] - https://en.wikipedia.org/wiki/ARM_Cortex-R
[2] - https://patchwork.ozlabs.org/patch/1205772/
Note:
[*] - i.MX28 is a pretty old ARM9 SoC, with not so much resources (now)
available. To use the device tree drivers, the OF_PLATDATA had to be
used. Moreover, some old drivers (like gpio, pinctrl, eth, etc.) had to
be converted to device tree support with OF_PLATDATA (please refer to
mxs_* prefix in drivers).
Now all those drivers are upstreamed, so it is just enough to apply the
patch which adds xea board.
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
