On 01/24/2014 08:44 AM, Thierry Reding wrote:
On Thu, Jan 23, 2014 at 05:42:55PM -0700, Stephen Warren wrote:
+static bool is_partition_powered(u32 mask) +{
- struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
- u32 reg;
- /* Get power gate status */
- reg = readl(&pmc->pmc_pwrgate_status);
- return (reg & mask) == mask;
+}
Why can't we pass in the partition ID? That way we don't even have to define the masks in the header file. It's pretty redundant.
+static void power_partition(u32 status, u32 partid) +{
- struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
- debug("%s: status = %08X, part ID = %08X\n", __func__, status, partid);
- /* Is the partition already on? */
- if (!is_partition_powered(status)) {
/* No, toggle the partition power state (OFF -> ON) */debug("power_partition, toggling state\n");writel(START_CP | partid, &pmc->pmc_pwrgate_toggle);/* Wait for the power to come up */while (!is_partition_powered(status));/* Give I/O signals time to stabilize */udelay(IO_STABILIZATION_DELAY);- }
+}
This is being called as follows:
power_partition(CRAIL, CRAILID); power_partition(C0NC, C0NCID); power_partition(CE0, CE0ID);
So instead of passing in (1 << CRAILID, CRAILID), why not just pass around the partition ID only and compute the status mask as needed?
Now that I mention it, I do have a vague recollection that I said the exact same thing during my initial review of Tom's patches.
Oh, I see what you mean now. I hadn't realized that "CRAIL == 1 << CRAILID", so hadn't understood what you meant before. I'll certainly fix that up.