[PATCH v4 2/4] mvebu: Dove: Instantiate cpufreq driver.

[Mark Rutland]

On Wed, Dec 04, 2013 at 03:02:28PM +0000, Andrew Lunn wrote:
> On Wed, Dec 04, 2013 at 02:36:28PM +0000, Mark Rutland wrote:
> > On Wed, Dec 04, 2013 at 02:17:19PM +0000, Andrew Lunn wrote:
> > > Add a platform driver definition to instantiate the dove cpufreq
> > > driver. Also indicate the ARCH has cpufreq support, so allowing the
> > > cpufreq framework to be enabled.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew at lunn.ch>
> > > Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth at gmail.com>
> > > ---
> > > Make resource names consistent.
> > > Fix length of ranges
> > > ---
> > >  arch/arm/Kconfig                       |  1 +
> > >  arch/arm/mach-dove/board-dt.c          |  3 +++
> > >  arch/arm/mach-dove/common.c            | 36 ++++++++++++++++++++++++++++++++++
> > >  arch/arm/mach-dove/common.h            |  1 +
> > >  arch/arm/mach-dove/include/mach/dove.h |  1 +
> > >  5 files changed, 42 insertions(+)
> > > 
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index c1f1a7eee953..908b02f8d901 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -500,6 +500,7 @@ config ARCH_IXP4XX
> > >  
> > >  config ARCH_DOVE
> > >  	bool "Marvell Dove"
> > > +	select ARCH_HAS_CPUFREQ
> > >  	select ARCH_REQUIRE_GPIOLIB
> > >  	select CPU_PJ4
> > >  	select GENERIC_CLOCKEVENTS
> > > diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
> > > index 49fa9abd09da..845e8b328432 100644
> > > --- a/arch/arm/mach-dove/board-dt.c
> > > +++ b/arch/arm/mach-dove/board-dt.c
> > > @@ -27,6 +27,9 @@ static void __init dove_dt_init(void)
> > >  	tauros2_init(0);
> > >  #endif
> > >  	BUG_ON(mvebu_mbus_dt_init());
> > > +
> > > +	dove_cpufreq_init();
> > > +
> > >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > >  }
> > >  
> > > diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
> > > index c122bcff9f7c..323fcd4b27b6 100644
> > > --- a/arch/arm/mach-dove/common.c
> > > +++ b/arch/arm/mach-dove/common.c
> > > @@ -344,6 +344,42 @@ void __init dove_sdio1_init(void)
> > >  	platform_device_register(&dove_sdio1);
> > >  }
> > >  
> > > +/*****************************************************************************
> > > + * CPU Frequency
> > > + ****************************************************************************/
> > > +static struct resource dove_cpufreq_resources[] = {
> > > +	[0] = {
> > > +		.start  = DOVE_PMU_PHYS_BASE,
> > > +		.end    = DOVE_PMU_PHYS_BASE + 0x7,
> > > +		.flags  = IORESOURCE_MEM,
> > > +		.name   = "cpufreq: DFS"
> > > +	},
> > > +	[1] = {
> > > +		.start  = DOVE_PMU_PHYS_BASE + 0x8000,
> > > +		.end    = DOVE_PMU_PHYS_BASE + 0x8003,
> > > +		.flags  = IORESOURCE_MEM,
> > > +		.name   = "cpufreq: PMU CR"
> > > +	},
> > > +	[2] = {
> > > +		.start  = DOVE_PMU_PHYS_BASE + 0x0044,
> > > +		.end    = DOVE_PMU_PHYS_BASE + 0x0047,
> > > +		.flags  = IORESOURCE_MEM,
> > > +		.name   = "cpufreq: PMU Clk Div"
> > > +	},
> > > +};
> > > +
> > > +static struct platform_device dove_cpufreq_device = {
> > > +	.name		= "dove-cpufreq",
> > > +	.id		= -1,
> > > +	.num_resources	= ARRAY_SIZE(dove_cpufreq_resources),
> > > +	.resource	= dove_cpufreq_resources,
> > > +};
> > > +
> > > +void __init dove_cpufreq_init(void)
> > > +{
> > > +	platform_device_register(&dove_cpufreq_device);
> > > +}
> > 
> > Please describe your power management unit in the DT. Your hardcoding
> > physical addresses that can easily be described in the DT, and you don't
> > appear to have an early probing requirement.
> 
> Hi Mark
> 
> We have been here before, with the kirkwood cpufreq driver. My v1 of
> that driver had the cpufreq driver fully described in DT. My binding
> got shot down and i was told that pseudo devices, like cpufreq,
> cpuidle, should not be in DT, and a platform driver was the way to go.
> So that is how kirkwood cpufreq works and dove follows that.
> 
> Do you want to reverse that decision now?

I'm not arguing for describing a pseudo-device. The power management
unit is a real device. If we used that to register a cpufreq driver, it
doesn't mean that we've registered a pseudo-device, but rather that we
made a decision to register the cpufreq portions based on the necessary
real devices being present.

This code registers a platform device that has PMU register fields. We
can easily describe the PMU in dt. The PMU _could_ be used by frameworks
other than cpufreq, so describing it as a cpufreq device is wrong.
Perhaps this was the problem with the proposed kirkwood binding?

Thanks,
Mark.