linux-kernel - Re: [PATCH 10/10 v6] ARM / shmobile: Support for I/O power domains for SH7372 (v8)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTi=UfhzhhQ1EcVNJPnoH_KQdXF5Ckg@mail.gmail.com>
Date:	Tue, 28 Jun 2011 08:21:20 +0900
From:	Magnus Damm <magnus.damm@...il.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Linux PM mailing list <linux-pm@...ts.linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Paul Walmsley <paul@...an.com>, Kevin Hilman <khilman@...com>,
	Alan Stern <stern@...land.harvard.edu>,
	LKML <linux-kernel@...r.kernel.org>, linux-sh@...r.kernel.org,
	Paul Mundt <lethal@...ux-sh.org>
Subject: Re: [PATCH 10/10 v6] ARM / shmobile: Support for I/O power domains
 for SH7372 (v8)

On Tue, Jun 28, 2011 at 4:25 AM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> On Monday, June 27, 2011, Magnus Damm wrote:
>> On Sun, Jun 26, 2011 at 6:31 AM, Rafael J. Wysocki <rjw@...k.pl> wrote:
>> > From: Rafael J. Wysocki <rjw@...k.pl>
>> >
>> > 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 <rjw@...k.pl>
>> > Acked-by: Paul Mundt <lethal@...ux-sh.org>
>> > ---
>>
>> Hi Rafael,
>>
>> Thanks for your work on this. I've been working on up-porting my A3RV
>> prototype, and I came across these minor details:
>>
>> > --- linux-2.6.orig/arch/arm/mach-shmobile/board-mackerel.c
>> > +++ linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
>> > @@ -1582,6 +1582,10 @@ static void __init mackerel_init(void)
>> >
>> >        platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
>> >
>> > +       sh7372_init_pm_domain(SH7372_A4LC);
>> > +       sh7372_add_device_to_domain(SH7372_A4LC, &lcdc_device);
>> > +       sh7372_add_device_to_domain(SH7372_A4LC, &hdmi_lcdc_device);
>> > +
>> >        hdmi_init_pm_clock();
>> >        sh7372_pm_init();
>> >  }
>> > Index: linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
>> > ===================================================================
>> > --- linux-2.6.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
>> > +++ linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
>> > @@ -12,6 +12,7 @@
>> >  #define __ASM_SH7372_H__
>> >
>> >  #include <linux/sh_clk.h>
>> > +#include <linux/pm_domain.h>
>> >
>> >  /*
>> >  * Pin Function Controller:
>> > @@ -470,4 +471,31 @@ extern struct clk sh7372_fsibck_clk;
>> >  extern struct clk sh7372_fsidiva_clk;
>> >  extern struct clk sh7372_fsidivb_clk;
>> >
>> > +struct platform_device;
>> > +
>> > +struct sh7372_pm_domain {
>> > +       struct generic_pm_domain genpd;
>> > +       unsigned int bit_shift;
>> > +};
>> > +
>> > +static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
>> > +{
>> > +       return container_of(d, struct sh7372_pm_domain, genpd);
>> > +}
>> > +
>> > +#ifdef CONFIG_PM
>> > +extern struct sh7372_pm_domain sh7372_a4lc_domain;
>> > +#define SH7372_A4LC    (&sh7372_a4lc_domain)
>> > +
>> > +extern void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd);
>> > +extern void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
>> > +                                       struct platform_device *pdev);
>> > +#else
>> > +#define SH7372_A4LC    NULL
>> > +
>> > +static inline void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd) {}
>> > +static inline void sh7372_add_device_to_domain(struct sh7372_pm_domain *pd,
>> > +                                              struct platform_device *pdev) {}
>> > +#endif /* CONFIG_PM */
>> > +
>> >  #endif /* __ASM_SH7372_H__ */
>>
>> Wouldn't it be easier to simply get rid of SH7372_A4LC?
>
> Not really, because the code won't build for both CONFIG_PM_RUNTIME and
> CONFIG_SUSPEND unset (resulting in CONFIG_PM unset).
>
>> Perhaps you have some special intention behind your #define, but for me the
>> following change is working just fine:
>
> Well, if CONFIG_PM is unset, sh7372_a4lc is not defined.

True, but in the CONFIG_PM=n case sh7372_a4lc is never used by
sh7372_init_pm_domain() or sh7372_add_device_to_domain().

How about letting the preprocessor do the work for us instead? This
certainly builds without SH7372_A4LC in case of CONFIG_PM=n:

#else
#define sh7372_init_pm_domain(pd) do { } while(0)
#define sh7372_add_device_to_domain(pd, pdev) do { } while(0)
#endif /* CONFIG_PM */

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/