[2/2] ARM: DTS: Add devicetree file for the Galaxy S2

Message ID	20200312153411.13535-2-paul@crapouillou.net (mailing list archive)
State	Superseded
Headers	show Return-Path: <SRS0=z6PS=45=vger.kernel.org=linux-samsung-soc-owner@kernel.org> From: Paul Cercueil <paul@crapouillou.net> To: Kukjin Kim <kgene@kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com> Cc: Stenkin Evgeniy <stenkinevgeniy@gmail.com>, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Cercueil <paul@crapouillou.net> Subject: [PATCH 2/2] ARM: DTS: Add devicetree file for the Galaxy S2 Date: Thu, 12 Mar 2020 16:34:11 +0100 Message-Id: <20200312153411.13535-2-paul@crapouillou.net> In-Reply-To: <20200312153411.13535-1-paul@crapouillou.net> References: <20200312153411.13535-1-paul@crapouillou.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk
Series	[1/2] dt-bindings: arm/samsung: Add compatible string for the Galaxy S2 \| expand [1/2] dt-bindings: arm/samsung: Add compatible string for the Galaxy S2 [2/2] ARM: DTS: Add devicetree file for the Galaxy S2

Paul Cercueil March 12, 2020, 3:34 p.m. UTC

From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>

Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 version).

Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/arm/boot/dts/Makefile             |   1 +
 arch/arm/boot/dts/exynos4210-i9100.dts | 816 +++++++++++++++++++++++++
 2 files changed, 817 insertions(+)
 create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts

Marek Szyprowski March 13, 2020, 8:03 a.m. UTC | #1

On 12.03.2020 16:34, Paul Cercueil wrote:
> From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>
> Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 version).
>
> Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   arch/arm/boot/dts/Makefile             |   1 +
>   arch/arm/boot/dts/exynos4210-i9100.dts | 816 +++++++++++++++++++++++++
>   2 files changed, 817 insertions(+)
>   create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index d6546d2676b9..522436d30690 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
>   	exynos3250-monk.dtb \
>   	exynos3250-rinato.dtb
>   dtb-$(CONFIG_ARCH_EXYNOS4) += \
> +	exynos4210-i9100.dtb \
>   	exynos4210-origen.dtb \
>   	exynos4210-smdkv310.dtb \
>   	exynos4210-trats.dtb \
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> new file mode 100644
> index 000000000000..c218e34959be
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -0,0 +1,816 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device tree
> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
> + *
> + * Device tree source file for Samsung's Galaxy S2 smartphone (i9100 version),
> + * which is based on Samsung's Exynos4210 SoC.
> + */
> +
> +/dts-v1/;
> +#include "exynos4210.dtsi"
> +#include "exynos4412-ppmu-common.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +
> +/ {
> +	model = "Samsung Galaxy S2 (i9100)";
> +	compatible = "samsung,i9100", "samsung,exynos4210", "samsung,exynos4";
> +
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x40000000 0x40000000>;
> +	};
> +
> +	vemmc_reg: regulator-0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VMEM_VDD_2.8V";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	tsp_reg: regulator-1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "TSP_FIXED_VOLTAGES";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
> +		startup-delay-us = <70000>;
> +		enable-active-high;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	cam_af_28v_reg: regulator-3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "8M_AF_2.8V_EN";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpk1 1 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	cam_io_en_reg: regulator-4 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "CAM_IO_EN";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpe2 1 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	cam_io_12v_reg: regulator-5 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "8M_1.2V_EN";
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +		gpio = <&gpe2 5 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	vt_core_15v_reg: regulator-6 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VT_CORE_1.5V";
> +		regulator-min-microvolt = <1500000>;
> +		regulator-max-microvolt = <1500000>;
> +		gpio = <&gpe2 2 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		vol-down {
> +			gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			label = "volume down";
> +			debounce-interval = <10>;
> +		};
> +
> +		vol-up {
> +			gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +			label = "volume up";
> +			debounce-interval = <10>;
> +		};
> +
> +		power {
> +			gpios = <&gpx2 7 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_POWER>;
> +			label = "power";
> +			debounce-interval = <10>;
> +			wakeup-source;
> +		};
> +
> +		ok {
> +			gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_OK>;
> +			label = "ok";
> +			debounce-interval = <10>;
> +		};
> +	};
> +
> +	wlan_pwrseq: sdhci3-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		reset-gpios = <&gpl1 2 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	i2c_max17042_fuel: i2c-gpio-3 {
> +		compatible = "i2c-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		sda-gpios = <&gpy4 0 GPIO_ACTIVE_HIGH>;
> +		scl-gpios = <&gpy4 1 GPIO_ACTIVE_HIGH>;
> +		i2c-gpio,delay-us = <5>;
> +
> +		battery@36 {
> +			compatible = "maxim,max17042";
> +
> +			interrupt-parent = <&gpx2>;
> +			interrupts = <3 2>;
> +
> +			pinctrl-0 = <&max17042_fuel_irq>;
> +			pinctrl-names = "default";
> +
> +			reg = <0x36>;
> +			maxim,over-heat-temp = <700>;
> +			maxim,over-volt = <4500>;
> +
> +		};
> +	};
> +
> +	spi-lcd {
> +		compatible = "spi-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		num-chipselects = <1>;
> +		cs-gpios = <&gpy4 3 GPIO_ACTIVE_HIGH>;
LD9040 CS is active low, so this board needs probably the same fix a 
Universal C210 board: https://patchwork.kernel.org/patch/11420231/ Are 
you sure it really works now with v5.6-rcX kernels? Or you just got the 
panel working only because the bootloader enabled it?
> +		sck-gpios = <&gpy3 1 GPIO_ACTIVE_HIGH>;
> +		mosi-gpios = <&gpy3 3 GPIO_ACTIVE_HIGH>;
> +
> +		lcd@0 {
> +			compatible = "samsung,ld9040";
> +			reg = <0>;
> +
> +			spi-max-frequency = <1200000>;
> +			spi-cpol;
> +			spi-cpha;
> +
> +			vdd3-supply = <&vmipi_reg>;
> +			vci-supply = <&vcclcd_reg>;
> +
> +			reset-gpios = <&gpy4 5 GPIO_ACTIVE_HIGH>;
> +			power-on-delay = <10>;
> +			reset-delay = <10>;
> +
> +			panel-width-mm = <90>;
> +			panel-height-mm = <154>;
> +
> +			display-timings {
> +				timing {
> +					clock-frequency = <23492370>;
> +					hactive = <480>;
> +					vactive = <800>;
> +					hback-porch = <16>;
> +					hfront-porch = <16>;
> +					vback-porch = <2>;
> +					vfront-porch = <28>;
> +					hsync-len = <2>;
> +					vsync-len = <1>;
> +					hsync-active = <0>;
> +					vsync-active = <0>;
> +					de-active = <0>;
> +					pixelclk-active = <0>;
> +				};
> +			};
> +
> +			port {
> +				lcd_ep: endpoint {
> +					remote-endpoint = <&fimd_dpi_ep>;
> +				};
> +			};
> +		};
> +	};
> +
> +	fixed-rate-clocks {
> +		xxti {
> +			compatible = "samsung,clock-xxti";
> +			clock-frequency = <0>;
> +		};
> +
> +		xusbxti {
> +			compatible = "samsung,clock-xusbxti";
> +			clock-frequency = <24000000>;
> +		};
> +	};
> +
> +	thermal-zones {
> +		cpu_thermal: cpu-thermal {
> +			cooling-maps {
> +				map0 {
> +					/* Corresponds to 800MHz */
> +					cooling-device = <&cpu0 2 2>;
> +				};
> +				map1 {
> +					/* Corresponds to 200MHz */
> +					cooling-device = <&cpu0 4 4>;
> +				};
> +			};
> +		};
> +	};
> +
> +};
> +
> +&camera {
> +	status = "okay";
> +};
> +
> +&cpu0 {
> +	cpu0-supply = <&varm_breg>;
> +};
> +
> +&exynos_usbphy {
> +	status = "okay";
> +
> +	vbus-supply = <&safe1_sreg>;
> +};
> +
> +&fimc_0 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock CLK_SCLK_FIMC0>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_1 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock CLK_SCLK_FIMC1>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_2 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock CLK_SCLK_FIMC2>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_3 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock CLK_SCLK_FIMC3>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimd {
> +	status = "okay";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	samsung,invert-vden;
> +	samsung,invert-vclk;
> +
> +	pinctrl-0 = <&lcd_clk>, <&lcd_data24>;
> +	pinctrl-names = "default";
> +
> +	port@3 {
> +		reg = <3>;
> +
> +		fimd_dpi_ep: endpoint {
> +			remote-endpoint = <&lcd_ep>;
> +		};
> +	};
> +};
> +
> +&hsotg {
> +	status = "okay";
> +
> +	dr_mode = "otg";
The the host mode really works with DWC2? Afair the host mode worked 
only when it was rerouted by PHY to the EHCI (at least on UniversalC210 
board, which is very similar to GS2).
> +	vusb_d-supply = <&vusb_reg>;
> +	vusb_a-supply = <&vusbdac_reg>;
> +};
> +
> +&ehci {
> +	status = "okay";
> +
> +	port@0 {
> +		status = "okay";
> +	};
> +
> +	port@1 {
> +		status = "okay";
> +	};
> +
> +	port@2 {
> +		status = "okay";
> +	};
> +};

This is the old, obsolete exynos-ehci binding. Please remove 'port' 
nodes and use standard USB phy bindings like it has been done in the 
following patch: https://patchwork.kernel.org/patch/11060465/

BTW, do you really need all 3 phys/ports?

