Hi Paul,
> -----Original Message-----
> From: Paul Walmsley [mailto:p...@pwsan.com]
> Sent: Monday, November 29, 2010 5:45 AM
> To: Rajendra Nayak
> Cc: linux-omap@vger.kernel.org; b-cous...@ti.com;
khil...@deeprootsystems.com
> Subject: Re: [PATCH 3/6] OMAP: powerdomain: Arch specific funcs for
state control
>
> Some comments below:
>
> On Tue, 16 Nov 2010, Rajendra Nayak wrote:
>
> > Define the following architecture specific funtions for omap2/3/4
> > .pwrdm_set_next_pwrst
> > .pwrdm_read_next_pwrst
> > .pwrdm_read_pwrst
> > .pwrdm_read_prev_pwrst
> >
> > Convert the platform-independent framework to call these functions.
> >
> > Signed-off-by: Rajendra Nayak <rna...@ti.com>
> > Cc: Paul Walmsley <p...@pwsan.com>
> > Cc: Benoit Cousson <b-cous...@ti.com>
> > Cc: Kevin Hilman <khil...@deeprootsystems.com>
> > ---
> > arch/arm/mach-omap2/Makefile | 5 +++
> > arch/arm/mach-omap2/io.c | 5 ++-
> > arch/arm/mach-omap2/powerdomains.h | 8 +++++
> > arch/arm/mach-omap2/powerdomains2xxx.c | 53
++++++++++++++++++++++++++++++++
> > arch/arm/mach-omap2/powerdomains44xx.c | 53
++++++++++++++++++++++++++++++++
> > arch/arm/plat-omap/powerdomain.c | 33 ++++++++++++++------
> > 6 files changed, 146 insertions(+), 11 deletions(-)
> > create mode 100644 arch/arm/mach-omap2/powerdomains2xxx.c
> > create mode 100644 arch/arm/mach-omap2/powerdomains44xx.c
> >
> > diff --git a/arch/arm/mach-omap2/Makefile
b/arch/arm/mach-omap2/Makefile
> > index 4bfadc5..5f843fc 100644
> > --- a/arch/arm/mach-omap2/Makefile
> > +++ b/arch/arm/mach-omap2/Makefile
> > @@ -94,6 +94,11 @@ obj-$(CONFIG_ARCH_OMAP2430) +=
omap_hwmod_2430_data.o
> > obj-$(CONFIG_ARCH_OMAP3) += omap_hwmod_3xxx_data.o
> > obj-$(CONFIG_ARCH_OMAP4) += omap_hwmod_44xx_data.o
> >
> > +#powerdomain framework
> > +obj-$(CONFIG_ARCH_OMAP2) += powerdomains2xxx.o
> > +obj-$(CONFIG_ARCH_OMAP3) += powerdomains2xxx.o
>
> We're going to need a separate file for OMAP3, because there are some
> PRCM powerdomain features that are not present on OMAP2.
>
> Please move the functions that are common between OMAP2xxx and OMAP3
into
> a separate file -- maybe something like powerdomain_2xxx_3xxx.o. Then
> powerdomain2xxx.c and powerdomain3xxx.c can just put pointers into
> pwrdm_functions in that file for the common stuff. Unfortunately, we
> won't be able to keep those common functions static at that point.
For this in the latest series that I posted, I have created just one
file powerdomain2xxx_3xxx.c which has all OMAP2xxx/3 common
funcs (named as omap2_pwrdm_*) and all OMAP3 specific funcs
as well (named as omap3_pwrdm_*).
There are different pwrdm_functions defined one for omap2
and another for omap3 (which was anyway needed to support
multiomap even if I did split them in separate files).
That way I have kept all functions static as well.
Let me know if you still feel we might need a
powerdomain2xxx.c and powerdomain3xxx.c additionally.
Regards,
Rajendra
>
> > +obj-$(CONFIG_ARCH_OMAP4) += powerdomains44xx.o
> > +
> > # EMU peripherals
> > obj-$(CONFIG_OMAP3_EMU) += emu.o
> >
> > diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> > index 76c531a..c68e989 100644
> > --- a/arch/arm/mach-omap2/io.c
> > +++ b/arch/arm/mach-omap2/io.c
> > @@ -316,7 +316,10 @@ void __init omap2_init_common_hw(struct
omap_sdrc_params *sdrc_cs0,
> > {
> > u8 skip_setup_idle = 0;
> >
> > - pwrdm_init(powerdomains_omap, NULL);
> > + if (cpu_is_omap24xx() || cpu_is_omap34xx())
> > + pwrdm_init(powerdomains_omap, &omap2_pwrdm_functions);
> > + else if (cpu_is_omap44xx())
> > + pwrdm_init(powerdomains_omap, &omap4_pwrdm_functions);
> > clkdm_init(clockdomains_omap, clkdm_autodeps);
> > if (cpu_is_omap242x())
> > omap2420_hwmod_init();
> > diff --git a/arch/arm/mach-omap2/powerdomains.h
b/arch/arm/mach-omap2/powerdomains.h
> > index 105cbca..b25b989 100644
> > --- a/arch/arm/mach-omap2/powerdomains.h
> > +++ b/arch/arm/mach-omap2/powerdomains.h
> > @@ -88,8 +88,16 @@ static struct powerdomain wkup_omap2_pwrdm = {
> > .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP24XX |
CHIP_IS_OMAP3430),
> > };
> >
> > +extern struct pwrdm_functions omap2_pwrdm_functions;
> > +#else
> > +static struct pwrdm_functions omap2_pwrdm_functions;
> > #endif
> >
> > +#ifdef CONFIG_ARCH_OMAP4
> > +extern struct pwrdm_functions omap4_pwrdm_functions;
> > +#else
> > +static struct pwrdm_functions omap4_pwrdm_functions;
> > +#endif
> >
> > /* As powerdomains are added or removed above, this list must also be
changed */
> > static struct powerdomain *powerdomains_omap[] __initdata = {
> > diff --git a/arch/arm/mach-omap2/powerdomains2xxx.c
b/arch/arm/mach-omap2/powerdomains2xxx.c
> > new file mode 100644
> > index 0000000..dfeb8af
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/powerdomains2xxx.c
> > @@ -0,0 +1,53 @@
> > +/*
> > + * OMAP2 and OMAP3 powerdomain control
> > + *
> > + * Copyright (C) 2009-2010 Texas Instruments, Inc.
> > + * Copyright (C) 2007-2009 Nokia Corporation
> > + *
> > + * Derived from mach-omap2/powerdomain.c written by Paul Walmsley
> > + * Rajendra Nayak <rna...@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <plat/powerdomain.h>
> > +#include <plat/prcm.h>
> > +#include "powerdomains.h"
> > +
> > +static int omap2_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8
pwrst)
> > +{
> > + prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> > + (pwrst << OMAP_POWERSTATE_SHIFT),
> > + pwrdm->prcm_offs, OMAP2_PM_PWSTCTRL);
> > + return 0;
> > +}
> > +
> > +static int omap2_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > + OMAP2_PM_PWSTCTRL, OMAP_POWERSTATE_MASK);
> > +}
> > +
> > +static int omap2_pwrdm_read_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > + OMAP2_PM_PWSTST, OMAP_POWERSTATEST_MASK);
> > +}
> > +
> > +static int omap2_pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs,
OMAP3430_PM_PREPWSTST,
> > + OMAP3430_LASTPOWERSTATEENTERED_MASK);
> > +}
>
> this is OMAP3 only...
>
> > +
> > +struct pwrdm_functions omap2_pwrdm_functions = {
> > + .pwrdm_set_next_pwrst = omap2_pwrdm_set_next_pwrst,
> > + .pwrdm_read_next_pwrst = omap2_pwrdm_read_next_pwrst,
> > + .pwrdm_read_pwrst = omap2_pwrdm_read_pwrst,
> > + .pwrdm_read_prev_pwrst = omap2_pwrdm_read_prev_pwrst,
> > +};
> > diff --git a/arch/arm/mach-omap2/powerdomains44xx.c
b/arch/arm/mach-omap2/powerdomains44xx.c
> > new file mode 100644
> > index 0000000..b3aaf9b
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/powerdomains44xx.c
> > @@ -0,0 +1,53 @@
> > +/*
> > + * OMAP4 powerdomain control
> > + *
> > + * Copyright (C) 2009-2010 Texas Instruments, Inc.
> > + * Copyright (C) 2007-2009 Nokia Corporation
> > + *
> > + * Derived from mach-omap2/powerdomain.c written by Paul Walmsley
> > + * Rajendra Nayak <rna...@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <plat/powerdomain.h>
> > +#include <plat/prcm.h>
> > +#include "powerdomains.h"
> > +
> > +static int omap4_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8
pwrst)
> > +{
> > + prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> > + (pwrst << OMAP_POWERSTATE_SHIFT),
> > + pwrdm->prcm_offs, OMAP4_PM_PWSTCTRL);
> > + return 0;
> > +}
> > +
> > +static int omap4_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > + OMAP4_PM_PWSTCTRL, OMAP_POWERSTATE_MASK);
> > +}
> > +
> > +static int omap4_pwrdm_read_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > + OMAP4_PM_PWSTST, OMAP_POWERSTATEST_MASK);
> > +}
> > +
> > +static int omap4_pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
> > +{
> > + return prm_read_mod_bits_shift(pwrdm->prcm_offs, OMAP4_PM_PWSTST,
> > + OMAP4430_LASTPOWERSTATEENTERED_MASK);
> > +}
> > +
> > +struct pwrdm_functions omap4_pwrdm_functions = {
> > + .pwrdm_set_next_pwrst = omap4_pwrdm_set_next_pwrst,
> > + .pwrdm_read_next_pwrst = omap4_pwrdm_read_next_pwrst,
> > + .pwrdm_read_pwrst = omap4_pwrdm_read_pwrst,
> > + .pwrdm_read_prev_pwrst = omap4_pwrdm_read_prev_pwrst,
> > +};
> > diff --git a/arch/arm/plat-omap/powerdomain.c
b/arch/arm/plat-omap/powerdomain.c
> > index 9e2d712..73d6dad 100644
> > --- a/arch/arm/plat-omap/powerdomain.c
> > +++ b/arch/arm/plat-omap/powerdomain.c
> > @@ -438,6 +438,8 @@ int pwrdm_get_mem_bank_count(struct powerdomain
*pwrdm)
> > */
> > int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
> > {
> > + int ret = -EINVAL;
> > +
> > if (!pwrdm)
> > return -EINVAL;
> >
> > @@ -447,11 +449,10 @@ int pwrdm_set_next_pwrst(struct powerdomain
*pwrdm, u8 pwrst)
> > pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
> > pwrdm->name, pwrst);
> >
> > - prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
> > - (pwrst << OMAP_POWERSTATE_SHIFT),
> > - pwrdm->prcm_offs, pwrstctrl_reg_offs);
> > + if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
> > + ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
> >
> > - return 0;
> > + return ret;
> > }
> >
> > /**
> > @@ -464,11 +465,15 @@ int pwrdm_set_next_pwrst(struct powerdomain
*pwrdm, u8 pwrst)
> > */
> > int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
> > {
> > + int ret = -EINVAL;
> > +
> > if (!pwrdm)
> > return -EINVAL;
> >
> > - return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > - pwrstctrl_reg_offs,
OMAP_POWERSTATE_MASK);
> > + if (arch_pwrdm && arch_pwrdm->pwrdm_read_next_pwrst)
> > + ret = arch_pwrdm->pwrdm_read_next_pwrst(pwrdm);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -481,11 +486,15 @@ int pwrdm_read_next_pwrst(struct powerdomain
*pwrdm)
> > */
> > int pwrdm_read_pwrst(struct powerdomain *pwrdm)
> > {
> > + int ret = -EINVAL;
> > +
> > if (!pwrdm)
> > return -EINVAL;
> >
> > - return prm_read_mod_bits_shift(pwrdm->prcm_offs,
> > - pwrstst_reg_offs,
OMAP_POWERSTATEST_MASK);
> > + if (arch_pwrdm && arch_pwrdm->pwrdm_read_pwrst)
> > + ret = arch_pwrdm->pwrdm_read_pwrst(pwrdm);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -498,11 +507,15 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
> > */
> > int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
> > {
> > + int ret = -EINVAL;
> > +
> > if (!pwrdm)
> > return -EINVAL;
> >
> > - return prm_read_mod_bits_shift(pwrdm->prcm_offs,
OMAP3430_PM_PREPWSTST,
> > -
OMAP3430_LASTPOWERSTATEENTERED_MASK);
> > + if (arch_pwrdm && arch_pwrdm->pwrdm_read_prev_pwrst)
> > + ret = arch_pwrdm->pwrdm_read_prev_pwrst(pwrdm);
> > +
> > + return ret;
> > }
> >
> > /**
> > --
> > 1.7.0.4
> >
>
>
> - Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
