Re: [PATCH v2 4/4] arm64: dts: mediatek: mt8195-demo: simplify mt6360 nodes

[Krzysztof Kozlowski]

On 30/08/2023 13:15, Macpaul Lin wrote:
> The dts file for the MediaTek MT8195 demo board has been refactored
> to improve the configuration of the MT6360 multi-function PMIC.
> 
> The changes include:
>  - Addition of the mt6360.dtsi include file, which contains the common
>    configuration of the MT6360 for all mt8195 boards.
>  - Removal of the direct inclusion of the mt6360-regulator.h file.
>  - Removal of the common configuration of the MT6360 PMIC since
>    the included mt6360.dtsi is used.
>  - Add names according to the schematic of mt8195-demo board for
>    mt6360 nodes.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 173 ++++++++-----------
>  1 file changed, 74 insertions(+), 99 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> index 8aea6f5d72b3..d082d679dbbe 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> @@ -11,7 +11,6 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> -#include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
>  
>  / {
>  	model = "MediaTek MT8195 demo board";
> @@ -130,103 +129,9 @@
>  	mt6360: pmic@34 {
>  		compatible = "mediatek,mt6360";
>  		reg = <0x34>;
> -		interrupt-controller;
> +		pinctrl-0 = <&mt6360_pins>;
> +		pinctrl-names = "default";
>  		interrupts-extended = <&pio 101 IRQ_TYPE_EDGE_FALLING>;
> -		interrupt-names = "IRQB";
> -
> -		charger {
> -			compatible = "mediatek,mt6360-chg";
> -			richtek,vinovp-microvolt = <14500000>;
> -
> -			otg_vbus_regulator: usb-otg-vbus-regulator {
> -				regulator-compatible = "usb-otg-vbus";
> -				regulator-name = "usb-otg-vbus";
> -				regulator-min-microvolt = <4425000>;
> -				regulator-max-microvolt = <5825000>;
> -			};
> -		};
> -
> -		regulator {
> -			compatible = "mediatek,mt6360-regulator";
> -			LDO_VIN3-supply = <&mt6360_buck2>;
> -
> -			mt6360_buck1: buck1 {

Your patchset does not look bisectable. Does not look tested, either...
Why having duplicated regulators (?) and then removing correct
regulators and keeping wrong ones?

> -				regulator-compatible = "BUCK1";
> -				regulator-name = "mt6360,buck1";
> -				regulator-min-microvolt = <300000>;
> -				regulator-max-microvolt = <1300000>;
> -				regulator-allowed-modes = <MT6360_OPMODE_NORMAL
> -							   MT6360_OPMODE_LP
> -							   MT6360_OPMODE_ULP>;
> -				regulator-always-on;
> -			};

...

>  	};
>  };
>  
> @@ -259,8 +164,8 @@
>  	cap-sd-highspeed;
>  	sd-uhs-sdr50;
>  	sd-uhs-sdr104;
> -	vmmc-supply = <&mt6360_ldo5>;
> -	vqmmc-supply = <&mt6360_ldo3>;
> +	vmmc-supply = <&mt6360_vmch_pmu_ldo5_reg>;
> +	vqmmc-supply = <&mt6360_vmc_pmu_ldo3_reg>;
>  	status = "okay";
>  };
>  
> @@ -300,6 +205,67 @@
>  	regulator-always-on;
>  };
>  
> +#include "mt6360.dtsi"

Includes are in the top part of the DTS. Sometimes bottom, but never in
the middle.


Best regards,
Krzysztof