Hi Krzysztof
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski at canonical.com]
>Sent: Thursday, January 13, 2022 6:50 PM
>To: Alim Akhtar <alim.akhtar at samsung.com>; linux-arm-
>kernel at lists.infradead.org; linux-kernel at vger.kernel.org
>Cc: soc at kernel.org; linux-clk at vger.kernel.org; devicetree at vger.kernel.org;
>olof at lixom.net; linus.walleij at linaro.org; catalin.marinas at arm.com;
>robh+dt at kernel.org; s.nawrocki at samsung.com; linux-samsung-
>soc at vger.kernel.org; pankaj.dubey at samsung.com; linux-fsd at tesla.com;
>Shashank Prashar <s.prashar at samsung.com>; Aswani Reddy
><aswani.reddy at samsung.com>
>Subject: Re: [PATCH 15/23] arm64: dts: fsd: Add initial pinctrl support
>
>On 13/01/2022 13:11, Alim Akhtar wrote:
>> Add initial pin configuration nodes for FSD SoC.
>>
>> Cc: linux-fsd at tesla.com
>> Signed-off-by: Shashank Prashar <s.prashar at samsung.com>
>> Signed-off-by: Aswani Reddy <aswani.reddy at samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar at samsung.com>
>> ---
>> arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 338
>+++++++++++++++++++++
>> arch/arm64/boot/dts/tesla/fsd.dtsi | 22 ++
>> 2 files changed, 360 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
>> b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
>> new file mode 100644
>> index 000000000000..ec8d944af636
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
>> @@ -0,0 +1,338 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Tesla Full Self-Driving SoC device tree source
>> + *
>> + * Copyright (c) 2017-2021 Samsung Electronics Co., Ltd.
>> + * https://www.samsung.com
>> + * Copyright (c) 2017-2021 Tesla, Inc.
>> + * https://www.tesla.com
>> + */
>> +
>> +#include <dt-bindings/pinctrl/samsung.h>
>> +
>> +&pinctrl_fsys0 {
>> +
>
>No need for empty line.
>
Noted
>> + gpf0: gpf0 {
>
>FYI:
>It's ok now, but the nodes will have to be renamed to "xxx-gpio-bank" later.
>
Have rebased my v2 on your pinmux schema update, so these and below comments are addressed.
Thanks
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf1: gpf1 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf6: gpf6 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf4: gpf4 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf5: gpf5 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +};
>> +
>> +&pinctrl_peric {
>> +
>
>No need for empty line.
>
>> + gpc8: gpc8 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf2: gpf2 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf3: gpf3 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd0: gpd0 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb0: gpb0 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb1: gpb1 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb4: gpb4 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb5: gpb5 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb6: gpb6 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb7: gpb7 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd1: gpd1 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd2: gpd2 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd3: gpd3 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg0: gpg0 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg1: gpg1 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg2: gpg2 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg3: gpg3 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg4: gpg4 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg5: gpg5 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg6: gpg6 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpg7: gpg7 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + pwm0_out: pwm0-out {
>
>All pin configuretion node names with "-pins" suffix. Upcoming dtschema will
>require this.
>
>> + samsung,pins = "gpb6-1";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV2>;
>> + };
>> +
>> + pwm1_out: pwm1-out {
>> + samsung,pins = "gpb6-5";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV2>;
>> + };
>> +
>> + hs_i2c0_bus: hs-i2c0-bus {
>> + samsung,pins = "gpb0-0", "gpb0-1";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c1_bus: hs-i2c1-bus {
>> + samsung,pins = "gpb0-2", "gpb0-3";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c2_bus: hs-i2c2-bus {
>> + samsung,pins = "gpb0-4", "gpb0-5";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c3_bus: hs-i2c3-bus {
>> + samsung,pins = "gpb0-6", "gpb0-7";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c4_bus: hs-i2c4-bus {
>> + samsung,pins = "gpb1-0", "gpb1-1";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c5_bus: hs-i2c5-bus {
>> + samsung,pins = "gpb1-2", "gpb1-3";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c6_bus: hs-i2c6-bus {
>> + samsung,pins = "gpb1-4", "gpb1-5";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + hs_i2c7_bus: hs-i2c7-bus {
>> + samsung,pins = "gpb1-6", "gpb1-7";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + uart0_data: uart0-data {
>> + samsung,pins = "gpb7-0", "gpb7-1";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + uart1_data: uart1-data {
>> + samsung,pins = "gpb7-4", "gpb7-5";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + spi0_bus: spi0-bus {
>> + samsung,pins = "gpb4-0", "gpb4-2", "gpb4-3";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + spi1_bus: spi1-bus {
>> + samsung,pins = "gpb4-4", "gpb4-6", "gpb4-7";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +
>> + spi2_bus: spi2-bus {
>> + samsung,pins = "gpb5-0", "gpb5-2", "gpb5-3";
>> + samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
>> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>> + };
>> +};
>> +
>> +&pinctrl_pmu {
>> +
>
>No need for empty line.
>
>> + gpq0: gpq0 {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + };
>
>
>
>
>Best regards,
>Krzysztof