All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Wayne Chang" <waynec@nvidia.com>
Cc: olteanv@gmail.com, vkoul@kernel.org, robh@kernel.org,
	neil.armstrong@linaro.org, devicetree@vger.kernel.org,
	linux-phy@lists.infradead.org, conor+dt@kernel.org
Subject: Re: [PATCH 8/8] usb: host: xhci-tegra: Add Tegra264 XHCI support
Date: Mon, 29 Jun 2026 09:47:07 +0000	[thread overview]
Message-ID: <20260629094708.2FDA91F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260629093406.1118594-9-waynec@nvidia.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `tegra264_soc` struct explicitly sets `enable_firmware_messages` to `false`, contradicting the commit message that claims the only difference from `tegra234_soc` is `max_num_wakes`.
--

commit f38c91ea7436bccae268e6611e1931cff02f07ec
Author: Wayne Chang <waynec@nvidia.com>

usb: host: xhci-tegra: Add Tegra264 XHCI support

This commit adds XUSB host mode controller support for the Tegra264 SoC.
It defines a new tegra_xusb_soc struct for Tegra264 and adds it to the
OF match table.

>     The host controller is very similar to the existing Tegra234 XHCI,
>     except the number of max_num_wakes.

[ ... ]

> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 7d56f35ec9e71..9a55d5818efb0 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
[ ... ]
> @@ -2752,12 +2752,40 @@ static const struct tegra_xusb_soc tegra234_soc = {
[ ... ]
> +static const struct tegra_xusb_soc tegra264_soc = {
[ ... ]
> +	.lpm_support = true,
> +	.has_bar2 = true,
> +	.enable_firmware_messages = false,

[Severity: Medium]
Is this value intentional?

The commit message states that the only difference from Tegra234 is
max_num_wakes, but tegra234_soc sets enable_firmware_messages to true.

Setting this to false means the driver will skip sending the
MBOX_CMD_MSG_ENABLED command to the controller via
__tegra_xusb_enable_firmware_messages() during initialization.

If Tegra264 requires these messages for clock scaling or power management
like Tegra234 does, this omission might lead to performance or power
management issues. Could you clarify if this should be true, or update the
commit message to reflect this difference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629093406.1118594-1-waynec@nvidia.com?part=8

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Wayne Chang" <waynec@nvidia.com>
Cc: olteanv@gmail.com, vkoul@kernel.org, robh@kernel.org,
	neil.armstrong@linaro.org, devicetree@vger.kernel.org,
	linux-phy@lists.infradead.org, conor+dt@kernel.org
Subject: Re: [PATCH 8/8] usb: host: xhci-tegra: Add Tegra264 XHCI support
Date: Mon, 29 Jun 2026 09:47:07 +0000	[thread overview]
Message-ID: <20260629094708.2FDA91F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260629093406.1118594-9-waynec@nvidia.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `tegra264_soc` struct explicitly sets `enable_firmware_messages` to `false`, contradicting the commit message that claims the only difference from `tegra234_soc` is `max_num_wakes`.
--

commit f38c91ea7436bccae268e6611e1931cff02f07ec
Author: Wayne Chang <waynec@nvidia.com>

usb: host: xhci-tegra: Add Tegra264 XHCI support

This commit adds XUSB host mode controller support for the Tegra264 SoC.
It defines a new tegra_xusb_soc struct for Tegra264 and adds it to the
OF match table.

>     The host controller is very similar to the existing Tegra234 XHCI,
>     except the number of max_num_wakes.

[ ... ]

> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 7d56f35ec9e71..9a55d5818efb0 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
[ ... ]
> @@ -2752,12 +2752,40 @@ static const struct tegra_xusb_soc tegra234_soc = {
[ ... ]
> +static const struct tegra_xusb_soc tegra264_soc = {
[ ... ]
> +	.lpm_support = true,
> +	.has_bar2 = true,
> +	.enable_firmware_messages = false,

[Severity: Medium]
Is this value intentional?

The commit message states that the only difference from Tegra234 is
max_num_wakes, but tegra234_soc sets enable_firmware_messages to true.

Setting this to false means the driver will skip sending the
MBOX_CMD_MSG_ENABLED command to the controller via
__tegra_xusb_enable_firmware_messages() during initialization.

If Tegra264 requires these messages for clock scaling or power management
like Tegra234 does, this omission might lead to performance or power
management issues. Could you clarify if this should be true, or update the
commit message to reflect this difference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629093406.1118594-1-waynec@nvidia.com?part=8

  reply	other threads:[~2026-06-29  9:47 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29  9:33 [PATCH 0/8] Enable USB host functions on Jetson AGX Thor Wayne Chang
2026-06-29  9:33 ` Wayne Chang
2026-06-29  9:33 ` [PATCH 1/8] dt-bindings: phy: tegra-xusb: Add support for Tegra264 Wayne Chang
2026-06-29  9:33   ` Wayne Chang
2026-06-29  9:34 ` [PATCH 2/8] dt-bindings: usb: Add Tegra264 XUSB host support Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:34 ` [PATCH 3/8] arm64: tegra: Enable XUSB host function on Jetson AGX Thor Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:44   ` sashiko-bot
2026-06-29  9:44     ` sashiko-bot
2026-06-29  9:34 ` [PATCH 4/8] phy: tegra: xusb: Use devm_clk_get_optional to fetch USB2 tracking clock Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:46   ` sashiko-bot
2026-06-29  9:46     ` sashiko-bot
2026-06-29  9:34 ` [PATCH 5/8] phy: tegra: xusb: Increase timeout for USB2_TRK_COMPLETED polling Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:34 ` [PATCH 6/8] phy: tegra: xusb: Add Tegra264 support Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:45   ` sashiko-bot
2026-06-29  9:45     ` sashiko-bot
2026-06-29  9:34 ` [PATCH 7/8] usb: host: xhci-tegra: Skip MBOX MSG_ENABLED on Tegra264 Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:34 ` [PATCH 8/8] usb: host: xhci-tegra: Add Tegra264 XHCI support Wayne Chang
2026-06-29  9:34   ` Wayne Chang
2026-06-29  9:47   ` sashiko-bot [this message]
2026-06-29  9:47     ` 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=20260629094708.2FDA91F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    --cc=waynec@nvidia.com \
    /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.