Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jiqi Li" <lijq9@lenovo.com>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS
Date: Fri, 26 Jun 2026 03:47:07 +0000	[thread overview]
Message-ID: <20260626034708.3A62F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626033418.520762-3-lijq9@lenovo.com>

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Pin configurations are instantiated as standalone dummy nodes instead of subnodes of the pinctrl controllers.
- [High] SD card controller override fails due to a node name mismatch (`sdhci` vs `mmc`).
- [High] The pwm-fan status override fails because it is incorrectly nested under bus@0.
- [Medium] Device tree contains dead overrides for non-existent downstream nodes.
- [Medium] The fixed-regulators node incorrectly uses simple-bus with `#address-cells = <1>` for a sizeless child.
--

commit 124a29c4818b88a33b77f6bc336c64c7bd9a06ec
Author: Jiqi Li <lijq9@lenovo.com>

arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS

ThinkEdge SE70 is a fanless industrial edge gateway built around
NVIDIA Tegra194 Xavier NX SOM. This patch adds a standalone device
tree file describing core carrier board peripherals.

> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts b/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts
> new file mode 100644
> index 0000000000000..d44eb7c9b4742
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts

[ ... ]

> +	/* Fixed 3.3V regulator for external SD card slot */
> +	fixed-regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		ap2306gn_3v3_sd: ap2306gn-3v3-sd {

[Severity: Medium]
The fixed-regulators node is marked as compatible = "simple-bus" and sets
#address-cells = <1>, but its child ap2306gn-3v3-sd does not have a unit
address or a reg property.

Since the simple-bus binding requires children to have a reg or ranges
property, will this cause dtbs_check validation to fail?

[ ... ]

> +	bus@0 {
> +
> +		/* Custom pinmux configurations for 40-pin expansion header */
> +		pinmux@c302000 {
> +			status = "okay";
> +			touch-clk-pcc4 {
> +				nvidia,pins = "touch_clk_pcc4";
> +				nvidia,function = "rsvd2";
> +				nvidia,pull = <TEGRA_PIN_PULL_UP>;
> +				nvidia,tristate = <TEGRA_PIN_ENABLE>;
> +				nvidia,enable-input = <TEGRA_PIN_ENABLE>;
> +			};
> +		};
> +
> +		pinmux@2430028 {
> +			status = "okay";
> +			soc-gpio41-pq5 {

[Severity: High]
Is it intended to create standalone nodes at the exact register offsets of
the individual pins (pinmux@c302000 and pinmux@2430028), instead of placing
them as subnodes under the main pinctrl controllers (which reside at
pinmux@c300000 and pinmux@2430000 in tegra194.dtsi)?

Because they are standalone nodes and lack a pinctrl-0 reference, the pinctrl
driver won't parse them, which will cause these pinmux configurations to be
ignored.

[ ... ]

> +		/* SDMMC3 for external user SD card slot with dedicated 3.3V power */
> +		sdhci@3440000 {
> +			vmmc-supply = <&ap2306gn_3v3_sd>;
> +			cd-gpios = <&gpio_aon TEGRA194_AON_GPIO(CC, 4) GPIO_ACTIVE_HIGH>;
> +			status = "okay";
> +		};

[Severity: High]
In the upstream tegra194.dtsi, the SDMMC3 controller is defined as
mmc@3440000.

Does using sdhci@3440000 here create a new, inert dummy node instead of
overriding the existing mmc@3440000 node? This would leave the actual
controller in its default disabled state.

> +
> +		/* Disable fan and tachometer hardware not populated on SE70 carrier board */
> +		pwm-fan {
> +			status = "disabled";
> +		};

[Severity: High]
In tegra194-p3509-0000.dtsi, the pwm-fan node is located at the root of the
device tree (/pwm-fan).

Since this override is nested inside /bus@0/, doesn't this create a new dummy
node at /bus@0/pwm-fan while leaving the actual root /pwm-fan node enabled?

> +		thermal-fan-est {
> +			status = "disabled";
> +		};
> +		tachometer@39c0000 {
> +			status = "disabled";
> +		};

[Severity: Medium]
It looks like nodes such as thermal-fan-est, tachometer@39c0000,
generic-pwm-tachometer, spi@3210000, and spi@3230000 do not exist in the
upstream tegra194 device tree sources.

Does overriding them here just create new dummy disabled nodes instead of
affecting real hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626033418.520762-1-lijq9@lenovo.com?part=2

  reply	other threads:[~2026-06-26  3:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26  3:34 [RFC v2 PATCH 0/2] Add Lenovo ThinkEdge SE70 carrier board support Jiqi Li
2026-06-26  3:34 ` [PATCH 1/2] dt-bindings: arm: tegra: Add lenovo,thinkedge-se70 compatible string Jiqi Li
2026-06-26  3:43   ` sashiko-bot
2026-06-26  3:34 ` [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS Jiqi Li
2026-06-26  3:47   ` sashiko-bot [this message]
  -- strict thread matches above, loose matches on Subject: below --
2026-06-26 10:56 [RFC v4 PATCH 0/2] Add Lenovo ThinkEdge SE70 carrier board support Jiqi Li
2026-06-26 10:56 ` [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS Jiqi Li
2026-06-26 11:10   ` sashiko-bot
2026-06-26  9:13 [RFC v3 PATCH 0/2] Add Lenovo ThinkEdge SE70 carrier board support Jiqi Li
2026-06-26  9:13 ` [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS Jiqi Li
2026-06-26  9:28   ` sashiko-bot
2026-06-26  1:09 [RFC PATCH 0/2] Add Lenovo ThinkEdge SE70 carrier board support Jiqi Li
2026-06-26  1:09 ` [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS Jiqi Li
2026-06-26  1:23   ` sashiko-bot

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=20260626034708.3A62F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lijq9@lenovo.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox