From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Fri, 24 Jan 2014 10:47:04 -0700
Subject: [U-Boot] [PATCH V2 08/13] ARM: tegra: add SPL/AVP (arm720t) CPU
 files for Tegra124
In-Reply-To: <20140124154453.GH25720@ulmo.nvidia.com>
References: <1390524180-15008-1-git-send-email-swarren@wwwdotorg.org>
	<1390524180-15008-8-git-send-email-swarren@wwwdotorg.org>
	<20140124154453.GH25720@ulmo.nvidia.com>
Message-ID: <52E2A718.1030002@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.