All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Long Zhao" <longzhao@ambarella.com>
Cc: linux-clk@vger.kernel.org, mfd@lists.linux.dev,
	devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org,
	lee@kernel.org
Subject: Re: [PATCH v10 4/5] arm64: ambarella: add ARCH_AMBARELLA and CV75 EVK DT
Date: Mon, 19 Oct 2026 05:05:12 +0000	[thread overview]
Message-ID: <sashiko-outbox-176658@kernel.org> (raw)
In-Reply-To: <20261019-cv75-soc-v10-4-d386065e2dd5@ambarella.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: ambarella: uart0 permanently defers probe due to missing pinctrl
- [Medium] arm64: dts: ambarella: undocumented compatible strings and properties in cv75
- [Medium] arm64: dts: ambarella: gpio nodes use unmatched periphid and unprobed pinctrl
--

commit 7e36a79675aa487cf1bb15e171011853976e17db
Author: Long Zhao <longzhao@ambarella.com>

arm64: ambarella: add ARCH_AMBARELLA and CV75 EVK DT

This commit adds the ARCH_AMBARELLA platform configuration and introduces
the device tree files for the Ambarella CV75 SoC and CV75 EVK board to
support initial console bring-up.

> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index d2acfac730037..16d7e3a4c2d62 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -26,6 +26,12 @@ config ARCH_SUNXI
>  	help
>  	  This enables support for Allwinner sunxi based SoCs like the A64.
>  
> +config ARCH_AMBARELLA
> +	bool "Ambarella SoC family"
> +	select PINCTRL
> +	help
> +	  This enables support for the ARM64 based Ambarella SoCs like CV75.
> +
>  config ARCH_ALPINE
>  	bool "Annapurna Labs Alpine platform"
>  	select ALPINE_MSI if PCI

[ ... ]

> diff --git a/arch/arm64/boot/dts/ambarella/cv75-evk.dts b/arch/arm64/boot/dts/ambarella/cv75-evk.dts
> new file mode 100644
> index 0000000000000..42a0fe24b83bd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ambarella/cv75-evk.dts
> @@ -0,0 +1,30 @@

[ ... ]

> +	chosen {
> +		stdout-path = &uart0;
> +	};
> +
> +	memory@200000 {
> +		device_type = "memory";
> +		reg = <0x0 0x00200000 0x0 0x07e00000>;
> +	};
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/ambarella/cv75.dtsi b/arch/arm64/boot/dts/ambarella/cv75.dtsi
> new file mode 100644
> index 0000000000000..c36b6af48237a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ambarella/cv75.dtsi
> @@ -0,0 +1,177 @@

[ ... ]

> +		ahb@ffe0000000 {
> +			compatible = "simple-bus";
> +			reg = <0xff 0xe0000000 0x0 0x01000000>;
> +			ranges = <0xe0000000 0xff 0xe0000000 0x01000000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			s_scratchpad_syscon: scratchpad@e002e000 {
> +				compatible = "ambarella,cv75-sec-scratchpad", "syscon";
> +				reg = <0xe002e000 0x1000>;
> +			};
> +		};
> +
> +		apb@ffe4000000 {
> +			compatible = "simple-bus";
> +			reg = <0xff 0xe4000000 0x0 0x01000000>;
> +			ranges = <0xe4000000 0xff 0xe4000000 0x01000000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			uart0: serial@e4000000 {
> +				compatible = "ambarella,cv75-uart", "snps,dw-apb-uart";
> +				reg = <0xe4000000 0x1000>;
> +				clocks = <&rct CV75_GCLK_UART0>;
> +				interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> +				reg-shift = <2>;
> +				reg-io-width = <4>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&uart0_pins>;
> +
> +				status = "disabled";
> +			};
> +
> +			pinctrl: pinctrl@e4010000 {
> +				compatible = "ambarella,cv75-pinctrl";
> +				reg = <0xe4010000 0x1000>;
> +				ambarella,drive-strength-syscon = <&rct>;
> +				ambarella,pull-syscon = <&s_scratchpad_syscon>;
> +
> +				uart0_pins: uart0-pins {
> +					function = "uart0";
> +					groups = "uart0";
> +				};
> +			};

[Severity: High]
Will uart0 permanently defer probing on boot because no driver exists for
"ambarella,cv75-pinctrl"?

In arch/arm64/boot/dts/ambarella/cv75-evk.dts, uart0 is enabled as the
console via stdout-path = &uart0. In cv75.dtsi, uart0 specifies
pinctrl-0 = <&uart0_pins>, which is a child of pinctrl@e4010000
(compatible = "ambarella,cv75-pinctrl"). However, there is no driver in
the kernel tree matching "ambarella,cv75-pinctrl".

With CONFIG_ARCH_AMBARELLA selecting CONFIG_PINCTRL and arm64 defconfig
enabling CONFIG_MODULES=y, fw_devlink (via parse_pinctrl_n() ->
device_links_check_suppliers()) will hold off probing uart0 waiting for
the pinctrl supplier. Even after fw_devlink times out or if fw_devlink=off
is passed, the probe path runs:

really_probe()
  -> pinctrl_bind_pins()
    -> devm_pinctrl_get()
      -> create_pinctrl()
        -> pinctrl_dt_to_map()
          -> dt_to_map_one_config()

In drivers/pinctrl/devicetree.c:dt_to_map_one_config(), when
get_pinctrl_dev_from_of_node() fails to find a registered pinctrl_dev for
pinctrl@e4010000 and walks up to the root node without pinctrl-use-default
set:

		if (!np_pctldev || of_node_is_root(np_pctldev)) {
			of_node_put(np_pctldev);
			ret = -ENODEV;
			/* keep deferring if modules are enabled */
			if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
				ret = -EPROBE_DEFER;
			return ret;
		}

This returns -EPROBE_DEFER to pinctrl_bind_pins(), aborting really_probe()
before dw8250_probe() can bind the serial console. Should pinctrl-0 and
pinctrl-names be omitted from uart0 (or the pinctrl node disabled/omitted)
until the pinctrl driver is present?

> +
> +			gpio0: gpio@e4013000 {
> +				compatible = "ambarella,cv75-gpio", "arm,primecell";
> +				reg = <0xe4013000 0x1000>;
> +				clocks = <&rct CV75_GCLK_APB>;
> +				clock-names = "apb_pclk";
> +				interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				gpio-ranges = <&pinctrl 0 0 32>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				arm,primecell-periphid = <0x000e8061>;
> +			};
> +
> +			gpio1: gpio@e4014000 {
> +				compatible = "ambarella,cv75-gpio", "arm,primecell";
> +				reg = <0xe4014000 0x1000>;
> +				clocks = <&rct CV75_GCLK_APB>;
> +				clock-names = "apb_pclk";
> +				interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				gpio-ranges = <&pinctrl 0 32 32>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				arm,primecell-periphid = <0x000e8061>;
> +			};
> +
> +			gpio2: gpio@e4015000 {
> +				compatible = "ambarella,cv75-gpio", "arm,primecell";
> +				reg = <0xe4015000 0x1000>;
> +				clocks = <&rct CV75_GCLK_APB>;
> +				clock-names = "apb_pclk";
> +				interrupts = <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				gpio-ranges = <&pinctrl 0 64 32>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				arm,primecell-periphid = <0x000e8061>;
> +			};
> +		};

