On 03/21/2014 04:45 PM, Alexandre Belloni wrote:
[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.
Hmm, maybe I should look up valid vcodiv myself, but your vcodiv_berlin2
has 9 values and I guess they are all valid, aren't they?
The next possible, larger mask where 0-8 fits in, is 0xf. You used that
above and that reveals 16 possible indices.
The only option for shrinking the table that I see, would be min/max
allowed indices, but that is as useful as having a slightly larger
table.
+ .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.
Maybe, I wasn't clear enough. Just assign .foo_mask and .foo_shift in
subsequent lines of code, i.e.
static struct berlin_pllmap berlin_pll_map = {
.vcodiv = vcodiv_berlin2,
.fbdiv_mask = 0x1FF,
.fbdiv_shift = 6,
.rfdiv_mask = 0x1F,
.rfdiv_shift = 1,
.divsel_mask = 0xF,
.divsel_shift = 7,
};
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
