From: sashiko-bot@kernel.org
To: "Jiqi Li" <lijq9@lenovo.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH 2/2] arm64: tegra: Add Lenovo ThinkEdge SE70 carrier board DTS
Date: Fri, 26 Jun 2026 09:28:00 +0000 [thread overview]
Message-ID: <20260626092801.3A6811F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626091349.570091-3-lijq9@lenovo.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing `no-1-8-v` property or 1.8V-capable `vqmmc-supply` for `mmc@3440000` while UHS modes are enabled in the base DTSI.
- [High] Missing `bus-width` property for the SD card slot (`mmc@3440000`).
- [Medium] Device tree schema violation for 'fixed-regulators' wrapper and its child node.
- [Medium] The patch fails to disable the unused SPI peripheral as claimed in the commit message.
- [Medium] Missing `disable-wp` property in the SD card node (`mmc@3440000`).
--
commit d7f4007a1b474f4e8527dff37fcaa0da82f91d42
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.
> No fan, PWM, tachometer, extra camera/spi peripherals are present
> on this passively cooled platform, so unused nodes are explicitly
> disabled following mainline device tree best practices.
[Severity: Medium]
Does this patch actually disable the extra SPI peripherals as claimed?
The tegra194-p3509-0000.dtsi explicitly enables the spi@3270000 node, but
there doesn't appear to be an override to disable it in this file. The
unpopulated SPI flash driver will probe on boot and waste resources.
> 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..44a7ae9a05bfb
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-lenovo-thinkedge-se70.dts
[ ... ]
> @@ -14,6 +14,6 @@
> + /* Fixed 3.3V regulator for external SD card slot */
> + fixed-regulators {
> + compatible = "simple-bus";
> + ap2306gn_3v3_sd: ap2306gn-3v3-sd {
> + compatible = "regulator-fixed";
> + regulator-name = "ap2306gn-3v3-sd";
[Severity: Medium]
Does this fixed-regulators node comply with the device tree schemas?
Using simple-bus requires the #address-cells, #size-cells, and ranges
properties, which are missing here. Additionally, the ap2306gn-3v3-sd child
node does not start with the regulator- prefix as required by the
fixed-regulator.yaml schema.
This seems to contradict the commit message claim that dt_binding_check
passed without errors.
[ ... ]
> @@ -95,6 +95,6 @@
> + /* SDMMC3 for external user SD card slot with dedicated 3.3V power */
> + mmc@3440000 {
> + vmmc-supply = <&ap2306gn_3v3_sd>;
> + cd-gpios = <&gpio_aon TEGRA194_AON_GPIO(CC, 4) GPIO_ACTIVE_HIGH>;
> + status = "okay";
> + };
[Severity: High]
Does this configuration need a no-1-8-v property or a 1.8V-capable
vqmmc-supply?
The tegra194.dtsi enables UHS-I modes for mmc@3440000, which require 1.8V
signaling. Since this node only defines a fixed 3.3V supply without disabling
1.8V modes, won't the MMC core try to switch the SoC pads to 1.8V signaling
while the card is powered at 3.3V? This could cause I/O failures and
hardware overstress.
[Severity: High]
Should this node specify a bus-width property?
If the bus-width property is omitted, the MMC subsystem defaults to 1-bit
mode. Since the parent sdmmc3 node in tegra194.dtsi does not specify a
bus-width, will this SD card operate in a degraded 1-bit mode, artificially
crippling read and write performance?
[Severity: Medium]
Could this node be missing a disable-wp property?
If the board leaves the internal WP pin floating and disable-wp is absent,
could the kernel randomly mount the SD card as read-only?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626091349.570091-1-lijq9@lenovo.com?part=2
next prev parent reply other threads:[~2026-06-26 9:28 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 1/2] dt-bindings: arm: tegra: Add lenovo,thinkedge-se70 compatible string 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 [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 3:34 [RFC v2 PATCH 0/2] Add Lenovo ThinkEdge SE70 carrier board support Jiqi Li
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
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=20260626092801.3A6811F000E9@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