| Message ID | 1408346196-30419-12-git-send-email-thierry.reding@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
On 08/18/2014 01:16 AM, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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 <swarren@nvidia.com>
On Wed, Aug 20, 2014 at 12:24:11PM -0600, Stephen Warren wrote: > On 08/18/2014 01:16 AM, Thierry Reding wrote: > >From: Thierry Reding <treding@nvidia.com> > > > >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? It seems like the PWRGATE_STATUS register can be polled to determine when the operation is finished. I'll implement a loop with a small timeout here. We probably need to do the same in the kernel. Apparently for newer SoCs (starting with Tegra114) the meaning of the PWRGATE_TOGGLE_START bit has also changed. It should be polled until cleared before starting a new operation and once an operation has been started (PWRGATE_TOGGLE_START set) the bit should be polled until it is cleared again to check that the request was accepted by the PMC. > >+ 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? As far as I can tell this was carried over for register compatibility. The bits for the PCIE and VDEC partitions are still reversed in Tegra124. > >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? The list of valid partitions varies per chip, but the list of values remains compatible. For the same reason we have a list of partitions in the Linux kernel, but they are checked for validity against the per-SoC list. Not sure if we need to go that far with U-Boot. Thierry
diff --git a/arch/arm/cpu/tegra-common/Makefile b/arch/arm/cpu/tegra-common/Makefile index 892556e64451..fed618ba8fc9 100644 --- a/arch/arm/cpu/tegra-common/Makefile +++ b/arch/arm/cpu/tegra-common/Makefile @@ -13,4 +13,5 @@ obj-y += cache.o obj-y += clock.o obj-y += lowlevel_init.o obj-y += pinmux-common.o +obj-y += powergate.o obj-$(CONFIG_DISPLAY_CPUINFO) += sys_info.o diff --git a/arch/arm/cpu/tegra-common/powergate.c b/arch/arm/cpu/tegra-common/powergate.c new file mode 100644 index 000000000000..2097bbcde365 --- /dev/null +++ b/arch/arm/cpu/tegra-common/powergate.c @@ -0,0 +1,80 @@ +#include <common.h> + +#include <asm/io.h> +#include <asm/types.h> + +#include <asm/arch/powergate.h> +#include <asm/arch/tegra.h> + +#define PWRGATE_TOGGLE 0x30 +#define PWRGATE_TOGGLE_START (1 << 8) + +#define PWRGATE_STATUS 0x38 + +static int tegra_powergate_set(enum tegra_powergate id, bool state) +{ + unsigned long value; + bool old_state; + + value = readl(NV_PA_PMC_BASE + PWRGATE_STATUS); + old_state = value & (1 << id); + + if (state == old_state) + return 0; + + writel(PWRGATE_TOGGLE_START | id, NV_PA_PMC_BASE + PWRGATE_TOGGLE); + + return 0; +} + +static int tegra_powergate_power_on(enum tegra_powergate id) +{ + return tegra_powergate_set(id, true); +} + +int tegra_powergate_power_off(enum tegra_powergate id) +{ + return tegra_powergate_set(id, false); +} + +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; + + writel(value, NV_PA_PMC_BASE + 0x34); + + return 0; +} + +int tegra_powergate_sequence_power_up(enum tegra_powergate id, + enum periph_id periph) +{ + int err; + + reset_set_enable(periph, 1); + + err = tegra_powergate_power_on(id); + if (err < 0) + return err; + + clock_enable(periph); + + udelay(10); + + err = tegra_powergate_remove_clamping(id); + if (err < 0) + return err; + + udelay(10); + + reset_set_enable(periph, 0); + + return 0; +} diff --git a/arch/arm/include/asm/arch-tegra/powergate.h b/arch/arm/include/asm/arch-tegra/powergate.h new file mode 100644 index 000000000000..53bed37d42c1 --- /dev/null +++ b/arch/arm/include/asm/arch-tegra/powergate.h @@ -0,0 +1,27 @@ +#ifndef _TEGRA_POWERGATE_H_ +#define _TEGRA_POWERGATE_H_ + +#include <asm/arch/clock.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, +}; + +int tegra_powergate_sequence_power_up(enum tegra_powergate id, + enum periph_id periph); +int tegra_powergate_power_off(enum tegra_powergate id); + +#endif diff --git a/arch/arm/include/asm/arch-tegra114/powergate.h b/arch/arm/include/asm/arch-tegra114/powergate.h new file mode 100644 index 000000000000..260ea801b81b --- /dev/null +++ b/arch/arm/include/asm/arch-tegra114/powergate.h @@ -0,0 +1,6 @@ +#ifndef _TEGRA114_POWERGATE_H_ +#define _TEGRA114_POWERGATE_H_ + +#include <asm/arch-tegra/powergate.h> + +#endif /* _TEGRA114_POWERGATE_H_ */ diff --git a/arch/arm/include/asm/arch-tegra124/powergate.h b/arch/arm/include/asm/arch-tegra124/powergate.h new file mode 100644 index 000000000000..8a0cfbaf9665 --- /dev/null +++ b/arch/arm/include/asm/arch-tegra124/powergate.h @@ -0,0 +1,6 @@ +#ifndef _TEGRA124_POWERGATE_H_ +#define _TEGRA124_POWERGATE_H_ + +#include <asm/arch-tegra/powergate.h> + +#endif /* _TEGRA124_POWERGATE_H_ */ diff --git a/arch/arm/include/asm/arch-tegra20/powergate.h b/arch/arm/include/asm/arch-tegra20/powergate.h new file mode 100644 index 000000000000..439d88b7022a --- /dev/null +++ b/arch/arm/include/asm/arch-tegra20/powergate.h @@ -0,0 +1,6 @@ +#ifndef _TEGRA20_POWERGATE_H_ +#define _TEGRA20_POWERGATE_H_ + +#include <asm/arch-tegra/powergate.h> + +#endif /* _TEGRA20_POWERGATE_H_ */ diff --git a/arch/arm/include/asm/arch-tegra30/powergate.h b/arch/arm/include/asm/arch-tegra30/powergate.h new file mode 100644 index 000000000000..c70e44b6216d --- /dev/null +++ b/arch/arm/include/asm/arch-tegra30/powergate.h @@ -0,0 +1,6 @@ +#ifndef _TEGRA30_POWERGATE_H_ +#define _TEGRA30_POWERGATE_H_ + +#include <asm/arch-tegra/powergate.h> + +#endif /* _TEGRA30_POWERGATE_H_ */