From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Wed, 20 Aug 2014 12:24:11 -0600 Subject: [U-Boot] [PATCH 11/23] ARM: tegra: Implement powergate support In-Reply-To: <1408346196-30419-12-git-send-email-thierry.reding@gmail.com> References: <1408346196-30419-1-git-send-email-thierry.reding@gmail.com> <1408346196-30419-12-git-send-email-thierry.reding@gmail.com> Message-ID: <53F4E7CB.6020707@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/18/2014 01:16 AM, Thierry Reding wrote: > From: Thierry Reding > > Implement the powergate API that allows various power partitions to be > power up and down. > diff --git a/arch/arm/cpu/tegra-common/powergate.c b/arch/arm/cpu/tegra-common/powergate.c > +static int tegra_powergate_set(enum tegra_powergate id, bool state) > + writel(PWRGATE_TOGGLE_START | id, NV_PA_PMC_BASE + PWRGATE_TOGGLE); Since the power-down/up is an asynchronous operation, don't you need to wait for it to complete here? > + return 0; > +} > +static int tegra_powergate_remove_clamping(enum tegra_powergate id) > +{ > + unsigned long value; > + > + if (id == TEGRA_POWERGATE_VDEC) > + value = 1 << TEGRA_POWERGATE_PCIE; > + else if (id == TEGRA_POWERGATE_PCIE) > + value = 1 << TEGRA_POWERGATE_VDEC; > + else > + value = 1 << id; A comment indicating why there's a special case here would be useful. Isn't the special-case (HW design bug) restricted to Tegra20, or did it carry over into later chips in order to maintain HW register compatibility? > diff --git a/arch/arm/include/asm/arch-tegra/powergate.h b/arch/arm/include/asm/arch-tegra/powergate.h > +enum tegra_powergate { > + TEGRA_POWERGATE_CPU, > + TEGRA_POWERGATE_3D, > + TEGRA_POWERGATE_VENC, > + TEGRA_POWERGATE_PCIE, > + TEGRA_POWERGATE_VDEC, > + TEGRA_POWERGATE_L2, > + TEGRA_POWERGATE_MPE, > + TEGRA_POWERGATE_HEG, > + TEGRA_POWERGATE_SATA, > + TEGRA_POWERGATE_CPU1, > + TEGRA_POWERGATE_CPU2, > + TEGRA_POWERGATE_CPU3, > + TEGRA_POWERGATE_CELP, > + TEGRA_POWERGATE_3D1, > +}; I thought the list of partitions varied a bit by chip? Overall the code structure looks good, and on the condition I'm wrong about the variations between chips, Acked-by: Stephen Warren