diff mbox series

[1/4] arm64: dts: mediatek: mt8195-demo: fix the memory size to 8GB

Message ID 20230825114623.16884-1-macpaul.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series [1/4] arm64: dts: mediatek: mt8195-demo: fix the memory size to 8GB | expand

Commit Message

Macpaul Lin Aug. 25, 2023, 11:46 a.m. UTC
The onboard dram of mt8195-demo board is 8GB.

Cc: stable@vger.kernel.org      # 6.1, 6.4
Fixes: 6147314aeedc ("arm64: dts: mediatek: Add device-tree for MT8195 Demo board")
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Aug. 30, 2023, 11:30 a.m. UTC | #1
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@mediatek.com>
> ---
>  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
Macpaul Lin Aug. 31, 2023, 4:06 a.m. UTC | #2
On 8/30/23 19:30, Krzysztof Kozlowski wrote:
> 	
> 
> External email : Please do not click links or open attachments until you 
> have verified the sender or the content.
> 
> 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@mediatek.com>
>> ---
>>  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
> 

MT6360 is an external component controlled by I2C.
On some mt8195 boards, it is connected to I2C6.
But on some smart phone boards, they are connected to I2C5.
I agree to put the includes in the top or in the bottom,
but it to include I2C node together in mt6360.dtsi will be necessary
to avoid build error. It might introduce mt6360-i2c5.dtsi
or mt6360-i2c6.dtsi in the future.

I think the better approach is to expand the common nodes in the board's 
dts, rather than organizing them into dtsi.

Please drop these 2 patches for adding mt6360.dtsi and modification for 
mt8195-demo.dts (patch 3/4 and 4/4) for the patch set.

Thanks for the review. :)
Macpaul Lin
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
index b2485ddfd33b..ff363ab925e9 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
@@ -48,7 +48,7 @@ 
 
 	memory@40000000 {
 		device_type = "memory";
-		reg = <0 0x40000000 0 0x80000000>;
+		reg = <0 0x40000000 0x2 0x00000000>;
 	};
 
 	reserved-memory {