All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>
Cc: wsa+renesas@sang-engineering.com, Frank.Li@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	linux-sunxi@lists.linux.dev, imx@lists.linux.dev,
	conor+dt@kernel.org, robh@kernel.org,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()
Date: Tue, 14 Jul 2026 15:42:24 +0000	[thread overview]
Message-ID: <20260714154224.E7BCA1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260713-qcom-sa8255p-emac-v13-6-119f8699ef8e@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Updating `ethqos->phy_mode` before clearing the SGMII loopback causes the loopback to remain erroneously enabled when switching away from 2500BASE-X.
--

--- Patch [6]: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish() ---
commit 3dfd6663f759c10c24659b5275b88ff5f976b420
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()

This commit updates the driver to overwrite ethqos->phy_mode with the
resolved phylink interface during the mac_finish() callback. This ensures
the stored mode matches runtime changes instead of being permanently stuck
at the device tree default.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 47b70b5e706f2..9d19203ae4bd7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -632,6 +632,12 @@ static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv,
>  	struct qcom_ethqos *ethqos = priv;
>  	int ret = 0;
>  
> +	/* The interface mode is only known once phylink has resolved it and
> +	 * may change at runtime, so keep it in sync for later
> +	 * serdes_powerup()/serdes_powerdown() calls.
> +	 */
> +	ethqos->phy_mode = interface;
> +
>  	qcom_ethqos_set_sgmii_loopback(ethqos, false);

[Severity: High]
Does updating ethqos->phy_mode here prevent the loopback from being disabled?

If the link negotiates from 2500BASE-X down to SGMII, this update changes
ethqos->phy_mode to SGMII before the loopback is disabled.

Looking at qcom_ethqos_set_sgmii_loopback(), it checks the current mode to
determine if it needs to act:

qcom_ethqos_set_sgmii_loopback() {
    if (!ethqos->needs_sgmii_loopback ||
        ethqos->phy_mode != PHY_INTERFACE_MODE_2500BASEX)
            return;
    ...
}

Will this cause the function to return early without clearing the loopback
when switching away from 2500BASE-X, leaving outgoing packets reflected
internally instead of transmitting to the wire?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-sa8255p-emac-v13-0-119f8699ef8e@oss.qualcomm.com?part=6

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>
Cc: wsa+renesas@sang-engineering.com, Frank.Li@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	linux-sunxi@lists.linux.dev, imx@lists.linux.dev,
	conor+dt@kernel.org, robh@kernel.org,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()
Date: Tue, 14 Jul 2026 15:42:24 +0000	[thread overview]
Message-ID: <20260714154224.E7BCA1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260713-qcom-sa8255p-emac-v13-6-119f8699ef8e@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Updating `ethqos->phy_mode` before clearing the SGMII loopback causes the loopback to remain erroneously enabled when switching away from 2500BASE-X.
--

--- Patch [6]: [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish() ---
commit 3dfd6663f759c10c24659b5275b88ff5f976b420
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish()

This commit updates the driver to overwrite ethqos->phy_mode with the
resolved phylink interface during the mac_finish() callback. This ensures
the stored mode matches runtime changes instead of being permanently stuck
at the device tree default.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 47b70b5e706f2..9d19203ae4bd7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -632,6 +632,12 @@ static int ethqos_mac_finish_serdes(struct net_device *ndev, void *priv,
>  	struct qcom_ethqos *ethqos = priv;
>  	int ret = 0;
>  
> +	/* The interface mode is only known once phylink has resolved it and
> +	 * may change at runtime, so keep it in sync for later
> +	 * serdes_powerup()/serdes_powerdown() calls.
> +	 */
> +	ethqos->phy_mode = interface;
> +
>  	qcom_ethqos_set_sgmii_loopback(ethqos, false);

[Severity: High]
Does updating ethqos->phy_mode here prevent the loopback from being disabled?

If the link negotiates from 2500BASE-X down to SGMII, this update changes
ethqos->phy_mode to SGMII before the loopback is disabled.

Looking at qcom_ethqos_set_sgmii_loopback(), it checks the current mode to
determine if it needs to act:

qcom_ethqos_set_sgmii_loopback() {
    if (!ethqos->needs_sgmii_loopback ||
        ethqos->phy_mode != PHY_INTERFACE_MODE_2500BASEX)
            return;
    ...
}

Will this cause the function to return early without clearing the loopback
when switching away from 2500BASE-X, leaving outgoing packets reflected
internally instead of transmitting to the wire?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-sa8255p-emac-v13-0-119f8699ef8e@oss.qualcomm.com?part=6

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2026-07-14 15:42 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-13 13:20 [PATCH net-next v13 00/10] net: stmmac: qcom-ethqos: add support for SCMI power domains Bartosz Golaszewski
2026-07-13 13:20 ` Bartosz Golaszewski
2026-07-13 13:20 ` Bartosz Golaszewski
2026-07-13 13:20 ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 01/10] net: phy: aquantia: fix system interface type not updated in forced mode Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 02/10] dt-bindings: phy: document the serdes PHY on sa8255p Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 03/10] phy: qcom: add the SGMII SerDes PHY driver for SCMI systems Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-14 15:42   ` sashiko-bot
2026-07-14 15:42     ` sashiko-bot
2026-07-13 13:20 ` [PATCH net-next v13 04/10] dt-bindings: net: qcom: document the ethqos device for SCMI-based systems Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-14 15:42   ` sashiko-bot
2026-07-14 15:42     ` sashiko-bot
2026-07-13 13:20 ` [PATCH net-next v13 05/10] net: stmmac: qcom-ethqos: set serdes mode before powerup Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 06/10] net: stmmac: qcom-ethqos: update phy_mode to the resolved interface in mac_finish() Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-14 15:42   ` sashiko-bot [this message]
2026-07-14 15:42     ` sashiko-bot
2026-07-13 13:20 ` [PATCH net-next v13 07/10] net: stmmac: qcom-ethqos: fix SGMII loopback not set on resume after speed change Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 08/10] net: stmmac: qcom-ethqos: reuse the address of ethqos_emac_driver_data Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 09/10] net: stmmac: qcom-ethqos: factor out linux-level setup into a separate function Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20 ` [PATCH net-next v13 10/10] net: stmmac: qcom-ethqos: add support for sa8255p Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-13 13:20   ` Bartosz Golaszewski
2026-07-14 15:42   ` sashiko-bot
2026-07-14 15:42     ` 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=20260714154224.E7BCA1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wsa+renesas@sang-engineering.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.