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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
