Re: [PATCH v11 4/5] arm64: ambarella: add ARCH_AMBARELLA and CV75 EVK DT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 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
- [Medium] arm64: dts: ambarella: apb simple-bus reg blocks child AMBA gpio registration
- [Low] arm64: dts: ambarella: cpu@1 unit-address mismatches reg value 0x100 in cv75
--

commit 2577b890c9c87bc36cb977c573e3558792596572
Author: Long Zhao <longzhao@ambarella.com>

arm64: ambarella: add ARCH_AMBARELLA and CV75 EVK DT

This commit enables ARCH_AMBARELLA and adds a minimal device tree for the
Ambarella CV75 SoC and EVK board with RCT clocks, UART, pinctrl, and GPIO
banks.

> 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 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Ambarella CV75 SoC
> + *
> + * Boot peripheral: UART0 console.
> + * Clocks: a single ambarella,cv75-rct provider.
> + */
> +
> +#include <dt-bindings/clock/ambarella,cv75-rct.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	compatible = "ambarella,cv75";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a76";
> +			device_type = "cpu";
> +			reg = <0x000>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu@1 {
> +			compatible = "arm,cortex-a76";
> +			device_type = "cpu";
> +			reg = <0x100>;
> +			enable-method = "psci";
> +		};

[Severity: Low]
In arch/arm64/boot/dts/ambarella/cv75.dtsi, the second CPU node is named
cpu@1 while its reg property is <0x100>. Does this trigger a
unit_address_vs_reg warning during W=1 builds and dtbs_check? Should the
node unit-address match the reg property as cpu@100 (or reg = <0x1> if
MPIDR Aff0 is 1)?

[ ... ]

> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		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>;

[Severity: Medium]
Does specifying reg = <0xff 0xe4000000 0x0 0x01000000> on the
apb@ffe4000000 simple-bus node block registration of the child AMBA GPIO
devices (gpio0, gpio1, and gpio2) with -EBUSY?

During of_platform_default_populate(), of_platform_bus_create() creates a
platform_device for apb@ffe4000000, and platform_device_add() calls
insert_resource(&iomem_resource, r), reserving [mem 0xffe4000000-0xffe4ffffff]
as a top-level child of iomem_resource.

When of_platform_bus_create() then iterates over the child "arm,primecell"
nodes (gpio@e4013000, gpio@e4014000, and gpio@e4015000), it calls:

of_amba_device_create()
  amba_device_add(dev, &iomem_resource)
    request_resource(&iomem_resource, &dev->res)

Because __request_resource() checks direct children of iomem_resource, it
collides with the parent bus reservation and returns -EBUSY, causing
amba_device_add() to fail for each GPIO controller.

> +
> +			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]
As reported by Sashiko on v10 [1], will uart0 (enabled as stdout-path in
arch/arm64/boot/dts/ambarella/cv75-evk.dts) permanently defer probing with
-EPROBE_DEFER because no driver exists for "ambarella,cv75-pinctrl"?

In arch/arm64/Kconfig.platforms, ARCH_AMBARELLA selects PINCTRL, and the
arm64 defconfig enables both CONFIG_ARCH_AMBARELLA=y and CONFIG_MODULES=y.
Because uart0 declares pinctrl-0 = <&uart0_pins> referencing
pinctrl@e4010000, fw_devlink (parse_pinctrl_n() ->
device_links_check_suppliers()) blocks uart0 waiting for the pinctrl
supplier.

Even if fw_devlink times out or is disabled, really_probe() calls:

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() finds no registered pinctrl_dev for
pinctrl@e4010000 and walks up to the root node without pinctrl-use-default:

		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 whenever CONFIG_MODULES is enabled, preventing
dw8250_probe() from ever binding the serial console.

> +
> +			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>;
> +			};

[Severity: Medium]
As reported by Sashiko on v10 [1], can gpio0, gpio1, and gpio2 bind to any
driver in the kernel tree?

Because "arm,primecell" is in compatible, of_platform_bus_create()
instantiates these nodes as struct amba_device objects with periphid
0x000e8061. In drivers/amba/bus.c, amba_match() matches AMBA devices solely
against pcdrv->id_table via amba_lookup(). However, drivers/gpio/gpio-pl061.c
only matches periphid 0x00041061 (and supports 8 lines per block rather than
32):

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

In addition, each GPIO node specifies gpio-ranges referencing &pinctrl
("ambarella,cv75-pinctrl"), which has no driver in the tree, so
of_gpiochip_add_pin_range() would also defer probing with -EPROBE_DEFER when
of_pinctrl_get() fails to find a registered pinctrl device.

[Severity: Medium]
As reported by Sashiko on v10 [1], does make dtbs_check fail on
cv75-evk.dtb due to undocumented compatible strings and properties in
arch/arm64/boot/dts/ambarella/cv75.dtsi?

Specifically, there do not appear to be Devicetree binding schemas covering:
- "ambarella,cv75-sec-scratchpad" in scratchpad@e002e000 (not in
  Documentation/devicetree/bindings/mfd/syscon.yaml)
- "ambarella,cv75-uart" in serial@e4000000 (not in
  Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml)
- "ambarella,cv75-pinctrl", ambarella,drive-strength-syscon, and
  ambarella,pull-syscon in pinctrl@e4010000
- "ambarella,cv75-gpio" in gpio@e4013000, gpio@e4014000, and gpio@e4015000
- the reg properties on the simple-bus nodes ahb@ffe0000000 and
  apb@ffe4000000

[1] https://lore.kernel.org/r/20261019-cv75-soc-v10-4-d386065e2dd5@ambarella.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-cv75-soc-v11-0-aa6ee0b2380d@ambarella.com?part=4