Re: [PATCH v3 13/13] ARM: dts: exynos: Add Samsung Galaxy Tab 3 8.0 boards

[Krzysztof Kozlowski]

On 01/05/2023 21:55, Artur Weber wrote:
> Introduce support for the Galaxy Tab 3 8.0 series of boards:
> 
>  - Samsung Galaxy Tab 3 8.0 WiFi (SM-T310/lt01wifi)
>  - Samsung Galaxy Tab 3 8.0 3G (SM-T311/lt013g)
>  - Samsung Galaxy Tab 3 8.0 LTE (SM-T315/lt01lte)
> 
> What works:
> 
>  - Display and backlight
>  - Touchscreen (without touchkeys)
>  - GPIO buttons, hall sensor
>  - WiFi and Bluetooth
>  - USB, fuel gauge, charging
>  - Accelerometer and magnetometer
>  - WiFi model only: light sensor
> 
> Signed-off-by: Artur Weber <aweber.kernel@xxxxxxxxx>

Thank you for your patch. There is something to discuss/improve.

> ---
> Changed in v3:
>  - Addressed review comments
>  - Removed rtc node (RTC is provided by PMIC)
>  - Added CPU thermal node
>  - Fixed dtb_check warnings
>  - Added common changes from next/dt
> 
> Display panel bindings are added by a separate patchset:
> "[PATCH 0/3] Add Samsung S6D7AA0 panel controller driver"[1]
> 
> LP855X node is adapted to changes from a separate patchset:
> "[PATCH 0/4] video: backlight: lp855x: modernize bindings"[2]
> 
> [1] https://lore.kernel.org/all/20230501185103.25939-1-aweber.kernel@xxxxxxxxx/
> [2] https://lore.kernel.org/all/20230429104534.28943-1-aweber.kernel@xxxxxxxxx/

New failures:

arch/arm/boot/dts/exynos4212-tab3-3g8.dtb: rtc@10070000: clocks: [[5,
346]] is too short

> ---
>  arch/arm/boot/dts/Makefile                  |    3 +
>  arch/arm/boot/dts/exynos4212-tab3-3g8.dts   |   29 +
>  arch/arm/boot/dts/exynos4212-tab3-lte8.dts  |   44 +
>  arch/arm/boot/dts/exynos4212-tab3-wifi8.dts |   26 +
>  arch/arm/boot/dts/exynos4212-tab3.dtsi      | 1171 +++++++++++++++++++
>  5 files changed, 1273 insertions(+)
>  create mode 100644 arch/arm/boot/dts/exynos4212-tab3-3g8.dts
>  create mode 100644 arch/arm/boot/dts/exynos4212-tab3-lte8.dts
>  create mode 100644 arch/arm/boot/dts/exynos4212-tab3-wifi8.dts
>  create mode 100644 arch/arm/boot/dts/exynos4212-tab3.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index efe4152e5846..e5f63b636637 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -217,6 +217,9 @@ dtb-$(CONFIG_ARCH_EXYNOS4) += \
>  	exynos4210-smdkv310.dtb \
>  	exynos4210-trats.dtb \
>  	exynos4210-universal_c210.dtb \
> +	exynos4212-tab3-3g8.dtb \
> +	exynos4212-tab3-lte8.dtb \
> +	exynos4212-tab3-wifi8.dtb \
>  	exynos4412-i9300.dtb \
>  	exynos4412-i9305.dtb \
>  	exynos4412-itop-elite.dtb \
> diff --git a/arch/arm/boot/dts/exynos4212-tab3-3g8.dts b/arch/arm/boot/dts/exynos4212-tab3-3g8.dts
> new file mode 100644
> index 000000000000..6d890353ae76
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos4212-tab3-3g8.dts
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's Exynos4212 based Galaxy Tab 3 8.0 3G board device tree
> + * source
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + */
> +
> +/dts-v1/;
> +#include "exynos4212-tab3.dtsi"
> +
> +/ {
> +	model = "Samsung Galaxy Tab 3 8.0 3G (SM-T311) based on Exynos4212";
> +	compatible = "samsung,t311", "samsung,tab3", "samsung,exynos4212", "samsung,exynos4";
> +	chassis-type = "tablet";
> +};
> +
> +/* Pin control sleep state overrides */
> +&sleep0 {
> +		PIN_SLP(gpb-5, INPUT, UP);

Too much indentent.

> +};
> +
> +&sleep1 {
> +		PIN_SLP(gpl0-0, OUT0, NONE);

Same here.

> +		PIN_SLP(gpl1-0, OUT0, NONE);
> +		PIN_SLP(gpl2-4, OUT0, NONE);
> +		PIN_SLP(gpm3-3, OUT1, NONE);
> +};
> diff --git a/arch/arm/boot/dts/exynos4212-tab3-lte8.dts b/arch/arm/boot/dts/exynos4212-tab3-lte8.dts
> new file mode 100644
> index 000000000000..c5ec68c292b0
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos4212-tab3-lte8.dts
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Samsung's Exynos4212 based Galaxy Tab 3 8.0 LTE board device tree
> + * source
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + */
> +
> +/dts-v1/;
> +#include "exynos4212-tab3.dtsi"
> +
> +/ {
> +	model = "Samsung Galaxy Tab 3 8.0 LTE (SM-T315) based on Exynos4212";
> +	compatible = "samsung,t315", "samsung,tab3", "samsung,exynos4212", "samsung,exynos4";
> +	chassis-type = "tablet";
> +};
> +
> +/* Pin control sleep state overrides */
> +&sleep0 {
> +		PIN_SLP(gpa0-4, INPUT, UP);
> +		PIN_SLP(gpa0-5, INPUT, UP);

Same.

> +
> +		PIN_SLP(gpb-5, INPUT, UP);
> +
> +		PIN_SLP(gpc0-0, PREV, NONE);
> +		PIN_SLP(gpc1-3, INPUT, NONE);
> +
> +		PIN_SLP(gpf1-6, INPUT, NONE);
> +		PIN_SLP(gpf2-2, PREV, NONE);
> +};
> +
> +&sleep1 {
> +		PIN_SLP(gpl0-0, PREV, NONE);
> +
> +		PIN_SLP(gpl1-0, PREV, NONE);
> +
> +		PIN_SLP(gpl2-1, INPUT, DOWN);
> +		PIN_SLP(gpl2-2, INPUT, DOWN);
> +		PIN_SLP(gpl2-4, OUT0, NONE);
> +		PIN_SLP(gpl2-5, PREV, NONE);
> +
> +		PIN_SLP(gpm3-3, OUT1, NONE);

...

> +			buck5_reg: BUCK5 {
> +				regulator-name = "VMEM_1.2V_AP";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt	= <1200000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +				op_mode = <1>;
> +			};
> +
> +			buck6_reg: BUCK6 {
> +				regulator-name = "CAM_ISP_CORE_1.2V";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				op_mode = <1>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +		};
> +
> +		s5m8767_osc: clocks {
> +			compatible = "samsung,s5m8767-clk";
> +			#clock-cells = <1>;
> +			clock-output-names = "en32khz_ap",
> +						 "en32khz_cp",
> +						 "en32khz_bt";

Are these aligned with opening "?

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


Best regards,
Krzysztof