linux-kernel - Re: [PATCH v4 5/7] clk: Introduce davinci clocks

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 Jan 2018 17:58:29 +0530
From:   Sekhar Nori <nsekhar@...com>
To:     David Lechner <david@...hnology.com>,
        <linux-arm-kernel@...ts.infradead.org>
CC:     Kevin Hilman <khilman@...nel.org>, Adam Ford <aford173@...il.com>,
        <linux-kernel@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: [PATCH v4 5/7] clk: Introduce davinci clocks

On Wednesday 03 January 2018 03:01 AM, David Lechner wrote:
> Forgot to cc linux-clk, so doing that now...
> 
> 
> On 12/31/2017 05:39 PM, David Lechner wrote:
>> This introduces new drivers for arch/arm/mach-davinci. The code is based
>> on the clock drivers from there and adapted to use the common clock
>> framework.
>>
>> Signed-off-by: David Lechner <david@...hnology.com>
>> ---
>>   drivers/clk/Makefile                      |   1 +
>>   drivers/clk/davinci/Makefile              |   3 +
>>   drivers/clk/davinci/da8xx-cfgchip-clk.c   | 380
>> ++++++++++++++++++++++++++++++
>>   drivers/clk/davinci/pll.c                 | 333
>> ++++++++++++++++++++++++++
>>   drivers/clk/davinci/psc.c                 | 217 +++++++++++++++++
>>   include/linux/clk/davinci.h               |  46 ++++
>>   include/linux/platform_data/davinci_clk.h |  25 ++
>>   7 files changed, 1005 insertions(+)

This is a pretty huge patch and I think each of cfgchip, pll and PSC
clocks deserve a patch of their own.

On the PLL patch, please describe how the PLL implementation on DaVinci
is different from Keystone, so no reuse is really possible. Similarly
for the PSC patch (no non-DT support in keystone etc).

>> diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
>> new file mode 100644
>> index 0000000..8ae85ee
>> --- /dev/null
>> +++ b/drivers/clk/davinci/psc.c
>> @@ -0,0 +1,217 @@

>> +static void psc_config(struct davinci_psc_clk *psc,
>> +               enum davinci_psc_state next_state)
>> +{
>> +    u32 epcpr, ptcmd, pdstat, pdctl, mdstat, mdctl, ptstat;
>> +
>> +    mdctl = readl(psc->base + MDCTL + 4 * psc->lpsc);
>> +    mdctl &= ~MDSTAT_STATE_MASK;
>> +    mdctl |= next_state;
>> +    /* TODO: old davinci clocks for da850 set MDCTL_FORCE bit for
>> sata and
>> +     * dsp here. Is this really needed?
>> +     */
>> +    writel(mdctl, psc->base + MDCTL + 4 * psc->lpsc);
>> +
>> +    pdstat = readl(psc->base + PDSTAT + 4 * psc->pd);
>> +    if ((pdstat & PDSTAT_STATE_MASK) == 0) {
>> +        pdctl = readl(psc->base + PDSTAT + 4 * psc->pd);
>> +        pdctl |= PDCTL_NEXT;
>> +        writel(pdctl, psc->base + PDSTAT + 4 * psc->pd);
>> +
>> +        ptcmd = BIT(psc->pd);
>> +        writel(ptcmd, psc->base + PTCMD);
>> +
>> +        do {
>> +            epcpr = __raw_readl(psc->base + EPCPR);
>> +        } while (!(epcpr & BIT(psc->pd)));
>> +
>> +        pdctl = __raw_readl(psc->base + PDCTL + 4 * psc->pd);
>> +        pdctl |= PDCTL_EPCGOOD;
>> +        __raw_writel(pdctl, psc->base + PDCTL + 4 * psc->pd);

Can we shift to regmap here too? Then the polling loops like above can
be converted to regmap_read_poll_timeout() too like you have done elsewhere.

Thanks,
Sekhar