From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Tue, 10 Dec 2019 01:07:03 +0100 Subject: [PATCH 09/20] clk: imx: pllv3: add support for PLLV3_AV type In-Reply-To: References: <20191204174439.69934-1-giulio.benetti@benettiengineering.com> <20191204174439.69934-10-giulio.benetti@benettiengineering.com> <20191208160524.232bd633@jawa> Message-ID: <20191210010703.510ef9be@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, 9 Dec 2019 18:13:04 +0100 Giulio Benetti wrote: > Hi Lukasz, > > On 12/8/19 4:05 PM, Lukasz Majewski wrote: > > On Wed, 4 Dec 2019 18:44:28 +0100 > > Giulio Benetti wrote: > > > >> Add support for PLLV3 AV type. > >> > >> Signed-off-by: Giulio Benetti > >> --- > >> 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 > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -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). 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. 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. - If OF_PLATDATA is used - then it is possible to NOT compile libfdt and friends, which brings the substantial footprint reduction. > > 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 :-). > > 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: lukma at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: