> -----Original Message-----
> From: Hilman, Kevin
> Sent: Saturday, January 07, 2012 12:57 AM
> To: Hiremath, Vaibhav
> Cc: [email protected]; Paul Walmsley; Nayak, Rajendra; Tony
> Lindgren
> Subject: Re: [RFC PATCH] arm:omap:omap4: Remove hardcoded reg-offs for
> PWRSTCTRL & PWRSTST
>
> Vaibhav Hiremath <[email protected]> writes:
>
> > PRM module in AM33XX is closer to OMAP4 PRM module, so it makes complete
> > sense to reuse all the code from existing OMAP4 implementation.
> > Having said that, ther is a catch here with respect to AM33XX device,
> >
> > The register offset in PRM module is not consistent
> > across (crazy IP integration), for example,
> >
> > PRM_XXX PWRSTCTRL PWRSTST RSTCTRL RSTST
> > ===============================================
> > PRM_PER_MOD: 0x0C, 0x08, 0x00, 0x04
> > PRM_WKUP_MOD: 0x04, 0x08, 0x00, 0x0C
> > PRM_MPU_MOD: 0x00, 0x04, 0x08, NA
> > PRM_DEVICE_MOD: NA, NA, 0x00, 0x08
> >
> > So in order to reuse the existing OMAP4 code, we have to add
> > separate entry for register offsets, especially
> > PWRSTCTRL & PWRSTST.
>
> > This patch removes the existing hard-coded way of providing
> > offset to omap4_prminst_xxx API's and instead use offsets
> > provided in powerdomainsxxxx_data.
>
> Thanks, I think this is a much better approach.
>
Kevin, Thanks a lot for review ...
> However, for ease of review, I think it should still be broken up a bit
> more. First, create a patch that just changes the existing OMAP4 code
> to use the new fields without changing any functionality.
> powerdomain.h, pwerdomain44xx.c, powerdomains44xx_data.c
>
> Then, the AM33x support should be added in a separate patch.
>
Ok, point taken.
I will divide it into 3 logical patches.
> [...]
>
> > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
> omap2/omap_hwmod.c
> > index 5192cab..4fd53d4 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > @@ -1288,14 +1288,14 @@ static int _assert_hardreset(struct omap_hwmod
> *oh, const char *name)
> > if (IS_ERR_VALUE(ret))
> > return ret;
> >
> > - if (cpu_is_omap24xx() || cpu_is_omap34xx())
> > - return omap2_prm_assert_hardreset(oh->prcm.omap2.module_offs,
> > - ohri.rst_shift);
>
> Why are moving this OMAP2/3 code below?
>
> I assume it is because cpu_is_omap34xx() is true for AM33xx? If so, a
> comment is probably needed here so we don't mess with the ordering.
>
Ok, will add comment in next version.
Thanks,
Vaibhav
> > - else if (cpu_is_omap44xx())
> > + if (cpu_is_omap44xx() || cpu_is_am33xx())
> > return omap4_prminst_assert_hardreset(ohri.rst_shift,
> > oh->clkdm->pwrdm.ptr->prcm_partition,
> > oh->clkdm->pwrdm.ptr->prcm_offs,
> > oh->prcm.omap4.rstctrl_offs);
> > + else if (cpu_is_omap24xx() || cpu_is_omap34xx())
> > + return omap2_prm_assert_hardreset(oh->prcm.omap2.module_offs,
> > + ohri.rst_shift);
> > else
> > return -EINVAL;
> > }
> > @@ -1322,11 +1322,7 @@ static int _deassert_hardreset(struct omap_hwmod
> *oh, const char *name)
> > if (IS_ERR_VALUE(ret))
> > return ret;
> >
> > - if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> > - ret = omap2_prm_deassert_hardreset(oh->prcm.omap2.module_offs,
> > - ohri.rst_shift,
> > - ohri.st_shift);
> > - } else if (cpu_is_omap44xx()) {
> > + if (cpu_is_omap44xx() || cpu_is_am33xx()) {
> > if (ohri.st_shift)
> > pr_err("omap_hwmod: %s: %s: hwmod data error: OMAP4 does
> not support st_shift\n",
> > oh->name, name);
> > @@ -1334,6 +1330,10 @@ static int _deassert_hardreset(struct omap_hwmod
> *oh, const char *name)
> > oh->clkdm->pwrdm.ptr->prcm_partition,
> > oh->clkdm->pwrdm.ptr->prcm_offs,
> > oh->prcm.omap4.rstctrl_offs);
> > + } else if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
> > + ret = omap2_prm_deassert_hardreset(oh->prcm.omap2.module_offs,
> > + ohri.rst_shift,
> > + ohri.st_shift);
>
> same comment as above.
>
> Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
