linux-kernel - Re: [PATCH v4 2/5] MIPS: DTS: CI20: fix PMU definitions for ACT8600

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1582992575.3.2@crapouillou.net>
Date:   Sat, 29 Feb 2020 13:09:35 -0300
From:   Paul Cercueil <paul@...pouillou.net>
To:     "H. Nikolaus Schaller" <hns@...delico.com>
Cc:     Paul Boddie <paul@...die.org.uk>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paulburton@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kees Cook <keescook@...omium.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        devicetree@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-kernel@...r.kernel.org, letux-kernel@...nphoenux.org,
        kernel@...a-handheld.com, stable@...r.kernel.org
Subject: Re: [PATCH v4 2/5] MIPS: DTS: CI20: fix PMU definitions for ACT8600

Hi Nikolaus,


Le ven., févr. 28, 2020 at 19:02, H. Nikolaus Schaller 
<hns@...delico.com> a écrit :
> There is a ACT8600 on the CI20 board and the bindings of the
> ACT8865 driver have changed without updating the CI20 device
> tree. Therefore the PMU can not be probed successfully and
> is running in power-on reset state.
> 
> Fix DT to match the latest act8865-regulator bindings.
> 
> Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
> Cc: stable@...r.kernel.org
> Signed-off-by: H. Nikolaus Schaller <hns@...delico.com>
> ---
>  arch/mips/boot/dts/ingenic/ci20.dts | 48 
> ++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts 
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 59c104289ece..44741e927d2b 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -4,6 +4,8 @@
>  #include "jz4780.dtsi"
>  #include <dt-bindings/clock/ingenic,tcu.h>
>  #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>

This include should be in patch 3/5 where it's first used.

With that fixed:
Reviewed-by: Paul Cercueil <paul@...pouillou.net>

for the whole series.

Cheers,
-Paul

> +#include <dt-bindings/regulator/active-semi,8865-regulator.h>
> 
>  / {
>  	compatible = "img,ci20", "ingenic,jz4780";
> @@ -166,65 +168,81 @@
>  		reg = <0x5a>;
>  		status = "okay";
> 
> +/*
> +Optional input supply properties:
> +- for act8600:
> +  - vp1-supply: The input supply for DCDC_REG1
> +  - vp2-supply: The input supply for DCDC_REG2
> +  - vp3-supply: The input supply for DCDC_REG3
> +  - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7 
> and LDO_REG8
> +  SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
> +*/
> +
>  		regulators {
>  			vddcore: SUDCDC1 {
> -				regulator-name = "VDDCORE";
> +				regulator-name = "DCDC_REG1";
>  				regulator-min-microvolt = <1100000>;
>  				regulator-max-microvolt = <1100000>;
>  				regulator-always-on;
>  			};
>  			vddmem: SUDCDC2 {
> -				regulator-name = "VDDMEM";
> +				regulator-name = "DCDC_REG2";
>  				regulator-min-microvolt = <1500000>;
>  				regulator-max-microvolt = <1500000>;
>  				regulator-always-on;
>  			};
>  			vcc_33: SUDCDC3 {
> -				regulator-name = "VCC33";
> +				regulator-name = "DCDC_REG3";
>  				regulator-min-microvolt = <3300000>;
>  				regulator-max-microvolt = <3300000>;
>  				regulator-always-on;
>  			};
>  			vcc_50: SUDCDC4 {
> -				regulator-name = "VCC50";
> +				regulator-name = "SUDCDC_REG4";
>  				regulator-min-microvolt = <5000000>;
>  				regulator-max-microvolt = <5000000>;
>  				regulator-always-on;
>  			};
>  			vcc_25: LDO_REG5 {
> -				regulator-name = "VCC25";
> +				regulator-name = "LDO_REG5";
>  				regulator-min-microvolt = <2500000>;
>  				regulator-max-microvolt = <2500000>;
>  				regulator-always-on;
>  			};
>  			wifi_io: LDO_REG6 {
> -				regulator-name = "WIFIIO";
> +				regulator-name = "LDO_REG6";
>  				regulator-min-microvolt = <2500000>;
>  				regulator-max-microvolt = <2500000>;
>  				regulator-always-on;
>  			};
>  			vcc_28: LDO_REG7 {
> -				regulator-name = "VCC28";
> +				regulator-name = "LDO_REG7";
>  				regulator-min-microvolt = <2800000>;
>  				regulator-max-microvolt = <2800000>;
>  				regulator-always-on;
>  			};
>  			vcc_15: LDO_REG8 {
> -				regulator-name = "VCC15";
> +				regulator-name = "LDO_REG8";
>  				regulator-min-microvolt = <1500000>;
>  				regulator-max-microvolt = <1500000>;
>  				regulator-always-on;
>  			};
> -			vcc_18: LDO_REG9 {
> -				regulator-name = "VCC18";
> -				regulator-min-microvolt = <1800000>;
> -				regulator-max-microvolt = <1800000>;
> +			vrtc_18: LDO_REG9 {
> +				regulator-name = "LDO_REG9";
> +				/* Despite the datasheet stating 3.3V for REG9 and
> +				   driver expecting that, REG9 outputs 1.8V.
> +				   Likely the CI20 uses a chip variant.
> +				   Since it is a simple on/off LDO the exact values
> +				   do not matter.
> +				*/
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
>  				regulator-always-on;
>  			};
>  			vcc_11: LDO_REG10 {
> -				regulator-name = "VCC11";
> -				regulator-min-microvolt = <1100000>;
> -				regulator-max-microvolt = <1100000>;
> +				regulator-name = "LDO_REG10";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
>  				regulator-always-on;
>  			};
>  		};
> --
> 2.23.0
>