linux-kernel - Re: [PATCH 10/17] clk: sunxi=ng: add support for R329 R-CCU

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:   Wed, 25 Aug 2021 19:20:54 -0500
From:   Samuel Holland <samuel@...lland.org>
To:     Jernej Škrabec <jernej.skrabec@...il.com>,
        Maxime Ripard <mripard@...nel.org>
Cc:     devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Icenowy Zheng <icenowy@...eed.com>,
        Rob Herring <robh+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Andre Przywara <andre.przywara@....com>
Subject: Re: [PATCH 10/17] clk: sunxi=ng: add support for R329 R-CCU

On 8/19/21 11:34 PM, Jernej Škrabec wrote:
>>> +static void __init sun50i_r329_r_ccu_setup(struct device_node *node)
>>> +{
>>> +	void __iomem *reg;
>>> +	u32 val;
>>> +	int i;
>>> +
>>> +	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
>>> +	if (IS_ERR(reg)) {
>>> +		pr_err("%pOF: Could not map clock registers\n", node);
>>> +		return;
>>> +	}
>>> +
>>> +	/* Enable the lock bits and the output enable bits on all PLLs */
>>> +	for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
>>> +		val = readl(reg + pll_regs[i]);
>>> +		val |= BIT(29) | BIT(27);
>>> +		writel(val, reg + pll_regs[i]);
>>> +	}
>>> +
>>> +	/*
>>> +	 * Force the I/O dividers of PLL-AUDIO1 to reset default value
>>> +	 *
>>> +	 * See the comment before pll-audio1 definition for the reason.
>>> +	 */
>>> +
>>> +	val = readl(reg + SUN50I_R329_PLL_AUDIO1_REG);
>>> +	val &= ~BIT(1);
>>> +	val |= BIT(0);
>>> +	writel(val, reg + SUN50I_R329_PLL_AUDIO1_REG);
>>> +
>>> +	i = sunxi_ccu_probe(node, reg, &sun50i_r329_r_ccu_desc);
>>> +	if (i)
>>> +		pr_err("%pOF: probing clocks fails: %d\n", node, i);
>>> +}
>>> +
>>> +CLK_OF_DECLARE(sun50i_r329_r_ccu, "allwinner,sun50i-r329-r-ccu",
>>> +	       sun50i_r329_r_ccu_setup);
>>
>> Please make this a platform driver. There is no particular reason why it
>> needs to be an early OF clock provider.
> 
> Why? It's good to have it as early clock provider. It has no dependencies and 
> other drivers that depends on it, like IR, can be deferred, if this is loaded 
> later.

Another reason is so the driver can be built as a module. Each of these
CCU drivers has 30k-70k of data in it (lots of pointers, plus lots of
relocations). So it saves some RAM to only load the ones you need,
especially if that is none of them.

Regards,
Samuel