[Severity: Medium]
Are binding schemas missing for the newly introduced compatible strings and
properties in cv75.dtsi?

Running make dtbs_check on cv75-evk.dtb will fail validation because
several compatible strings and vendor properties are not documented under
Documentation/devicetree/bindings/:

- scratchpad@e002e000 uses compatible = "ambarella,cv75-sec-scratchpad",
  "syscon", which is not listed in
  Documentation/devicetree/bindings/mfd/syscon.yaml.
- serial@e4000000 uses compatible = "ambarella,cv75-uart",
  "snps,dw-apb-uart", which is not listed in
  Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml.
- pinctrl@e4010000 uses compatible = "ambarella,cv75-pinctrl" along with
  ambarella,drive-strength-syscon and ambarella,pull-syscon, which have no
  binding schema in the tree.
- gpio@e4013000, gpio@e4014000, and gpio@e4015000 use
  compatible = "ambarella,cv75-gpio", "arm,primecell", which is not
  documented in any binding schema.

[Severity: Medium]
How are gpio0, gpio1, and gpio2 expected to bind to a driver with
arm,primecell-periphid = <0x000e8061>?

Because "arm,primecell" is in compatible, of_platform_bus_create()
registers gpio0, gpio1, and gpio2 as struct amba_device instances on the
AMBA bus, where amba_match() -> amba_lookup() matches exclusively against
periphid. No amba_driver in the tree matches 0x000e8061; for example,
drivers/gpio/gpio-pl061.c has:

static const struct amba_id pl061_ids[] = {
	{
		.id	= 0x00041061,
		.mask	= 0x000fffff,
	},
	{ 0, 0 },
};

(and PL061 only supports 8 GPIO lines per block rather than 32). In
addition, gpio-ranges = <&pinctrl ... 32> references the unprobed
pinctrl@e4010000 node, which would cause of_gpiochip_add_pin_range() ->
of_pinctrl_get() to return -EPROBE_DEFER. Is there a separate GPIO and
pinctrl driver series that these nodes depend on?

[ ... ]

> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index ad3bd3d9507b2..454f07a6a6a6f 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -36,6 +36,7 @@ CONFIG_CRASH_DUMP=y
>  CONFIG_ARCH_ACTIONS=y
>  CONFIG_ARCH_AIROHA=y
>  CONFIG_ARCH_SUNXI=y
> +CONFIG_ARCH_AMBARELLA=y
>  CONFIG_ARCH_ALPINE=y
>  CONFIG_ARCH_APPLE=y
>  CONFIG_ARCH_ARTPEC=y

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-cv75-soc-v10-0-d386065e2dd5@ambarella.com?part=4

  reply	other threads:[~2026-10-19  5:05 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19  4:55 [PATCH v10 0/5] arm64: add Ambarella CV75 SoC support Long Zhao via B4 Relay
2026-10-19  4:55 ` Long Zhao
2026-10-19  4:55 ` [PATCH v10 1/5] dt-bindings: arm: add Ambarella CV75 platforms Long Zhao via B4 Relay
2026-10-19  4:55   ` Long Zhao
2026-10-19  4:55 ` [PATCH v10 2/5] dt-bindings: clock: add Ambarella CV75 RCT Long Zhao via B4 Relay
2026-10-19  4:55   ` Long Zhao
2026-10-19  4:55 ` [PATCH v10 3/5] clk: ambarella: add CV75 RCT clock controller Long Zhao via B4 Relay
2026-10-19  4:55   ` Long Zhao
2026-10-19  4:55 ` [PATCH v10 4/5] arm64: ambarella: add ARCH_AMBARELLA and CV75 EVK DT Long Zhao via B4 Relay
2026-10-19  4:55   ` Long Zhao
2026-10-19  5:05   ` sashiko-bot [this message]
2026-10-19  4:55 ` [PATCH v10 5/5] MAINTAINERS: add ARM/AMBARELLA SoC support Long Zhao via B4 Relay
2026-10-19  4:55   ` Long Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-176658@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=longzhao@ambarella.com \
    --cc=mfd@lists.linux.dev \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.