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@xxxxxxxxxx> a écrit > : > > On Thu, Mar 12, 2020 at 04:34:11PM +0100, Paul Cercueil wrote: > > > From: Stenkin Evgeniy <stenkinevgeniy@xxxxxxxxx> > > > > > > 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@xxxxxxxxx> > > > Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx> > > > --- > > > 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@xxxxxxxxx> > > > + * Copyright (c) 2020 Paul Cercueil <paul@xxxxxxxxxxxxxxx> > > > + * > > > + * 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
