linux-kernel - Re: [PATCH v2 2/5] arm64: dts: exynos: add initial devicetree support for exynos7870

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <199646ca71487cd6d2593c9b8bd7fa69@disroot.org>
Date: Tue, 04 Feb 2025 19:12:48 +0000
From: Kaustabh Chakraborty <kauschluss@...root.org>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
 Conor Dooley <conor@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, Kees
 Cook <kees@...nel.org>, Tony Luck <tony.luck@...el.com>, "Guilherme G.
 Piccoli" <gpiccoli@...lia.com>, Sergey Lisov <sleirsgoevy@...il.com>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org, Kaustabh Chakraborty
 <kauschluss@...root.org>
Subject: Re: [PATCH v2 2/5] arm64: dts: exynos: add initial devicetree support
 for exynos7870

On 2025-02-04 18:22, Ivaylo Ivanov wrote:
> On 2/3/25 22:46, Kaustabh Chakraborty wrote:
>> Exynos7870 is an arm64 SoC manufactured by Samsung and announced in
>> 2016. It is present in multiple mid-range Samsung phones and tablets.
>>
>> Add basic devicetree support for the SoC, which includes CMUs, pin
>> controllers, I2C, UART, DW-MMC, and USB-DRD.
>>
>> Co-developed-by: Sergey Lisov <sleirsgoevy@...il.com>
>> Signed-off-by: Sergey Lisov <sleirsgoevy@...il.com>
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@...root.org>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos7870-pinctrl.dtsi | 1035 ++++++++++++++++++++
>>  arch/arm64/boot/dts/exynos/exynos7870.dtsi         |  722 ++++++++++++++
>>  2 files changed, 1757 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7870-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos7870-pinctrl.dtsi
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..28ff409c4fdc5f766d92617ea2df7be2112c28d1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/exynos/exynos7870-pinctrl.dtsi
>> @@ -0,0 +1,1035 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Samsung Exynos7870 SoC pin-mux and pin-config device tree source
>> + *
>> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include "exynos-pinctrl.h"
>> +
>> +&pinctrl0 {
> 
> I haven't had enough time to look deeper, but these are my 2 cents:
> 
> 
> Can you label them according to their block name rather than numbers?
> For example, pinctrl_abox (make sure to keep them alphabetically sorted
> as well).

Can do. The rationale behind keeping it numeric was that the names were
kind of misleading in some cases.

> 
>> +	etc0: etc0-gpio-bank {
>> +		gpio-controller;

...

>> diff --git a/arch/arm64/boot/dts/exynos/exynos7870.dtsi b/arch/arm64/boot/dts/exynos/exynos7870.dtsi
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..11129e37fc86ebaee01684ed6841c932dd6cbc8a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/exynos/exynos7870.dtsi
>> @@ -0,0 +1,722 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Samsung Exynos7870 SoC device tree source
>> + *
>> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
>> + */
>> +
>> +#include <dt-bindings/clock/exynos7870.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/soc/samsung,boot-mode.h>
>> +

...

>> +
>> +			reboot-mode {
>> +				compatible = "syscon-reboot-mode";
>> +				offset = <0x080c>;
>> +				mode-bootloader = <EXYNOS7870_BOOT_BOOTLOADER>;
>> +				mode-download = <EXYNOS7870_BOOT_DOWNLOAD>;
>> +				mode-recovery = <EXYNOS7870_BOOT_RECOVERY>;
>> +			};
>> +		};
>> +

...

>> +
>> +#include "exynos7870-pinctrl.dtsi"
>> +#include "arm/samsung/exynos-syscon-restart.dtsi"
> 
> Didn't this already include a reboot node?

Yes, I believe you've confused it with the *reboot-mode* node as defined
above.

> 
> Best regards,
> Ivaylo
> 
>>