Re: [PATCH] ARM: tegra: cardhu: add dt entry for PMIC TPS65911.

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 07/04/2012 09:07 AM, Laxman Dewangan wrote:
> Tegra30 based platform "cardhu" have the power management
> IC TPS65911 for the regulator.
> Adding DT entry for this device.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I recall there were some differences between Cardhu A02 and A04. Does
this patch apply equally to both?

Please have a look at the recent threads re: various Tegra20 boards'
regulator .dts files, and see the results at:

git://nv-tegra.nvidia.com/user/swarren/linux-2.6 linux-next_common

Most of these comments are driven by comments made in the review of
those patches.

In the patch description, can you please specify where you got all the
values from, and any discrepancies.

> diff --git a/arch/arm/boot/dts/tegra30-cardhu.dts b/arch/arm/boot/dts/tegra30-cardhu.dts

> +				vdd1_reg: regulator@0 {
> +					reg = <0>;
> +					regulator-compatible = "vdd1";
> +					regulator-name = "vdd_1v2_gen";

It'd be good to list all the signals that these regulators drive
directly; the schematics "rename" the signals quite a lot.

> +					regulator-min-microvolt = < 600000>;
> +					regulator-max-microvolt = <1500000>;

Similarly, the contraints should list the exact voltage that's required
of the regulators, not a range. Right now, there is no DVFS in the
mainline kernel, so even for rails where DVFS could be used in the
future, we should specify the single voltage we want these rails to run
at without DVFS for now.

> +					regulator-always-on;
> +					regulator-boot-on;

These properties aren't in the same order in all the nodes. It'd be nice
to have them ordered the same way everywhere.

I'm not sure if it really makes sense to specify regulator-boot-on,
since there's no way to know what SW has run before the kernel which
might have turned off the regulator; regulator-always-on seems to cover
all necessary use-cases.