diff mbox series

ARM: dts: imx6ul-evk: Fix peripheral regulator

Message ID ccd182469476d43739ebdb4eedb2497bf72146ea.1576067880.git.leonard.crestez@nxp.com (mailing list archive)
State Superseded
Headers show
Series ARM: dts: imx6ul-evk: Fix peripheral regulator | expand

Commit Message

Leonard Crestez Dec. 11, 2019, 12:41 p.m. UTC
Many peripherals are affected by gpio5/2, not just sensors. One of those
is ethernet phy so network boot is current broken.

Fix by renaming reg_sensors and marking it as "always on". Also add a
comment asking for careful testing if this is to be made dynamic in the
future.

The "peri-3v3" naming mirrors imx6sx and schematics.

Fixes: 09e2b1048954 ("ARM: dts: imx6ul-14x14-evk: Add sensors' GPIO regulator")
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Marco Felsch Dec. 11, 2019, 1:03 p.m. UTC | #1
Hi Leonard,

On 19-12-11 14:41, Leonard Crestez wrote:
> Many peripherals are affected by gpio5/2, not just sensors. One of those
> is ethernet phy so network boot is current broken.
> 
> Fix by renaming reg_sensors and marking it as "always on". Also add a
> comment asking for careful testing if this is to be made dynamic in the
> future.
> 
> The "peri-3v3" naming mirrors imx6sx and schematics.

Nitpick: You named it "peri_3v3".

