Message ID | 20220113121143.22280-1-alim.akhtar@samsung.com |
---|---|
Headers | show |
Series | Add support for Tesla Full Self-Driving (FSD) SoC | expand |
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
On 13/01/2022 13:11, Alim Akhtar wrote: > Add dt-schema documentation for Tesla FSD SoC clock controller. > > Cc: linux-fsd@tesla.com > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > .../bindings/clock/tesla,fsd-clock.yaml | 212 ++++++++++++++++++ > 1 file changed, 212 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml > > diff --git a/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml b/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml > new file mode 100644 > index 000000000000..58f341e5004d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml > @@ -0,0 +1,212 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/tesla,fsd-clock.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Tesla FSD (Full Self-Driving) SoC clock controller > + > +maintainers: > + - Alim Akhtar <alim.akhtar@samsung.com> > + - linux-fsd@tesla.com > + > +description: | > + FSD clock controller consist of several clock management unit > + (CMU), which generates clocks for various inteernal SoC blocks. > + The root clock comes from external OSC clock (24 MHz). > + > + All available clocks are defined as preprocessor macros in > + 'dt-bindings/clock/fsd-clk.h' header. > + > +properties: > + compatible: > + enum: > + - tesla,fsd-clock-cmu > + - tesla,fsd-clock-imem > + - tesla,fsd-clock-peric > + - tesla,fsd-clock-fsys0 > + - tesla,fsd-clock-fsys1 > + - tesla,fsd-clock-mfc > + - tesla,fsd-clock-cam_csi > + > + clocks: > + minItems: 1 > + maxItems: 6 > + > + clock-names: > + minItems: 1 > + maxItems: 6 > + > + "#clock-cells": > + const: 1 > + > + reg: > + maxItems: 1 > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: tesla,fsd-clock-cmu > + Nitpick: Drop the white-spaces between if-then. It's easier to spot the if-blocks if they are together. > + then: > + properties: > + clocks: > + items: > + - description: External reference clock (24 MHz) > + Drop this whitespace as well. Rest looks good to me, except the discussion about the compatible. Best regards, Krzysztof
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
On 13/01/2022 13:11, Alim Akhtar wrote: > Add vendor prefix for the Tesla (https://www.tesla.com) > > Cc: linux-fsd@tesla.com > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions( This should be the first patch in the series. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
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
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
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
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
>-----Original Message----- >From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com] >Sent: Thursday, January 13, 2022 6:11 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 >Subject: Re: [PATCH 01/23] dt-bindings: clock: Document FSD CMU bindings > >On 13/01/2022 13:11, Alim Akhtar wrote: >> Add dt-schema documentation for Tesla FSD SoC clock controller. >> >> Cc: linux-fsd@tesla.com >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> .../bindings/clock/tesla,fsd-clock.yaml | 212 ++++++++++++++++++ >> 1 file changed, 212 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml >> b/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml >> new file mode 100644 >> index 000000000000..58f341e5004d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/tesla,fsd-clock.yaml >> @@ -0,0 +1,212 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 >> +--- >> +$id: >> +https://protect2.fireeye.com/v1/url?k=7e607c7a-1f1d943d-7e61f735-74fe >> +485fff30-a4acf0e03cbf256d&q=1&e=05b30de9-b535-49f7-9359- >77edd951da07& >> +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Ftesla%2Cfsd- >clock.y >> +aml%23 >> +$schema: >> +https://protect2.fireeye.com/v1/url?k=5c769dcb-3d0b758c-5c771684-74fe >> +485fff30-b4007a892a5a3e44&q=1&e=05b30de9-b535-49f7-9359- >77edd951da07& >> +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23 >> + >> +title: Tesla FSD (Full Self-Driving) SoC clock controller >> + >> +maintainers: >> + - Alim Akhtar <alim.akhtar@samsung.com> >> + - linux-fsd@tesla.com >> + >> +description: | >> + FSD clock controller consist of several clock management unit >> + (CMU), which generates clocks for various inteernal SoC blocks. >> + The root clock comes from external OSC clock (24 MHz). >> + >> + All available clocks are defined as preprocessor macros in >> + 'dt-bindings/clock/fsd-clk.h' header. >> + >> +properties: >> + compatible: >> + enum: >> + - tesla,fsd-clock-cmu >> + - tesla,fsd-clock-imem >> + - tesla,fsd-clock-peric >> + - tesla,fsd-clock-fsys0 >> + - tesla,fsd-clock-fsys1 >> + - tesla,fsd-clock-mfc >> + - tesla,fsd-clock-cam_csi >> + >> + clocks: >> + minItems: 1 >> + maxItems: 6 >> + >> + clock-names: >> + minItems: 1 >> + maxItems: 6 >> + >> + "#clock-cells": >> + const: 1 >> + >> + reg: >> + maxItems: 1 >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: tesla,fsd-clock-cmu >> + > >Nitpick: Drop the white-spaces between if-then. It's easier to spot the if- >blocks if they are together. > Thanks will fix in next version >> + then: >> + properties: >> + clocks: >> + items: >> + - description: External reference clock (24 MHz) >> + > >Drop this whitespace as well. > Noted >Rest looks good to me, except the discussion about the compatible. > Have replied to the original question thread. > >Best regards, >Krzysztof
>-----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
>-----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
>-----Original Message----- >From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com] >Sent: Thursday, January 13, 2022 6:28 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 >Subject: Re: [PATCH 12/23] dt-bindings: add vendor prefix for Tesla > >On 13/01/2022 13:11, Alim Akhtar wrote: >> Add vendor prefix for the Tesla (https://www.tesla.com) >> >> Cc: linux-fsd@tesla.com >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ >> 1 file changed, 2 insertions( > >This should be the first patch in the series. > >Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > Thanks, will put this as first patch in this series > >Best regards, >Krzysztof
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
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
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
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
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