public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: olof@lixom•net (Olof Johansson)
To: Arnd Bergmann <arnd@arndb•de>
Cc: linuxppc-dev@ozlabs•org, egor@pasemi•com, paulus@samba•org,
	cpufreq@lists•linux.org.uk
Subject: Re: [PATCH] [2.6.22] pasemi: cpufreq driver
Date: Wed, 25 Apr 2007 18:57:57 -0500	[thread overview]
Message-ID: <20070425235757.GA20905@lixom.net> (raw)
In-Reply-To: <200704260147.35878.arnd@arndb.de>

On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> On Wednesday 25 April 2007, Olof Johansson wrote:
> 
> > +/* This should eventually come out of the device tree */
> > +#define SDCPWR_BASE		0xfc104000
> > +#define SDCPWR_SIZE		0x1000
> > +
> > +#define SDCASR_BASE		0xfc120000
> > +#define SDCASR_SIZE		0x2000
> 
> the comment is right. Why don't you do it then? ;-)

Leftovers from when it was a regular platform driver.

> > +static int pas_cpufreq_cpu_init (struct cpufreq_policy *policy)
> > +{
> > +	u32 *max_freq;
> > +	int i, cur_astate;
> > +	struct device_node *cpu;
> > +
> > +	cpu = of_get_cpu_node(policy->cpu, NULL);
> > +
> > +	if(!cpu)
> > +		return -ENODEV;
> > +
> > +	sdcpwr_mapbase = ioremap(SDCPWR_BASE, SDCPWR_SIZE);
> > +	sdcasr_mapbase = ioremap(SDCASR_BASE, SDCASR_SIZE);
> > +	if (!sdcpwr_mapbase || !sdcasr_mapbase)
> > +		panic("SDCMAP: Cannot map registers!");
> 
> I can't see any check in here that finds out if you are actually running on
> the right hardware. 
> The proper way to implement this driver would be to register an
> of_platform_driver for the cpufreq device and then get the register
> addresses from there,

Yup.

> > @@ -155,7 +155,7 @@ source "drivers/cpufreq/Kconfig"
> >  
> >  config CPU_FREQ_PMAC
> >  	bool "Support for Apple PowerBooks"
> > -	depends on CPU_FREQ && ADB_PMU && PPC32
> > +	depends on ADB_PMU && PPC32
> >  	select CPU_FREQ_TABLE
> >  	help
> >  	  This adds support for frequency switching on Apple PowerBooks,
> > @@ -164,11 +164,21 @@ config CPU_FREQ_PMAC
> >  
> >  config CPU_FREQ_PMAC64
> >  	bool "Support for some Apple G5s"
> > -	depends on CPU_FREQ && PPC64
> > +	depends on PPC64
> >  	select CPU_FREQ_TABLE
> >  	help
> >  	  This adds support for frequency switching on Apple iMac G5,
> >  	  and some of the more recent desktop G5 machines as well.
> 
> Why this change?

They're redundant, but they should have been in a separate patch. I
forgot to revert them before rediffing.

> > +config PPC_PASEMI_CPUFREQ
> > +	bool "Support for PA Semi PWRficient"
> > +	depends on PPC_PASEMI
> > +	default y
> > +	select CPU_FREQ_TABLE
> > +	help
> > +	  This adds the support for frequency switching on PA Semi
> > +	  PWRficient processors.
> > +
> >  endmenu
> 
> Why bool and not tristate?

1) The other cpufreq drivers are bool
2) See the idle loop interaction: It can go tristate once we have runtime
selection of idle loop, but until then we need it for idle=doze.


-Olof

  reply	other threads:[~2007-04-25 23:57 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-25 20:46 [PATCH] [2.6.22] pasemi: cpufreq driver Olof Johansson
2007-04-25 23:47 ` Arnd Bergmann
2007-04-25 23:57   ` Olof Johansson [this message]
2007-04-26  1:57     ` Olof Johansson
2007-04-26  6:56   ` cbe_cpufreq crashes my machine Olof Johansson
2007-04-26  8:39     ` Benjamin Herrenschmidt
2007-04-26 23:07     ` [PATCH] cell: cbe_cpufreq cleanup and crash fix Olof Johansson
2007-04-27  5:22       ` [Cbe-oss-dev] " Akinobu Mita
2007-04-27  5:32         ` Olof Johansson
2007-04-27  5:33       ` [PATCH v2] " Olof Johansson
2007-04-27  7:55         ` Christian Krafft
2007-04-26  5:37 ` [PATCH v2] [2.6.22] pasemi: cpufreq driver Olof Johansson
2007-04-26  8:55   ` Arnd Bergmann
2007-04-26 16:48     ` Olof Johansson
2007-04-26 17:11       ` Arnd Bergmann
2007-04-26 19:05         ` Segher Boessenkool
2007-04-26 20:38           ` Olof Johansson
2007-04-27  0:10             ` Segher Boessenkool
2007-04-26 20:26         ` Olof Johansson
2007-04-26 20:43           ` Arnd Bergmann
2007-04-26 10:26   ` Johannes Berg
2007-04-26 20:37     ` Olof Johansson
2007-04-27  9:40       ` Johannes Berg
2007-04-27 18:09         ` Olof Johansson
2007-04-27  5:46   ` [PATCH v3] " Olof Johansson
2007-04-29  2:50     ` Dominik Brodowski
2007-04-29  3:40       ` Stephen Rothwell
2007-04-29  4:39         ` Olof Johansson
2007-04-29  4:42       ` Olof Johansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070425235757.GA20905@lixom.net \
    --to=olof@lixom$(echo .)net \
    --cc=arnd@arndb$(echo .)de \
    --cc=cpufreq@lists$(echo .)linux.org.uk \
    --cc=egor@pasemi$(echo .)com \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    --cc=paulus@samba$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox