[PATCH 1/4] cpufreq: Add a cpufreq driver for Marvell Dove

[Andrew Lunn]

> >+	if (freqs.old != freqs.new) {
> >+		local_irq_disable();
> >+
> >+		/* Mask IRQ and FIQ to CPU */
> >+		cr = readl(priv.pmu_cr);
> >+		cr |= MASK_IRQ | MASK_FIQ;
> >+		writel(cr, priv.pmu_cr);
> >+
> >+		/* Set/Clear the CPU_SLOW_EN bit */
> >+		reg = readl_relaxed(priv.dfs + DFS_CR);
> >+		reg &= ~L2_RATIO_MASK;
> >+
> >+		switch (state) {
> >+		case STATE_CPU_FREQ:
> >+			reg |= priv.xpratio;
> >+			reg &= ~CPU_SLOW_EN;
> >+			break;
> >+		case STATE_DDR_FREQ:
> >+			reg |= (priv.dpratio | CPU_SLOW_EN);
> 
> Does slow mode really (only) depend on the value written into CPUL2CR?
> Dove FS isn't that chatty about it and I cannot really test right now.
> But reading it, I think there is two CPU PLL to {L2,DDR} ratios that you
> can switch between by (de-)asserting CPU_SLOW_EN.

Hi Sebastian

The lack of details in the datasheet was very annoying. I spent many
evenings looking at a frozen up cubox. In the end i had to risk eye
cancer and look at the Marvell vendor code. It always sets these
values on each transition, and it even calculated one of the ratios
each time the frequency changed. I never actually tried setting them
once and then leaving them alone. Worth a try once i have the hardware
available.

> >+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+	priv.dfs = devm_ioremap_resource(&pdev->dev, res);
> >+	if (IS_ERR(priv.dfs))
> >+		return PTR_ERR(priv.dfs);
> >+
> >+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >+	priv.pmu_cr = devm_ioremap_resource(&pdev->dev, res);
> >+	if (IS_ERR(priv.pmu_cr))
> 
> Cleanup path for most of the resources here and below is kind of
> missing. 

Well, i'm using devm_ioremap_resources().  The cleanup for that should
happen automagically. And platform_get_resource() is just a lookup
function.

> Also, maybe give names to the different areas and not depend
> on ordering?

Actually, they already have names, but you probably have not go to
that patch yet. So swapping to platform_get_resource_byname() would be
simple.

Thanks
	Andrew