> +
> +
> +&i2c_3 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <100000>;
> +
> +	pinctrl-0 = <&i2c3_bus>;
> +	pinctrl-names = "default";
> +
> +	mxt224-touchscreen@4a {
> +		compatible = "atmel,maxtouch";
> +		reg = <0x4a>;
> +
> +		interrupt-parent = <&gpx0>;
> +		interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> +
> +		avdd-supply = <&tsp_reg>;
> +		vdd-supply = <&tsp_reg>;
> +	};
> +};
> +
> +&i2c_7 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <400000>;
> +
> +	pinctrl-0 = <&i2c7_bus>;
> +	pinctrl-names = "default";
> +
> +	ak8975@c {
> +		compatible = "asahi-kasei,ak8975";
> +		reg = <0x0c>;
> +
> +		gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +
> +&i2c_5 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <100000>;
> +
> +	pinctrl-0 = <&i2c5_bus>;
> +	pinctrl-names = "default";
> +
> +	max8997_pmic@66 {
> +		compatible = "maxim,max8997-pmic";
> +		reg = <0x66>;
> +
> +		interrupts-extended = <&gpx0 7 0>, <&gpx2 3 2>;
> +		interrupt-parent = <&gpx0>;
> +		interrupts = <7 IRQ_TYPE_NONE>;
> +
> +		otg_en-gpios = <&gpx3 3 0>;
> +		usb_sel-gpios = <&gpl0 6 0>;
> +
> +		max8997,pmic-buck1-uses-gpio-dvs;
> +		max8997,pmic-buck2-uses-gpio-dvs;
> +		max8997,pmic-buck5-uses-gpio-dvs;
> +
> +		max8997,pmic-ignore-gpiodvs-side-effect;
> +		max8997,pmic-buck125-default-dvs-idx = <0>;
> +
> +		max8997,pmic-buck125-dvs-gpios = <&gpx0 5 GPIO_ACTIVE_HIGH>,
> +						 <&gpx0 6 GPIO_ACTIVE_HIGH>,
> +						 <&gpl0 0 GPIO_ACTIVE_HIGH>;
> +
> +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
> +						 <1250000>, <1200000>,
> +						 <1150000>, <1100000>,
> +						 <1000000>, <950000>;
> +
> +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1000000>,
> +						 <950000>,  <900000>,
> +						 <1100000>, <1000000>,
> +						 <950000>,  <900000>;
> +
> +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>;
> +
> +		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
> +		pinctrl-names = "default";
> +
> +		regulators {
> +			vadc_reg: LDO1 {
> +				regulator-name = "VADC_3.3V_C210";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +
> +			};
> +			valive_reg: LDO2 {
> +				regulator-name = "VALIVE_1.1V_C210";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +
> +			};
> +
> +			vusb_reg: LDO3 {
> +				regulator-name = "VUSB_1.1V_C210";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +			};
> +
> +			vmipi_reg: LDO4 {
> +				regulator-name = "VMIPI_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			vhsic_reg: LDO5 {
> +				regulator-name = "VHSIC_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			vpda_reg: LDO6 {
> +				regulator-name = "VCC_1.8V_PDA";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			vcam_reg: LDO7 {
> +				regulator-name = "CAM_ISP_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vusbdac_reg: LDO8 {
> +				regulator-name = "VUSB+VDAC_3.3V_C210";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			vccpda_reg: LDO9 {
> +				regulator-name = "VCC_2.8V_PDA";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +			};
> +
> +			vtouch_reg: LDO11 {
> +				regulator-name = "TOUCH_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +			};
> +
> +			vpll_reg: LDO10 {
> +				regulator-name = "VPLL_1.1V";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +			};
> +
> +			vtcam_reg: LDO12 {
> +				regulator-name = "VT_CAM_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vcclcd_reg: LDO13 {
> +				regulator-name = "VCC_3.0V_LCD";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +			};
> +
> +			vmotor_reg: LDO14 {
> +				regulator-name = "VCC_2.8V_MOTOR";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			vled_reg: LDO15 {
> +				regulator-name = "LED_A_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			camsensor_reg: LDO16 {
> +				regulator-name = "CAM_SENSOR_IO_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vtf_reg: LDO17 {
> +				regulator-name = "VTF_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			vtouchled_reg: LDO18 {
> +				regulator-name = "TOUCH_LED_3.3V";
> +				regulator-min-microvolt = <2500000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			vddq_reg: LDO21 {
> +				regulator-name = "VDDQ_M1M2_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			varm_breg: BUCK1 {
> +				//regulator-name = "VARM_1.2V_C210";
> +				regulator-name = "vdd_arm";
> +				regulator-min-microvolt = <65000>;
> +				regulator-max-microvolt = <2225000>;
> +				regulator-always-on;
> +			};
> +
> +			vint_breg: BUCK2 {
> +				regulator-name = "VINT_1.1V_C210";
> +				regulator-min-microvolt = <65000>;
> +				regulator-max-microvolt = <2225000>;
> +				regulator-always-on;
> +			};
> +
> +			vg3d_breg: BUCK3 {
> +				regulator-name = "G3D_1.1V";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			camisp_breg: BUCK4 {
> +				regulator-name = "CAM_ISP_CORE_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +			};
> +
> +			vmem_breg: BUCK5 {
> +				regulator-name = "VMEM_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			vccsub_breg: BUCK7 {
> +				regulator-name = "VCC_SUB_2.0V";
> +				regulator-min-microvolt = <2000000>;
> +				regulator-max-microvolt = <2000000>;
> +				regulator-always-on;
> +			};
> +
> +			safe1_sreg: ESAFEOUT1 {
> +				regulator-name = "SAFEOUT1";
> +			};
> +
> +			safe2_sreg: ESAFEOUT2 {
> +				regulator-name = "SAFEOUT2";
> +				regulator-boot-on;
> +			};
> +
> +			charger_reg: CHARGER {
> +				regulator-name = "CHARGER";
> +				regulator-min-microamp = <60000>;
> +				regulator-max-microamp = <2580000>;
> +				regulator-always-on;
> +			};
> +
> +			chargercv_reg: CHARGER_CV {
> +				regulator-name = "CHARGER_CV";
> +				regulator-min-microvolt = <3800000>;
> +				regulator-max-microvolt = <4100000>;
> +				regulator-always-on;
> +			};
> +
> +		};
> +
> +	};
> +};
> +
> +
> +&gpu {
> +	status = "okay";
> +
> +	mali-supply = <&vg3d_breg>;
> +	regulator-microvolt-offset = <50000>;
> +	regulator-microsecs-delay = <50>;
> +};
> +
> +
> +&sdhci_0 {
> +	status = "okay";
> +
> +	bus-width = <8>;
> +	non-removable;
> +	vmmc-supply = <&vemmc_reg>;
> +
> +	pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_bus8>;
> +	pinctrl-names = "default";
> +};
> +
> +&sdhci_2 {
> +	status = "okay";
> +
> +	bus-width = <4>;
> +	cd-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&vtf_reg>;
> +
> +	pinctrl-0 = <&sd2_clk>, <&sd2_cmd>, <&sd2_bus4>;
> +	pinctrl-names = "default";
> +};
> +
> +&ehci {
> +	port@0 {
> +		status = "okay";
> +	};
> +
> +	port@1 {
> +		status = "okay";
> +	};
> +
> +	port@2 {
> +		status = "okay";
> +	};
> +
> +};

This is duplicated 'ehci' node, probable an artifact from the earlier 
rebase. Please remove it.

> +
> +
> +&sdhci_3 {
> +	status = "okay";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	non-removable;
> +	bus-width = <4>;
> +	mmc-pwrseq = <&wlan_pwrseq>;
> +	vmmc-supply = <&vtf_reg>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sd3_clk>, <&sd3_cmd>, <&sd3_bus4>;
> +
> +	brcmf: wifi@1 {
> +		compatible = "brcm,bcm4330-fmac";
> +		reg = <1>;
> +
> +		interrupt-parent = <&gpx2>;
> +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "host-wake";
> +	};
> +};
> +
> +
> +&serial_0 {
> +	status = "okay";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&bt_en>, <&bt_res>, <&uart0_data>, <&uart0_fctl>;
> +
> +	bluetooth {
> +		compatible = "brcm,bcm4330-bt";
> +
> +		shutdown-gpios = <&gpl0 4 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpl1 0 GPIO_ACTIVE_HIGH>;
> +		device-wakeup-gpios = <&gpx3 1 GPIO_ACTIVE_HIGH>;
> +		host-wakeup-gpios = <&gpx2 6 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&serial_1 {
> +	status = "okay";
> +};
> +
> +&serial_2 {
> +	status = "okay";
> +};
> +
> +&serial_3 {
> +	status = "okay";
> +};
> +
> +&tmu {
> +	status = "okay";
> +};
> +
> +
> +&pinctrl_0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sleep0>;
> +
> +
> +	sleep0: sleep-states {
> +		gpa0-0 {
> +			samsung,pins = "gpa0-0";
> +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
> +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
> +		};
> +
> +		gpa0-1 {
> +			samsung,pins = "gpa0-1";
> +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT0>;
> +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
> +		};
> +
> +		gpa0-2 {
> +			samsung,pins = "gpa0-2";
> +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
> +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
> +		};
> +
> +		gpa0-3 {
> +			samsung,pins = "gpa0-3";
> +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT1>;
> +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
> +		};
> +	};
> +};
> +
> +&pinctrl_1 {
> +	mhl_int: mhl-int {
> +		samsung,pins = "gpf3-5";
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +	};
> +
> +	i2c_mhl_bus: i2c-mhl-bus {
> +		samsung,pins = "gpf0-4", "gpf0-6";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_DOWN>;
> +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +	};
> +
> +	usb_sel: usb-sel {
> +		samsung,pins = "gpl0-6";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +		samsung,pin-val = <0>;
> +	};
> +
> +	bt_en: bt-en {
> +		samsung,pins = "gpl0-4";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
> +		samsung,pin-val = <0>;
> +	};
> +
> +	bt_res: bt-res {
> +		samsung,pins = "gpl1-0";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
> +		samsung,pin-val = <0>;
> +	};
> +
> +
> +	otg_gp: otg-gp {
> +		samsung,pins = "gpx3-3";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
> +		samsung,pin-val = <0>;
> +	};
> +
> +	mag_mhl_gpio: mag-mhl-gpio {
> +		samsung,pins = "gpd0-2";
> +		samsung,pin-function = <EXYNOS_PIN_FUNC_3>;
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +	};
> +
> +
> +	max8997_irq: max8997-irq {
> +		samsung,pins = "gpx0-7";
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +	};
> +
> +	max17042_fuel_irq: max17042-fuel-irq {
> +		samsung,pins = "gpx2-3";
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +	};
> +
> +	tsp224_irq: tsp224-irq {
> +		samsung,pins = "gpx0-4";
> +		samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
> +	};
> +};

Best regards

Krzysztof Kozlowski March 13, 2020, 9 a.m. UTC | #2

On Thu, Mar 12, 2020 at 04:34:11PM +0100, Paul Cercueil wrote:
> From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> 
> Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 version).

Thanks for the patch!

It is an GT-I9100.

+Cc Marek (I have few questions further)


> 
> Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  arch/arm/boot/dts/Makefile             |   1 +
>  arch/arm/boot/dts/exynos4210-i9100.dts | 816 +++++++++++++++++++++++++
>  2 files changed, 817 insertions(+)
>  create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index d6546d2676b9..522436d30690 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
>  	exynos3250-monk.dtb \
>  	exynos3250-rinato.dtb
>  dtb-$(CONFIG_ARCH_EXYNOS4) += \
> +	exynos4210-i9100.dtb \
>  	exynos4210-origen.dtb \
>  	exynos4210-smdkv310.dtb \
>  	exynos4210-trats.dtb \
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> new file mode 100644
> index 000000000000..c218e34959be
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -0,0 +1,816 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device tree

Ditto

> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
> + *
> + * Device tree source file for Samsung's Galaxy S2 smartphone (i9100 version),
> + * which is based on Samsung's Exynos4210 SoC.

Duplicated description, you mentioned it before.

> + */
> +
> +/dts-v1/;
> +#include "exynos4210.dtsi"
> +#include "exynos4412-ppmu-common.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +
> +/ {
> +	model = "Samsung Galaxy S2 (i9100)";

GT-I9100

> +	compatible = "samsung,i9100", "samsung,exynos4210", "samsung,exynos4";

I think it shares a lot with Trats board...

Marek, how similar these
designs are? Maybe it is worth to split some parts and make common part?

> +
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x40000000 0x40000000>;
> +	};
> +

You do not have a chosen node, at least for stdout serial selection.
Probably you should have one... bootargs I guess could come from
bootloader.


> +	vemmc_reg: regulator-0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VMEM_VDD_2.8V";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;

This looks like a stub/fake regulator. Probably it shoould be replaced
with proper regulator coming from PMIC (few seems to be missing there).

I don't have the schematics so I cannot judge... I see Trats has the
same.

Marek, mayybe you know, is it really a separate regulator?

> +	};
> +
> +	tsp_reg: regulator-1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "TSP_FIXED_VOLTAGES";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
> +		startup-delay-us = <70000>;
> +		enable-active-high;
> +		regulator-boot-on;
> +		regulator-always-on;

always-on and boot-on should not be needed. You have a consumer for this
regulator.

> +	};
> +
> +	cam_af_28v_reg: regulator-3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "8M_AF_2.8V_EN";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpk1 1 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	cam_io_en_reg: regulator-4 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "CAM_IO_EN";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&gpe2 1 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	cam_io_12v_reg: regulator-5 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "8M_1.2V_EN";
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +		gpio = <&gpe2 5 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	vt_core_15v_reg: regulator-6 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VT_CORE_1.5V";
> +		regulator-min-microvolt = <1500000>;
> +		regulator-max-microvolt = <1500000>;
> +		gpio = <&gpe2 2 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		vol-down {
> +			gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			label = "volume down";
> +			debounce-interval = <10>;
> +		};
> +
> +		vol-up {
> +			gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +			label = "volume up";
> +			debounce-interval = <10>;
> +		};
> +
> +		power {
> +			gpios = <&gpx2 7 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_POWER>;
> +			label = "power";
> +			debounce-interval = <10>;
> +			wakeup-source;
> +		};
> +
> +		ok {
> +			gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_OK>;
> +			label = "ok";
> +			debounce-interval = <10>;
> +		};
> +	};
> +
> +	wlan_pwrseq: sdhci3-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		reset-gpios = <&gpl1 2 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	i2c_max17042_fuel: i2c-gpio-3 {

i2c-gpio-3 -> i2c-gpio

> +		compatible = "i2c-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		sda-gpios = <&gpy4 0 GPIO_ACTIVE_HIGH>;
> +		scl-gpios = <&gpy4 1 GPIO_ACTIVE_HIGH>;
> +		i2c-gpio,delay-us = <5>;
> +
> +		battery@36 {
> +			compatible = "maxim,max17042";
> +
> +			interrupt-parent = <&gpx2>;
> +			interrupts = <3 2>;

s/2/IRQ_TYPE_EDGE_FALLING/

> +
> +			pinctrl-0 = <&max17042_fuel_irq>;
> +			pinctrl-names = "default";
> +
> +			reg = <0x36>;
> +			maxim,over-heat-temp = <700>;
> +			maxim,over-volt = <4500>;
> +

Remove empty line.

> +		};
> +	};
> +
> +	spi-lcd {
> +		compatible = "spi-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		num-chipselects = <1>;
> +		cs-gpios = <&gpy4 3 GPIO_ACTIVE_HIGH>;
> +		sck-gpios = <&gpy3 1 GPIO_ACTIVE_HIGH>;
> +		mosi-gpios = <&gpy3 3 GPIO_ACTIVE_HIGH>;
> +
> +		lcd@0 {
> +			compatible = "samsung,ld9040";
> +			reg = <0>;
> +
> +			spi-max-frequency = <1200000>;
> +			spi-cpol;
> +			spi-cpha;
> +
> +			vdd3-supply = <&vmipi_reg>;
> +			vci-supply = <&vcclcd_reg>;
> +
> +			reset-gpios = <&gpy4 5 GPIO_ACTIVE_HIGH>;
> +			power-on-delay = <10>;
> +			reset-delay = <10>;
> +
> +			panel-width-mm = <90>;
> +			panel-height-mm = <154>;
> +
> +			display-timings {
> +				timing {
> +					clock-frequency = <23492370>;
> +					hactive = <480>;
> +					vactive = <800>;
> +					hback-porch = <16>;
> +					hfront-porch = <16>;
> +					vback-porch = <2>;
> +					vfront-porch = <28>;
> +					hsync-len = <2>;
> +					vsync-len = <1>;
> +					hsync-active = <0>;
> +					vsync-active = <0>;
> +					de-active = <0>;
> +					pixelclk-active = <0>;
> +				};
> +			};
> +
> +			port {
> +				lcd_ep: endpoint {
> +					remote-endpoint = <&fimd_dpi_ep>;
> +				};
> +			};
> +		};
> +	};
> +
> +	fixed-rate-clocks {
> +		xxti {
> +			compatible = "samsung,clock-xxti";
> +			clock-frequency = <0>;
> +		};
> +
> +		xusbxti {
> +			compatible = "samsung,clock-xusbxti";
> +			clock-frequency = <24000000>;
> +		};
> +	};
> +
> +	thermal-zones {
> +		cpu_thermal: cpu-thermal {
> +			cooling-maps {
> +				map0 {
> +					/* Corresponds to 800MHz */
> +					cooling-device = <&cpu0 2 2>;
> +				};
> +				map1 {
> +					/* Corresponds to 200MHz */
> +					cooling-device = <&cpu0 4 4>;
> +				};
> +			};
> +		};
> +	};
> +
> +};
> +
> +&camera {
> +	status = "okay";
> +};
> +
> +&cpu0 {
> +	cpu0-supply = <&varm_breg>;
> +};
> +
> +&exynos_usbphy {
> +	status = "okay";
> +
> +	vbus-supply = <&safe1_sreg>;
> +};
> +
> +&fimc_0 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock CLK_SCLK_FIMC0>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_1 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock CLK_SCLK_FIMC1>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_2 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock CLK_SCLK_FIMC2>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimc_3 {
> +	status = "okay";
> +
> +	assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock CLK_SCLK_FIMC3>;
> +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
> +	assigned-clock-rates = <0>, <160000000>;
> +};
> +
> +&fimd {
> +	status = "okay";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	samsung,invert-vden;
> +	samsung,invert-vclk;
> +
> +	pinctrl-0 = <&lcd_clk>, <&lcd_data24>;
> +	pinctrl-names = "default";
> +
> +	port@3 {
> +		reg = <3>;
> +
> +		fimd_dpi_ep: endpoint {
> +			remote-endpoint = <&lcd_ep>;
> +		};
> +	};
> +};
> +
> +&hsotg {
> +	status = "okay";
> +
> +	dr_mode = "otg";
> +	vusb_d-supply = <&vusb_reg>;
> +	vusb_a-supply = <&vusbdac_reg>;
> +};
> +
> +&ehci {

Order the phandle/nodes in alphabetical order, please. So:

&ehci ...
&hsoth ...
&i2c ...

etc.

> +	status = "okay";
> +
> +	port@0 {
> +		status = "okay";
> +	};
> +
> +	port@1 {
> +		status = "okay";
> +	};
> +
> +	port@2 {
> +		status = "okay";
> +	};
> +};
> +

Double empty line.

> +
> +&i2c_3 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <100000>;
> +
> +	pinctrl-0 = <&i2c3_bus>;
> +	pinctrl-names = "default";
> +
> +	mxt224-touchscreen@4a {
> +		compatible = "atmel,maxtouch";
> +		reg = <0x4a>;
> +
> +		interrupt-parent = <&gpx0>;
> +		interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> +
> +		avdd-supply = <&tsp_reg>;
> +		vdd-supply = <&tsp_reg>;
> +	};
> +};
> +
> +&i2c_7 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <400000>;
> +
> +	pinctrl-0 = <&i2c7_bus>;
> +	pinctrl-names = "default";
> +
> +	ak8975@c {
> +		compatible = "asahi-kasei,ak8975";
> +		reg = <0x0c>;
> +
> +		gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +
> +&i2c_5 {
> +	status = "okay";
> +
> +	samsung,i2c-sda-delay = <100>;
> +	samsung,i2c-slave-addr = <0x10>;
> +	samsung,i2c-max-bus-freq = <100000>;
> +
> +	pinctrl-0 = <&i2c5_bus>;
> +	pinctrl-names = "default";
> +
> +	max8997_pmic@66 {
> +		compatible = "maxim,max8997-pmic";
> +		reg = <0x66>;
> +
> +		interrupts-extended = <&gpx0 7 0>, <&gpx2 3 2>;
> +		interrupt-parent = <&gpx0>;
> +		interrupts = <7 IRQ_TYPE_NONE>;
> +
> +		otg_en-gpios = <&gpx3 3 0>;
> +		usb_sel-gpios = <&gpl0 6 0>;
> +
> +		max8997,pmic-buck1-uses-gpio-dvs;
> +		max8997,pmic-buck2-uses-gpio-dvs;
> +		max8997,pmic-buck5-uses-gpio-dvs;
> +
> +		max8997,pmic-ignore-gpiodvs-side-effect;
> +		max8997,pmic-buck125-default-dvs-idx = <0>;
> +
> +		max8997,pmic-buck125-dvs-gpios = <&gpx0 5 GPIO_ACTIVE_HIGH>,
> +						 <&gpx0 6 GPIO_ACTIVE_HIGH>,
> +						 <&gpl0 0 GPIO_ACTIVE_HIGH>;
> +
> +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
> +						 <1250000>, <1200000>,
> +						 <1150000>, <1100000>,
> +						 <1000000>, <950000>;
> +
> +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1000000>,
> +						 <950000>,  <900000>,
> +						 <1100000>, <1000000>,
> +						 <950000>,  <900000>;
> +
> +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>;
> +
> +		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
> +		pinctrl-names = "default";
> +
> +		regulators {
> +			vadc_reg: LDO1 {
> +				regulator-name = "VADC_3.3V_C210";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +
> +			};

Add empty line.

> +			valive_reg: LDO2 {
> +				regulator-name = "VALIVE_1.1V_C210";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +
> +			};
> +
> +			vusb_reg: LDO3 {
> +				regulator-name = "VUSB_1.1V_C210";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +			};
> +
> +			vmipi_reg: LDO4 {
> +				regulator-name = "VMIPI_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			vhsic_reg: LDO5 {
> +				regulator-name = "VHSIC_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			vpda_reg: LDO6 {
> +				regulator-name = "VCC_1.8V_PDA";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			vcam_reg: LDO7 {
> +				regulator-name = "CAM_ISP_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vusbdac_reg: LDO8 {
> +				regulator-name = "VUSB+VDAC_3.3V_C210";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			vccpda_reg: LDO9 {
> +				regulator-name = "VCC_2.8V_PDA";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +			};
> +
> +			vtouch_reg: LDO11 {
> +				regulator-name = "TOUCH_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +			};
> +
> +			vpll_reg: LDO10 {
> +				regulator-name = "VPLL_1.1V";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +			};
> +
> +			vtcam_reg: LDO12 {
> +				regulator-name = "VT_CAM_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vcclcd_reg: LDO13 {
> +				regulator-name = "VCC_3.0V_LCD";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +			};
> +
> +			vmotor_reg: LDO14 {
> +				regulator-name = "VCC_2.8V_MOTOR";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			vled_reg: LDO15 {
> +				regulator-name = "LED_A_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			camsensor_reg: LDO16 {
> +				regulator-name = "CAM_SENSOR_IO_1.8V";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +
> +			vtf_reg: LDO17 {
> +				regulator-name = "VTF_2.8V";
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +			};
> +
> +			vtouchled_reg: LDO18 {
> +				regulator-name = "TOUCH_LED_3.3V";
> +				regulator-min-microvolt = <2500000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			vddq_reg: LDO21 {
> +				regulator-name = "VDDQ_M1M2_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			varm_breg: BUCK1 {
> +				//regulator-name = "VARM_1.2V_C210";
> +				regulator-name = "vdd_arm";
> +				regulator-min-microvolt = <65000>;
> +				regulator-max-microvolt = <2225000>;
> +				regulator-always-on;
> +			};
> +
> +			vint_breg: BUCK2 {
> +				regulator-name = "VINT_1.1V_C210";
> +				regulator-min-microvolt = <65000>;
> +				regulator-max-microvolt = <2225000>;
> +				regulator-always-on;
> +			};
> +
> +			vg3d_breg: BUCK3 {
> +				regulator-name = "G3D_1.1V";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			camisp_breg: BUCK4 {
> +				regulator-name = "CAM_ISP_CORE_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +			};
> +
> +			vmem_breg: BUCK5 {
> +				regulator-name = "VMEM_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			vccsub_breg: BUCK7 {
> +				regulator-name = "VCC_SUB_2.0V";
> +				regulator-min-microvolt = <2000000>;
> +				regulator-max-microvolt = <2000000>;
> +				regulator-always-on;
> +			};
> +
> +			safe1_sreg: ESAFEOUT1 {
> +				regulator-name = "SAFEOUT1";
> +			};
> +
> +			safe2_sreg: ESAFEOUT2 {
> +				regulator-name = "SAFEOUT2";
> +				regulator-boot-on;
> +			};
> +
> +			charger_reg: CHARGER {
> +				regulator-name = "CHARGER";
> +				regulator-min-microamp = <60000>;
> +				regulator-max-microamp = <2580000>;
> +				regulator-always-on;
> +			};
> +
> +			chargercv_reg: CHARGER_CV {
> +				regulator-name = "CHARGER_CV";
> +				regulator-min-microvolt = <3800000>;
> +				regulator-max-microvolt = <4100000>;
> +				regulator-always-on;
> +			};
> +

Empty line, not needed.

> +		};
> +

Empty line, not needed.

> +	};
> +};
> +
> +

Empty line, not needed. Please fix them in all places.

> +&gpu {
> +	status = "okay";
> +
> +	mali-supply = <&vg3d_breg>;
> +	regulator-microvolt-offset = <50000>;
> +	regulator-microsecs-delay = <50>;
> +};
> +

and more...

> +
> +&sdhci_0 {
> +	status = "okay";
> +
> +	bus-width = <8>;
> +	non-removable;
> +	vmmc-supply = <&vemmc_reg>;
> +
> +	pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_bus8>;
> +	pinctrl-names = "default";
> +};
> +
> +&sdhci_2 {
> +	status = "okay";
> +
> +	bus-width = <4>;
> +	cd-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&vtf_reg>;
> +
> +	pinctrl-0 = <&sd2_clk>, <&sd2_cmd>, <&sd2_bus4>;
> +	pinctrl-names = "default";
> +};
> +
> +&ehci {

Duplicated.

Best regards,
Krzysztof

Paul Cercueil March 13, 2020, 2:29 p.m. UTC | #3

Hi Marek,


Le ven., mars 13, 2020 at 09:03, Marek Szyprowski 
<m.szyprowski@samsung.com> a écrit :
> 
> On 12.03.2020 16:34, Paul Cercueil wrote:
>>  From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>> 
>>  Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 
>> version).
>> 
>>  Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>    arch/arm/boot/dts/Makefile             |   1 +
>>    arch/arm/boot/dts/exynos4210-i9100.dts | 816 
>> +++++++++++++++++++++++++
>>    2 files changed, 817 insertions(+)
>>    create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
>> 
>>  diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>  index d6546d2676b9..522436d30690 100644
>>  --- a/arch/arm/boot/dts/Makefile
>>  +++ b/arch/arm/boot/dts/Makefile
>>  @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
>>    	exynos3250-monk.dtb \
>>    	exynos3250-rinato.dtb
>>    dtb-$(CONFIG_ARCH_EXYNOS4) += \
>>  +	exynos4210-i9100.dtb \
>>    	exynos4210-origen.dtb \
>>    	exynos4210-smdkv310.dtb \
>>    	exynos4210-trats.dtb \
>>  diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts 
>> b/arch/arm/boot/dts/exynos4210-i9100.dts
>>  new file mode 100644
>>  index 000000000000..c218e34959be
>>  --- /dev/null
>>  +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
>>  @@ -0,0 +1,816 @@
>>  +// SPDX-License-Identifier: GPL-2.0
>>  +/*
>>  + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device tree
>>  + *
>>  + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>>  + *		http://www.samsung.com
>>  + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>  + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
>>  + *
>>  + * Device tree source file for Samsung's Galaxy S2 smartphone 
>> (i9100 version),
>>  + * which is based on Samsung's Exynos4210 SoC.
>>  + */
>>  +
>>  +/dts-v1/;
>>  +#include "exynos4210.dtsi"
>>  +#include "exynos4412-ppmu-common.dtsi"
>>  +
>>  +#include <dt-bindings/gpio/gpio.h>
>>  +#include <dt-bindings/input/linux-event-codes.h>
>>  +
>>  +/ {
>>  +	model = "Samsung Galaxy S2 (i9100)";
>>  +	compatible = "samsung,i9100", "samsung,exynos4210", 
>> "samsung,exynos4";
>>  +
>>  +
>>  +	memory@40000000 {
>>  +		device_type = "memory";
>>  +		reg = <0x40000000 0x40000000>;
>>  +	};
>>  +
>>  +	vemmc_reg: regulator-0 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "VMEM_VDD_2.8V";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	tsp_reg: regulator-1 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "TSP_FIXED_VOLTAGES";
>>  +		regulator-min-microvolt = <3300000>;
>>  +		regulator-max-microvolt = <3300000>;
>>  +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>  +		startup-delay-us = <70000>;
>>  +		enable-active-high;
>>  +		regulator-boot-on;
>>  +		regulator-always-on;
>>  +	};
>>  +
>>  +	cam_af_28v_reg: regulator-3 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "8M_AF_2.8V_EN";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpk1 1 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	cam_io_en_reg: regulator-4 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "CAM_IO_EN";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpe2 1 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	cam_io_12v_reg: regulator-5 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "8M_1.2V_EN";
>>  +		regulator-min-microvolt = <1200000>;
>>  +		regulator-max-microvolt = <1200000>;
>>  +		gpio = <&gpe2 5 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	vt_core_15v_reg: regulator-6 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "VT_CORE_1.5V";
>>  +		regulator-min-microvolt = <1500000>;
>>  +		regulator-max-microvolt = <1500000>;
>>  +		gpio = <&gpe2 2 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	gpio-keys {
>>  +		compatible = "gpio-keys";
>>  +
>>  +		vol-down {
>>  +			gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_VOLUMEDOWN>;
>>  +			label = "volume down";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +
>>  +		vol-up {
>>  +			gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_VOLUMEUP>;
>>  +			label = "volume up";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +
>>  +		power {
>>  +			gpios = <&gpx2 7 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_POWER>;
>>  +			label = "power";
>>  +			debounce-interval = <10>;
>>  +			wakeup-source;
>>  +		};
>>  +
>>  +		ok {
>>  +			gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_OK>;
>>  +			label = "ok";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +	};
>>  +
>>  +	wlan_pwrseq: sdhci3-pwrseq {
>>  +		compatible = "mmc-pwrseq-simple";
>>  +		reset-gpios = <&gpl1 2 GPIO_ACTIVE_LOW>;
>>  +	};
>>  +
>>  +	i2c_max17042_fuel: i2c-gpio-3 {
>>  +		compatible = "i2c-gpio";
>>  +		#address-cells = <1>;
>>  +		#size-cells = <0>;
>>  +
>>  +		sda-gpios = <&gpy4 0 GPIO_ACTIVE_HIGH>;
>>  +		scl-gpios = <&gpy4 1 GPIO_ACTIVE_HIGH>;
>>  +		i2c-gpio,delay-us = <5>;
>>  +
>>  +		battery@36 {
>>  +			compatible = "maxim,max17042";
>>  +
>>  +			interrupt-parent = <&gpx2>;
>>  +			interrupts = <3 2>;
>>  +
>>  +			pinctrl-0 = <&max17042_fuel_irq>;
>>  +			pinctrl-names = "default";
>>  +
>>  +			reg = <0x36>;
>>  +			maxim,over-heat-temp = <700>;
>>  +			maxim,over-volt = <4500>;
>>  +
>>  +		};
>>  +	};
>>  +
>>  +	spi-lcd {
>>  +		compatible = "spi-gpio";
>>  +		#address-cells = <1>;
>>  +		#size-cells = <0>;
>>  +
>>  +		num-chipselects = <1>;
>>  +		cs-gpios = <&gpy4 3 GPIO_ACTIVE_HIGH>;
> LD9040 CS is active low, so this board needs probably the same fix a
> Universal C210 board: https://patchwork.kernel.org/patch/11420231/ Are
> you sure it really works now with v5.6-rcX kernels? Or you just got 
> the
> panel working only because the bootloader enabled it?

Sorry but the fix looks bogus to me. The spi-gpio driver calls 
gpiod_set_value(0) when the CS is going *active*, and 
gpiod_set_value(1) when the CS is going *inactive*. If you use the 
GPIO_ACTIVE_LOW flag, the levels are inverted, and the CS is high when 
active, which is the opposite of what you want.


>>  +		sck-gpios = <&gpy3 1 GPIO_ACTIVE_HIGH>;
>>  +		mosi-gpios = <&gpy3 3 GPIO_ACTIVE_HIGH>;
>>  +
>>  +		lcd@0 {
>>  +			compatible = "samsung,ld9040";
>>  +			reg = <0>;
>>  +
>>  +			spi-max-frequency = <1200000>;
>>  +			spi-cpol;
>>  +			spi-cpha;
>>  +
>>  +			vdd3-supply = <&vmipi_reg>;
>>  +			vci-supply = <&vcclcd_reg>;
>>  +
>>  +			reset-gpios = <&gpy4 5 GPIO_ACTIVE_HIGH>;
>>  +			power-on-delay = <10>;
>>  +			reset-delay = <10>;
>>  +
>>  +			panel-width-mm = <90>;
>>  +			panel-height-mm = <154>;
>>  +
>>  +			display-timings {
>>  +				timing {
>>  +					clock-frequency = <23492370>;
>>  +					hactive = <480>;
>>  +					vactive = <800>;
>>  +					hback-porch = <16>;
>>  +					hfront-porch = <16>;
>>  +					vback-porch = <2>;
>>  +					vfront-porch = <28>;
>>  +					hsync-len = <2>;
>>  +					vsync-len = <1>;
>>  +					hsync-active = <0>;
>>  +					vsync-active = <0>;
>>  +					de-active = <0>;
>>  +					pixelclk-active = <0>;
>>  +				};
>>  +			};
>>  +
>>  +			port {
>>  +				lcd_ep: endpoint {
>>  +					remote-endpoint = <&fimd_dpi_ep>;
>>  +				};
>>  +			};
>>  +		};
>>  +	};
>>  +
>>  +	fixed-rate-clocks {
>>  +		xxti {
>>  +			compatible = "samsung,clock-xxti";
>>  +			clock-frequency = <0>;
>>  +		};
>>  +
>>  +		xusbxti {
>>  +			compatible = "samsung,clock-xusbxti";
>>  +			clock-frequency = <24000000>;
>>  +		};
>>  +	};
>>  +
>>  +	thermal-zones {
>>  +		cpu_thermal: cpu-thermal {
>>  +			cooling-maps {
>>  +				map0 {
>>  +					/* Corresponds to 800MHz */
>>  +					cooling-device = <&cpu0 2 2>;
>>  +				};
>>  +				map1 {
>>  +					/* Corresponds to 200MHz */
>>  +					cooling-device = <&cpu0 4 4>;
>>  +				};
>>  +			};
>>  +		};
>>  +	};
>>  +
>>  +};
>>  +
>>  +&camera {
>>  +	status = "okay";
>>  +};
>>  +
>>  +&cpu0 {
>>  +	cpu0-supply = <&varm_breg>;
>>  +};
>>  +
>>  +&exynos_usbphy {
>>  +	status = "okay";
>>  +
>>  +	vbus-supply = <&safe1_sreg>;
>>  +};
>>  +
>>  +&fimc_0 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock 
>> CLK_SCLK_FIMC0>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_1 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock 
>> CLK_SCLK_FIMC1>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_2 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock 
>> CLK_SCLK_FIMC2>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_3 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock 
>> CLK_SCLK_FIMC3>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimd {
>>  +	status = "okay";
>>  +	#address-cells = <1>;
>>  +	#size-cells = <0>;
>>  +
>>  +	samsung,invert-vden;
>>  +	samsung,invert-vclk;
>>  +
>>  +	pinctrl-0 = <&lcd_clk>, <&lcd_data24>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	port@3 {
>>  +		reg = <3>;
>>  +
>>  +		fimd_dpi_ep: endpoint {
>>  +			remote-endpoint = <&lcd_ep>;
>>  +		};
>>  +	};
>>  +};
>>  +
>>  +&hsotg {
>>  +	status = "okay";
>>  +
>>  +	dr_mode = "otg";
> The the host mode really works with DWC2? Afair the host mode worked
> only when it was rerouted by PHY to the EHCI (at least on 
> UniversalC210
> board, which is very similar to GS2).

I have to say that I didn't try OTG / host, but it does work fine in 
gadget mode.
I can change it to dr_mode = "peripheral" for now, if you prefer.

>>  +	vusb_d-supply = <&vusb_reg>;
>>  +	vusb_a-supply = <&vusbdac_reg>;
>>  +};
>>  +
>>  +&ehci {
>>  +	status = "okay";
>>  +
>>  +	port@0 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@1 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@2 {
>>  +		status = "okay";
>>  +	};
>>  +};
> 
> This is the old, obsolete exynos-ehci binding. Please remove 'port'
> nodes and use standard USB phy bindings like it has been done in the
> following patch: https://patchwork.kernel.org/patch/11060465/

Will do.

> BTW, do you really need all 3 phys/ports?

No idea!

>>  +
>>  +
>>  +&i2c_3 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <100000>;
>>  +
>>  +	pinctrl-0 = <&i2c3_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	mxt224-touchscreen@4a {
>>  +		compatible = "atmel,maxtouch";
>>  +		reg = <0x4a>;
>>  +
>>  +		interrupt-parent = <&gpx0>;
>>  +		interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>>  +
>>  +		avdd-supply = <&tsp_reg>;
>>  +		vdd-supply = <&tsp_reg>;
>>  +	};
>>  +};
>>  +
>>  +&i2c_7 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <400000>;
>>  +
>>  +	pinctrl-0 = <&i2c7_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	ak8975@c {
>>  +		compatible = "asahi-kasei,ak8975";
>>  +		reg = <0x0c>;
>>  +
>>  +		gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
>>  +	};
>>  +};
>>  +
>>  +
>>  +&i2c_5 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <100000>;
>>  +
>>  +	pinctrl-0 = <&i2c5_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	max8997_pmic@66 {
>>  +		compatible = "maxim,max8997-pmic";
>>  +		reg = <0x66>;
>>  +
>>  +		interrupts-extended = <&gpx0 7 0>, <&gpx2 3 2>;
>>  +		interrupt-parent = <&gpx0>;
>>  +		interrupts = <7 IRQ_TYPE_NONE>;
>>  +
>>  +		otg_en-gpios = <&gpx3 3 0>;
>>  +		usb_sel-gpios = <&gpl0 6 0>;
>>  +
>>  +		max8997,pmic-buck1-uses-gpio-dvs;
>>  +		max8997,pmic-buck2-uses-gpio-dvs;
>>  +		max8997,pmic-buck5-uses-gpio-dvs;
>>  +
>>  +		max8997,pmic-ignore-gpiodvs-side-effect;
>>  +		max8997,pmic-buck125-default-dvs-idx = <0>;
>>  +
>>  +		max8997,pmic-buck125-dvs-gpios = <&gpx0 5 GPIO_ACTIVE_HIGH>,
>>  +						 <&gpx0 6 GPIO_ACTIVE_HIGH>,
>>  +						 <&gpl0 0 GPIO_ACTIVE_HIGH>;
>>  +
>>  +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
>>  +						 <1250000>, <1200000>,
>>  +						 <1150000>, <1100000>,
>>  +						 <1000000>, <950000>;
>>  +
>>  +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1000000>,
>>  +						 <950000>,  <900000>,
>>  +						 <1100000>, <1000000>,
>>  +						 <950000>,  <900000>;
>>  +
>>  +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>;
>>  +
>>  +		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
>>  +		pinctrl-names = "default";
>>  +
>>  +		regulators {
>>  +			vadc_reg: LDO1 {
>>  +				regulator-name = "VADC_3.3V_C210";
>>  +				regulator-min-microvolt = <3300000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +				regulator-always-on;
>>  +
>>  +			};
>>  +			valive_reg: LDO2 {
>>  +				regulator-name = "VALIVE_1.1V_C210";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +				regulator-always-on;
>>  +
>>  +			};
>>  +
>>  +			vusb_reg: LDO3 {
>>  +				regulator-name = "VUSB_1.1V_C210";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +			};
>>  +
>>  +			vmipi_reg: LDO4 {
>>  +				regulator-name = "VMIPI_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vhsic_reg: LDO5 {
>>  +				regulator-name = "VHSIC_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vpda_reg: LDO6 {
>>  +				regulator-name = "VCC_1.8V_PDA";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vcam_reg: LDO7 {
>>  +				regulator-name = "CAM_ISP_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vusbdac_reg: LDO8 {
>>  +				regulator-name = "VUSB+VDAC_3.3V_C210";
>>  +				regulator-min-microvolt = <3300000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +			};
>>  +
>>  +			vccpda_reg: LDO9 {
>>  +				regulator-name = "VCC_2.8V_PDA";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vtouch_reg: LDO11 {
>>  +				regulator-name = "TOUCH_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vpll_reg: LDO10 {
>>  +				regulator-name = "VPLL_1.1V";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vtcam_reg: LDO12 {
>>  +				regulator-name = "VT_CAM_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vcclcd_reg: LDO13 {
>>  +				regulator-name = "VCC_3.0V_LCD";
>>  +				regulator-min-microvolt = <3000000>;
>>  +				regulator-max-microvolt = <3000000>;
>>  +			};
>>  +
>>  +			vmotor_reg: LDO14 {
>>  +				regulator-name = "VCC_2.8V_MOTOR";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			vled_reg: LDO15 {
>>  +				regulator-name = "LED_A_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			camsensor_reg: LDO16 {
>>  +				regulator-name = "CAM_SENSOR_IO_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vtf_reg: LDO17 {
>>  +				regulator-name = "VTF_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			vtouchled_reg: LDO18 {
>>  +				regulator-name = "TOUCH_LED_3.3V";
>>  +				regulator-min-microvolt = <2500000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +			};
>>  +
>>  +			vddq_reg: LDO21 {
>>  +				regulator-name = "VDDQ_M1M2_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			varm_breg: BUCK1 {
>>  +				//regulator-name = "VARM_1.2V_C210";
>>  +				regulator-name = "vdd_arm";
>>  +				regulator-min-microvolt = <65000>;
>>  +				regulator-max-microvolt = <2225000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vint_breg: BUCK2 {
>>  +				regulator-name = "VINT_1.1V_C210";
>>  +				regulator-min-microvolt = <65000>;
>>  +				regulator-max-microvolt = <2225000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vg3d_breg: BUCK3 {
>>  +				regulator-name = "G3D_1.1V";
>>  +				regulator-min-microvolt = <900000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			camisp_breg: BUCK4 {
>>  +				regulator-name = "CAM_ISP_CORE_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +			};
>>  +
>>  +			vmem_breg: BUCK5 {
>>  +				regulator-name = "VMEM_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vccsub_breg: BUCK7 {
>>  +				regulator-name = "VCC_SUB_2.0V";
>>  +				regulator-min-microvolt = <2000000>;
>>  +				regulator-max-microvolt = <2000000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			safe1_sreg: ESAFEOUT1 {
>>  +				regulator-name = "SAFEOUT1";
>>  +			};
>>  +
>>  +			safe2_sreg: ESAFEOUT2 {
>>  +				regulator-name = "SAFEOUT2";
>>  +				regulator-boot-on;
>>  +			};
>>  +
>>  +			charger_reg: CHARGER {
>>  +				regulator-name = "CHARGER";
>>  +				regulator-min-microamp = <60000>;
>>  +				regulator-max-microamp = <2580000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			chargercv_reg: CHARGER_CV {
>>  +				regulator-name = "CHARGER_CV";
>>  +				regulator-min-microvolt = <3800000>;
>>  +				regulator-max-microvolt = <4100000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +		};
>>  +
>>  +	};
>>  +};
>>  +
>>  +
>>  +&gpu {
>>  +	status = "okay";
>>  +
>>  +	mali-supply = <&vg3d_breg>;
>>  +	regulator-microvolt-offset = <50000>;
>>  +	regulator-microsecs-delay = <50>;
>>  +};
>>  +
>>  +
>>  +&sdhci_0 {
>>  +	status = "okay";
>>  +
>>  +	bus-width = <8>;
>>  +	non-removable;
>>  +	vmmc-supply = <&vemmc_reg>;
>>  +
>>  +	pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_bus8>;
>>  +	pinctrl-names = "default";
>>  +};
>>  +
>>  +&sdhci_2 {
>>  +	status = "okay";
>>  +
>>  +	bus-width = <4>;
>>  +	cd-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>;
>>  +	vmmc-supply = <&vtf_reg>;
>>  +
>>  +	pinctrl-0 = <&sd2_clk>, <&sd2_cmd>, <&sd2_bus4>;
>>  +	pinctrl-names = "default";
>>  +};
>>  +
>>  +&ehci {
>>  +	port@0 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@1 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@2 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +};
> 
> This is duplicated 'ehci' node, probable an artifact from the earlier
> rebase. Please remove it.

Right, didn't even notice it.

Will remove it and send a V2.

Cheers,
-Paul

>>  +
>>  +
>>  +&sdhci_3 {
>>  +	status = "okay";
>>  +
>>  +	#address-cells = <1>;
>>  +	#size-cells = <0>;
>>  +
>>  +	non-removable;
>>  +	bus-width = <4>;
>>  +	mmc-pwrseq = <&wlan_pwrseq>;
>>  +	vmmc-supply = <&vtf_reg>;
>>  +
>>  +	pinctrl-names = "default";
>>  +	pinctrl-0 = <&sd3_clk>, <&sd3_cmd>, <&sd3_bus4>;
>>  +
>>  +	brcmf: wifi@1 {
>>  +		compatible = "brcm,bcm4330-fmac";
>>  +		reg = <1>;
>>  +
>>  +		interrupt-parent = <&gpx2>;
>>  +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>>  +		interrupt-names = "host-wake";
>>  +	};
>>  +};
>>  +
>>  +
>>  +&serial_0 {
>>  +	status = "okay";
>>  +
>>  +	pinctrl-names = "default";
>>  +	pinctrl-0 = <&bt_en>, <&bt_res>, <&uart0_data>, <&uart0_fctl>;
>>  +
>>  +	bluetooth {
>>  +		compatible = "brcm,bcm4330-bt";
>>  +
>>  +		shutdown-gpios = <&gpl0 4 GPIO_ACTIVE_HIGH>;
>>  +		reset-gpios = <&gpl1 0 GPIO_ACTIVE_HIGH>;
>>  +		device-wakeup-gpios = <&gpx3 1 GPIO_ACTIVE_HIGH>;
>>  +		host-wakeup-gpios = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>  +	};
>>  +};
>>  +
>>  +&serial_1 {
>>  +	status = "okay";
>>  +};
>>  +
>>  +&serial_2 {
>>  +	status = "okay";
>>  +};
>>  +
>>  +&serial_3 {
>>  +	status = "okay";
>>  +};
>>  +
>>  +&tmu {
>>  +	status = "okay";
>>  +};
>>  +
>>  +
>>  +&pinctrl_0 {
>>  +	pinctrl-names = "default";
>>  +	pinctrl-0 = <&sleep0>;
>>  +
>>  +
>>  +	sleep0: sleep-states {
>>  +		gpa0-0 {
>>  +			samsung,pins = "gpa0-0";
>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>  +		};
>>  +
>>  +		gpa0-1 {
>>  +			samsung,pins = "gpa0-1";
>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT0>;
>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>  +		};
>>  +
>>  +		gpa0-2 {
>>  +			samsung,pins = "gpa0-2";
>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>  +		};
>>  +
>>  +		gpa0-3 {
>>  +			samsung,pins = "gpa0-3";
>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT1>;
>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>  +		};
>>  +	};
>>  +};
>>  +
>>  +&pinctrl_1 {
>>  +	mhl_int: mhl-int {
>>  +		samsung,pins = "gpf3-5";
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +	};
>>  +
>>  +	i2c_mhl_bus: i2c-mhl-bus {
>>  +		samsung,pins = "gpf0-4", "gpf0-6";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_DOWN>;
>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>  +	};
>>  +
>>  +	usb_sel: usb-sel {
>>  +		samsung,pins = "gpl0-6";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>  +		samsung,pin-val = <0>;
>>  +	};
>>  +
>>  +	bt_en: bt-en {
>>  +		samsung,pins = "gpl0-4";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
>>  +		samsung,pin-val = <0>;
>>  +	};
>>  +
>>  +	bt_res: bt-res {
>>  +		samsung,pins = "gpl1-0";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
>>  +		samsung,pin-val = <0>;
>>  +	};
>>  +
>>  +
>>  +	otg_gp: otg-gp {
>>  +		samsung,pins = "gpx3-3";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>  +		samsung,pin-val = <0>;
>>  +	};
>>  +
>>  +	mag_mhl_gpio: mag-mhl-gpio {
>>  +		samsung,pins = "gpd0-2";
>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_3>;
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +	};
>>  +
>>  +
>>  +	max8997_irq: max8997-irq {
>>  +		samsung,pins = "gpx0-7";
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +	};
>>  +
>>  +	max17042_fuel_irq: max17042-fuel-irq {
>>  +		samsung,pins = "gpx2-3";
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>  +	};
>>  +
>>  +	tsp224_irq: tsp224-irq {
>>  +		samsung,pins = "gpx0-4";
>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>>  +	};
>>  +};
> 
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Paul Cercueil March 13, 2020, 2:33 p.m. UTC | #4

Hi Krzysztof,


Le ven., mars 13, 2020 at 10:00, Krzysztof Kozlowski <krzk@kernel.org> 
a écrit :
> On Thu, Mar 12, 2020 at 04:34:11PM +0100, Paul Cercueil wrote:
>>  From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>> 
>>  Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 
>> version).
> 
> Thanks for the patch!
> 
> It is an GT-I9100.
> 
> +Cc Marek (I have few questions further)
> 
> 
>> 
>>  Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   arch/arm/boot/dts/Makefile             |   1 +
>>   arch/arm/boot/dts/exynos4210-i9100.dts | 816 
>> +++++++++++++++++++++++++
>>   2 files changed, 817 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
>> 
>>  diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>  index d6546d2676b9..522436d30690 100644
>>  --- a/arch/arm/boot/dts/Makefile
>>  +++ b/arch/arm/boot/dts/Makefile
>>  @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
>>   	exynos3250-monk.dtb \
>>   	exynos3250-rinato.dtb
>>   dtb-$(CONFIG_ARCH_EXYNOS4) += \
>>  +	exynos4210-i9100.dtb \
>>   	exynos4210-origen.dtb \
>>   	exynos4210-smdkv310.dtb \
>>   	exynos4210-trats.dtb \
>>  diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts 
>> b/arch/arm/boot/dts/exynos4210-i9100.dts
>>  new file mode 100644
>>  index 000000000000..c218e34959be
>>  --- /dev/null
>>  +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
>>  @@ -0,0 +1,816 @@
>>  +// SPDX-License-Identifier: GPL-2.0
>>  +/*
>>  + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device tree
> 
> Ditto
> 
>>  + *
>>  + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>>  + *		http://www.samsung.com
>>  + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>  + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
>>  + *
>>  + * Device tree source file for Samsung's Galaxy S2 smartphone 
>> (i9100 version),
>>  + * which is based on Samsung's Exynos4210 SoC.
> 
> Duplicated description, you mentioned it before.
> 
>>  + */
>>  +
>>  +/dts-v1/;
>>  +#include "exynos4210.dtsi"
>>  +#include "exynos4412-ppmu-common.dtsi"
>>  +
>>  +#include <dt-bindings/gpio/gpio.h>
>>  +#include <dt-bindings/input/linux-event-codes.h>
>>  +
>>  +/ {
>>  +	model = "Samsung Galaxy S2 (i9100)";
> 
> GT-I9100
> 
>>  +	compatible = "samsung,i9100", "samsung,exynos4210", 
>> "samsung,exynos4";
> 
> I think it shares a lot with Trats board...
> 
> Marek, how similar these
> designs are? Maybe it is worth to split some parts and make common 
> part?

I can try to do it, but it'll be only by comparing the devicetree files.

>>  +
>>  +
>>  +	memory@40000000 {
>>  +		device_type = "memory";
>>  +		reg = <0x40000000 0x40000000>;
>>  +	};
>>  +
> 
> You do not have a chosen node, at least for stdout serial selection.
> Probably you should have one... bootargs I guess could come from
> bootloader.

Problem is, I have no idea which UART is available via the USB jig :)

>>  +	vemmc_reg: regulator-0 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "VMEM_VDD_2.8V";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
> 
> This looks like a stub/fake regulator. Probably it shoould be replaced
> with proper regulator coming from PMIC (few seems to be missing 
> there).
> 
> I don't have the schematics so I cannot judge... I see Trats has the
> same.
> 
> Marek, mayybe you know, is it really a separate regulator?
> 
>>  +	};
>>  +
>>  +	tsp_reg: regulator-1 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "TSP_FIXED_VOLTAGES";
>>  +		regulator-min-microvolt = <3300000>;
>>  +		regulator-max-microvolt = <3300000>;
>>  +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>  +		startup-delay-us = <70000>;
>>  +		enable-active-high;
>>  +		regulator-boot-on;
>>  +		regulator-always-on;
> 
> always-on and boot-on should not be needed. You have a consumer for 
> this
> regulator.
> 
>>  +	};
>>  +
>>  +	cam_af_28v_reg: regulator-3 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "8M_AF_2.8V_EN";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpk1 1 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	cam_io_en_reg: regulator-4 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "CAM_IO_EN";
>>  +		regulator-min-microvolt = <2800000>;
>>  +		regulator-max-microvolt = <2800000>;
>>  +		gpio = <&gpe2 1 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	cam_io_12v_reg: regulator-5 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "8M_1.2V_EN";
>>  +		regulator-min-microvolt = <1200000>;
>>  +		regulator-max-microvolt = <1200000>;
>>  +		gpio = <&gpe2 5 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	vt_core_15v_reg: regulator-6 {
>>  +		compatible = "regulator-fixed";
>>  +		regulator-name = "VT_CORE_1.5V";
>>  +		regulator-min-microvolt = <1500000>;
>>  +		regulator-max-microvolt = <1500000>;
>>  +		gpio = <&gpe2 2 GPIO_ACTIVE_HIGH>;
>>  +		enable-active-high;
>>  +	};
>>  +
>>  +	gpio-keys {
>>  +		compatible = "gpio-keys";
>>  +
>>  +		vol-down {
>>  +			gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_VOLUMEDOWN>;
>>  +			label = "volume down";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +
>>  +		vol-up {
>>  +			gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_VOLUMEUP>;
>>  +			label = "volume up";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +
>>  +		power {
>>  +			gpios = <&gpx2 7 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_POWER>;
>>  +			label = "power";
>>  +			debounce-interval = <10>;
>>  +			wakeup-source;
>>  +		};
>>  +
>>  +		ok {
>>  +			gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
>>  +			linux,code = <KEY_OK>;
>>  +			label = "ok";
>>  +			debounce-interval = <10>;
>>  +		};
>>  +	};
>>  +
>>  +	wlan_pwrseq: sdhci3-pwrseq {
>>  +		compatible = "mmc-pwrseq-simple";
>>  +		reset-gpios = <&gpl1 2 GPIO_ACTIVE_LOW>;
>>  +	};
>>  +
>>  +	i2c_max17042_fuel: i2c-gpio-3 {
> 
> i2c-gpio-3 -> i2c-gpio
> 
>>  +		compatible = "i2c-gpio";
>>  +		#address-cells = <1>;
>>  +		#size-cells = <0>;
>>  +
>>  +		sda-gpios = <&gpy4 0 GPIO_ACTIVE_HIGH>;
>>  +		scl-gpios = <&gpy4 1 GPIO_ACTIVE_HIGH>;
>>  +		i2c-gpio,delay-us = <5>;
>>  +
>>  +		battery@36 {
>>  +			compatible = "maxim,max17042";
>>  +
>>  +			interrupt-parent = <&gpx2>;
>>  +			interrupts = <3 2>;
> 
> s/2/IRQ_TYPE_EDGE_FALLING/
> 
>>  +
>>  +			pinctrl-0 = <&max17042_fuel_irq>;
>>  +			pinctrl-names = "default";
>>  +
>>  +			reg = <0x36>;
>>  +			maxim,over-heat-temp = <700>;
>>  +			maxim,over-volt = <4500>;
>>  +
> 
> Remove empty line.
> 
>>  +		};
>>  +	};
>>  +
>>  +	spi-lcd {
>>  +		compatible = "spi-gpio";
>>  +		#address-cells = <1>;
>>  +		#size-cells = <0>;
>>  +
>>  +		num-chipselects = <1>;
>>  +		cs-gpios = <&gpy4 3 GPIO_ACTIVE_HIGH>;
>>  +		sck-gpios = <&gpy3 1 GPIO_ACTIVE_HIGH>;
>>  +		mosi-gpios = <&gpy3 3 GPIO_ACTIVE_HIGH>;
>>  +
>>  +		lcd@0 {
>>  +			compatible = "samsung,ld9040";
>>  +			reg = <0>;
>>  +
>>  +			spi-max-frequency = <1200000>;
>>  +			spi-cpol;
>>  +			spi-cpha;
>>  +
>>  +			vdd3-supply = <&vmipi_reg>;
>>  +			vci-supply = <&vcclcd_reg>;
>>  +
>>  +			reset-gpios = <&gpy4 5 GPIO_ACTIVE_HIGH>;
>>  +			power-on-delay = <10>;
>>  +			reset-delay = <10>;
>>  +
>>  +			panel-width-mm = <90>;
>>  +			panel-height-mm = <154>;
>>  +
>>  +			display-timings {
>>  +				timing {
>>  +					clock-frequency = <23492370>;
>>  +					hactive = <480>;
>>  +					vactive = <800>;
>>  +					hback-porch = <16>;
>>  +					hfront-porch = <16>;
>>  +					vback-porch = <2>;
>>  +					vfront-porch = <28>;
>>  +					hsync-len = <2>;
>>  +					vsync-len = <1>;
>>  +					hsync-active = <0>;
>>  +					vsync-active = <0>;
>>  +					de-active = <0>;
>>  +					pixelclk-active = <0>;
>>  +				};
>>  +			};
>>  +
>>  +			port {
>>  +				lcd_ep: endpoint {
>>  +					remote-endpoint = <&fimd_dpi_ep>;
>>  +				};
>>  +			};
>>  +		};
>>  +	};
>>  +
>>  +	fixed-rate-clocks {
>>  +		xxti {
>>  +			compatible = "samsung,clock-xxti";
>>  +			clock-frequency = <0>;
>>  +		};
>>  +
>>  +		xusbxti {
>>  +			compatible = "samsung,clock-xusbxti";
>>  +			clock-frequency = <24000000>;
>>  +		};
>>  +	};
>>  +
>>  +	thermal-zones {
>>  +		cpu_thermal: cpu-thermal {
>>  +			cooling-maps {
>>  +				map0 {
>>  +					/* Corresponds to 800MHz */
>>  +					cooling-device = <&cpu0 2 2>;
>>  +				};
>>  +				map1 {
>>  +					/* Corresponds to 200MHz */
>>  +					cooling-device = <&cpu0 4 4>;
>>  +				};
>>  +			};
>>  +		};
>>  +	};
>>  +
>>  +};
>>  +
>>  +&camera {
>>  +	status = "okay";
>>  +};
>>  +
>>  +&cpu0 {
>>  +	cpu0-supply = <&varm_breg>;
>>  +};
>>  +
>>  +&exynos_usbphy {
>>  +	status = "okay";
>>  +
>>  +	vbus-supply = <&safe1_sreg>;
>>  +};
>>  +
>>  +&fimc_0 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock 
>> CLK_SCLK_FIMC0>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_1 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock 
>> CLK_SCLK_FIMC1>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_2 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock 
>> CLK_SCLK_FIMC2>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimc_3 {
>>  +	status = "okay";
>>  +
>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock 
>> CLK_SCLK_FIMC3>;
>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>  +	assigned-clock-rates = <0>, <160000000>;
>>  +};
>>  +
>>  +&fimd {
>>  +	status = "okay";
>>  +	#address-cells = <1>;
>>  +	#size-cells = <0>;
>>  +
>>  +	samsung,invert-vden;
>>  +	samsung,invert-vclk;
>>  +
>>  +	pinctrl-0 = <&lcd_clk>, <&lcd_data24>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	port@3 {
>>  +		reg = <3>;
>>  +
>>  +		fimd_dpi_ep: endpoint {
>>  +			remote-endpoint = <&lcd_ep>;
>>  +		};
>>  +	};
>>  +};
>>  +
>>  +&hsotg {
>>  +	status = "okay";
>>  +
>>  +	dr_mode = "otg";
>>  +	vusb_d-supply = <&vusb_reg>;
>>  +	vusb_a-supply = <&vusbdac_reg>;
>>  +};
>>  +
>>  +&ehci {
> 
> Order the phandle/nodes in alphabetical order, please. So:
> 
> &ehci ...
> &hsoth ...
> &i2c ...
> 
> etc.
> 
>>  +	status = "okay";
>>  +
>>  +	port@0 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@1 {
>>  +		status = "okay";
>>  +	};
>>  +
>>  +	port@2 {
>>  +		status = "okay";
>>  +	};
>>  +};
>>  +
> 
> Double empty line.
> 
>>  +
>>  +&i2c_3 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <100000>;
>>  +
>>  +	pinctrl-0 = <&i2c3_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	mxt224-touchscreen@4a {
>>  +		compatible = "atmel,maxtouch";
>>  +		reg = <0x4a>;
>>  +
>>  +		interrupt-parent = <&gpx0>;
>>  +		interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>>  +
>>  +		avdd-supply = <&tsp_reg>;
>>  +		vdd-supply = <&tsp_reg>;
>>  +	};
>>  +};
>>  +
>>  +&i2c_7 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <400000>;
>>  +
>>  +	pinctrl-0 = <&i2c7_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	ak8975@c {
>>  +		compatible = "asahi-kasei,ak8975";
>>  +		reg = <0x0c>;
>>  +
>>  +		gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
>>  +	};
>>  +};
>>  +
>>  +
>>  +&i2c_5 {
>>  +	status = "okay";
>>  +
>>  +	samsung,i2c-sda-delay = <100>;
>>  +	samsung,i2c-slave-addr = <0x10>;
>>  +	samsung,i2c-max-bus-freq = <100000>;
>>  +
>>  +	pinctrl-0 = <&i2c5_bus>;
>>  +	pinctrl-names = "default";
>>  +
>>  +	max8997_pmic@66 {
>>  +		compatible = "maxim,max8997-pmic";
>>  +		reg = <0x66>;
>>  +
>>  +		interrupts-extended = <&gpx0 7 0>, <&gpx2 3 2>;
>>  +		interrupt-parent = <&gpx0>;
>>  +		interrupts = <7 IRQ_TYPE_NONE>;
>>  +
>>  +		otg_en-gpios = <&gpx3 3 0>;
>>  +		usb_sel-gpios = <&gpl0 6 0>;
>>  +
>>  +		max8997,pmic-buck1-uses-gpio-dvs;
>>  +		max8997,pmic-buck2-uses-gpio-dvs;
>>  +		max8997,pmic-buck5-uses-gpio-dvs;
>>  +
>>  +		max8997,pmic-ignore-gpiodvs-side-effect;
>>  +		max8997,pmic-buck125-default-dvs-idx = <0>;
>>  +
>>  +		max8997,pmic-buck125-dvs-gpios = <&gpx0 5 GPIO_ACTIVE_HIGH>,
>>  +						 <&gpx0 6 GPIO_ACTIVE_HIGH>,
>>  +						 <&gpl0 0 GPIO_ACTIVE_HIGH>;
>>  +
>>  +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
>>  +						 <1250000>, <1200000>,
>>  +						 <1150000>, <1100000>,
>>  +						 <1000000>, <950000>;
>>  +
>>  +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1000000>,
>>  +						 <950000>,  <900000>,
>>  +						 <1100000>, <1000000>,
>>  +						 <950000>,  <900000>;
>>  +
>>  +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>,
>>  +						 <1200000>, <1200000>;
>>  +
>>  +		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
>>  +		pinctrl-names = "default";
>>  +
>>  +		regulators {
>>  +			vadc_reg: LDO1 {
>>  +				regulator-name = "VADC_3.3V_C210";
>>  +				regulator-min-microvolt = <3300000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +				regulator-always-on;
>>  +
>>  +			};
> 
> Add empty line.
> 
>>  +			valive_reg: LDO2 {
>>  +				regulator-name = "VALIVE_1.1V_C210";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +				regulator-always-on;
>>  +
>>  +			};
>>  +
>>  +			vusb_reg: LDO3 {
>>  +				regulator-name = "VUSB_1.1V_C210";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +			};
>>  +
>>  +			vmipi_reg: LDO4 {
>>  +				regulator-name = "VMIPI_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vhsic_reg: LDO5 {
>>  +				regulator-name = "VHSIC_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vpda_reg: LDO6 {
>>  +				regulator-name = "VCC_1.8V_PDA";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vcam_reg: LDO7 {
>>  +				regulator-name = "CAM_ISP_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vusbdac_reg: LDO8 {
>>  +				regulator-name = "VUSB+VDAC_3.3V_C210";
>>  +				regulator-min-microvolt = <3300000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +			};
>>  +
>>  +			vccpda_reg: LDO9 {
>>  +				regulator-name = "VCC_2.8V_PDA";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vtouch_reg: LDO11 {
>>  +				regulator-name = "TOUCH_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vpll_reg: LDO10 {
>>  +				regulator-name = "VPLL_1.1V";
>>  +				regulator-min-microvolt = <1100000>;
>>  +				regulator-max-microvolt = <1100000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vtcam_reg: LDO12 {
>>  +				regulator-name = "VT_CAM_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vcclcd_reg: LDO13 {
>>  +				regulator-name = "VCC_3.0V_LCD";
>>  +				regulator-min-microvolt = <3000000>;
>>  +				regulator-max-microvolt = <3000000>;
>>  +			};
>>  +
>>  +			vmotor_reg: LDO14 {
>>  +				regulator-name = "VCC_2.8V_MOTOR";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			vled_reg: LDO15 {
>>  +				regulator-name = "LED_A_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			camsensor_reg: LDO16 {
>>  +				regulator-name = "CAM_SENSOR_IO_1.8V";
>>  +				regulator-min-microvolt = <1800000>;
>>  +				regulator-max-microvolt = <1800000>;
>>  +			};
>>  +
>>  +			vtf_reg: LDO17 {
>>  +				regulator-name = "VTF_2.8V";
>>  +				regulator-min-microvolt = <2800000>;
>>  +				regulator-max-microvolt = <2800000>;
>>  +			};
>>  +
>>  +			vtouchled_reg: LDO18 {
>>  +				regulator-name = "TOUCH_LED_3.3V";
>>  +				regulator-min-microvolt = <2500000>;
>>  +				regulator-max-microvolt = <3300000>;
>>  +			};
>>  +
>>  +			vddq_reg: LDO21 {
>>  +				regulator-name = "VDDQ_M1M2_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			varm_breg: BUCK1 {
>>  +				//regulator-name = "VARM_1.2V_C210";
>>  +				regulator-name = "vdd_arm";
>>  +				regulator-min-microvolt = <65000>;
>>  +				regulator-max-microvolt = <2225000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vint_breg: BUCK2 {
>>  +				regulator-name = "VINT_1.1V_C210";
>>  +				regulator-min-microvolt = <65000>;
>>  +				regulator-max-microvolt = <2225000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vg3d_breg: BUCK3 {
>>  +				regulator-name = "G3D_1.1V";
>>  +				regulator-min-microvolt = <900000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			camisp_breg: BUCK4 {
>>  +				regulator-name = "CAM_ISP_CORE_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +			};
>>  +
>>  +			vmem_breg: BUCK5 {
>>  +				regulator-name = "VMEM_1.2V";
>>  +				regulator-min-microvolt = <1200000>;
>>  +				regulator-max-microvolt = <1200000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			vccsub_breg: BUCK7 {
>>  +				regulator-name = "VCC_SUB_2.0V";
>>  +				regulator-min-microvolt = <2000000>;
>>  +				regulator-max-microvolt = <2000000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			safe1_sreg: ESAFEOUT1 {
>>  +				regulator-name = "SAFEOUT1";
>>  +			};
>>  +
>>  +			safe2_sreg: ESAFEOUT2 {
>>  +				regulator-name = "SAFEOUT2";
>>  +				regulator-boot-on;
>>  +			};
>>  +
>>  +			charger_reg: CHARGER {
>>  +				regulator-name = "CHARGER";
>>  +				regulator-min-microamp = <60000>;
>>  +				regulator-max-microamp = <2580000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
>>  +			chargercv_reg: CHARGER_CV {
>>  +				regulator-name = "CHARGER_CV";
>>  +				regulator-min-microvolt = <3800000>;
>>  +				regulator-max-microvolt = <4100000>;
>>  +				regulator-always-on;
>>  +			};
>>  +
> 
> Empty line, not needed.
> 
>>  +		};
>>  +
> 
> Empty line, not needed.
> 
>>  +	};
>>  +};
>>  +
>>  +
> 
> Empty line, not needed. Please fix them in all places.
> 
>>  +&gpu {
>>  +	status = "okay";
>>  +
>>  +	mali-supply = <&vg3d_breg>;
>>  +	regulator-microvolt-offset = <50000>;
>>  +	regulator-microsecs-delay = <50>;
>>  +};
>>  +
> 
> and more...
> 
>>  +
>>  +&sdhci_0 {
>>  +	status = "okay";
>>  +
>>  +	bus-width = <8>;
>>  +	non-removable;
>>  +	vmmc-supply = <&vemmc_reg>;
>>  +
>>  +	pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_bus8>;
>>  +	pinctrl-names = "default";
>>  +};
>>  +
>>  +&sdhci_2 {
>>  +	status = "okay";
>>  +
>>  +	bus-width = <4>;
>>  +	cd-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>;
>>  +	vmmc-supply = <&vtf_reg>;
>>  +
>>  +	pinctrl-0 = <&sd2_clk>, <&sd2_cmd>, <&sd2_bus4>;
>>  +	pinctrl-names = "default";
>>  +};
>>  +
>>  +&ehci {
> 
> Duplicated.
> 
> Best regards,
> Krzysztof

For all other comments: ACK, will fix this mess in V2.

Cheers,
-Paul

Paul Cercueil March 13, 2020, 2:36 p.m. UTC | #5

Le ven., mars 13, 2020 at 15:29, Paul Cercueil <paul@crapouillou.net> a 
écrit :
> Hi Marek,
> 
> 
> Le ven., mars 13, 2020 at 09:03, Marek Szyprowski 
> <m.szyprowski@samsung.com> a écrit :
>> 
>> On 12.03.2020 16:34, Paul Cercueil wrote:
>>>  From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>> 
>>>  Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 
>>> version).
>>> 
>>>  Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>  ---
>>>    arch/arm/boot/dts/Makefile             |   1 +
>>>    arch/arm/boot/dts/exynos4210-i9100.dts | 816 
>>> +++++++++++++++++++++++++
>>>    2 files changed, 817 insertions(+)
>>>    create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
>>> 
>>>  diff --git a/arch/arm/boot/dts/Makefile 
>>> b/arch/arm/boot/dts/Makefile
>>>  index d6546d2676b9..522436d30690 100644
>>>  --- a/arch/arm/boot/dts/Makefile
>>>  +++ b/arch/arm/boot/dts/Makefile
>>>  @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
>>>    	exynos3250-monk.dtb \
>>>    	exynos3250-rinato.dtb
>>>    dtb-$(CONFIG_ARCH_EXYNOS4) += \
>>>  +	exynos4210-i9100.dtb \
>>>    	exynos4210-origen.dtb \
>>>    	exynos4210-smdkv310.dtb \
>>>    	exynos4210-trats.dtb \
>>>  diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts 
>>> b/arch/arm/boot/dts/exynos4210-i9100.dts
>>>  new file mode 100644
>>>  index 000000000000..c218e34959be
>>>  --- /dev/null
>>>  +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
>>>  @@ -0,0 +1,816 @@
>>>  +// SPDX-License-Identifier: GPL-2.0
>>>  +/*
>>>  + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device 
>>> tree
>>>  + *
>>>  + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>>>  + *		http://www.samsung.com
>>>  + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>>  + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
>>>  + *
>>>  + * Device tree source file for Samsung's Galaxy S2 smartphone 
>>> (i9100 version),
>>>  + * which is based on Samsung's Exynos4210 SoC.
>>>  + */
>>>  +
>>>  +/dts-v1/;
>>>  +#include "exynos4210.dtsi"
>>>  +#include "exynos4412-ppmu-common.dtsi"
>>>  +
>>>  +#include <dt-bindings/gpio/gpio.h>
>>>  +#include <dt-bindings/input/linux-event-codes.h>
>>>  +
>>>  +/ {
>>>  +	model = "Samsung Galaxy S2 (i9100)";
>>>  +	compatible = "samsung,i9100", "samsung,exynos4210", 
>>> "samsung,exynos4";
>>>  +
>>>  +
>>>  +	memory@40000000 {
>>>  +		device_type = "memory";
>>>  +		reg = <0x40000000 0x40000000>;
>>>  +	};
>>>  +
>>>  +	vemmc_reg: regulator-0 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "VMEM_VDD_2.8V";
>>>  +		regulator-min-microvolt = <2800000>;
>>>  +		regulator-max-microvolt = <2800000>;
>>>  +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>>>  +		enable-active-high;
>>>  +	};
>>>  +
>>>  +	tsp_reg: regulator-1 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "TSP_FIXED_VOLTAGES";
>>>  +		regulator-min-microvolt = <3300000>;
>>>  +		regulator-max-microvolt = <3300000>;
>>>  +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>  +		startup-delay-us = <70000>;
>>>  +		enable-active-high;
>>>  +		regulator-boot-on;
>>>  +		regulator-always-on;
>>>  +	};
>>>  +
>>>  +	cam_af_28v_reg: regulator-3 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "8M_AF_2.8V_EN";
>>>  +		regulator-min-microvolt = <2800000>;
>>>  +		regulator-max-microvolt = <2800000>;
>>>  +		gpio = <&gpk1 1 GPIO_ACTIVE_HIGH>;
>>>  +		enable-active-high;
>>>  +	};
>>>  +
>>>  +	cam_io_en_reg: regulator-4 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "CAM_IO_EN";
>>>  +		regulator-min-microvolt = <2800000>;
>>>  +		regulator-max-microvolt = <2800000>;
>>>  +		gpio = <&gpe2 1 GPIO_ACTIVE_HIGH>;
>>>  +		enable-active-high;
>>>  +	};
>>>  +
>>>  +	cam_io_12v_reg: regulator-5 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "8M_1.2V_EN";
>>>  +		regulator-min-microvolt = <1200000>;
>>>  +		regulator-max-microvolt = <1200000>;
>>>  +		gpio = <&gpe2 5 GPIO_ACTIVE_HIGH>;
>>>  +		enable-active-high;
>>>  +	};
>>>  +
>>>  +	vt_core_15v_reg: regulator-6 {
>>>  +		compatible = "regulator-fixed";
>>>  +		regulator-name = "VT_CORE_1.5V";
>>>  +		regulator-min-microvolt = <1500000>;
>>>  +		regulator-max-microvolt = <1500000>;
>>>  +		gpio = <&gpe2 2 GPIO_ACTIVE_HIGH>;
>>>  +		enable-active-high;
>>>  +	};
>>>  +
>>>  +	gpio-keys {
>>>  +		compatible = "gpio-keys";
>>>  +
>>>  +		vol-down {
>>>  +			gpios = <&gpx2 1 GPIO_ACTIVE_LOW>;
>>>  +			linux,code = <KEY_VOLUMEDOWN>;
>>>  +			label = "volume down";
>>>  +			debounce-interval = <10>;
>>>  +		};
>>>  +
>>>  +		vol-up {
>>>  +			gpios = <&gpx2 0 GPIO_ACTIVE_LOW>;
>>>  +			linux,code = <KEY_VOLUMEUP>;
>>>  +			label = "volume up";
>>>  +			debounce-interval = <10>;
>>>  +		};
>>>  +
>>>  +		power {
>>>  +			gpios = <&gpx2 7 GPIO_ACTIVE_LOW>;
>>>  +			linux,code = <KEY_POWER>;
>>>  +			label = "power";
>>>  +			debounce-interval = <10>;
>>>  +			wakeup-source;
>>>  +		};
>>>  +
>>>  +		ok {
>>>  +			gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
>>>  +			linux,code = <KEY_OK>;
>>>  +			label = "ok";
>>>  +			debounce-interval = <10>;
>>>  +		};
>>>  +	};
>>>  +
>>>  +	wlan_pwrseq: sdhci3-pwrseq {
>>>  +		compatible = "mmc-pwrseq-simple";
>>>  +		reset-gpios = <&gpl1 2 GPIO_ACTIVE_LOW>;
>>>  +	};
>>>  +
>>>  +	i2c_max17042_fuel: i2c-gpio-3 {
>>>  +		compatible = "i2c-gpio";
>>>  +		#address-cells = <1>;
>>>  +		#size-cells = <0>;
>>>  +
>>>  +		sda-gpios = <&gpy4 0 GPIO_ACTIVE_HIGH>;
>>>  +		scl-gpios = <&gpy4 1 GPIO_ACTIVE_HIGH>;
>>>  +		i2c-gpio,delay-us = <5>;
>>>  +
>>>  +		battery@36 {
>>>  +			compatible = "maxim,max17042";
>>>  +
>>>  +			interrupt-parent = <&gpx2>;
>>>  +			interrupts = <3 2>;
>>>  +
>>>  +			pinctrl-0 = <&max17042_fuel_irq>;
>>>  +			pinctrl-names = "default";
>>>  +
>>>  +			reg = <0x36>;
>>>  +			maxim,over-heat-temp = <700>;
>>>  +			maxim,over-volt = <4500>;
>>>  +
>>>  +		};
>>>  +	};
>>>  +
>>>  +	spi-lcd {
>>>  +		compatible = "spi-gpio";
>>>  +		#address-cells = <1>;
>>>  +		#size-cells = <0>;
>>>  +
>>>  +		num-chipselects = <1>;
>>>  +		cs-gpios = <&gpy4 3 GPIO_ACTIVE_HIGH>;
>> LD9040 CS is active low, so this board needs probably the same fix a
>> Universal C210 board: https://patchwork.kernel.org/patch/11420231/ 
>> Are
>> you sure it really works now with v5.6-rcX kernels? Or you just got 
>> the
>> panel working only because the bootloader enabled it?
> 
> Sorry but the fix looks bogus to me. The spi-gpio driver calls 
> gpiod_set_value(0) when the CS is going *active*, and 
> gpiod_set_value(1) when the CS is going *inactive*. If you use the 
> GPIO_ACTIVE_LOW flag, the levels are inverted, and the CS is high 
> when active, which is the opposite of what you want.

Correction: the driver sets the SPI_CS_HIGH flag. I guess that's what's 
new. So indeed GPIO_ACTIVE_LOW should be used.

-Paul

>>>  +		sck-gpios = <&gpy3 1 GPIO_ACTIVE_HIGH>;
>>>  +		mosi-gpios = <&gpy3 3 GPIO_ACTIVE_HIGH>;
>>>  +
>>>  +		lcd@0 {
>>>  +			compatible = "samsung,ld9040";
>>>  +			reg = <0>;
>>>  +
>>>  +			spi-max-frequency = <1200000>;
>>>  +			spi-cpol;
>>>  +			spi-cpha;
>>>  +
>>>  +			vdd3-supply = <&vmipi_reg>;
>>>  +			vci-supply = <&vcclcd_reg>;
>>>  +
>>>  +			reset-gpios = <&gpy4 5 GPIO_ACTIVE_HIGH>;
>>>  +			power-on-delay = <10>;
>>>  +			reset-delay = <10>;
>>>  +
>>>  +			panel-width-mm = <90>;
>>>  +			panel-height-mm = <154>;
>>>  +
>>>  +			display-timings {
>>>  +				timing {
>>>  +					clock-frequency = <23492370>;
>>>  +					hactive = <480>;
>>>  +					vactive = <800>;
>>>  +					hback-porch = <16>;
>>>  +					hfront-porch = <16>;
>>>  +					vback-porch = <2>;
>>>  +					vfront-porch = <28>;
>>>  +					hsync-len = <2>;
>>>  +					vsync-len = <1>;
>>>  +					hsync-active = <0>;
>>>  +					vsync-active = <0>;
>>>  +					de-active = <0>;
>>>  +					pixelclk-active = <0>;
>>>  +				};
>>>  +			};
>>>  +
>>>  +			port {
>>>  +				lcd_ep: endpoint {
>>>  +					remote-endpoint = <&fimd_dpi_ep>;
>>>  +				};
>>>  +			};
>>>  +		};
>>>  +	};
>>>  +
>>>  +	fixed-rate-clocks {
>>>  +		xxti {
>>>  +			compatible = "samsung,clock-xxti";
>>>  +			clock-frequency = <0>;
>>>  +		};
>>>  +
>>>  +		xusbxti {
>>>  +			compatible = "samsung,clock-xusbxti";
>>>  +			clock-frequency = <24000000>;
>>>  +		};
>>>  +	};
>>>  +
>>>  +	thermal-zones {
>>>  +		cpu_thermal: cpu-thermal {
>>>  +			cooling-maps {
>>>  +				map0 {
>>>  +					/* Corresponds to 800MHz */
>>>  +					cooling-device = <&cpu0 2 2>;
>>>  +				};
>>>  +				map1 {
>>>  +					/* Corresponds to 200MHz */
>>>  +					cooling-device = <&cpu0 4 4>;
>>>  +				};
>>>  +			};
>>>  +		};
>>>  +	};
>>>  +
>>>  +};
>>>  +
>>>  +&camera {
>>>  +	status = "okay";
>>>  +};
>>>  +
>>>  +&cpu0 {
>>>  +	cpu0-supply = <&varm_breg>;
>>>  +};
>>>  +
>>>  +&exynos_usbphy {
>>>  +	status = "okay";
>>>  +
>>>  +	vbus-supply = <&safe1_sreg>;
>>>  +};
>>>  +
>>>  +&fimc_0 {
>>>  +	status = "okay";
>>>  +
>>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC0>, <&clock 
>>> CLK_SCLK_FIMC0>;
>>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>>  +	assigned-clock-rates = <0>, <160000000>;
>>>  +};
>>>  +
>>>  +&fimc_1 {
>>>  +	status = "okay";
>>>  +
>>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC1>, <&clock 
>>> CLK_SCLK_FIMC1>;
>>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>>  +	assigned-clock-rates = <0>, <160000000>;
>>>  +};
>>>  +
>>>  +&fimc_2 {
>>>  +	status = "okay";
>>>  +
>>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC2>, <&clock 
>>> CLK_SCLK_FIMC2>;
>>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>>  +	assigned-clock-rates = <0>, <160000000>;
>>>  +};
>>>  +
>>>  +&fimc_3 {
>>>  +	status = "okay";
>>>  +
>>>  +	assigned-clocks = <&clock CLK_MOUT_FIMC3>, <&clock 
>>> CLK_SCLK_FIMC3>;
>>>  +	assigned-clock-parents = <&clock CLK_SCLK_MPLL>;
>>>  +	assigned-clock-rates = <0>, <160000000>;
>>>  +};
>>>  +
>>>  +&fimd {
>>>  +	status = "okay";
>>>  +	#address-cells = <1>;
>>>  +	#size-cells = <0>;
>>>  +
>>>  +	samsung,invert-vden;
>>>  +	samsung,invert-vclk;
>>>  +
>>>  +	pinctrl-0 = <&lcd_clk>, <&lcd_data24>;
>>>  +	pinctrl-names = "default";
>>>  +
>>>  +	port@3 {
>>>  +		reg = <3>;
>>>  +
>>>  +		fimd_dpi_ep: endpoint {
>>>  +			remote-endpoint = <&lcd_ep>;
>>>  +		};
>>>  +	};
>>>  +};
>>>  +
>>>  +&hsotg {
>>>  +	status = "okay";
>>>  +
>>>  +	dr_mode = "otg";
>> The the host mode really works with DWC2? Afair the host mode worked
>> only when it was rerouted by PHY to the EHCI (at least on 
>> UniversalC210
>> board, which is very similar to GS2).
> 
> I have to say that I didn't try OTG / host, but it does work fine in 
> gadget mode.
> I can change it to dr_mode = "peripheral" for now, if you prefer.
> 
>>>  +	vusb_d-supply = <&vusb_reg>;
>>>  +	vusb_a-supply = <&vusbdac_reg>;
>>>  +};
>>>  +
>>>  +&ehci {
>>>  +	status = "okay";
>>>  +
>>>  +	port@0 {
>>>  +		status = "okay";
>>>  +	};
>>>  +
>>>  +	port@1 {
>>>  +		status = "okay";
>>>  +	};
>>>  +
>>>  +	port@2 {
>>>  +		status = "okay";
>>>  +	};
>>>  +};
>> 
>> This is the old, obsolete exynos-ehci binding. Please remove 'port'
>> nodes and use standard USB phy bindings like it has been done in the
>> following patch: https://patchwork.kernel.org/patch/11060465/
> 
> Will do.
> 
>> BTW, do you really need all 3 phys/ports?
> 
> No idea!
> 
>>>  +
>>>  +
>>>  +&i2c_3 {
>>>  +	status = "okay";
>>>  +
>>>  +	samsung,i2c-sda-delay = <100>;
>>>  +	samsung,i2c-slave-addr = <0x10>;
>>>  +	samsung,i2c-max-bus-freq = <100000>;
>>>  +
>>>  +	pinctrl-0 = <&i2c3_bus>;
>>>  +	pinctrl-names = "default";
>>>  +
>>>  +	mxt224-touchscreen@4a {
>>>  +		compatible = "atmel,maxtouch";
>>>  +		reg = <0x4a>;
>>>  +
>>>  +		interrupt-parent = <&gpx0>;
>>>  +		interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>>>  +
>>>  +		avdd-supply = <&tsp_reg>;
>>>  +		vdd-supply = <&tsp_reg>;
>>>  +	};
>>>  +};
>>>  +
>>>  +&i2c_7 {
>>>  +	status = "okay";
>>>  +
>>>  +	samsung,i2c-sda-delay = <100>;
>>>  +	samsung,i2c-slave-addr = <0x10>;
>>>  +	samsung,i2c-max-bus-freq = <400000>;
>>>  +
>>>  +	pinctrl-0 = <&i2c7_bus>;
>>>  +	pinctrl-names = "default";
>>>  +
>>>  +	ak8975@c {
>>>  +		compatible = "asahi-kasei,ak8975";
>>>  +		reg = <0x0c>;
>>>  +
>>>  +		gpios = <&gpx2 2 GPIO_ACTIVE_HIGH>;
>>>  +	};
>>>  +};
>>>  +
>>>  +
>>>  +&i2c_5 {
>>>  +	status = "okay";
>>>  +
>>>  +	samsung,i2c-sda-delay = <100>;
>>>  +	samsung,i2c-slave-addr = <0x10>;
>>>  +	samsung,i2c-max-bus-freq = <100000>;
>>>  +
>>>  +	pinctrl-0 = <&i2c5_bus>;
>>>  +	pinctrl-names = "default";
>>>  +
>>>  +	max8997_pmic@66 {
>>>  +		compatible = "maxim,max8997-pmic";
>>>  +		reg = <0x66>;
>>>  +
>>>  +		interrupts-extended = <&gpx0 7 0>, <&gpx2 3 2>;
>>>  +		interrupt-parent = <&gpx0>;
>>>  +		interrupts = <7 IRQ_TYPE_NONE>;
>>>  +
>>>  +		otg_en-gpios = <&gpx3 3 0>;
>>>  +		usb_sel-gpios = <&gpl0 6 0>;
>>>  +
>>>  +		max8997,pmic-buck1-uses-gpio-dvs;
>>>  +		max8997,pmic-buck2-uses-gpio-dvs;
>>>  +		max8997,pmic-buck5-uses-gpio-dvs;
>>>  +
>>>  +		max8997,pmic-ignore-gpiodvs-side-effect;
>>>  +		max8997,pmic-buck125-default-dvs-idx = <0>;
>>>  +
>>>  +		max8997,pmic-buck125-dvs-gpios = <&gpx0 5 GPIO_ACTIVE_HIGH>,
>>>  +						 <&gpx0 6 GPIO_ACTIVE_HIGH>,
>>>  +						 <&gpl0 0 GPIO_ACTIVE_HIGH>;
>>>  +
>>>  +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
>>>  +						 <1250000>, <1200000>,
>>>  +						 <1150000>, <1100000>,
>>>  +						 <1000000>, <950000>;
>>>  +
>>>  +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1000000>,
>>>  +						 <950000>,  <900000>,
>>>  +						 <1100000>, <1000000>,
>>>  +						 <950000>,  <900000>;
>>>  +
>>>  +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
>>>  +						 <1200000>, <1200000>,
>>>  +						 <1200000>, <1200000>,
>>>  +						 <1200000>, <1200000>;
>>>  +
>>>  +		pinctrl-0 = <&max8997_irq>, <&otg_gp>, <&usb_sel>;
>>>  +		pinctrl-names = "default";
>>>  +
>>>  +		regulators {
>>>  +			vadc_reg: LDO1 {
>>>  +				regulator-name = "VADC_3.3V_C210";
>>>  +				regulator-min-microvolt = <3300000>;
>>>  +				regulator-max-microvolt = <3300000>;
>>>  +				regulator-always-on;
>>>  +
>>>  +			};
>>>  +			valive_reg: LDO2 {
>>>  +				regulator-name = "VALIVE_1.1V_C210";
>>>  +				regulator-min-microvolt = <1100000>;
>>>  +				regulator-max-microvolt = <1100000>;
>>>  +				regulator-always-on;
>>>  +
>>>  +			};
>>>  +
>>>  +			vusb_reg: LDO3 {
>>>  +				regulator-name = "VUSB_1.1V_C210";
>>>  +				regulator-min-microvolt = <1100000>;
>>>  +				regulator-max-microvolt = <1100000>;
>>>  +			};
>>>  +
>>>  +			vmipi_reg: LDO4 {
>>>  +				regulator-name = "VMIPI_1.8V";
>>>  +				regulator-min-microvolt = <1800000>;
>>>  +				regulator-max-microvolt = <1800000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vhsic_reg: LDO5 {
>>>  +				regulator-name = "VHSIC_1.2V";
>>>  +				regulator-min-microvolt = <1200000>;
>>>  +				regulator-max-microvolt = <1200000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vpda_reg: LDO6 {
>>>  +				regulator-name = "VCC_1.8V_PDA";
>>>  +				regulator-min-microvolt = <1800000>;
>>>  +				regulator-max-microvolt = <1800000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vcam_reg: LDO7 {
>>>  +				regulator-name = "CAM_ISP_1.8V";
>>>  +				regulator-min-microvolt = <1800000>;
>>>  +				regulator-max-microvolt = <1800000>;
>>>  +			};
>>>  +
>>>  +			vusbdac_reg: LDO8 {
>>>  +				regulator-name = "VUSB+VDAC_3.3V_C210";
>>>  +				regulator-min-microvolt = <3300000>;
>>>  +				regulator-max-microvolt = <3300000>;
>>>  +			};
>>>  +
>>>  +			vccpda_reg: LDO9 {
>>>  +				regulator-name = "VCC_2.8V_PDA";
>>>  +				regulator-min-microvolt = <2800000>;
>>>  +				regulator-max-microvolt = <2800000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vtouch_reg: LDO11 {
>>>  +				regulator-name = "TOUCH_2.8V";
>>>  +				regulator-min-microvolt = <2800000>;
>>>  +				regulator-max-microvolt = <2800000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vpll_reg: LDO10 {
>>>  +				regulator-name = "VPLL_1.1V";
>>>  +				regulator-min-microvolt = <1100000>;
>>>  +				regulator-max-microvolt = <1100000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vtcam_reg: LDO12 {
>>>  +				regulator-name = "VT_CAM_1.8V";
>>>  +				regulator-min-microvolt = <1800000>;
>>>  +				regulator-max-microvolt = <1800000>;
>>>  +			};
>>>  +
>>>  +			vcclcd_reg: LDO13 {
>>>  +				regulator-name = "VCC_3.0V_LCD";
>>>  +				regulator-min-microvolt = <3000000>;
>>>  +				regulator-max-microvolt = <3000000>;
>>>  +			};
>>>  +
>>>  +			vmotor_reg: LDO14 {
>>>  +				regulator-name = "VCC_2.8V_MOTOR";
>>>  +				regulator-min-microvolt = <2800000>;
>>>  +				regulator-max-microvolt = <2800000>;
>>>  +			};
>>>  +
>>>  +			vled_reg: LDO15 {
>>>  +				regulator-name = "LED_A_2.8V";
>>>  +				regulator-min-microvolt = <2800000>;
>>>  +				regulator-max-microvolt = <2800000>;
>>>  +			};
>>>  +
>>>  +			camsensor_reg: LDO16 {
>>>  +				regulator-name = "CAM_SENSOR_IO_1.8V";
>>>  +				regulator-min-microvolt = <1800000>;
>>>  +				regulator-max-microvolt = <1800000>;
>>>  +			};
>>>  +
>>>  +			vtf_reg: LDO17 {
>>>  +				regulator-name = "VTF_2.8V";
>>>  +				regulator-min-microvolt = <2800000>;
>>>  +				regulator-max-microvolt = <2800000>;
>>>  +			};
>>>  +
>>>  +			vtouchled_reg: LDO18 {
>>>  +				regulator-name = "TOUCH_LED_3.3V";
>>>  +				regulator-min-microvolt = <2500000>;
>>>  +				regulator-max-microvolt = <3300000>;
>>>  +			};
>>>  +
>>>  +			vddq_reg: LDO21 {
>>>  +				regulator-name = "VDDQ_M1M2_1.2V";
>>>  +				regulator-min-microvolt = <1200000>;
>>>  +				regulator-max-microvolt = <1200000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			varm_breg: BUCK1 {
>>>  +				//regulator-name = "VARM_1.2V_C210";
>>>  +				regulator-name = "vdd_arm";
>>>  +				regulator-min-microvolt = <65000>;
>>>  +				regulator-max-microvolt = <2225000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vint_breg: BUCK2 {
>>>  +				regulator-name = "VINT_1.1V_C210";
>>>  +				regulator-min-microvolt = <65000>;
>>>  +				regulator-max-microvolt = <2225000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vg3d_breg: BUCK3 {
>>>  +				regulator-name = "G3D_1.1V";
>>>  +				regulator-min-microvolt = <900000>;
>>>  +				regulator-max-microvolt = <1200000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			camisp_breg: BUCK4 {
>>>  +				regulator-name = "CAM_ISP_CORE_1.2V";
>>>  +				regulator-min-microvolt = <1200000>;
>>>  +				regulator-max-microvolt = <1200000>;
>>>  +			};
>>>  +
>>>  +			vmem_breg: BUCK5 {
>>>  +				regulator-name = "VMEM_1.2V";
>>>  +				regulator-min-microvolt = <1200000>;
>>>  +				regulator-max-microvolt = <1200000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			vccsub_breg: BUCK7 {
>>>  +				regulator-name = "VCC_SUB_2.0V";
>>>  +				regulator-min-microvolt = <2000000>;
>>>  +				regulator-max-microvolt = <2000000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			safe1_sreg: ESAFEOUT1 {
>>>  +				regulator-name = "SAFEOUT1";
>>>  +			};
>>>  +
>>>  +			safe2_sreg: ESAFEOUT2 {
>>>  +				regulator-name = "SAFEOUT2";
>>>  +				regulator-boot-on;
>>>  +			};
>>>  +
>>>  +			charger_reg: CHARGER {
>>>  +				regulator-name = "CHARGER";
>>>  +				regulator-min-microamp = <60000>;
>>>  +				regulator-max-microamp = <2580000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +			chargercv_reg: CHARGER_CV {
>>>  +				regulator-name = "CHARGER_CV";
>>>  +				regulator-min-microvolt = <3800000>;
>>>  +				regulator-max-microvolt = <4100000>;
>>>  +				regulator-always-on;
>>>  +			};
>>>  +
>>>  +		};
>>>  +
>>>  +	};
>>>  +};
>>>  +
>>>  +
>>>  +&gpu {
>>>  +	status = "okay";
>>>  +
>>>  +	mali-supply = <&vg3d_breg>;
>>>  +	regulator-microvolt-offset = <50000>;
>>>  +	regulator-microsecs-delay = <50>;
>>>  +};
>>>  +
>>>  +
>>>  +&sdhci_0 {
>>>  +	status = "okay";
>>>  +
>>>  +	bus-width = <8>;
>>>  +	non-removable;
>>>  +	vmmc-supply = <&vemmc_reg>;
>>>  +
>>>  +	pinctrl-0 = <&sd0_clk>, <&sd0_cmd>, <&sd0_bus8>;
>>>  +	pinctrl-names = "default";
>>>  +};
>>>  +
>>>  +&sdhci_2 {
>>>  +	status = "okay";
>>>  +
>>>  +	bus-width = <4>;
>>>  +	cd-gpios = <&gpx3 4 GPIO_ACTIVE_LOW>;
>>>  +	vmmc-supply = <&vtf_reg>;
>>>  +
>>>  +	pinctrl-0 = <&sd2_clk>, <&sd2_cmd>, <&sd2_bus4>;
>>>  +	pinctrl-names = "default";
>>>  +};
>>>  +
>>>  +&ehci {
>>>  +	port@0 {
>>>  +		status = "okay";
>>>  +	};
>>>  +
>>>  +	port@1 {
>>>  +		status = "okay";
>>>  +	};
>>>  +
>>>  +	port@2 {
>>>  +		status = "okay";
>>>  +	};
>>>  +
>>>  +};
>> 
>> This is duplicated 'ehci' node, probable an artifact from the earlier
>> rebase. Please remove it.
> 
> Right, didn't even notice it.
> 
> Will remove it and send a V2.
> 
> Cheers,
> -Paul
> 
>>>  +
>>>  +
>>>  +&sdhci_3 {
>>>  +	status = "okay";
>>>  +
>>>  +	#address-cells = <1>;
>>>  +	#size-cells = <0>;
>>>  +
>>>  +	non-removable;
>>>  +	bus-width = <4>;
>>>  +	mmc-pwrseq = <&wlan_pwrseq>;
>>>  +	vmmc-supply = <&vtf_reg>;
>>>  +
>>>  +	pinctrl-names = "default";
>>>  +	pinctrl-0 = <&sd3_clk>, <&sd3_cmd>, <&sd3_bus4>;
>>>  +
>>>  +	brcmf: wifi@1 {
>>>  +		compatible = "brcm,bcm4330-fmac";
>>>  +		reg = <1>;
>>>  +
>>>  +		interrupt-parent = <&gpx2>;
>>>  +		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>>>  +		interrupt-names = "host-wake";
>>>  +	};
>>>  +};
>>>  +
>>>  +
>>>  +&serial_0 {
>>>  +	status = "okay";
>>>  +
>>>  +	pinctrl-names = "default";
>>>  +	pinctrl-0 = <&bt_en>, <&bt_res>, <&uart0_data>, <&uart0_fctl>;
>>>  +
>>>  +	bluetooth {
>>>  +		compatible = "brcm,bcm4330-bt";
>>>  +
>>>  +		shutdown-gpios = <&gpl0 4 GPIO_ACTIVE_HIGH>;
>>>  +		reset-gpios = <&gpl1 0 GPIO_ACTIVE_HIGH>;
>>>  +		device-wakeup-gpios = <&gpx3 1 GPIO_ACTIVE_HIGH>;
>>>  +		host-wakeup-gpios = <&gpx2 6 GPIO_ACTIVE_HIGH>;
>>>  +	};
>>>  +};
>>>  +
>>>  +&serial_1 {
>>>  +	status = "okay";
>>>  +};
>>>  +
>>>  +&serial_2 {
>>>  +	status = "okay";
>>>  +};
>>>  +
>>>  +&serial_3 {
>>>  +	status = "okay";
>>>  +};
>>>  +
>>>  +&tmu {
>>>  +	status = "okay";
>>>  +};
>>>  +
>>>  +
>>>  +&pinctrl_0 {
>>>  +	pinctrl-names = "default";
>>>  +	pinctrl-0 = <&sleep0>;
>>>  +
>>>  +
>>>  +	sleep0: sleep-states {
>>>  +		gpa0-0 {
>>>  +			samsung,pins = "gpa0-0";
>>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
>>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>>  +		};
>>>  +
>>>  +		gpa0-1 {
>>>  +			samsung,pins = "gpa0-1";
>>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT0>;
>>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>>  +		};
>>>  +
>>>  +		gpa0-2 {
>>>  +			samsung,pins = "gpa0-2";
>>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
>>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>>  +		};
>>>  +
>>>  +		gpa0-3 {
>>>  +			samsung,pins = "gpa0-3";
>>>  +			samsung,pin-con-pdn = <EXYNOS_PIN_PDN_OUT1>;
>>>  +			samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>>  +		};
>>>  +	};
>>>  +};
>>>  +
>>>  +&pinctrl_1 {
>>>  +	mhl_int: mhl-int {
>>>  +		samsung,pins = "gpf3-5";
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +	};
>>>  +
>>>  +	i2c_mhl_bus: i2c-mhl-bus {
>>>  +		samsung,pins = "gpf0-4", "gpf0-6";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_DOWN>;
>>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>>  +	};
>>>  +
>>>  +	usb_sel: usb-sel {
>>>  +		samsung,pins = "gpl0-6";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>>  +		samsung,pin-val = <0>;
>>>  +	};
>>>  +
>>>  +	bt_en: bt-en {
>>>  +		samsung,pins = "gpl0-4";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
>>>  +		samsung,pin-val = <0>;
>>>  +	};
>>>  +
>>>  +	bt_res: bt-res {
>>>  +		samsung,pins = "gpl1-0";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV4>;
>>>  +		samsung,pin-val = <0>;
>>>  +	};
>>>  +
>>>  +
>>>  +	otg_gp: otg-gp {
>>>  +		samsung,pins = "gpx3-3";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>>  +		samsung,pin-val = <0>;
>>>  +	};
>>>  +
>>>  +	mag_mhl_gpio: mag-mhl-gpio {
>>>  +		samsung,pins = "gpd0-2";
>>>  +		samsung,pin-function = <EXYNOS_PIN_FUNC_3>;
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +	};
>>>  +
>>>  +
>>>  +	max8997_irq: max8997-irq {
>>>  +		samsung,pins = "gpx0-7";
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +	};
>>>  +
>>>  +	max17042_fuel_irq: max17042-fuel-irq {
>>>  +		samsung,pins = "gpx2-3";
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>>  +	};
>>>  +
>>>  +	tsp224_irq: tsp224-irq {
>>>  +		samsung,pins = "gpx0-4";
>>>  +		samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>>>  +	};
>>>  +};
>> 
>> Best regards
>> --
>> Marek Szyprowski, PhD
>> Samsung R&D Institute Poland
>> 
>

Marek Szyprowski March 13, 2020, 3:24 p.m. UTC | #6

Hi Paul,

On 13.03.2020 15:33, Paul Cercueil wrote:
> Hi Krzysztof,
>
>
> Le ven., mars 13, 2020 at 10:00, Krzysztof Kozlowski <krzk@kernel.org> 
> a écrit :
>> On Thu, Mar 12, 2020 at 04:34:11PM +0100, Paul Cercueil wrote:
>>>  From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
>>>
>>>  Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100 
>>> version).

 > [...]

>  +
>>>  +
>>>  +    memory@40000000 {
>>>  +        device_type = "memory";
>>>  +        reg = <0x40000000 0x40000000>;
>>>  +    };
>>>  +
>>
>> You do not have a chosen node, at least for stdout serial selection.
>> Probably you should have one... bootargs I guess could come from
>> bootloader.
>
> Problem is, I have no idea which UART is available via the USB jig :)

See exynos4210-trats.dts and exynos4210-universal_c210.dts, they are 
similar to i9100 and share a lots. They all use serial2 for debug UART 
via USB jig.

>>>  +    vemmc_reg: regulator-0 {
>>>  +        compatible = "regulator-fixed";
>>>  +        regulator-name = "VMEM_VDD_2.8V";
>>>  +        regulator-min-microvolt = <2800000>;
>>>  +        regulator-max-microvolt = <2800000>;
>>>  +        gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
>>>  +        enable-active-high;
>>
>> This looks like a stub/fake regulator. Probably it shoould be replaced
>> with proper regulator coming from PMIC (few seems to be missing there).
>>
>> I don't have the schematics so I cannot judge... I see Trats has the
>> same.
>>
>> Marek, mayybe you know, is it really a separate regulator?

Trats has the same regulator and according to the schematic it is 
supplied from VBAT and controlled via eMMC_EN GPIO pin.

> > [...]

Best regards

Krzysztof Kozlowski March 18, 2020, 3:23 p.m. UTC | #7

On Wed, Mar 18, 2020 at 03:25:22PM +0100, Paul Cercueil wrote:
> Hi Krzysztof,
> 
> 
> Le ven. 13 mars 2020 à 10:00, Krzysztof Kozlowski <krzk@kernel.org> a écrit
> :
> > On Thu, Mar 12, 2020 at 04:34:11PM +0100, Paul Cercueil wrote:
> > >  From: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> > > 
> > >  Add devicetree file for the Exynos 4210 based Galaxy S2 (i9100
> > > version).
> > 
> > Thanks for the patch!
> > 
> > It is an GT-I9100.
> > 
> > +Cc Marek (I have few questions further)
> > 
> > 
> > > 
> > >  Signed-off-by: Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  ---
> > >   arch/arm/boot/dts/Makefile             |   1 +
> > >   arch/arm/boot/dts/exynos4210-i9100.dts | 816
> > > +++++++++++++++++++++++++
> > >   2 files changed, 817 insertions(+)
> > >   create mode 100644 arch/arm/boot/dts/exynos4210-i9100.dts
> > > 
> > >  diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > >  index d6546d2676b9..522436d30690 100644
> > >  --- a/arch/arm/boot/dts/Makefile
> > >  +++ b/arch/arm/boot/dts/Makefile
> > >  @@ -181,6 +181,7 @@ dtb-$(CONFIG_ARCH_EXYNOS3) += \
> > >   	exynos3250-monk.dtb \
> > >   	exynos3250-rinato.dtb
> > >   dtb-$(CONFIG_ARCH_EXYNOS4) += \
> > >  +	exynos4210-i9100.dtb \
> > >   	exynos4210-origen.dtb \
> > >   	exynos4210-smdkv310.dtb \
> > >   	exynos4210-trats.dtb \
> > >  diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > b/arch/arm/boot/dts/exynos4210-i9100.dts
> > >  new file mode 100644
> > >  index 000000000000..c218e34959be
> > >  --- /dev/null
> > >  +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > >  @@ -0,0 +1,816 @@
> > >  +// SPDX-License-Identifier: GPL-2.0
> > >  +/*
> > >  + * Samsung's Exynos4210 based Galaxy S2 (i9100 version) device tree
> > 
> > Ditto
> > 
> > >  + *
> > >  + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> > >  + *		http://www.samsung.com
> > >  + * Copyright (c) 2020 Stenkin Evgeniy <stenkinevgeniy@gmail.com>
> > >  + * Copyright (c) 2020 Paul Cercueil <paul@crapouillou.net>
> > >  + *
> > >  + * Device tree source file for Samsung's Galaxy S2 smartphone
> > > (i9100 version),
> > >  + * which is based on Samsung's Exynos4210 SoC.
> > 
> > Duplicated description, you mentioned it before.
> > 
> > >  + */
> > >  +
> > >  +/dts-v1/;
> > >  +#include "exynos4210.dtsi"
> > >  +#include "exynos4412-ppmu-common.dtsi"
> > >  +
> > >  +#include <dt-bindings/gpio/gpio.h>
> > >  +#include <dt-bindings/input/linux-event-codes.h>
> > >  +
> > >  +/ {
> > >  +	model = "Samsung Galaxy S2 (i9100)";
> > 
> > GT-I9100
> > 
> > >  +	compatible = "samsung,i9100", "samsung,exynos4210",
> > > "samsung,exynos4";
> > 
> > I think it shares a lot with Trats board...
> > 
> > Marek, how similar these
> > designs are? Maybe it is worth to split some parts and make common part?
> > 
> > >  +
> > >  +
> > >  +	memory@40000000 {
> > >  +		device_type = "memory";
> > >  +		reg = <0x40000000 0x40000000>;
> > >  +	};
> > >  +
> > 
> > You do not have a chosen node, at least for stdout serial selection.
> > Probably you should have one... bootargs I guess could come from
> > bootloader.
> > 
> > 
> > >  +	vemmc_reg: regulator-0 {
> > >  +		compatible = "regulator-fixed";
> > >  +		regulator-name = "VMEM_VDD_2.8V";
> > >  +		regulator-min-microvolt = <2800000>;
> > >  +		regulator-max-microvolt = <2800000>;
> > >  +		gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
> > >  +		enable-active-high;
> > 
> > This looks like a stub/fake regulator. Probably it shoould be replaced
> > with proper regulator coming from PMIC (few seems to be missing there).
> > 
> > I don't have the schematics so I cannot judge... I see Trats has the
> > same.
> > 
> > Marek, mayybe you know, is it really a separate regulator?
> > 
> > >  +	};
> > >  +
> > >  +	tsp_reg: regulator-1 {
> > >  +		compatible = "regulator-fixed";
> > >  +		regulator-name = "TSP_FIXED_VOLTAGES";
> > >  +		regulator-min-microvolt = <3300000>;
> > >  +		regulator-max-microvolt = <3300000>;
> > >  +		gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
> > >  +		startup-delay-us = <70000>;
> > >  +		enable-active-high;
> > >  +		regulator-boot-on;
> > >  +		regulator-always-on;
> > 
> > always-on and boot-on should not be needed. You have a consumer for this
> > regulator.
> 
> About this: the touchscreen driver does not use a regulator, so I believe
> that's why these properties were here.
> 
> I sent patches upstream to address the issue:
> https://lkml.org/lkml/2020/3/15/94
> 
> I believe this means I cannot merge the i9100 devicetree until it is acked.

This can stay because it is correct with current bindings and driver.
However you should remove avdd-supply/vdd-supply from touchscreen node
because they are not used/supported.

You can re-add them once bindings get accepted.

Best regards,
Krzysztof

Marek Szyprowski March 31, 2020, 5:36 a.m. UTC | #8

Hi Paul,

On 2020-03-18 15:25, Paul Cercueil wrote:
>>>  +    };
>>>  +
>>>  +    tsp_reg: regulator-1 {
>>>  +        compatible = "regulator-fixed";
>>>  +        regulator-name = "TSP_FIXED_VOLTAGES";
>>>  +        regulator-min-microvolt = <3300000>;
>>>  +        regulator-max-microvolt = <3300000>;
>>>  +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>  +        startup-delay-us = <70000>;
>>>  +        enable-active-high;
>>>  +        regulator-boot-on;
>>>  +        regulator-always-on;
>>
>> always-on and boot-on should not be needed. You have a consumer for this
>> regulator.
>
> About this: the touchscreen driver does not use a regulator, so I 
> believe that's why these properties were here.
>
> I sent patches upstream to address the issue: 
> https://lkml.org/lkml/2020/3/15/94
>
> I believe this means I cannot merge the i9100 devicetree until it is 
> acked.

One more information - similar change has been already posted, but it 
looks it got lost then: https://patchwork.kernel.org/patch/10550903/

Best regards

Paul Cercueil March 31, 2020, 1:09 p.m. UTC | #9

Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>

Hi Marek,

Le mar. 31 mars 2020 à 7:36, Marek Szyprowski 
<m.szyprowski@samsung.com> a écrit :
> Hi Paul,
> 
> On 2020-03-18 15:25, Paul Cercueil wrote:
>>>>   +    };
>>>>   +
>>>>   +    tsp_reg: regulator-1 {
>>>>   +        compatible = "regulator-fixed";
>>>>   +        regulator-name = "TSP_FIXED_VOLTAGES";
>>>>   +        regulator-min-microvolt = <3300000>;
>>>>   +        regulator-max-microvolt = <3300000>;
>>>>   +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>>   +        startup-delay-us = <70000>;
>>>>   +        enable-active-high;
>>>>   +        regulator-boot-on;
>>>>   +        regulator-always-on;
>>> 
>>>  always-on and boot-on should not be needed. You have a consumer 
>>> for this
>>>  regulator.
>> 
>>  About this: the touchscreen driver does not use a regulator, so I
>>  believe that's why these properties were here.
>> 
>>  I sent patches upstream to address the issue:
>>  https://lkml.org/lkml/2020/3/15/94
>> 
>>  I believe this means I cannot merge the i9100 devicetree until it is
>>  acked.
> 
> One more information - similar change has been already posted, but it
> looks it got lost then: https://patchwork.kernel.org/patch/10550903/

I was aware of this patch, but didn't know it was sent upstream.

This other patch uses two regulators, vdd/avdd but doesn't give any 
reason why.

Paweł, is that really needed?

-Paul


> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Marek Szyprowski March 31, 2020, 1:55 p.m. UTC | #10

Hi Paul,

On 2020-03-31 15:09, Paul Cercueil wrote:
> Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>
> Hi Marek,
>
> Le mar. 31 mars 2020 à 7:36, Marek Szyprowski 
> <m.szyprowski@samsung.com> a écrit :
>> Hi Paul,
>>
>> On 2020-03-18 15:25, Paul Cercueil wrote:
>>>>>   +    };
>>>>>   +
>>>>>   +    tsp_reg: regulator-1 {
>>>>>   +        compatible = "regulator-fixed";
>>>>>   +        regulator-name = "TSP_FIXED_VOLTAGES";
>>>>>   +        regulator-min-microvolt = <3300000>;
>>>>>   +        regulator-max-microvolt = <3300000>;
>>>>>   +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>>>   +        startup-delay-us = <70000>;
>>>>>   +        enable-active-high;
>>>>>   +        regulator-boot-on;
>>>>>   +        regulator-always-on;
>>>>
>>>>  always-on and boot-on should not be needed. You have a consumer 
>>>> for this
>>>>  regulator.
>>>
>>>  About this: the touchscreen driver does not use a regulator, so I
>>>  believe that's why these properties were here.
>>>
>>>  I sent patches upstream to address the issue:
>>>  https://protect2.fireeye.com/url?k=e8aedc29-b53072b3-e8af5766-0cc47a336fae-759579fd576d8382&u=https://lkml.org/lkml/2020/3/15/94 
>>>
>>>
>>>  I believe this means I cannot merge the i9100 devicetree until it is
>>>  acked.
>>
>> One more information - similar change has been already posted, but it
>> looks it got lost then: https://patchwork.kernel.org/patch/10550903/
>
> I was aware of this patch, but didn't know it was sent upstream.
>
> This other patch uses two regulators, vdd/avdd but doesn't give any 
> reason why.
>
I've checked the UniversalC210 schematic, which uses the same 
touchscreen chip. There are 2 supplies to the touchscreen chip: 2.8V VDD 
and 3.3V AVDD. Both are enabled by the same GPIO pin though. There is 
however no reset GPIO pin there.

> Paweł, is that really needed?
>
Best regards

Paweł Chmiel March 31, 2020, 3:29 p.m. UTC | #11

On Tue, 2020-03-31 at 15:55 +0200, Marek Szyprowski wrote:
> Hi Paul,
> 
> On 2020-03-31 15:09, Paul Cercueil wrote:
> > Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > 
> > Hi Marek,
> > 
> > Le mar. 31 mars 2020 à 7:36, Marek Szyprowski 
> > <m.szyprowski@samsung.com> a écrit :
> > > Hi Paul,
> > > 
> > > On 2020-03-18 15:25, Paul Cercueil wrote:
> > > > > >   +    };
> > > > > >   +
> > > > > >   +    tsp_reg: regulator-1 {
> > > > > >   +        compatible = "regulator-fixed";
> > > > > >   +        regulator-name = "TSP_FIXED_VOLTAGES";
> > > > > >   +        regulator-min-microvolt = <3300000>;
> > > > > >   +        regulator-max-microvolt = <3300000>;
> > > > > >   +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
> > > > > >   +        startup-delay-us = <70000>;
> > > > > >   +        enable-active-high;
> > > > > >   +        regulator-boot-on;
> > > > > >   +        regulator-always-on;
> > > > > 
> > > > >  always-on and boot-on should not be needed. You have a consumer 
> > > > > for this
> > > > >  regulator.
> > > > 
> > > >  About this: the touchscreen driver does not use a regulator, so I
> > > >  believe that's why these properties were here.
> > > > 
> > > >  I sent patches upstream to address the issue:
> > > >  https://protect2.fireeye.com/url?k=e8aedc29-b53072b3-e8af5766-0cc47a336fae-759579fd576d8382&u=https://lkml.org/lkml/2020/3/15/94 
> > > > 
> > > > 
> > > >  I believe this means I cannot merge the i9100 devicetree until it is
> > > >  acked.
> > > 
> > > One more information - similar change has been already posted, but it
> > > looks it got lost then: https://patchwork.kernel.org/patch/10550903/
> > 
> > I was aware of this patch, but didn't know it was sent upstream.
> > 
> > This other patch uses two regulators, vdd/avdd but doesn't give any 
> > reason why.
> > 
> I've checked the UniversalC210 schematic, which uses the same 
> touchscreen chip. There are 2 supplies to the touchscreen chip: 2.8V VDD 
> and 3.3V AVDD. Both are enabled by the same GPIO pin though. There is 
> however no reset GPIO pin there.
Hi
Don't remember now how it worked on Galaxy S1, but it looks like it has
the same setup - two regulators enabled by one GPIO pin.
> 
> > Paweł, is that really needed?
> > 
> Best regards

Marek Szyprowski March 31, 2020, 7:29 p.m. UTC | #12

Hi

On 2020-03-31 17:29, Paweł Chmiel wrote:
> On Tue, 2020-03-31 at 15:55 +0200, Marek Szyprowski wrote:
>> Hi Paul,
>>
>> On 2020-03-31 15:09, Paul Cercueil wrote:
>>> Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
>>>
>>> Hi Marek,
>>>
>>> Le mar. 31 mars 2020 à 7:36, Marek Szyprowski
>>> <m.szyprowski@samsung.com> a écrit :
>>>> Hi Paul,
>>>>
>>>> On 2020-03-18 15:25, Paul Cercueil wrote:
>>>>>>>    +    };
>>>>>>>    +
>>>>>>>    +    tsp_reg: regulator-1 {
>>>>>>>    +        compatible = "regulator-fixed";
>>>>>>>    +        regulator-name = "TSP_FIXED_VOLTAGES";
>>>>>>>    +        regulator-min-microvolt = <3300000>;
>>>>>>>    +        regulator-max-microvolt = <3300000>;
>>>>>>>    +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>>>>>    +        startup-delay-us = <70000>;
>>>>>>>    +        enable-active-high;
>>>>>>>    +        regulator-boot-on;
>>>>>>>    +        regulator-always-on;
>>>>>>   always-on and boot-on should not be needed. You have a consumer
>>>>>> for this
>>>>>>   regulator.
>>>>>   About this: the touchscreen driver does not use a regulator, so I
>>>>>   believe that's why these properties were here.
>>>>>
>>>>>   I sent patches upstream to address the issue:
>>>>>   https://protect2.fireeye.com/url?k=e8aedc29-b53072b3-e8af5766-0cc47a336fae-759579fd576d8382&u=https://lkml.org/lkml/2020/3/15/94
>>>>>
>>>>>
>>>>>   I believe this means I cannot merge the i9100 devicetree until it is
>>>>>   acked.
>>>> One more information - similar change has been already posted, but it
>>>> looks it got lost then: https://patchwork.kernel.org/patch/10550903/
>>> I was aware of this patch, but didn't know it was sent upstream.
>>>
>>> This other patch uses two regulators, vdd/avdd but doesn't give any
>>> reason why.
>>>
>> I've checked the UniversalC210 schematic, which uses the same
>> touchscreen chip. There are 2 supplies to the touchscreen chip: 2.8V VDD
>> and 3.3V AVDD. Both are enabled by the same GPIO pin though. There is
>> however no reset GPIO pin there.
> Hi
> Don't remember now how it worked on Galaxy S1, but it looks like it has
> the same setup - two regulators enabled by one GPIO pin.

It is quite common for the regulators to share the enable GPIO line, so 
this is not an issue. Regulator framework supports such configuration 
for ages. I'm curious about the reset GPIO line, which were made 
mandatory for the regulators control in your patch. I didn't find it on 
any schematic of the devices with this touch screen, but I don't have a 
schematic of the Galaxy S1. IMHO the reset signal is simply derived from 
the supply regulators with some simple circuit, but I don't have 
schematic of that part.

Best regards

Paweł Chmiel March 31, 2020, 7:50 p.m. UTC | #13

On Tue, 2020-03-31 at 21:29 +0200, Marek Szyprowski wrote:
> Hi
> 
> On 2020-03-31 17:29, Paweł Chmiel wrote:
> > On Tue, 2020-03-31 at 15:55 +0200, Marek Szyprowski wrote:
> > > Hi Paul,
> > > 
> > > On 2020-03-31 15:09, Paul Cercueil wrote:
> > > > Cc: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > > > 
> > > > Hi Marek,
> > > > 
> > > > Le mar. 31 mars 2020 à 7:36, Marek Szyprowski
> > > > <m.szyprowski@samsung.com> a écrit :
> > > > > Hi Paul,
> > > > > 
> > > > > On 2020-03-18 15:25, Paul Cercueil wrote:
> > > > > > > >    +    };
> > > > > > > >    +
> > > > > > > >    +    tsp_reg: regulator-1 {
> > > > > > > >    +        compatible = "regulator-fixed";
> > > > > > > >    +        regulator-name = "TSP_FIXED_VOLTAGES";
> > > > > > > >    +        regulator-min-microvolt = <3300000>;
> > > > > > > >    +        regulator-max-microvolt = <3300000>;
> > > > > > > >    +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
> > > > > > > >    +        startup-delay-us = <70000>;
> > > > > > > >    +        enable-active-high;
> > > > > > > >    +        regulator-boot-on;
> > > > > > > >    +        regulator-always-on;
> > > > > > >   always-on and boot-on should not be needed. You have a consumer
> > > > > > > for this
> > > > > > >   regulator.
> > > > > >   About this: the touchscreen driver does not use a regulator, so I
> > > > > >   believe that's why these properties were here.
> > > > > > 
> > > > > >   I sent patches upstream to address the issue:
> > > > > >   https://protect2.fireeye.com/url?k=e8aedc29-b53072b3-e8af5766-0cc47a336fae-759579fd576d8382&u=https://lkml.org/lkml/2020/3/15/94
> > > > > > 
> > > > > > 
> > > > > >   I believe this means I cannot merge the i9100 devicetree until it is
> > > > > >   acked.
> > > > > One more information - similar change has been already posted, but it
> > > > > looks it got lost then: https://patchwork.kernel.org/patch/10550903/
> > > > I was aware of this patch, but didn't know it was sent upstream.
> > > > 
> > > > This other patch uses two regulators, vdd/avdd but doesn't give any
> > > > reason why.
> > > > 
> > > I've checked the UniversalC210 schematic, which uses the same
> > > touchscreen chip. There are 2 supplies to the touchscreen chip: 2.8V VDD
> > > and 3.3V AVDD. Both are enabled by the same GPIO pin though. There is
> > > however no reset GPIO pin there.
> > Hi
> > Don't remember now how it worked on Galaxy S1, but it looks like it has
> > the same setup - two regulators enabled by one GPIO pin.
> 
> It is quite common for the regulators to share the enable GPIO line, so 
> this is not an issue. Regulator framework supports such configuration 
> for ages. I'm curious about the reset GPIO line, which were made 
> mandatory for the regulators control in your patch. I didn't find it on 
> any schematic of the devices with this touch screen, but I don't have a 
> schematic of the Galaxy S1. IMHO the reset signal is simply derived from 
> the supply regulators with some simple circuit, but I don't have 
> schematic of that part.
Hi
Regarding this change - it was suggested during review of v1 version of
those patches. It's first reply here 
https://patchwork.kernel.org/patch/10524007/

> 
> Best regards

Marek Szyprowski April 1, 2020, 10:42 a.m. UTC | #14

Hi Paweł,

On 2020-03-31 21:50, Paweł Chmiel wrote:
> On Tue, 2020-03-31 at 21:29 +0200, Marek Szyprowski wrote:
>> On 2020-03-31 17:29, Paweł Chmiel wrote:
>>> On Tue, 2020-03-31 at 15:55 +0200, Marek Szyprowski wrote:
>>>> On 2020-03-31 15:09, Paul Cercueil wrote:
>>>>> Le mar. 31 mars 2020 à 7:36, Marek Szyprowski
>>>>> <m.szyprowski@samsung.com> a écrit :
>>>>>> On 2020-03-18 15:25, Paul Cercueil wrote:
>>>>>>>>>     +    };
>>>>>>>>>     +
>>>>>>>>>     +    tsp_reg: regulator-1 {
>>>>>>>>>     +        compatible = "regulator-fixed";
>>>>>>>>>     +        regulator-name = "TSP_FIXED_VOLTAGES";
>>>>>>>>>     +        regulator-min-microvolt = <3300000>;
>>>>>>>>>     +        regulator-max-microvolt = <3300000>;
>>>>>>>>>     +        gpio = <&gpl0 3 GPIO_ACTIVE_HIGH>;
>>>>>>>>>     +        startup-delay-us = <70000>;
>>>>>>>>>     +        enable-active-high;
>>>>>>>>>     +        regulator-boot-on;
>>>>>>>>>     +        regulator-always-on;
>>>>>>>>    always-on and boot-on should not be needed. You have a consumer
>>>>>>>> for this
>>>>>>>>    regulator.
>>>>>>>    About this: the touchscreen driver does not use a regulator, so I
>>>>>>>    believe that's why these properties were here.
>>>>>>>
>>>>>>>    I sent patches upstream to address the issue:
>>>>>>>    https://protect2.fireeye.com/url?k=e8aedc29-b53072b3-e8af5766-0cc47a336fae-759579fd576d8382&u=https://lkml.org/lkml/2020/3/15/94
>>>>>>>
>>>>>>>
>>>>>>>    I believe this means I cannot merge the i9100 devicetree until it is
>>>>>>>    acked.
>>>>>> One more information - similar change has been already posted, but it
>>>>>> looks it got lost then: https://patchwork.kernel.org/patch/10550903/
>>>>> I was aware of this patch, but didn't know it was sent upstream.
>>>>>
>>>>> This other patch uses two regulators, vdd/avdd but doesn't give any
>>>>> reason why.
>>>>>
>>>> I've checked the UniversalC210 schematic, which uses the same
>>>> touchscreen chip. There are 2 supplies to the touchscreen chip: 2.8V VDD
>>>> and 3.3V AVDD. Both are enabled by the same GPIO pin though. There is
>>>> however no reset GPIO pin there.
>>> Hi
>>> Don't remember now how it worked on Galaxy S1, but it looks like it has
>>> the same setup - two regulators enabled by one GPIO pin.
>> It is quite common for the regulators to share the enable GPIO line, so
>> this is not an issue. Regulator framework supports such configuration
>> for ages. I'm curious about the reset GPIO line, which were made
>> mandatory for the regulators control in your patch. I didn't find it on
>> any schematic of the devices with this touch screen, but I don't have a
>> schematic of the Galaxy S1. IMHO the reset signal is simply derived from
>> the supply regulators with some simple circuit, but I don't have
>> schematic of that part.
> Hi
> Regarding this change - it was suggested during review of v1 version of
> those patches. It's first reply here
> https://patchwork.kernel.org/patch/10524007/

Well, I know that this has been requested in v1 review, but I thought 
that you have tested v3 of your patch on I9000, thus I was curious about 
that reset GPIO line.

Best regards

[2/2] ARM: DTS: Add devicetree file for the Galaxy S2

Commit Message

Comments

Patch