linux-kernel - Re: [PATCH v4] clk: at91: add generated clock driver

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]

Message-ID: <20150729091531.2f167170@bbrezillon>
Date:	Wed, 29 Jul 2015 09:15:31 +0200
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Nicolas Ferre <nicolas.ferre@...el.com>
Cc:	<linux-arm-kernel@...ts.infradead.org>, <mturquette@...libre.com>,
	<sboyd@...eaurora.org>, <linux-kernel@...r.kernel.org>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	Ludovic Desroches <ludovic.desroches@...el.com>,
	Josh Wu <josh.wu@...el.com>, <pebolle@...cali.nl>
Subject: Re: [PATCH v4] clk: at91: add generated clock driver

Hi Nicolas,

On Tue, 28 Jul 2015 18:08:05 +0200
Nicolas Ferre <nicolas.ferre@...el.com> wrote:


> +static void clk_generated_startup(struct clk_generated *gck)
> +{
> +	struct at91_pmc *pmc = gck->pmc;
> +	u32 tmp;
> +
> +	pmc_lock(pmc);
> +	pmc_write(pmc, AT91_PMC_PCR, (gck->id & AT91_PMC_PCR_PID_MASK));
> +	tmp = pmc_read(pmc, AT91_PMC_PCR);
> +	pmc_unlock(pmc);
> +
> +	gck->parent_id = (tmp & AT91_PMC_PCR_GCKCSS_MASK)
> +					>> AT91_PMC_PCR_GCKCSS_OFFSET;
> +	/*
> +	 * make sure that what we read in hardware is coherent with
> +	 * what we've just probed
> +	 */
> +	if (gck->parent_id >= __clk_get_num_parents(gck->hw.clk))
> +		gck->parent_id = 0;

Hm, I'm not sure this is correct. Here, you're just faking the
fact that your current parent is the first one in the parent list while
it actually points to the 6th entry. Not only your rate will be false
(the one calculated in ->round_rate()), but you're also changing the
behavior of the clk_set_rate() and clk_set_parent() operation (AFAIR,
if you try to change to the first parent, the core code will think it's
already properly configured and will never call ->set_parent()).

This leaves 2 solutions here:
- implement the missing clk driver and add an entry in the parent
  list
- select the 1st parent clk (I mean, change the register value) if the
  hardware points to the 6th one.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/