From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5) Date: Thu, 16 Jun 2011 01:06:40 +0200 Message-ID: <201106160106.40553.rjw__598.390369168733$1308179237$gmane$org@sisk.pl> References: <201106112223.04972.rjw@sisk.pl> <201106142316.02812.rjw@sisk.pl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Magnus Damm Cc: linux-sh@vger.kernel.org, Greg Kroah-Hartman , LKML , Linux PM mailing list List-Id: linux-pm@vger.kernel.org On Wednesday, June 15, 2011, Magnus Damm wrote: > On Wed, Jun 15, 2011 at 6:16 AM, Rafael J. Wysocki wrote: > > On Tuesday, June 14, 2011, Magnus Damm wrote: > >> On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki wrote: > >> > From: Rafael J. Wysocki > >> > > >> > Use the generic power domains support introduced by the previous > >> > patch to implement support for power domains on SH7372. > >> > > >> > Signed-off-by: Rafael J. Wysocki > >> > --- > >> > >> Thanks for your work on this. I just tried this on my Mackerel board, > >> but I can't seem to get the pd_power_up() and pd_power_down() > >> callbacks to be executed. It is probably a misconfiguration from my > >> side. > > > > They trigger for me e.g. after doing > > > > # echo 3 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank > > > > Attached is the .config I've been using. > > Thanks, I can trigger using sysfs and your kernel configuration. Good. > However, I assumed it also would work when the sceen saver kicked in. > I recall it being fbcon that controls the screen save, perhaps > something else. So just wait a bit and see if you also can reproduce > it. The console gets black but the power is still on... I noticed that, but I think it simply means pm_runtime_put() isn't called in that code path. > Also forcing to go back to powered-on state (see below) doesn't work that well: > # echo 0 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank > > It looks like we loose the panning information somehow. Most likely a > LCDC driver bug. Unless the driver callbacks are not being invoked as > expected. That should be easy to verify, I'll do that. > Also, there is garbage in on the screen if FB_SH_MOBILE_MERAM is > enabled. The MERAM hardware is a 1.5 MiB memory block that can be used > as a LCD cache. It sits in the same hardware power domain as the > LCDCs. I don't think the MERAM software supports power down > unfortunately. Disabling MERAM support removes the garbage on the > screen. Well, I can't really comment here. > >> Here's some feedback on the sh7372-specific code: > >> > >> > --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig > >> > +++ linux-2.6/arch/arm/mach-shmobile/Kconfig > >> > @@ -19,6 +19,7 @@ config ARCH_SH7372 > >> > select CPU_V7 > >> > select SH_CLK_CPG > >> > select ARCH_WANT_OPTIONAL_GPIOLIB > >> > + select PM_GENERIC_DOMAINS > >> > >> We want to support a single ARM binary for multiple boards, > > > > Surely CONFIG_ARCH_SH7372 will be set in that binary? > > > >> so this should be enabled for all SoCs in mach-shmobile as a whole. > > > > OK, where exactly do you want me to move it? > > Ideally to ARCH_SHMOBILE in arch/arm/Kconfig. OK > >> > --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c > >> > +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c > >> > @@ -15,16 +15,97 @@ > >> > #include > >> > #include > >> > #include > >> > +#include > >> > +#include > >> > #include > >> > #include > >> > #include > >> > #include > >> > +#include > >> > > >> > #define SMFRAM 0xe6a70000 > >> > #define SYSTBCR 0xe6150024 > >> > #define SBAR 0xe6180020 > >> > #define APARMBAREA 0xe6f10020 > >> > > >> > +#define SPDCR 0xe6180008 > >> > +#define SWUCR 0xe6180014 > >> > +#define PSTR 0xe6180080 > >> > + > >> > +struct sh7372_domain_data { > >> > + unsigned int bit_shift; > >> > +}; > >> > >> Is it possible to make struct sh7372_domain_data include struct > >> generic_pm_domain? > > > > It should be possible to do that. > > > > Do I understand it correctly that you want one structure definition per > > power domain instead of the two? > > Yes, at least that's what I would do to keep the data together. I > don't care that much though, so feel free to implement it however > you'd like. OK, I think I can merge them. Thanks, Rafael