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  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:	Fri, 21 Mar 2014 16:45:39 +0100
From:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Cc:	Mike Turquette <mturquette@...aro.org>, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>
Subject: Re: [PATCH 1/5] clk: berlin: add support for berlin plls

[all commentis I agree on are snipped]

On 21/03/2014 at 13:49:32 +0100, Sebastian Hesselbarth wrote :
> On 03/21/2014 12:43 PM, Alexandre Belloni wrote:
> obj-y += pll.o
> obj-$(CONFIG_MACH_BERLIN_BG2)	+= pll-berlin2.o
> obj-$(CONFIG_MACH_BERLIN_BG2CD)	+= pll-berlin2.o
> obj-$(CONFIG_MACH_BERLIN_BG2Q)	+= pll-berlin2q.o
> 
> Which reminds me, that we forgot to add MACH_BERLIN_BG2Q to
> arch/arm/mach-berlin/Kconfig. Can you spin a patch?
> 

I will do that.

> >+static const u8 vcodiv_berlin2[] = {10, 15, 20, 25, 30, 40, 50, 60, 80};
> >+
> >+static struct berlin_pllmap berlin_pll_map = {
> >+	.vcodiv = vcodiv_berlin2,
> >+	.fbdiv_mask = 0x1FF,
> >+	.rfdiv_mask = 0x1F,
> >+	.divsel_mask = 0xF,
> 
> divsel_mask allows 16 possible values, vcodiv_berlin2[] only provides
> 9, and common pll driver below does not know how many valid vcodiv
> values are passed. That can become dangerous..
> 
> I'd rather extend vcodiv_berlin2 to full divsel range and provide
> safe (=1) divisiors. This way wrong/new register values will only
> break clock frequency derived.
> 

Good catch ! Then, what about simply shrinking the mask so that we don't
overflow the table. We'll put it back to its supposed real value whant
we know what are the remaining divisors (my guess is that they are already
all listed here). I would say that we are getting the divisor wrong if
divsel > 8 anyway.

> >+	.fbdiv_shift = 6,
> >+	.rfdiv_shift = 1,
> >+	.divsel_shift = 7,
> 
> Have .foo_mask and .foo_shift together?
> 

This will make the struct larger but I don't really have an opinion.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android 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/

Powered by blists - more mailing lists