> Fixes: 09e2b1048954 ("ARM: dts: imx6ul-14x14-evk: Add sensors' GPIO regulator")
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
> index e5dafb49ef12..96853f42c050 100644
> --- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
> +++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
> @@ -28,18 +28,23 @@
>  		regulator-max-microvolt = <3300000>;
>  		gpio = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>  		enable-active-high;
>  	};
>  
> -	reg_sensors: regulator-sensors {
> +	reg_peri_3v3: regulator-peri-3v3 {
>  		compatible = "regulator-fixed";
>  		pinctrl-names = "default";
> -		pinctrl-0 = <&pinctrl_sensors_reg>;
> -		regulator-name = "sensors-supply";
> +		pinctrl-0 = <&pinctrl_peri_3v3>;
> +		regulator-name = "peri_3v3";
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
>  		gpio = <&gpio5 2 GPIO_ACTIVE_LOW>;
> +		/*
> +		 * If you want to want to make this dynamic please
> +		 * check schematics and test all affected peripherals
> +		 */

Should we list the peripherals here so it is easire to find them?

Regards,
  Marco 

> +		regulator-always-on;
>  	};
>  
>  	reg_can_3v3: regulator-can-3v3 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "can-3v3";
> @@ -157,17 +162,19 @@
>  		ethphy0: ethernet-phy@2 {
>  			reg = <2>;
>  			micrel,led-mode = <1>;
>  			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>  			clock-names = "rmii-ref";
> +			supply = <&reg_peri_3v3>;
>  		};
>  
>  		ethphy1: ethernet-phy@1 {
>  			reg = <1>;
>  			micrel,led-mode = <1>;
>  			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
>  			clock-names = "rmii-ref";
> +			supply = <&reg_peri_3v3>;
>  		};
>  	};
>  };
>  
>  &can1 {
> @@ -191,12 +198,12 @@
>  	status = "okay";
>  
>  	magnetometer@e {
>  		compatible = "fsl,mag3110";
>  		reg = <0x0e>;
> -		vdd-supply = <&reg_sensors>;
> -		vddio-supply = <&reg_sensors>;
> +		vdd-supply = <&reg_peri_3v3>;
> +		vddio-supply = <&reg_peri_3v3>;
>  	};
>  };
>  
>  &lcdif {
>  	assigned-clocks = <&clks IMX6UL_CLK_LCDIF_PRE_SEL>;
> @@ -460,11 +467,11 @@
>  			MX6UL_PAD_JTAG_TMS__SAI2_MCLK		0x17088
>  			MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04	0x17059
>  		>;
>  	};
>  
> -	pinctrl_sensors_reg: sensorsreggrp {
> +	pinctrl_peri_3v3: peri3v3grp {
>  		fsl,pins = <
>  			MX6UL_PAD_SNVS_TAMPER2__GPIO5_IO02	0x1b0b0
>  		>;
>  	};
>  
> -- 
> 2.17.1
> 
>
Leonard Crestez Dec. 11, 2019, 2:05 p.m. UTC | #2
On 11.12.2019 15:03, Marco Felsch wrote:
> Hi Leonard,
> 
> On 19-12-11 14:41, Leonard Crestez wrote:
>> Many peripherals are affected by gpio5/2, not just sensors. One of those
>> is ethernet phy so network boot is current broken.
>>
>> Fix by renaming reg_sensors and marking it as "always on". Also add a
>> comment asking for careful testing if this is to be made dynamic in the
>> future.
>>
>> The "peri-3v3" naming mirrors imx6sx and schematics.
> 
> Nitpick: You named it "peri_3v3".

Yes. Is there any recommended naming convention for regulator-name? 
Currently there's a chaotic mix of upper/lower case and dashes/underscores.

>> Fixes: 09e2b1048954 ("ARM: dts: imx6ul-14x14-evk: Add sensors' GPIO regulator")
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> ---
>>   arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 19 +++++++++++++------
>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
>> index e5dafb49ef12..96853f42c050 100644
>> --- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
>> +++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
>> @@ -28,18 +28,23 @@
>>   		regulator-max-microvolt = <3300000>;
>>   		gpio = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>   		enable-active-high;
>>   	};
>>   
>> -	reg_sensors: regulator-sensors {
>> +	reg_peri_3v3: regulator-peri-3v3 {
>>   		compatible = "regulator-fixed";
>>   		pinctrl-names = "default";
>> -		pinctrl-0 = <&pinctrl_sensors_reg>;
>> -		regulator-name = "sensors-supply";
>> +		pinctrl-0 = <&pinctrl_peri_3v3>;
>> +		regulator-name = "peri_3v3";
>>   		regulator-min-microvolt = <3300000>;
>>   		regulator-max-microvolt = <3300000>;
>>   		gpio = <&gpio5 2 GPIO_ACTIVE_LOW>;
>> +		/*
>> +		 * If you want to want to make this dynamic please
>> +		 * check schematics and test all affected peripherals
>> +		 */
> 
> Should we list the peripherals here so it is easire to find them?

My expectation is that people who adjust dts regulators will search 
through schematics pdf, the line is called "VPERI_3V3".

--
Regards,
Leonard
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
index e5dafb49ef12..96853f42c050 100644
--- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
+++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
@@ -28,18 +28,23 @@ 
 		regulator-max-microvolt = <3300000>;
 		gpio = <&gpio1 9 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 	};
 
-	reg_sensors: regulator-sensors {
+	reg_peri_3v3: regulator-peri-3v3 {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_sensors_reg>;
-		regulator-name = "sensors-supply";
+		pinctrl-0 = <&pinctrl_peri_3v3>;
+		regulator-name = "peri_3v3";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		gpio = <&gpio5 2 GPIO_ACTIVE_LOW>;
+		/*
+		 * If you want to want to make this dynamic please
+		 * check schematics and test all affected peripherals
+		 */
+		regulator-always-on;
 	};
 
 	reg_can_3v3: regulator-can-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "can-3v3";
@@ -157,17 +162,19 @@ 
 		ethphy0: ethernet-phy@2 {
 			reg = <2>;
 			micrel,led-mode = <1>;
 			clocks = <&clks IMX6UL_CLK_ENET_REF>;
 			clock-names = "rmii-ref";
+			supply = <&reg_peri_3v3>;
 		};
 
 		ethphy1: ethernet-phy@1 {
 			reg = <1>;
 			micrel,led-mode = <1>;
 			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
 			clock-names = "rmii-ref";
+			supply = <&reg_peri_3v3>;
 		};
 	};
 };
 
 &can1 {
@@ -191,12 +198,12 @@ 
 	status = "okay";
 
 	magnetometer@e {
 		compatible = "fsl,mag3110";
 		reg = <0x0e>;
-		vdd-supply = <&reg_sensors>;
-		vddio-supply = <&reg_sensors>;
+		vdd-supply = <&reg_peri_3v3>;
+		vddio-supply = <&reg_peri_3v3>;
 	};
 };
 
 &lcdif {
 	assigned-clocks = <&clks IMX6UL_CLK_LCDIF_PRE_SEL>;
@@ -460,11 +467,11 @@ 
 			MX6UL_PAD_JTAG_TMS__SAI2_MCLK		0x17088
 			MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04	0x17059
 		>;
 	};
 
-	pinctrl_sensors_reg: sensorsreggrp {
+	pinctrl_peri_3v3: peri3v3grp {
 		fsl,pins = <
 			MX6UL_PAD_SNVS_TAMPER2__GPIO5_IO02	0x1b0b0
 		>;
 	};