mbox series

[00/23] Add support for Tesla Full Self-Driving (FSD) SoC

Message ID 20220113121143.22280-1-alim.akhtar@samsung.com
Headers show
Series Add support for Tesla Full Self-Driving (FSD) SoC | expand

Message

Alim Akhtar Jan. 13, 2022, 12:11 p.m. UTC
This patch set adds basic support for the Tesla Full Self-Driving (FSD)
SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
as well as several IPs.

Patches 1 to 8 provide support for the clock controller
(which is designed similarly to Exynos SoCs).

The remaining changes provide pinmux support, initial device tree support,
and SPI, ADC, and MCT IP functionality.


Alim Akhtar (21):
  dt-bindings: clock: Document FSD CMU bindings
  dt-bindings: clock: Add bindings definitions for FSD CMU blocks
  clk: samsung: fsd: Add initial clock support
  clk: samsung: fsd: Add cmu_peric block clock information
  clk: samsung: fsd: Add cmu_fsys0 clock information
  clk: samsung: fsd: Add cmu_fsys1 clock information
  clk: samsung: fsd: Add cmu_imem block clock information
  clk: samsung: fsd: Add cmu_mfc block clock information
  clk: samsung: fsd: Add cam_csi block clock information
  dt-bindings: pinctrl: samsung: Add compatible for Tesla FSD SoC
  pinctrl: samsung: add FSD SoC specific data
  dt-bindings: add vendor prefix for Tesla
  dt-bindings: arm: add Tesla FSD ARM SoC
  arm64: dts: fsd: Add initial device tree support
  arm64: dts: fsd: Add initial pinctrl support
  arm64: defconfig: Enable Tesla FSD SoC
  Documentation: bindings: Add fsd spi compatible in dt-bindings
    document
  spi: s3c64xx: Add spi port configuration for Tesla FSD SoC
  dt-bindings: iio: adc: exynos-adc: Add ADC-V3 variant
  iio: adc: exynos-adc: Add support for ADC V3 controller
  arm64: dts: fsd: Add ADC device tree node

Aswani Reddy (2):
  arm64: dts: fsd: Add SPI device nodes
  clocksource: exynos_mct: Add support for handling three clusters

 .../devicetree/bindings/arm/tesla.yaml        |   25 +
 .../bindings/clock/tesla,fsd-clock.yaml       |  212 ++
 .../bindings/iio/adc/samsung,exynos-adc.yaml  |    1 +
 .../bindings/pinctrl/samsung-pinctrl.txt      |    1 +
 .../devicetree/bindings/spi/spi-samsung.txt   |    1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 MAINTAINERS                                   |    8 +
 arch/arm64/Kconfig.platforms                  |    6 +
 arch/arm64/boot/dts/Makefile                  |    1 +
 arch/arm64/boot/dts/tesla/Makefile            |    3 +
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi    |  338 +++
 arch/arm64/boot/dts/tesla/fsd.dts             |  156 ++
 arch/arm64/boot/dts/tesla/fsd.dtsi            |  805 +++++++
 arch/arm64/configs/defconfig                  |    1 +
 drivers/clk/samsung/Makefile                  |    1 +
 drivers/clk/samsung/clk-fsd.c                 | 1858 +++++++++++++++++
 drivers/clk/samsung/clk-pll.c                 |    1 +
 drivers/clk/samsung/clk-pll.h                 |    1 +
 drivers/clocksource/exynos_mct.c              |    6 +-
 drivers/iio/adc/exynos_adc.c                  |   74 +-
 .../pinctrl/samsung/pinctrl-exynos-arm64.c    |   71 +
 drivers/pinctrl/samsung/pinctrl-samsung.c     |    2 +
 drivers/pinctrl/samsung/pinctrl-samsung.h     |    1 +
 drivers/spi/spi-s3c64xx.c                     |   13 +
 include/dt-bindings/clock/fsd-clk.h           |  146 ++
 25 files changed, 3731 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/tesla.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml
 create mode 100644 arch/arm64/boot/dts/tesla/Makefile
 create mode 100644 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
 create mode 100644 arch/arm64/boot/dts/tesla/fsd.dts
 create mode 100644 arch/arm64/boot/dts/tesla/fsd.dtsi
 create mode 100644 drivers/clk/samsung/clk-fsd.c
 create mode 100644 include/dt-bindings/clock/fsd-clk.h


base-commit: c9e6606c7fe92b50a02ce51dda82586ebdf99b48

Comments

Krzysztof Kozlowski Jan. 13, 2022, 12:31 p.m. UTC | #1
On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch set adds basic support for the Tesla Full Self-Driving (FSD)
> SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
> as well as several IPs.
> 
> Patches 1 to 8 provide support for the clock controller
> (which is designed similarly to Exynos SoCs).
> 
> The remaining changes provide pinmux support, initial device tree support,
> and SPI, ADC, and MCT IP functionality.

Does FSD have some version number? The FDS, especially in compatibles,
looks quite generic, so what will happen if a newer SoC comes later? You
would have:
 - tesla,fsd-pinctrl
 - tesla,fsd-xxxx-pinctrl (where xxxx could be some new version)

This will be extra confusing, because fsd-pinctrl looks like the generic
one, while it is specific...

Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 12:49 p.m. UTC | #2
On 13/01/2022 13:11, Alim Akhtar wrote:
> Add initial clock support for FSD (Full Self-Driving) SoC
> which is required to bring-up platforms based on this SoC.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/clk/samsung/Makefile  |   1 +
>  drivers/clk/samsung/clk-fsd.c | 308 ++++++++++++++++++++++++++++++++++
>  drivers/clk/samsung/clk-pll.c |   1 +
>  drivers/clk/samsung/clk-pll.h |   1 +
>  4 files changed, 311 insertions(+)
>  create mode 100644 drivers/clk/samsung/clk-fsd.c
> 
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index c46cf11e4d0b..d66b2ede004c 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-exynos-audss.o
>  obj-$(CONFIG_EXYNOS_CLKOUT)	+= clk-exynos-clkout.o
>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7.o
>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos850.o
> +obj-$(CONFIG_ARCH_TESLA_FSD)         += clk-fsd.o

It should be rather it's own CONFIG_TESLA_FSD_CLK option, just like
other Exynos designs. This keeps unified approach with existing Samsung
clock Kconfig.

>  obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o
>  obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o
>  obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o
> diff --git a/drivers/clk/samsung/clk-fsd.c b/drivers/clk/samsung/clk-fsd.c
> new file mode 100644
> index 000000000000..e47523106d9e
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-fsd.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Common Clock Framework support for FSD SoC.
> + *
> + * Copyright (c) 2017-2022 Samsung Electronics Co., Ltd.
> + *             https://www.samsung.com
> + * Copyright (c) 2017-2022 Tesla, Inc.
> + *             https://www.tesla.com
> + *

Drop the line break with empty * comment.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +
> +#include "clk.h"
> +#include <dt-bindings/clock/fsd-clk.h>

dt-bindings headers before local clk.h.

> +
> +/* Register Offset definitions for CMU_CMU (0x11c10000) */



Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 12:55 p.m. UTC | #3
On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch adds CMU_PERIC block clock information needed
> for various IPs functions found in this block.

Here and in all other commits, please do not use "This patch". Instead:
https://elixir.bootlin.com/linux/v5.13/source/Documentation/process/submitting-patches.rst#L89

> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com>
> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/clk/samsung/clk-fsd.c | 464 +++++++++++++++++++++++++++++++++-
>  1 file changed, 463 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-fsd.c b/drivers/clk/samsung/clk-fsd.c
> index e47523106d9e..6da20966ba99 100644
> --- a/drivers/clk/samsung/clk-fsd.c
> +++ b/drivers/clk/samsung/clk-fsd.c
> @@ -9,12 +9,59 @@
>   *
>   */
>  
> -#include <linux/clk-provider.h>
>  #include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>

Please order the includes alphabetically.

>  
>  #include "clk.h"
>  #include <dt-bindings/clock/fsd-clk.h>
>  
> +/* Gate register bits */
> +#define GATE_MANUAL		BIT(20)
> +#define GATE_ENABLE_HWACG	BIT(28)
> +
> +/* Gate register offsets range */
> +#define GATE_OFF_START		0x2000
> +#define GATE_OFF_END		0x2fff
> +
> +/**
> + * fsd_init_clocks - Set clocks initial configuration
> + * @np:			CMU device tree node with "reg" property (CMU addr)
> + * @reg_offs:		Register offsets array for clocks to init
> + * @reg_offs_len:	Number of register offsets in reg_offs array
> + *
> + * Set manual control mode for all gate clocks.
> + */
> +static void __init fsd_init_clocks(struct device_node *np,
> +		const unsigned long *reg_offs, size_t reg_offs_len)

The same as exynos_arm64_init_clocks - please re-use instead of duplicating.

> +{
> +	void __iomem *reg_base;
> +	size_t i;
> +
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base)
> +		panic("%s: failed to map registers\n", __func__);
> +
> +	for (i = 0; i < reg_offs_len; ++i) {
> +		void __iomem *reg = reg_base + reg_offs[i];
> +		u32 val;
> +
> +		/* Modify only gate clock registers */
> +		if (reg_offs[i] < GATE_OFF_START || reg_offs[i] > GATE_OFF_END)
> +			continue;
> +
> +		val = readl(reg);
> +		val |= GATE_MANUAL;
> +		val &= ~GATE_ENABLE_HWACG;
> +		writel(val, reg);
> +	}
> +
> +	iounmap(reg_base);
> +}
> +

(...)

