Re: [PATCH v2 1/2] ARM64: dts: meson-gxbb: add MMC support

Hi Rob,

Rob Herring [off-list ref] writes:

On Wed, Aug 03, 2016 at 04:18:42PM -0700, Kevin Hilman wrote:

quoted

Add binding and basic support for the SD/eMMC controller on Amlogic
S905/GXBB devices.

Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

[...]

quoted

+	mmc_iv: gpio-regulator {
+		compatible = "regulator-gpio";
+
+		regulator-name = "mmc-gpio-supply";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&gpio_ao GPIOAO_3 GPIO_ACTIVE_HIGH>;
+		gpio-states = <0 1>;
+
+		/*
+		 * Based on ODROID-C2 schematics:
+		 * signal name: IO_TF_3V3N_1V8, GPIOAO bit 3
+		 */
+		states = <3300000 0
+			  1800000 1>;

There are multiple things wrong in this node based on the regulator-gpio 
documentation.

I see I got the property name wrong, and the number of elements wrong,
and this fixes those issues:

-               gpio-states = <0 1>;
+               gpios-states = <0>;

By "multiple", did you mean more than those 2?  If so, I'm not seeing
what else is wrong.

[...]

quoted

+	mmc_iv: gpio-regulator {
+		compatible = "regulator-gpio";
+
+		regulator-name = "mmc-gpio-supply";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
+		gpio-states = <0 1>;
+
+		/* Based on P200 schematics, signal CARD_1.8V/3.3V_CTR */
+		states = <1800000 0
+			  3300000 1>;
+	};
+
+	vddio_boot: regulator {

[...]

quoted

+	vcc_3v3: regulator {

This won't even compile as you have the same node name twice.

Oops, it's wrong indeed, but in fact, it does compile, otherwise I
would've caught that..  Even if I remove the labels, it still compiles.
Interesting.

Anyways, I'll make those unique node-names in the next spin.

Thanks for the review,

Kevin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html