Hi Paul,
So why should getting rid of some *unused* data have any testing
overhead?
Your definition of 'unused' seems to be different than mine:
$ egrep -rn 'c(lk|)->clkdm' arch/arm/
arch/arm/mach-omap2/omap_hwmod.c:560: if (oh->_clk->clkdm&& oh->_clk->clkdm->flags& CLKDM_NO_AUTODEPS)
arch/arm/mach-omap2/omap_hwmod.c:563: return clkdm_add_sleepdep(oh->_clk->clkdm, init_oh->_clk->clkdm);
arch/arm/mach-omap2/omap_hwmod.c:584: if (oh->_clk->clkdm&& oh->_clk->clkdm->flags& CLKDM_NO_AUTODEPS)
arch/arm/mach-omap2/omap_hwmod.c:587: return clkdm_del_sleepdep(oh->_clk->clkdm, init_oh->_clk->clkdm);
I did have a look at these (which are part of _add/_del_initiator_dep()
functions) while I was working on $SUBJECT patch, and was not very sure
of what these functions are expected to do.
They check for a CLKDM_NO_AUTODEPS flag which is not defined across any
clockdomain data files across any OMAP2+ platform.
What it then means is they add a sleep/static dependency for the
modules/hwmods clkdm with mpu on *all* platforms.
The AUTODEPS on the other hand are needed only on OMAP3 I guess, and
AUTODEPS need a sleep/wakeup (Not just a sleep dependency that
the above functions add) dependency not just with MPU but also with
DSP, besides AUTODEPS are already handled very well in the clockdomain
framework for OMAP3.
arch/arm/mach-omap2/omap_hwmod.c:612: if (!oh->_clk->clkdm)
arch/arm/mach-omap2/omap_hwmod.c:2998: if (!c->clkdm)
arch/arm/mach-omap2/omap_hwmod.c:3001: return c->clkdm->pwrdm.ptr;
I have fixed some of these dereferencing in hwmod to derive a clkdm/
pwrdm for a given hwmod by giving a precedence to a clkdm directly
associated with the hwmod and if missing only then looking for
something through the clk route. What I did miss is to update the
OMAP2/3 hwmod data file for some of the clks from where I was removing
the clkdm assocaitions (There are about ~10 clocks from $SUBJECT patch
which figure in hwmod data files out of the 75 from which I get rid of
the clkdms pointers)
arch/arm/mach-omap2/clock.c:96: if (!clk->clkdm_name)
arch/arm/mach-omap2/clock.c:99: clkdm = clkdm_lookup(clk->clkdm_name);
arch/arm/mach-omap2/clock.c:102: clk->name, clk->clkdm_name);
arch/arm/mach-omap2/clock.c:103: clk->clkdm = clkdm;
arch/arm/mach-omap2/clock.c:106: "clkdm %s\n", clk->name, clk->clkdm_name);
These are part of the init code to resolve clkdm_name into a clkdm
pointer.
arch/arm/mach-omap2/clock.c:292: if (clkdm_control&& clk->clkdm)
arch/arm/mach-omap2/clock.c:293: clkdm_clk_disable(clk->clkdm, clk);
arch/arm/mach-omap2/clock.c:332: if (clkdm_control&& clk->clkdm) {
arch/arm/mach-omap2/clock.c:333: ret = clkdm_clk_enable(clk->clkdm, clk);
arch/arm/mach-omap2/clock.c:336: "%d\n", clk->name, clk->clkdm->name, ret);
arch/arm/mach-omap2/clock.c:354: if (clkdm_control&& clk->clkdm)
arch/arm/mach-omap2/clock.c:355: clkdm_clk_disable(clk->clkdm, clk);
These are only applicable for gate clocks.
arch/arm/mach-omap2/clock.c:441: if (clk->clkdm != NULL)
arch/arm/mach-omap2/clock.c:442: pwrdm_state_switch(clk->clkdm->pwrdm.ptr);
This again part of disable unused clocks should also matter only for
gate clocks.
That is just the result of a casual grep, not even a code analysis.
Removing this data is not like removing some macros where one can get a
compiler error if they turn out to be needed. Problems with this ares
only likely to show up at runtime.
By the way, I hope you're testing the patches that you send to the lists,
at the very least to the minimal PM testing that I do that is documented
e.g. here:
http://www.pwsan.com/omap/bootlogs/20120508/prcm_devel_a_3.5__0135f6a04642c192bdf4b36e06937d3387e174ff/
yes, I am, atleast with the platforms that I have access to,
2430sdp/N800/3430sdp/3630beagle-xm/4430panda/4460panda.
I don't have any 35xxevm/3517evm/5912osk or cmt3517 platforms.
regards,
Rajendra
Otherwise the testing burden is just getting pushed to other people who
already have too much to do.
...
So to repeat myself on this:
1. The patch that removes some of the struct clk clkdm_name strings should
be part of or otherwise grouped with the CCF conversion patch series
2. The CCF conversion patch series needs to be fully PM-tested
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