> +/**
> + * fsd_cmu_probe - Probe function for FSD platform clocks
> + * @pdev: Pointer to platform device
> + *
> + * Configure clock hierarchy for clock domains of FSD platform
> + */
> +static int __init fsd_cmu_probe(struct platform_device *pdev)
> +{
> +	const struct samsung_cmu_info *info;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +
> +	info = of_device_get_match_data(dev);
> +	fsd_init_clocks(np, info->clk_regs, info->nr_clk_regs);
> +	samsung_cmu_register_one(np, info);
> +
> +	/* Keep bus clock running, so it's possible to access CMU registers */
> +	if (info->clk_name) {
> +		struct clk *bus_clk;
> +
> +		bus_clk = clk_get(dev, info->clk_name);
> +		if (IS_ERR(bus_clk)) {
> +			pr_err("%s: could not find bus clock %s; err = %ld\n",
> +			       __func__, info->clk_name, PTR_ERR(bus_clk));
> +		} else {
> +			clk_prepare_enable(bus_clk);
> +		}
> +	}
> +
> +	return 0;
> +}

Please re-use exynos_arm64_register_cmu(). This will also solve my
previous comment about exynos_arm64_init_clocks().

> +
> +/* CMUs which belong to Power Domains and need runtime PM to be implemented */
> +static const struct of_device_id fsd_cmu_of_match[] = {
> +	{
> +		.compatible = "tesla,fsd-clock-peric",
> +		.data = &peric_cmu_info,
> +	}, {
> +	},
> +};
> +
> +static struct platform_driver fsd_cmu_driver __refdata = {
> +	.driver	= {
> +		.name = "fsd-cmu",
> +		.of_match_table = fsd_cmu_of_match,
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe = fsd_cmu_probe,
> +};
> +
> +static int __init fsd_cmu_init(void)
> +{
> +	return platform_driver_register(&fsd_cmu_driver);
> +}
> +core_initcall(fsd_cmu_init);
> 


Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 12:57 p.m. UTC | #4
On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch adds Tesla FSD SoC specific data to enable pinctrl.
> FSD SoC has similar pinctrl controller as found in the most
> samsung/exynos SoCs.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 71 +++++++++++++++++++
>  drivers/pinctrl/samsung/pinctrl-samsung.c     |  2 +
>  drivers/pinctrl/samsung/pinctrl-samsung.h     |  1 +
>  3 files changed, 74 insertions(+)
> 
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index 6b77fd24571e..b9175b4911ac 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -646,3 +646,74 @@ const struct samsung_pinctrl_of_match_data exynosautov9_of_data __initconst = {
>  	.ctrl		= exynosautov9_pin_ctrl,
>  	.num_ctrl	= ARRAY_SIZE(exynosautov9_pin_ctrl),
>  };
> +
> +/*
> + * Pinctrl driver data for Tesla FSD SoC. FSD SoC includes three
> + * gpio/pin-mux/pinconfig controllers.
> + */
> +
> +/* pin banks of FSD pin-controller 0 (FSYS) */
> +static const struct samsung_pin_bank_data fsd_pin_banks0[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x00, "gpf0", 0x00),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x20, "gpf1", 0x04),
> +	EXYNOS850_PIN_BANK_EINTG(3, 0x40, "gpf6", 0x08),
> +	EXYNOS850_PIN_BANK_EINTG(2, 0x60, "gpf4", 0x0c),
> +	EXYNOS850_PIN_BANK_EINTG(6, 0x80, "gpf5", 0x10),
> +};
> +
> +/* pin banks of FSD pin-controller 1 (PERIC) */
> +static const struct samsung_pin_bank_data fsd_pin_banks1[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTG(4, 0x000, "gpc8", 0x00),
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x020, "gpf2", 0x04),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x040, "gpf3", 0x08),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x060, "gpd0", 0x0c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x080, "gpb0", 0x10),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x0a0, "gpb1", 0x14),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x0c0, "gpb4", 0x18),
> +	EXYNOS850_PIN_BANK_EINTG(4, 0x0e0, "gpb5", 0x1c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x100, "gpb6", 0x20),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x120, "gpb7", 0x24),
> +	EXYNOS850_PIN_BANK_EINTG(5, 0x140, "gpd1", 0x28),
> +	EXYNOS850_PIN_BANK_EINTG(5, 0x160, "gpd2", 0x2c),
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x180, "gpd3", 0x30),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1a0, "gpg0", 0x34),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1c0, "gpg1", 0x38),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1e0, "gpg2", 0x3c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x200, "gpg3", 0x40),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x220, "gpg4", 0x44),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x240, "gpg5", 0x48),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x260, "gpg6", 0x4c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x280, "gpg7", 0x50),
> +};
> +
> +/* pin banks of FSD pin-controller 2 (PMU) */
> +static const struct samsung_pin_bank_data fsd_pin_banks2[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTN(3, 0x00, "gpq0"),
> +};
> +
> +const struct samsung_pin_ctrl fsd_pin_ctrl[] __initconst = {
> +	{
> +		/* pin-controller instance 0 FSYS0 data */
> +		.pin_banks	= fsd_pin_banks0,
> +		.nr_banks	= ARRAY_SIZE(fsd_pin_banks0),

No wake-up interrupts (eint_wkup_init)? It's fine not to have them but
just looks incomplete.

In general looks ok, except discussion about compatibles.

> +		.eint_gpio_init = exynos_eint_gpio_init,
> +		.suspend	= exynos_pinctrl_suspend,
> +		.resume		= exynos_pinctrl_resume,
Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 1:16 p.m. UTC | #5
On 13/01/2022 13:11, Alim Akhtar wrote:
> Add initial device tree support for "Full Self-Driving" (FSD) SoC
> This SoC contain three clusters of four cortex-a72 CPUs and various
> peripheral IPs.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Arjun K V <arjun.kv@samsung.com>
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
> Signed-off-by: Shashank Prashar <s.prashar@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  MAINTAINERS                        |   8 +
>  arch/arm64/Kconfig.platforms       |   6 +
>  arch/arm64/boot/dts/Makefile       |   1 +
>  arch/arm64/boot/dts/tesla/Makefile |   3 +
>  arch/arm64/boot/dts/tesla/fsd.dts  | 140 ++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi | 715 +++++++++++++++++++++++++++++
>  6 files changed, 873 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/tesla/Makefile
>  create mode 100644 arch/arm64/boot/dts/tesla/fsd.dts
>  create mode 100644 arch/arm64/boot/dts/tesla/fsd.dtsi
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb18ce7168aa..02d56909c5e2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2726,6 +2726,14 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/media/tegra-cec.txt
>  F:	drivers/media/cec/platform/tegra/
>  
> +ARM/TESLA FSD SoC SUPPORT
> +M:	Alim Akhtar <alim.akhtar@samsung.com>
> +M:	linux-fsd@tesla.com
> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> +L:	linux-samsung-soc@vger.kernel.org
> +S:	Maintained
> +F:	arch/arm64/boot/dts/tesla*
> +

+Cc Arnd,

Please Cc all SoC maintainers in new sub-architecture submissions, so
Olof and Arnd.

>  ARM/TETON BGA MACHINE SUPPORT
>  M:	"Mark F. Brown" <mark.brown314@gmail.com>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 54e3910e8b9b..bb8a047c2359 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -267,6 +267,12 @@ config ARCH_TEGRA
>  	help
>  	  This enables support for the NVIDIA Tegra SoC family.
>  
> +config ARCH_TESLA_FSD
> +	bool "ARMv8 based Tesla platform"
> +	select ARCH_EXYNOS

How similar it is? I think it is better to duplicate Exynos
selections/options here, instead of selecting entire ARCH. If this would
require "depends on ARCH_EXYNOS || ARCH_TESLA_FSD" everywhere in the
drivers, it's a hint that it is not a separate SoC but it is an Exynos,
so it might not need a new sub-architecture.


> +	help
> +	  Support for ARMv8 based Tesla platforms.
> +
>  config ARCH_SPRD
>  	bool "Spreadtrum SoC platform"
>  	help
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 639e01a4d855..1ba04e31a438 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -27,6 +27,7 @@ subdir-y += rockchip
>  subdir-y += socionext
>  subdir-y += sprd
>  subdir-y += synaptics
> +subdir-y += tesla
>  subdir-y += ti
>  subdir-y += toshiba
>  subdir-y += xilinx
> diff --git a/arch/arm64/boot/dts/tesla/Makefile b/arch/arm64/boot/dts/tesla/Makefile
> new file mode 100644
> index 000000000000..a9818cda6b08
> --- /dev/null
> +++ b/arch/arm64/boot/dts/tesla/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_TESLA_FSD) += \
> +	fsd.dtb
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dts b/arch/arm64/boot/dts/tesla/fsd.dts
> new file mode 100644
> index 000000000000..e9bbd3284de9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/tesla/fsd.dts

No, this is not a fsd.dts, but some board. You call FSD a SoC, so this
should have different name.

> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Tesla FSD board 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
> + */
> +
> +/dts-v1/;
> +#include "fsd.dtsi"
> +
> +/ {
> +	model = "Tesla Full Self-Driving (FSD) SoC";

Wrong model, this is DTS, not DTSI.

> +	compatible = "tesla,fsd";

Missing compatible for board.

> +
> +	aliases {
> +		serial0 = &serial_0;
> +		serial1 = &serial_1;
> +	};
> +
> +	chosen {
> +		stdout-path = &serial_0;
> +		linux,initrd-start = <0xE0000000>;
> +		linux,initrd-end = <0xE4F00000>;
> +		bootargs = "console=ttySAC0,115200n8
> +			earlycon=exynos4210,0x14180000 root=/dev/ram0
> +			init=/linuxrc";

These are not bootargs matching SoC. earlycon is purely a debug option.
console is duplicating stdout-path. root and init are also not a part of
DTSI or DTS.

> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x80000000 0x2 0x00000000>;
> +	};
> +};
> +
> +&fin_pll {
> +	clock-frequency = <24000000>;
> +};
> +
> +&serial_0 {
> +	status = "okay";
> +};
> +
> +&serial_1 {
> +	status = "okay";
> +};
> +
> +&clock_cmu {
> +	status = "okay";
> +};
> +
> +&clock_imem {
> +	status = "okay";
> +};
> +
> +&clock_peric {
> +	status = "okay";
> +};

The labels/overrides are with weird order... looks like a mess. Anyway,
clocks should be always enabled, why board has to enable them?

> +
> +&smmu_isp {
> +	status = "okay";
> +};
> +
> +&clock_fsys0 {
> +	status = "okay";
> +};
> +
> +&clock_fsys1 {
> +	status = "okay";
> +};
> +
> +&smmu_peric {
> +	status = "okay";

The same as clocks.

> +};
> +
> +&smmu_imem {
> +	status = "okay";
> +};
> +
> +&smmu_fsys0 {
> +	status = "okay";
> +};
> +
> +&hsi2c_0 {
> +	status = "okay";

This looks wrong. There is nothing attached.

> +};
> +
> +&hsi2c_1 {
> +	status = "okay";

Same here and other I2C.

> +};
> +
> +&hsi2c_2 {
> +	status = "okay";
> +};
> +
> +&hsi2c_3 {
> +	status = "okay";
> +};
> +
> +&hsi2c_4 {
> +	status = "okay";
> +};
> +
> +&hsi2c_5 {
> +	status = "okay";
> +};
> +
> +&hsi2c_6 {
> +	status = "okay";
> +};
> +
> +&hsi2c_7 {
> +	status = "okay";
> +};
> +
> +&pwm_0 {
> +	status = "okay";

Nope, nope.

> +};
> +
> +&pwm_1 {
> +	status = "okay";
> +};
> +
> +&mdma0 {
> +	status = "okay";

Can you imagine SoC with disabled DMA?

> +};
> +
> +&mdma1 {
> +	status = "okay";
> +};
> +
> +&pdma0 {
> +	status = "okay";
> +};
> +
> +&pdma1 {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> new file mode 100644
> index 000000000000..47cd9f20566e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -0,0 +1,715 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Tesla Full Self-Driving SoC device tree source
> + *
> + * Copyright (c) 2017-2022 Samsung Electronics Co., Ltd.
> + *		https://www.samsung.com
> + * Copyright (c) 2017-2022 Tesla, Inc.
> + *		https://www.tesla.com
> + */
> +
> +#include <dt-bindings/clock/fsd-clk.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	compatible = "tesla,fsd";

Same compatible as board - nope.

> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		watchdog0 = &watchdog_0;
> +		watchdog1 = &watchdog_1;
> +		watchdog2 = &watchdog_2;

I think watchdogs do not need/use aliases.

> +		hsi2c0 = &hsi2c_0;
> +		hsi2c1 = &hsi2c_1;
> +		hsi2c2 = &hsi2c_2;
> +		hsi2c3 = &hsi2c_3;
> +		hsi2c4 = &hsi2c_4;
> +		hsi2c5 = &hsi2c_5;
> +		hsi2c6 = &hsi2c_6;
> +		hsi2c7 = &hsi2c_7;
> +	};
> +
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpucl0_0>;
> +				};
> +				core1 {
> +					cpu = <&cpucl0_1>;
> +				};
> +				core2 {
> +					cpu = <&cpucl0_2>;
> +				};
> +				core3 {
> +					cpu = <&cpucl0_3>;
> +				};
> +			};
> +
> +			cluster1 {
> +				core0 {
> +					cpu = <&cpucl1_0>;
> +				};
> +				core1 {
> +					cpu = <&cpucl1_1>;
> +				};
> +				core2 {
> +					cpu = <&cpucl1_2>;
> +				};
> +				core3 {
> +					cpu = <&cpucl1_3>;
> +				};
> +			};
> +
> +			cluster2 {
> +				core0 {
> +					cpu = <&cpucl2_0>;
> +				};
> +				core1 {
> +					cpu = <&cpucl2_1>;
> +				};
> +				core2 {
> +					cpu = <&cpucl2_2>;
> +				};
> +				core3 {
> +					cpu = <&cpucl2_3>;
> +				};
> +			};
> +		};
> +
> +		/* Cluster 0 */
> +		cpucl0_0: cpu@0 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x000>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl0_1: cpu@1 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x001>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl0_2: cpu@2 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x002>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl0_3: cpu@3 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x003>;
> +				enable-method = "psci";
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		/* Cluster 1 */
> +		cpucl1_0: cpu@100 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x100>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl1_1: cpu@101 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x101>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl1_2: cpu@102 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x102>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl1_3: cpu@103 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x103>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		/* Cluster 2 */
> +		cpucl2_0: cpu@200 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x200>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl2_1: cpu@201 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x201>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl2_2: cpu@202 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x202>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		cpucl2_3: cpu@203 {
> +				device_type = "cpu";
> +				compatible = "arm,cortex-a72";
> +				reg = <0x0 0x203>;
> +				enable-method = "psci";
> +				clock-frequency = <2400000000>;
> +				cpu-idle-states = <&CPU_SLEEP>;
> +				next-level-cache = <&L2_0>;
> +		};
> +
> +		idle-states {
> +			entry-method = "arm,psci";
> +
> +			CPU_SLEEP: cpu-sleep {
> +				idle-state-name = "c2";
> +				compatible = "arm,idle-state";
> +				local-timer-stop;
> +				arm,psci-suspend-param = <0x0010000>;
> +				entry-latency-us = <30>;
> +				exit-latency-us = <75>;
> +				min-residency-us = <300>;
> +				status = "okay";

No need for status.

> +			};
> +		};
> +
> +		L2_0: l2-cache0 {

lowercase letters in label.

> +			compatible = "cache";

You miss here some properties. Does your DTSI/DTS pass dtschema validation?

> +		};
> +	};
> +
> +	arm-pmu {
> +		compatible = "arm,armv8-pmuv3";
> +		interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 385 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 386 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 387 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-affinity = <&cpucl0_0>, <&cpucl0_1>, <&cpucl0_2>,
> +				     <&cpucl0_3>, <&cpucl1_0>, <&cpucl1_1>,
> +				     <&cpucl1_2>, <&cpucl1_3>, <&cpucl2_0>,
> +				     <&cpucl2_1>, <&cpucl2_2>, <&cpucl2_3>;
> +	};
> +
> +	psci {
> +		compatible = "arm,psci";
> +		method = "smc";
> +		cpu_on = <0xC4000003>;
> +		cpu_suspend = <0xC4000001>;
> +		cpu_off = <0x84000002>;
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,

Not sure - don't you need CPU mask?

> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +
> +	fin_pll: clock {
> +		compatible = "fixed-clock";
> +		clock-output-names = "fin_pll";
> +		#clock-cells = <0>;
> +	};
> +
> +	soc: soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges = <0x0 0x0 0x0 0x0 0x0 0x18000000>;
> +		dma-ranges = <0x0 0x0 0x0 0x0 0x10 0x0>;
> +
> +		gic: interrupt-controller@10400000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			reg =	<0x0 0x10400000 0x0 0x10000>, /* GICD */
> +				<0x0 0x10600000 0x0 0x200000>; /* GICR_RD+GICR_SGI */
> +		};
> +
> +		smmu_isp: iommu@12100000 {
> +			compatible = "arm,mmu-500";
> +			reg = <0x0 0x12100000 0x0 0x10000>;
> +			#iommu-cells = <2>;
> +			#global-interrupts = <11>;
> +			interrupts = <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>, /* Global secure fault */
> +				     <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>, /* Global non-secure fault */
> +				     <GIC_SPI 346 IRQ_TYPE_LEVEL_HIGH>, /* Combined secure interrupt */
> +				     <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>, /* Combined non-secure interrupt */
> +				     /* Performance counter interrupts */
> +				     <GIC_SPI 323 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_CSI   */
> +				     <GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_DP_0  */
> +				     <GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_DP_1  */
> +				     <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_ISP_0 */
> +				     <GIC_SPI 327 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_ISP_1 */
> +				     <GIC_SPI 328 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_MFC_0 */
> +				     <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>, /* for CAM_MFC_1 */
> +				     /* Per context non-secure context interrupts, 0-7 interrupts */
> +				     <GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_0 */
> +				     <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_1 */
> +				     <GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_2 */
> +				     <GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_3 */
> +				     <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_4 */
> +				     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_5 */
> +				     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_6 */
> +				     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>; /* for CONTEXT_7 */
> +			status = "disabled";
> +		};
> +
> +		smmu_imem: iommu@10200000 {
> +			compatible = "arm,mmu-500";
> +			reg = <0x0 0x10200000 0x0 0x10000>;
> +			#iommu-cells = <2>;
> +			#global-interrupts = <7>;
> +			interrupts = <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>, /* Global secure fault */
> +				     <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>, /* Global non-secure fault */
> +				     <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>, /* Combined secure interrupt */
> +				     <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>, /* Combined non-secure interrupt */
> +				     /* Performance counter interrupts */
> +				     <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>, /* for FSYS1_0 */
> +				     <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>, /* for FSYS1_1 */
> +				     <GIC_SPI 443 IRQ_TYPE_LEVEL_HIGH>, /* for IMEM_0  */
> +				     /* Per context non-secure context interrupts, 0-3 interrupts */
> +				     <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_0 */
> +				     <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_1 */
> +				     <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_2 */
> +				     <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>; /* for CONTEXT_3 */
> +			status = "disabled";
> +		};
> +
> +		smmu_peric: iommu@14900000 {
> +			compatible = "arm,mmu-500";
> +			reg = <0x0 0x14900000 0x0 0x10000>;
> +			#iommu-cells = <2>;
> +			#global-interrupts = <5>;
> +			interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, /* Global secure fault */
> +				     <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, /* Global non-secure fault */
> +				     <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, /* Combined secure interrupt */
> +				     <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, /* Combined non-secure interrupt */
> +				     /* Performance counter interrupts */
> +				     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, /* for PERIC */
> +				     /* Per context non-secure context interrupts, 0-1 interrupts */
> +				     <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_0 */
> +				     <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>; /* for CONTEXT_1 */
> +			status = "disabled";
> +		};
> +
> +		smmu_fsys0: iommu@15450000 {
> +			compatible = "arm,mmu-500";
> +			reg = <0x0 0x15450000 0x0 0x10000>;
> +			#iommu-cells = <2>;
> +			#global-interrupts = <5>;
> +			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>, /* Global secure fault */
> +				     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>, /* Global non-secure fault */
> +				     <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, /* Combined secure interrupt */
> +				     <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>, /* Combined non-secure interrupt */
> +				     /* Performance counter interrupts */
> +				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, /* for FSYS0   */
> +				     /* Per context non-secure context interrupts, 0-1 interrupts */
> +				     <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>, /* for CONTEXT_0 */
> +				     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; /* for CONTEXT_1 */
> +			status = "disabled";
> +		};
> +
> +		clock_cmu: clock-controller@11C10000 {
> +			compatible = "tesla,fsd-clock-cmu";
> +			reg = <0x0 0x11C10000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>;
> +			clock-names = "fin_pll";
> +			status = "disabled";

This explains why DTS has to enable basic SoC features... why do you
disable your clock controllers?

> +		};
> +
> +		clock_imem: clock-controller@10010000 {
> +			compatible = "tesla,fsd-clock-imem";
> +			reg = <0x0 0x10010000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>,
> +				<&clock_cmu DOUT_CMU_IMEM_TCUCLK>,
> +				<&clock_cmu DOUT_CMU_IMEM_ACLK>,
> +				<&clock_cmu DOUT_CMU_IMEM_DMACLK>;
> +			clock-names = "fin_pll",
> +				"dout_cmu_imem_tcuclk",
> +				"dout_cmu_imem_aclk",
> +				"dout_cmu_imem_dmaclk";
> +			status = "disabled";
> +		};
> +
> +		clock_peric: clock-controller@14010000 {
> +			compatible = "tesla,fsd-clock-peric";
> +			reg = <0x0 0x14010000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>,
> +				<&clock_cmu DOUT_CMU_PLL_SHARED0_DIV4>,
> +				<&clock_cmu DOUT_CMU_PERIC_SHARED1DIV36>,
> +				<&clock_cmu DOUT_CMU_PERIC_SHARED0DIV3_TBUCLK>,
> +				<&clock_cmu DOUT_CMU_PERIC_SHARED0DIV20>,
> +				<&clock_cmu DOUT_CMU_PERIC_SHARED1DIV4_DMACLK>;
> +			clock-names = "fin_pll",
> +				"dout_cmu_pll_shared0_div4",
> +				"dout_cmu_peric_shared1div36",
> +				"dout_cmu_peric_shared0div3_tbuclk",
> +				"dout_cmu_peric_shared0div20",
> +				"dout_cmu_peric_shared1div4_dmaclk";
> +			status = "disabled";
> +		};
> +
> +		clock_fsys0: clock-controller@15010000 {
> +			compatible = "tesla,fsd-clock-fsys0";
> +			reg = <0x0 0x15010000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>,
> +				<&clock_cmu DOUT_CMU_PLL_SHARED0_DIV6>,
> +				<&clock_cmu DOUT_CMU_FSYS0_SHARED1DIV4>,
> +				<&clock_cmu DOUT_CMU_FSYS0_SHARED0DIV4>;
> +			clock-names = "fin_pll",
> +				"dout_cmu_pll_shared0_div6",
> +				"dout_cmu_fsys0_shared1div4",
> +				"dout_cmu_fsys0_shared0div4";
> +			status = "disabled";
> +		};
> +
> +		clock_fsys1: clock-controller@16810000 {
> +			compatible = "tesla,fsd-clock-fsys1";
> +			reg = <0x0 0x16810000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>,
> +				<&clock_cmu DOUT_CMU_FSYS1_SHARED0DIV8>,
> +				<&clock_cmu DOUT_CMU_FSYS1_SHARED0DIV4>;
> +			clock-names = "fin_pll",
> +				"dout_cmu_fsys1_shared0div8",
> +				"dout_cmu_fsys1_shared0div4";
> +			status = "disabled";
> +		};
> +
> +		clock_mfc: clock-controller@12810000 {
> +			compatible = "tesla,fsd-clock-mfc";
> +			reg = <0x0 0x12810000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>;
> +			clock-names = "fin_pll";
> +			status = "disabled";
> +		};
> +
> +		clock_csi: clock-controller@12610000 {
> +			compatible = "tesla,fsd-clock-cam_csi";
> +			reg = <0x0 0x12610000 0x0 0x3000>;
> +			#clock-cells = <1>;
> +			clocks = <&fin_pll>;
> +			clock-names = "fin_pll";
> +			status = "disabled";
> +		};
> +
> +		mdma0: mdma@10100000 {

Node name: just dma to be generic. Schema does not enforce it but might
at some point.

> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x0 0x10100000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			#dma-channels = <8>;
> +			#dma-requests = <32>;
> +			clocks = <&clock_imem IMEM_DMA0_IPCLKPORT_ACLK>;
> +			clock-names = "apb_pclk";
> +			iommus = <&smmu_imem 0x800 0x0>;
> +			status = "disabled";
> +		};
> +
> +		mdma1: mdma@10110000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x0 0x10110000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			#dma-channels = <8>;
> +			#dma-requests = <32>;
> +			clocks = <&clock_imem IMEM_DMA1_IPCLKPORT_ACLK>;
> +			clock-names = "apb_pclk";
> +			iommus = <&smmu_imem 0x801 0x0>;
> +			status = "disabled";
> +		};
> +
> +		pdma0: pdma@14280000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x0 0x14280000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			#dma-channels = <8>;
> +			#dma-requests = <32>;
> +			clocks = <&clock_peric PERIC_DMA0_IPCLKPORT_ACLK>;
> +			clock-names = "apb_pclk";
> +			iommus = <&smmu_peric 0x2 0x0>;
> +			status = "disabled";
> +		};
> +
> +		pdma1: pdma@14290000 {
> +			compatible = "arm,pl330", "arm,primecell";
> +			reg = <0x0 0x14290000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			#dma-channels = <8>;
> +			#dma-requests = <32>;
> +			clocks = <&clock_peric PERIC_DMA1_IPCLKPORT_ACLK>;
> +			clock-names = "apb_pclk";
> +			iommus = <&smmu_peric 0x1 0x0>;
> +			status = "disabled";
> +		};
> +
> +		mct: mct@10040000 {
> +			compatible = "samsung,exynos4210-mct";
> +			reg = <0x0 0x10040000 0x0 0x800>;
> +			interrupts = <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&fin_pll>, <&clock_imem IMEM_MCT_PCLK>;
> +			clock-names = "fin_pll", "mct";
> +		};
> +
> +		serial_0: serial@14180000 {
> +			compatible = "samsung,exynos4210-uart";
> +			reg = <0x0 0x14180000 0x0 0x100>;
> +			interrupts = <GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>;
> +			dmas = <&pdma1 0>, <&pdma1 1>;
> +			dma-names = "tx", "rx";
> +			clocks = <&clock_peric PERIC_PCLK_UART0>,
> +				 <&clock_peric PERIC_SCLK_UART0>;
> +			clock-names = "uart", "clk_uart_baud0";
> +			status = "disabled";
> +		};
> +
> +		serial_1: serial@14190000 {
> +			compatible = "samsung,exynos4210-uart";
> +			reg = <0x0 0x14190000 0x0 0x100>;
> +			interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>;
> +			dmas = <&pdma1 2>, <&pdma1 3>;
> +			dma-names = "tx", "rx";
> +			clocks = <&clock_peric PERIC_PCLK_UART1>,
> +				 <&clock_peric PERIC_SCLK_UART1>;
> +			clock-names = "uart", "clk_uart_baud0";
> +			status = "disabled";
> +		};
> +
> +		pmu_system_controller: system-controller@11400000 {
> +			compatible = "samsung,exynos7-pmu", "syscon";
> +			reg = <0x0 0x11400000 0x0 0x5000>;
> +		};
> +
> +		watchdog_0: watchdog@100A0000 {

lowercase hex addresses. Here, in reg below and all other nodes.

> +			compatible = "samsung,exynos7-wdt";
> +			reg = <0x0 0x100A0000 0x0 0x100>;
> +			interrupts = <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>;
> +			samsung,syscon-phandle = <&pmu_system_controller>;
> +			clocks = <&fin_pll>;
> +			clock-names = "watchdog";
> +			interrupt-mode = <1>;

This property looks unsupported. Did you run dtschema checks?

Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 1:19 p.m. UTC | #6
On 13/01/2022 13:11, Alim Akhtar wrote:
> Add initial pin configuration nodes for FSD SoC.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Shashank Prashar <s.prashar@samsung.com>
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@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.

> +	gpf0: gpf0 {

FYI:
It's ok now, but the nodes will have to be renamed to "xxx-gpio-bank" later.

> +		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
Krzysztof Kozlowski Jan. 13, 2022, 1:28 p.m. UTC | #7
On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch adds compatible and port configuration for
> spi controller for Tesla Full Self-Driving SoC.
> 
> Cc: linux-fsd@tesla.com
> Cc: broonie@kernel.org
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 8755cd85e83c..8d0c1f03ab7a 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -1442,6 +1442,16 @@ static const struct s3c64xx_spi_port_config exynos5433_spi_port_config = {
>  	.quirks		= S3C64XX_SPI_QUIRK_CS_AUTO,
>  };
>  
> +static struct s3c64xx_spi_port_config fsd_spi_port_config = {
> +	.fifo_lvl_mask	= { 0x7f, 0x7f, 0x7f, 0x7f, 0x7f},
> +	.rx_lvl_offset	= 15,
> +	.tx_st_done	= 25,
> +	.high_speed	= true,
> +	.clk_from_cmu	= true,
> +	.clk_ioclk	= false,
> +	.quirks		= S3C64XX_SPI_QUIRK_CS_AUTO,
> +};
> +
>  static const struct platform_device_id s3c64xx_spi_driver_ids[] = {
>  	{
>  		.name		= "s3c2443-spi",
> @@ -1472,6 +1482,9 @@ static const struct of_device_id s3c64xx_spi_dt_match[] = {
>  	{ .compatible = "samsung,exynos5433-spi",
>  			.data = (void *)&exynos5433_spi_port_config,
>  	},
> +	{ .compatible = "tesla,fsd-spi",
> +			.data = (void *)&fsd_spi_port_config,
> +	},

Looks good, except the discussion about too generic compatible.


Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 1:30 p.m. UTC | #8
On 13/01/2022 13:11, Alim Akhtar wrote:
> From: Aswani Reddy <aswani.reddy@samsung.com>
> 
> This patch add device tree node for SPI IPs and needed
> GPIO pin configurations needed for SPI IP
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd.dts  | 12 +++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi | 57 ++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 1:33 p.m. UTC | #9
On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch adds ADC device tree node and enables the same
> on fsd platform.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd.dts  |  4 ++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi | 11 +++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dts b/arch/arm64/boot/dts/tesla/fsd.dts
> index 7f3bb6212e50..dd6c75fc3221 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd.dts
> @@ -150,3 +150,7 @@
>  &spi_2 {
>  	status = "okay";
>  };
> +
> +&adc {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index 7e687c6f74f6..058a9d381aed 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -788,6 +788,17 @@
>  			num-cs = <1>;
>  			status = "disabled";
>  		};
> +
> +		adc: adc@141A0000 {

lowercase hex numbers please.

> +			compatible = "samsung,exynos-adc-v3";
> +			reg = <0x0 0x141A0000 0x0 0x100>;
> +			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clock_peric PERIC_PCLK_ADCIF>;
> +			clock-names = "adc";
> +			#io-channel-cells = <1>;
> +			io-channel-ranges;
> +			status = "disabled";

This does not pass dtschema. NAK.

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


Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 13, 2022, 1:36 p.m. UTC | #10
On 13/01/2022 13:11, Alim Akhtar wrote:
> From: Aswani Reddy <aswani.reddy@samsung.com>
> 
> This patch adds support for handling thress clusters
> (upto 12 CPUs)
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 5e3e96d3d1b9..ba3af940a687 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -78,6 +78,10 @@ enum {
>  	MCT_L5_IRQ,
>  	MCT_L6_IRQ,
>  	MCT_L7_IRQ,
> +	MCT_L8_IRQ,
> +	MCT_L9_IRQ,
> +	MCT_L10_IRQ,
> +	MCT_L11_IRQ,

I think this should be variable, based on SoC compatible. The mistake
was done already when adding Exynos5420 support by Linaro/Samsung, but
at least let's correct it now.

Older MCTs/SoCs do not support 12 local interrupts, so they do not need
and should not register so many interrupts.


Best regards,
Krzysztof
Arnd Bergmann Jan. 13, 2022, 2:23 p.m. UTC | #11
On Thu, Jan 13, 2022 at 2:16 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> >  ARM/TETON BGA MACHINE SUPPORT
> >  M:   "Mark F. Brown" <mark.brown314@gmail.com>
> >  L:   linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index 54e3910e8b9b..bb8a047c2359 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -267,6 +267,12 @@ config ARCH_TEGRA
> >       help
> >         This enables support for the NVIDIA Tegra SoC family.
> >
> > +config ARCH_TESLA_FSD
> > +     bool "ARMv8 based Tesla platform"
> > +     select ARCH_EXYNOS
>
> How similar it is? I think it is better to duplicate Exynos
> selections/options here, instead of selecting entire ARCH. If this would
> require "depends on ARCH_EXYNOS || ARCH_TESLA_FSD" everywhere in the
> drivers, it's a hint that it is not a separate SoC but it is an Exynos,
> so it might not need a new sub-architecture.

Agreed, the SoC family options mainly exist so we can quickly enable or
disable drivers based on what a kernel is built for. If most of the drivers
for this SoC are shared with Exynos, I think having a single option is
sufficient, but it may be worth pointing out both in the help text.

If we want to have a separate option (mainly to help users find it), maybe
a 'depends on ARCH_EXYNOS' would be better. How many uses of
ARCH_TESLA_FSD are there?

        Arnd
Olof Johansson Jan. 13, 2022, 6:53 p.m. UTC | #12
On Thu, Jan 13, 2022 at 4:32 AM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 13/01/2022 13:11, Alim Akhtar wrote:
> > This patch set adds basic support for the Tesla Full Self-Driving (FSD)
> > SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
> > as well as several IPs.
> >
> > Patches 1 to 8 provide support for the clock controller
> > (which is designed similarly to Exynos SoCs).
> >
> > The remaining changes provide pinmux support, initial device tree support,
> > and SPI, ADC, and MCT IP functionality.
>
> Does FSD have some version number? The FDS, especially in compatibles,
> looks quite generic, so what will happen if a newer SoC comes later? You
> would have:
>  - tesla,fsd-pinctrl
>  - tesla,fsd-xxxx-pinctrl (where xxxx could be some new version)
>
> This will be extra confusing, because fsd-pinctrl looks like the generic
> one, while it is specific...

The public sources from Tesla on github uses "turbo,trav" here, but
that's also not a versioned name. The platform itself (hw3/hw31 -- 3.1
I presume?) has numbering, but that's system and not SoC:
https://github.com/teslamotors/linux/tree/tesla-4.14-hw3/arch/arm64/boot/dts/turbo

It would be easy to do "fsd2" for naming/numbering if needed for
future versions, for example. I'm not so worried about this,
especially if there's no corresponding internal version numbering that
this would map naturally to.


-Olof
Alim Akhtar Jan. 14, 2022, 5:41 a.m. UTC | #13
Hi Krzysztof,

>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Thursday, January 13, 2022 6:02 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com
>Subject: Re: [PATCH 00/23] Add support for Tesla Full Self-Driving (FSD) SoC
>
>On 13/01/2022 13:11, Alim Akhtar wrote:
>> This patch set adds basic support for the Tesla Full Self-Driving
>> (FSD) SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
>> as well as several IPs.
>>
>> Patches 1 to 8 provide support for the clock controller (which is
>> designed similarly to Exynos SoCs).
>>
>> The remaining changes provide pinmux support, initial device tree
>> support, and SPI, ADC, and MCT IP functionality.
>
>Does FSD have some version number? The FDS, especially in compatibles,
>looks quite generic, so what will happen if a newer SoC comes later? You
>would have:
> - tesla,fsd-pinctrl
> - tesla,fsd-xxxx-pinctrl (where xxxx could be some new version)
>
>This will be extra confusing, because fsd-pinctrl looks like the generic one,
>while it is specific...
>
AFAIK, there is no version for FSD SoC (like we see on Exynos or any other SoC)
In case something comes in future, may be just adopt as Olof suggested in the other thread like fsd2 etc..
>Best regards,
>Krzysztof
Alim Akhtar Jan. 14, 2022, 6:16 a.m. UTC | #14
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Thursday, January 13, 2022 6:20 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>Jayati Sahu <jayati.sahu@samsung.com>; Ajay Kumar
><ajaykumar.rs@samsung.com>
>Subject: Re: [PATCH 03/23] clk: samsung: fsd: Add initial clock support
>
>On 13/01/2022 13:11, Alim Akhtar wrote:
>> Add initial clock support for FSD (Full Self-Driving) SoC which is
>> required to bring-up platforms based on this SoC.
>>
>> Cc: linux-fsd@tesla.com
>> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>  drivers/clk/samsung/Makefile  |   1 +
>>  drivers/clk/samsung/clk-fsd.c | 308
>++++++++++++++++++++++++++++++++++
>>  drivers/clk/samsung/clk-pll.c |   1 +
>>  drivers/clk/samsung/clk-pll.h |   1 +
>>  4 files changed, 311 insertions(+)
>>  create mode 100644 drivers/clk/samsung/clk-fsd.c
>>
>> diff --git a/drivers/clk/samsung/Makefile
>> b/drivers/clk/samsung/Makefile index c46cf11e4d0b..d66b2ede004c 100644
>> --- a/drivers/clk/samsung/Makefile
>> +++ b/drivers/clk/samsung/Makefile
>> @@ -18,6 +18,7 @@ obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-
>exynos-audss.o
>>  obj-$(CONFIG_EXYNOS_CLKOUT)	+= clk-exynos-clkout.o
>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7.o
>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos850.o
>> +obj-$(CONFIG_ARCH_TESLA_FSD)         += clk-fsd.o
>
>It should be rather it's own CONFIG_TESLA_FSD_CLK option, just like other
>Exynos designs. This keeps unified approach with existing Samsung clock
>Kconfig.
>
Ok, will add a separate config for this 

>>  obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o
>> obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o
>> obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o diff --git
>> a/drivers/clk/samsung/clk-fsd.c b/drivers/clk/samsung/clk-fsd.c new
>> file mode 100644 index 000000000000..e47523106d9e
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-fsd.c
>> @@ -0,0 +1,308 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Common Clock Framework support for FSD SoC.
>> + *
>> + * Copyright (c) 2017-2022 Samsung Electronics Co., Ltd.
>> + *             https://www.samsung.com
>> + * Copyright (c) 2017-2022 Tesla, Inc.
>> + *             https://www.tesla.com
>> + *
>
>Drop the line break with empty * comment.
Will fix in next version
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/of.h>
>> +
>> +#include "clk.h"
>> +#include <dt-bindings/clock/fsd-clk.h>
>
>dt-bindings headers before local clk.h.
>
Noted, thanks
>> +
>> +/* Register Offset definitions for CMU_CMU (0x11c10000) */
>
>
>
>Best regards,
>Krzysztof
Alim Akhtar Jan. 14, 2022, 6:30 a.m. UTC | #15
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Thursday, January 13, 2022 6:25 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>Aswani Reddy <aswani.reddy@samsung.com>; Niyas Ahmed S T
><niyas.ahmed@samsung.com>; Chandrasekar R <rcsekar@samsung.com>;
>Jayati Sahu <jayati.sahu@samsung.com>; Sriranjani P
><sriranjani.p@samsung.com>; Ajay Kumar <ajaykumar.rs@samsung.com>
>Subject: Re: [PATCH 04/23] clk: samsung: fsd: Add cmu_peric block clock
>information
>
>On 13/01/2022 13:11, Alim Akhtar wrote:
>> This patch adds CMU_PERIC block clock information needed for various
>> IPs functions found in this block.
>
>Here and in all other commits, please do not use "This patch". Instead:
>https://protect2.fireeye.com/v1/url?k=5ec41fe1-015f26dc-5ec594ae-
>0cc47a31309a-72c796795ac37ef5&q=1&e=2a1e171b-f066-48ff-95a7-
>12605dbbf8a9&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.13%2Fso
>urce%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L89
>
Noted
>>
>> Cc: linux-fsd@tesla.com
>> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
>> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
>> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
>> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com>
>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-fsd.c | 464
>> +++++++++++++++++++++++++++++++++-
>>  1 file changed, 463 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-fsd.c
>> b/drivers/clk/samsung/clk-fsd.c index e47523106d9e..6da20966ba99
>> 100644
>> --- a/drivers/clk/samsung/clk-fsd.c
>> +++ b/drivers/clk/samsung/clk-fsd.c
>> @@ -9,12 +9,59 @@
>>   *
>>   */
>>
>> -#include <linux/clk-provider.h>
>>  #include <linux/of.h>
>> +#include <linux/clk.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>
>Please order the includes alphabetically.
>
Sure will fix this
>>
>>  #include "clk.h"
>>  #include <dt-bindings/clock/fsd-clk.h>
>>
>> +/* Gate register bits */
>> +#define GATE_MANUAL		BIT(20)
>> +#define GATE_ENABLE_HWACG	BIT(28)
>> +
>> +/* Gate register offsets range */
>> +#define GATE_OFF_START		0x2000
>> +#define GATE_OFF_END		0x2fff
>> +
>> +/**
>> + * fsd_init_clocks - Set clocks initial configuration
>> + * @np:			CMU device tree node with "reg" property
>(CMU addr)
>> + * @reg_offs:		Register offsets array for clocks to init
>> + * @reg_offs_len:	Number of register offsets in reg_offs array
>> + *
>> + * Set manual control mode for all gate clocks.
>> + */
>> +static void __init fsd_init_clocks(struct device_node *np,
>> +		const unsigned long *reg_offs, size_t reg_offs_len)
>
>The same as exynos_arm64_init_clocks - please re-use instead of duplicating.
>
Will re-base on latest tree to have these common functions
>> +{
>> +	void __iomem *reg_base;
>> +	size_t i;
>> +
>> +	reg_base = of_iomap(np, 0);
>> +	if (!reg_base)
>> +		panic("%s: failed to map registers\n", __func__);
>> +
>> +	for (i = 0; i < reg_offs_len; ++i) {
>> +		void __iomem *reg = reg_base + reg_offs[i];
>> +		u32 val;
>> +
>> +		/* Modify only gate clock registers */
>> +		if (reg_offs[i] < GATE_OFF_START || reg_offs[i] >
>GATE_OFF_END)
>> +			continue;
>> +
>> +		val = readl(reg);
>> +		val |= GATE_MANUAL;
>> +		val &= ~GATE_ENABLE_HWACG;
>> +		writel(val, reg);
>> +	}
>> +
>> +	iounmap(reg_base);
>> +}
>> +
>
>(...)
>
>> +/**
>> + * fsd_cmu_probe - Probe function for FSD platform clocks
>> + * @pdev: Pointer to platform device
>> + *
>> + * Configure clock hierarchy for clock domains of FSD platform  */
>> +static int __init fsd_cmu_probe(struct platform_device *pdev) {
>> +	const struct samsung_cmu_info *info;
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = dev->of_node;
>> +
>> +	info = of_device_get_match_data(dev);
>> +	fsd_init_clocks(np, info->clk_regs, info->nr_clk_regs);
>> +	samsung_cmu_register_one(np, info);
>> +
>> +	/* Keep bus clock running, so it's possible to access CMU registers */
>> +	if (info->clk_name) {
>> +		struct clk *bus_clk;
>> +
>> +		bus_clk = clk_get(dev, info->clk_name);
>> +		if (IS_ERR(bus_clk)) {
>> +			pr_err("%s: could not find bus clock %s; err = %ld\n",
>> +			       __func__, info->clk_name, PTR_ERR(bus_clk));
>> +		} else {
>> +			clk_prepare_enable(bus_clk);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>
>Please re-use exynos_arm64_register_cmu(). This will also solve my previous
>comment about exynos_arm64_init_clocks().
>
ok
>> +
>> +/* CMUs which belong to Power Domains and need runtime PM to be
>> +implemented */ static const struct of_device_id fsd_cmu_of_match[] = {
>> +	{
>> +		.compatible = "tesla,fsd-clock-peric",
>> +		.data = &peric_cmu_info,
>> +	}, {
>> +	},
>> +};
>> +
>> +static struct platform_driver fsd_cmu_driver __refdata = {
>> +	.driver	= {
>> +		.name = "fsd-cmu",
>> +		.of_match_table = fsd_cmu_of_match,
>> +		.suppress_bind_attrs = true,
>> +	},
>> +	.probe = fsd_cmu_probe,
>> +};
>> +
>> +static int __init fsd_cmu_init(void)
>> +{
>> +	return platform_driver_register(&fsd_cmu_driver);
>> +}
>> +core_initcall(fsd_cmu_init);
>>
>
>
>Best regards,
>Krzysztof
Alim Akhtar Jan. 14, 2022, 7:15 a.m. UTC | #16
>-----Original Message-----
>From: Mark Brown [mailto:broonie@kernel.org]
>Sent: Thursday, January 13, 2022 6:29 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>
>Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; krzysztof.kozlowski@canonical.com;
>s.nawrocki@samsung.com; linux-samsung-soc@vger.kernel.org;
>pankaj.dubey@samsung.com; linux-fsd@tesla.com; linux-
>spi@vger.kernel.org; Aswani Reddy <aswani.reddy@samsung.com>
>Subject: Re: [PATCH 18/23] spi: s3c64xx: Add spi port configuration for
Tesla
>FSD SoC
>
>On Thu, Jan 13, 2022 at 05:41:38PM +0530, Alim Akhtar wrote:
>
>> +	{ .compatible = "tesla,fsd-spi",
>> +			.data = (void *)&fsd_spi_port_config,
>> +	},
>
>This needs a DT bindings update to match.
Thanks for point it out, will update in v2.
Krzysztof Kozlowski Jan. 14, 2022, 7:34 a.m. UTC | #17
On 14/01/2022 06:41, Alim Akhtar wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>> Sent: Thursday, January 13, 2022 6:02 PM
>> To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>> olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>> robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>> soc@vger.kernel.org; pankaj.dubey@samsung.com
>> Subject: Re: [PATCH 00/23] Add support for Tesla Full Self-Driving (FSD) SoC
>>
>> On 13/01/2022 13:11, Alim Akhtar wrote:
>>> This patch set adds basic support for the Tesla Full Self-Driving
>>> (FSD) SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
>>> as well as several IPs.
>>>
>>> Patches 1 to 8 provide support for the clock controller (which is
>>> designed similarly to Exynos SoCs).
>>>
>>> The remaining changes provide pinmux support, initial device tree
>>> support, and SPI, ADC, and MCT IP functionality.
>>
>> Does FSD have some version number? The FDS, especially in compatibles,
>> looks quite generic, so what will happen if a newer SoC comes later? You
>> would have:
>> - tesla,fsd-pinctrl
>> - tesla,fsd-xxxx-pinctrl (where xxxx could be some new version)
>>
>> This will be extra confusing, because fsd-pinctrl looks like the generic one,
>> while it is specific...
>>
> AFAIK, there is no version for FSD SoC (like we see on Exynos or any other SoC)
> In case something comes in future, may be just adopt as Olof suggested in the other thread like fsd2 etc..
>> Best regards,
>> Krzysztof

The naming is still confusing. The SoC is FSD, compatible is "fsd" but
entire sub-architecture is also FSD called. Therefore it looks like
creating entire sub-architecture for only one SoC, which actually in
multiple pieces is or looks like Samsung Exynos (designed by Samsung,
using several blocks from Exynos SoC).

Best regards,
Krzysztof
Alim Akhtar Jan. 14, 2022, 8:13 a.m. UTC | #18
Hi Arnd and Krzysztof

>-----Original Message-----
>From: Arnd Bergmann [mailto:arnd@arndb.de]
>Sent: Thursday, January 13, 2022 7:54 PM
>To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>Cc: Alim Akhtar <alim.akhtar@samsung.com>; Linux ARM <linux-arm-
>kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
>kernel@vger.kernel.org>; SoC Team <soc@kernel.org>; linux-clk <linux-
>clk@vger.kernel.org>; DTML <devicetree@vger.kernel.org>; Olof Johansson
><olof@lixom.net>; Linus Walleij <linus.walleij@linaro.org>; Catalin Marinas
><catalin.marinas@arm.com>; Rob Herring <robh+dt@kernel.org>; Sylwester
>Nawrocki <s.nawrocki@samsung.com>; moderated list:ARM/SAMSUNG
>EXYNOS ARM ARCHITECTURES <linux-samsung-soc@vger.kernel.org>; Pankaj
>Dubey <pankaj.dubey@samsung.com>; linux-fsd@tesla.com; Arjun K V
><arjun.kv@samsung.com>; Aswani Reddy <aswani.reddy@samsung.com>;
>Ajay Kumar <ajaykumar.rs@samsung.com>; Sriranjani P
><sriranjani.p@samsung.com>; Chandrasekar R <rcsekar@samsung.com>;
>Shashank Prashar <s.prashar@samsung.com>; Arnd Bergmann
><arnd@arndb.de>
>Subject: Re: [PATCH 14/23] arm64: dts: fsd: Add initial device tree support
>
>On Thu, Jan 13, 2022 at 2:16 PM Krzysztof Kozlowski
><krzysztof.kozlowski@canonical.com> wrote:
>> >  ARM/TETON BGA MACHINE SUPPORT
>> >  M:   "Mark F. Brown" <mark.brown314@gmail.com>
>> >  L:   linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>> > diff --git a/arch/arm64/Kconfig.platforms
>> > b/arch/arm64/Kconfig.platforms index 54e3910e8b9b..bb8a047c2359
>> > 100644
>> > --- a/arch/arm64/Kconfig.platforms
>> > +++ b/arch/arm64/Kconfig.platforms
>> > @@ -267,6 +267,12 @@ config ARCH_TEGRA
>> >       help
>> >         This enables support for the NVIDIA Tegra SoC family.
>> >
>> > +config ARCH_TESLA_FSD
>> > +     bool "ARMv8 based Tesla platform"
>> > +     select ARCH_EXYNOS
>>
>> How similar it is? I think it is better to duplicate Exynos
>> selections/options here, instead of selecting entire ARCH. If this
>> would require "depends on ARCH_EXYNOS || ARCH_TESLA_FSD"
>everywhere in
>> the drivers, it's a hint that it is not a separate SoC but it is an
>> Exynos, so it might not need a new sub-architecture.
>
>Agreed, the SoC family options mainly exist so we can quickly enable or
>disable drivers based on what a kernel is built for. If most of the drivers for
>this SoC are shared with Exynos, I think having a single option is sufficient, but
>it may be worth pointing out both in the help text.
>
>If we want to have a separate option (mainly to help users find it), maybe a
>'depends on ARCH_EXYNOS' would be better. How many uses of
>ARCH_TESLA_FSD are there?
>
It is true that FSD shares few IPs with Exynos and it dose contains Telsa specific IPs/ PCIe/some of the PHYs/ GPUs (different then Exynos) etc. to name a few.
And drivers for those will be posted in upcoming phases by Samsung, Telsa etc.
AFA architecture is concerns both Exynos and FSD has completely different architecture (at least at HW level).
In my opinion, it is make sense to have a separate option for FSD.
And as Arnd suggested "'depends on ARCH_EXYNOS" may be the way forward.

>        Arnd
Pavel Machek Jan. 16, 2022, 9:23 a.m. UTC | #19
Hi!

> This patch set adds basic support for the Tesla Full Self-Driving (FSD)
> SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
> as well as several IPs.

I'm not thrilled by their naming. Intel does not produce "Intel
Fastest in world SoC", and this chip is not actually suitable for
autonomous driving :-(.

								Pavel
Linus Walleij Jan. 16, 2022, 12:05 p.m. UTC | #20
On Thu, Jan 13, 2022 at 1:24 PM Alim Akhtar <alim.akhtar@samsung.com> wrote:

> This patch adds Tesla FSD SoC specific data to enable pinctrl.
> FSD SoC has similar pinctrl controller as found in the most
> samsung/exynos SoCs.
>
> Cc: linux-fsd@tesla.com
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>

Looks good to me once Krysztof is happy with this I expect to get the
pin control portions from him by a pull request.
FWIW
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Linus Walleij Jan. 16, 2022, 12:12 p.m. UTC | #21
On Thu, Jan 13, 2022 at 1:24 PM Alim Akhtar <alim.akhtar@samsung.com> wrote:

> This patch adds compatible and port configuration for
> spi controller for Tesla Full Self-Driving SoC.
>
> Cc: linux-fsd@tesla.com
> Cc: broonie@kernel.org
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>

Note to self: now that I see that the Samsung "S3C" SPI controller,
which I was thinking of as "some kind of early 2000s legacy" is used
by the very latest silicon I bumped up fixing it to use GPIO descriptors
a bit on my TODO list.

Yours,
Linus Walleij
Alim Akhtar Jan. 17, 2022, 12:20 p.m. UTC | #22
Hi Jonathan

>-----Original Message-----
>From: Jonathan Cameron [mailto:jic23@kernel.org]
>Sent: Sunday, January 16, 2022 4:50 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>
>Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; krzysztof.kozlowski@canonical.com;
>s.nawrocki@samsung.com; linux-samsung-soc@vger.kernel.org;
>pankaj.dubey@samsung.com; linux-fsd@tesla.com; linux-
>iio@vger.kernel.org; Tamseel Shams <m.shams@samsung.com>
>Subject: Re: [PATCH 21/23] iio: adc: exynos-adc: Add support for ADC V3
>controller
>
>On Thu, 13 Jan 2022 17:41:41 +0530
>Alim Akhtar <alim.akhtar@samsung.com> wrote:
>
>> Exynos's ADC-V3 has some difference in registers set, number of
>> programmable channels (16 channel) etc. This patch adds support for
>> ADC-V3 controller version.
>>
>> Cc: linux-fsd@tesla.com
>> Cc: jic23@kernel.org
>> Cc: linux-iio@vger.kernel.org
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>
>Hi Alim,
>
>A few minor suggestions below.  I'm not seeing a binding update though...
>
>I'd also suggest that it would be more appropriate to break this out as a
>separate mini series from the main support so that it can be reviewed and
>merge separately. It's not ideal when a list just gets patch 21 of
>23 with no cover letter etc sent to it.
>
Thanks for the detailed review, I agree, will send as a separate patch set
only related with ADC support.
And addressing rest of your comments in this patch.

>Jonathan
>
>> ---
>>  drivers/iio/adc/exynos_adc.c | 74
>> +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 72 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/exynos_adc.c
>> b/drivers/iio/adc/exynos_adc.c index 3b3868aa2533..61752e798fd6 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -55,6 +55,11 @@
>>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
>>  #define ADC_V2_VER(x)		((x) + 0x20)
>>
>> +/* ADC_V3 register definitions */
>> +#define ADC_V3_DAT(x)			((x) + 0x08)
>> +#define ADC_V3_DAT_SUM(x)		((x) + 0x0C)
>> +#define ADC_V3_DBG_DATA(x)		((x) + 0x1C)
>> +
>>  /* Bit definitions for ADC_V1 */
>>  #define ADC_V1_CON_RES		(1u << 16)
>>  #define ADC_V1_CON_PRSCEN	(1u << 14)
>> @@ -92,6 +97,7 @@
>>
>>  /* Bit definitions for ADC_V2 */
>>  #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
>> +#define ADC_V2_CON1_SOFT_NON_RESET	(1u << 1)
>>
>>  #define ADC_V2_CON2_OSEL	(1u << 10)
>>  #define ADC_V2_CON2_ESEL	(1u << 9)
>> @@ -100,6 +106,7 @@
>>  #define ADC_V2_CON2_ACH_SEL(x)	(((x) & 0xF) << 0)
>>  #define ADC_V2_CON2_ACH_MASK	0xF
>>
>> +#define MAX_ADC_V3_CHANNELS		16
>>  #define MAX_ADC_V2_CHANNELS		10
>>  #define MAX_ADC_V1_CHANNELS		8
>>  #define MAX_EXYNOS3250_ADC_CHANNELS	2
>
>Given we have a mixture of required an unrequired elements in this
structure
>it might be a good idea to add some documentation.  Kernel-doc for the
>whole structure preferred.  Note this isn't necessarily something that
needs
>to be in this patch given the lack of docs predates this and with the
change to
>make
>adc_isr() required that I suggest below things aren't made worse by this
>patch.
>
>> @@ -164,6 +171,7 @@ struct exynos_adc_data {
>>  	void (*exit_hw)(struct exynos_adc *info);
>>  	void (*clear_irq)(struct exynos_adc *info);
>>  	void (*start_conv)(struct exynos_adc *info, unsigned long addr);
>> +	irqreturn_t (*adc_isr)(int irq, void *dev_id);
>>  };
>>
>>  static void exynos_adc_unprepare_clk(struct exynos_adc *info) @@
>> -484,6 +492,59 @@ static const struct exynos_adc_data exynos7_adc_data =
>{
>>  	.start_conv	= exynos_adc_v2_start_conv,
>>  };
>>
>> +static void exynos_adc_v3_init_hw(struct exynos_adc *info) {
>> +	u32 con2;
>> +
>> +	writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs));
>> +
>> +	writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info-
>>regs));
>> +
>> +	con2 = ADC_V2_CON2_C_TIME(6);
>> +	writel(con2, ADC_V2_CON2(info->regs));
>> +
>> +	/* Enable interrupts */
>> +	writel(1, ADC_V2_INT_EN(info->regs)); }
>> +
>> +static void exynos_adc_v3_exit_hw(struct exynos_adc *info) {
>> +	u32 con2;
>> +
>> +	con2 = readl(ADC_V2_CON2(info->regs));
>> +	con2 &= ~ADC_V2_CON2_C_TIME(7);
>> +	writel(con2, ADC_V2_CON2(info->regs));
>> +
>> +	/* Disable interrupts */
>> +	writel(0, ADC_V2_INT_EN(info->regs)); }
>> +
>> +static irqreturn_t exynos_adc_v3_isr(int irq, void *dev_id) {
>> +	struct exynos_adc *info = (struct exynos_adc *)dev_id;
>
>Shouldn't need the cast as cast from void * to another pointer is always
valid
>in C without the explicit cast.
>
>> +	u32 mask = info->data->mask;
>> +
>> +	info->value = readl(ADC_V3_DAT(info->regs)) & mask;
>> +
>> +	if (info->data->clear_irq)
>> +		info->data->clear_irq(info);
>
>Don't need this currently as v3_isr() is always matched with clear_isr()
being
>provided.  Having the check implies otherwise which is probably not a good
>thing to do until some future device support (maybe) needs it.
>
>> +
>> +	complete(&info->completion);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static const struct exynos_adc_data exynos_adc_v3_adc_data = {
>> +	.num_channels	= MAX_ADC_V3_CHANNELS,
>> +	.mask		= ADC_DATX_MASK, /* 12 bit ADC resolution */
>> +
>> +	.init_hw	= exynos_adc_v3_init_hw,
>> +	.exit_hw	= exynos_adc_v3_exit_hw,
>> +	.clear_irq	= exynos_adc_v2_clear_irq,
>> +	.start_conv	= exynos_adc_v2_start_conv,
>> +	.adc_isr	= exynos_adc_v3_isr,
>> +};
>> +
>>  static const struct of_device_id exynos_adc_match[] = {
>>  	{
>>  		.compatible = "samsung,s3c2410-adc", @@ -518,6 +579,9 @@
>static
>> const struct of_device_id exynos_adc_match[] = {
>>  	}, {
>>  		.compatible = "samsung,exynos7-adc",
>>  		.data = &exynos7_adc_data,
>> +	}, {
>> +		.compatible = "samsung,exynos-adc-v3",
>> +		.data = &exynos_adc_v3_adc_data,
>>  	},
>>  	{},
>>  };
>> @@ -719,6 +783,12 @@ static const struct iio_chan_spec
>exynos_adc_iio_channels[] = {
>>  	ADC_CHANNEL(7, "adc7"),
>>  	ADC_CHANNEL(8, "adc8"),
>>  	ADC_CHANNEL(9, "adc9"),
>> +	ADC_CHANNEL(10, "adc10"),
>> +	ADC_CHANNEL(11, "adc11"),
>> +	ADC_CHANNEL(12, "adc12"),
>> +	ADC_CHANNEL(13, "adc13"),
>> +	ADC_CHANNEL(14, "adc14"),
>> +	ADC_CHANNEL(15, "adc15"),
>>  };
>>
>>  static int exynos_adc_remove_devices(struct device *dev, void *c) @@
>> -885,8 +955,8 @@ static int exynos_adc_probe(struct platform_device
>> *pdev)
>>
>>  	mutex_init(&info->lock);
>>
>> -	ret = request_irq(info->irq, exynos_adc_isr,
>> -					0, dev_name(&pdev->dev), info);
>> +	ret = request_irq(info->irq, info->data->adc_isr ?
info->data->adc_isr
>:
>> +				exynos_adc_isr, 0, dev_name(&pdev->dev),
>info);
>
>I'd rather see the slightly larger change of providing adc_isr for existing
parts
>and the conditional part here going away.
>
>Jonathan
>
>
>>  	if (ret < 0) {
>>  		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
>>  							info->irq);
Alim Akhtar Jan. 17, 2022, 1:44 p.m. UTC | #23
Hi Krzysztof

>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Thursday, January 13, 2022 6:50 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>Shashank Prashar <s.prashar@samsung.com>; Aswani Reddy
><aswani.reddy@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@tesla.com
>> Signed-off-by: Shashank Prashar <s.prashar@samsung.com>
>> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@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
Krzysztof Kozlowski Jan. 17, 2022, 1:50 p.m. UTC | #24
On 17/01/2022 14:44, Alim Akhtar wrote:
> Hi Krzysztof
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>> Sent: Thursday, January 13, 2022 6:50 PM
>> To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>> olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>> robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>> soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>> Shashank Prashar <s.prashar@samsung.com>; Aswani Reddy
>> <aswani.reddy@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@tesla.com
>>> Signed-off-by: Shashank Prashar <s.prashar@samsung.com>
>>> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
>>> Signed-off-by: Alim Akhtar <alim.akhtar@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
> 

OK, but have in mind that -gpio-bank suffix is not needed now. This
depends on support in the pinctrl driver, which will be applied after
the merge window to different tree or branches than DTS is going to.
Therefore if I apply your DTS with "-gpio-bank" to my next/dt64, the
kernel won't find GPIo banks and won't properly boot. The linux-next
will be fine, just my next/dt64 won't be.

If you're fine with it - use "-gpio-bank" suffix. If you prefer my
next/dt64 to have a fully working Tesla SoC DTS, then stick to old node
naming and let's replace it later.

Best regards,
Krzysztof
Olof Johansson Jan. 17, 2022, 8:53 p.m. UTC | #25
On Sun, Jan 16, 2022 at 1:23 AM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > This patch set adds basic support for the Tesla Full Self-Driving (FSD)
> > SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
> > as well as several IPs.
>
> I'm not thrilled by their naming. Intel does not produce "Intel
> Fastest in world SoC"

If you say so. :)

> , and this chip is not actually suitable for
> autonomous driving :-(.

And AMD's Infinity Fabric isn't.... infinite. Things have names.

That discussion seems off-topic for this patchset. It references a
marketing name used by the company, and as such it makes sense to be
able to cross-reference:
https://www.tesla.com/support/full-self-driving-computer

Tesla seems to have moved away from the initial "Hardware 3" naming
scheme, so using this naming seems as good as any.


-Olof
Pavel Machek Jan. 17, 2022, 11:10 p.m. UTC | #26
On Mon 2022-01-17 12:53:48, Olof Johansson wrote:
> On Sun, Jan 16, 2022 at 1:23 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > Hi!
> >
> > > This patch set adds basic support for the Tesla Full Self-Driving (FSD)
> > > SoC. This SoC contains three clusters of four Cortex-A72 CPUs,
> > > as well as several IPs.
> >
> > I'm not thrilled by their naming. Intel does not produce "Intel
> > Fastest in world SoC"
> 
> If you say so. :)
> 
> > , and this chip is not actually suitable for
> > autonomous driving :-(.
> 
> And AMD's Infinity Fabric isn't.... infinite. Things have names.
> 
> That discussion seems off-topic for this patchset. It references a
> marketing name used by the company, and as such it makes sense to be
> able to cross-reference:
> https://www.tesla.com/support/full-self-driving-computer
> 
> Tesla seems to have moved away from the initial "Hardware 3" naming
> scheme, so using this naming seems as good as any.

I'd prefer to call it Tesla HW3. Even wikipedia has that name, no need
to do false advertising for Tesla, and we'll have good names for
HW2.5 and HW4 if it comes out. We normally use codenames, not
marketing names.

Best regards,
								Pavel
Alim Akhtar Jan. 18, 2022, 2:58 p.m. UTC | #27
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Monday, January 17, 2022 7:20 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>'Shashank Prashar' <s.prashar@samsung.com>; 'Aswani Reddy'
><aswani.reddy@samsung.com>
>Subject: Re: [PATCH 15/23] arm64: dts: fsd: Add initial pinctrl support
>
>On 17/01/2022 14:44, Alim Akhtar wrote:
>> Hi Krzysztof
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>>> Sent: Thursday, January 13, 2022 6:50 PM
>>> To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>>> Cc: soc@kernel.org; linux-clk@vger.kernel.org;
>>> devicetree@vger.kernel.org; olof@lixom.net; linus.walleij@linaro.org;
>>> catalin.marinas@arm.com;
>>> robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>>> soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>>> Shashank Prashar <s.prashar@samsung.com>; Aswani Reddy
>>> <aswani.reddy@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@tesla.com
>>>> Signed-off-by: Shashank Prashar <s.prashar@samsung.com>
>>>> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
>>>> Signed-off-by: Alim Akhtar <alim.akhtar@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
>>
>
>OK, but have in mind that -gpio-bank suffix is not needed now. This depends
>on support in the pinctrl driver, which will be applied after the merge window
>to different tree or branches than DTS is going to.
>Therefore if I apply your DTS with "-gpio-bank" to my next/dt64, the kernel
>won't find GPIo banks and won't properly boot. The linux-next will be fine,
>just my next/dt64 won't be.
>
Thanks, got it, for now let me send v2 on linux-next (as of today).
As required I will rebase to your next/dt64 and send again.
Which also means that my v2 will not be based on your new pinmux schema.
Probably we will move to that after the merge window.

>If you're fine with it - use "-gpio-bank" suffix. If you prefer my
>next/dt64 to have a fully working Tesla SoC DTS, then stick to old node naming
>and let's replace it later.
>
>Best regards,
>Krzysztof