linux-kernel - Re: [PATCH v7 3/5] clk: imx: add SCCG PLL type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 20 Sep 2018 16:53:57 -0700
From:   Andrey Smirnov <andrew.smirnov@...il.com>
To:     Abel Vesa <abel.vesa@....com>
Cc:     Lucas Stach <l.stach@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Dong Aisheng <aisheng.dong@....com>,
        Fabio Estevam <fabio.estevam@....com>,
        Anson Huang <anson.huang@....com>,
        Rob Herring <robh@...nel.org>, linux-imx@....com,
        abelvesa@...ux.com, Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Michael Turquette <mturquette@...libre.com>, sboyd@...nel.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH v7 3/5] clk: imx: add SCCG PLL type

On Thu, Sep 20, 2018 at 3:07 AM Abel Vesa <abel.vesa@....com> wrote:
>
> From: Lucas Stach <l.stach@...gutronix.de>
>
> The SCCG is a new PLL type introduced on i.MX8. Add support for this.
> The driver currently misses the PLL lock check, as the preliminary
> documentation mentions lock configurations, but is quiet about where
> to find the actual lock status signal.
>
> Signed-off-by: Lucas Stach <l.stach@...gutronix.de>
> Signed-off-by: Abel Vesa <abel.vesa@....com>
> ---
>  drivers/clk/imx/Makefile       |   3 +-
>  drivers/clk/imx/clk-sccg-pll.c | 246 +++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/imx/clk.h          |   9 ++
>  3 files changed, 257 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/imx/clk-sccg-pll.c
>
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index 4893c1f..b87513c 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -12,7 +12,8 @@ obj-y += \
>         clk-pllv1.o \
>         clk-pllv2.o \
>         clk-pllv3.o \
> -       clk-pfd.o
> +       clk-pfd.o \
> +       clk-sccg-pll.o
>
>  obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
>  obj-$(CONFIG_SOC_IMX21)  += clk-imx21.o
> diff --git a/drivers/clk/imx/clk-sccg-pll.c b/drivers/clk/imx/clk-sccg-pll.c
> new file mode 100644
> index 0000000..8d87ba5
> --- /dev/null
> +++ b/drivers/clk/imx/clk-sccg-pll.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2018 NXP.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +
> +#include "clk.h"
> +
> +/* PLL CFGs */
> +#define PLL_CFG0       0x0
> +#define PLL_CFG1       0x4
> +#define PLL_CFG2       0x8
> +
> +#define PLL_DIVF1_SHIFT        13
> +#define PLL_DIVF2_SHIFT        7
> +#define PLL_DIVF_MASK  0x3f
> +
> +#define PLL_DIVR1_SHIFT        25
> +#define PLL_DIVR2_SHIFT        19
> +#define PLL_DIVR1_MASK 0x3
> +#define PLL_DIVR2_MASK 0x3f
> +#define PLL_REF_SHIFT  0
> +#define PLL_REF_MASK   0x3
> +

Similar to previous patch, I'd suggest using FILED_GET()/GENMAKS for
the above fields

> +#define PLL_LOCK_MASK  BIT(31)
> +#define PLL_PD_MASK    BIT(7)
> +
> +#define OSC_25M                25000000
> +#define OSC_27M                27000000
> +
> +struct clk_sccg_pll {
> +       struct clk_hw   hw;
> +       void __iomem    *base;
> +};
> +
> +#define to_clk_sccg_pll(_hw) container_of(_hw, struct clk_sccg_pll, hw)
> +
> +static int clk_pll_wait_lock(struct clk_sccg_pll *pll)
> +{
> +       /* max lock time is 70us */
> +       int retry = 7;
> +
> +       /* Wait for PLL to lock */
> +       do {
> +               if (readl_relaxed(pll->base) & PLL_LOCK_MASK)
> +                       break;
> +               udelay(10);
> +               retry--;
> +       } while (retry);
> +

Can readx_poll_timeout_atomic() be used here instead?

Thanks,
Andrey Smirnov