From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D2A7F2DF128 for ; Thu, 12 Mar 2026 10:51:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773312672; cv=none; b=gWXD19156b4NSz8GHlDwdKZlvFGdtuqe8fWNjWvKXyHscV28CG+sbE3nwz5qCLJ7fVsbm1Dh4lyPtTdh8ayU02oEfgNM9ji5ei2DLBt5cxcSPxMhkHrAzXIA2HWyekZcUYkaRIaubrh+i81tb8mKm3k6V40XY5+g5AVpc/elH50= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773312672; c=relaxed/simple; bh=EQkW6+lM7nq1mEy4KqkJc8bvPxN1VhQdmSs8sInnBXs=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Rl1W8jLawgJGkGxMo9UaHTyUGgzNWLl2kffEY79V0v3zO9DqZKW1YGxMZ761c6xqJVxxYrnHxk60St1pvWSfpzh7T7IvzLrmEvxEkj/DUNht/MHc9BaRLfcTes3Yu/zh28+pEyoDatYQO1qsB39V9+lySDuVlTEtLkwDNQJGgps= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=OuTYXSN9; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=hsMy/Set; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="OuTYXSN9"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="hsMy/Set" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1773312669; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CsY/vQXT9rNKMEUwHJ7p9ZkQrv3cw72S9U06qV9qj28=; b=OuTYXSN9dIB3yL+VSZD5VhyCmw2eGAPZAAv3FKI1y0uTYgDZH2eOmySy3gbBr5jOEjOoq9 sxsM67bbHxSu2LdoQB6cy5Tg2Zqt50x0eQn5qt5Kq4ICJGWKQQ4PkMC0clPHJNSaYDcOog LnVWsXaTOqt3xoXimTekVBmJSG91SCw= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-159-1Og-RkdZPwO_gejMfMiWEA-1; Thu, 12 Mar 2026 06:51:08 -0400 X-MC-Unique: 1Og-RkdZPwO_gejMfMiWEA-1 X-Mimecast-MFC-AGG-ID: 1Og-RkdZPwO_gejMfMiWEA_1773312667 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-48378df3469so9488955e9.1 for ; Thu, 12 Mar 2026 03:51:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1773312667; x=1773917467; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=CsY/vQXT9rNKMEUwHJ7p9ZkQrv3cw72S9U06qV9qj28=; b=hsMy/SetE+gflMxauowE/FCbKtG10b5eMz4KW+HzAblRRBPNkNdppDJorgvhVUNwn5 HpzesIm9DkMLuJYn0ZYeFZKKLCis2tTSEFHPjtxBXU+Vv9rYYscvBVE3ZrGGxf3qPI37 Xzwr80CcYpnnhRcvrM4E3QZIyyxor9KkfoG0jAptBZ8l//rd9xON+j/QdWvSbyHINMKA 4qm9bg8rt1nkyi2O5RV6DiYU4beSBONVvRYLoVA+HzP89RpA/Ig1PrXnk874sEcxo0WM FDRZTGuh5Bw4u7MimJu+VKcogLfRLD6dfWc9PoCXCgPCQYa7Ms9KxFMRszLjoPkH9Rmm k5Xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1773312667; x=1773917467; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=CsY/vQXT9rNKMEUwHJ7p9ZkQrv3cw72S9U06qV9qj28=; b=cAMS/KerCI/hm/LvJ+hlTm6ncmE+Q1dn7vLGN3UBXVz0ZEEeSgXweXPsAtR29MjSns BNExzqkPP2NS3L3q5r63zEabr7GN39+6Jdu22x2YjrsUDgs/a3OZffF8IbCc5vJraC35 K4zx2JOZsXOiKliCLUe+Qsoz7aY2oUtGm2XR9x0INxFFIQtXicGznsw8125uhLZxTeyS +muKGLae1vFRzbEzf4SaBDil+UbY2eWhPjXbr7Ei2/mdzjZT37hTZmkMVF6tgQGHV4rP YVPesu+s2ks/Q4pKiJV/Ae7djV+0+7ogiHZUbjnr1Wh0T4JkZrJNOteL1z5qjR63fXoy SR0g== X-Forwarded-Encrypted: i=1; AJvYcCUX4c8qLFgxMqEmmA72iAeWoSmaWxDfFdZalBiylTIl5cn00KtKHtY3i8Tlgek2uXKE+whTrQWj4yvicKs=@vger.kernel.org X-Gm-Message-State: AOJu0Yzsc5ORUKSeRbAU4R8RE/L9KJN1elH0dF3pj8djEhlJ5XKtDqk1 +mYgHnJmXF1YSUuUOQogEsmdGsciD3bPRfB5w8uUD8ljVJb7L9cSh02njOgUKKrENurr5VLQISY fZ7Bl+BILNxEn5FkGCpHNt6vyAdp8tunL0rWYQBI/aJCps8UzmnIIsI821Zu8rpsj1w== X-Gm-Gg: ATEYQzxCqWAttYXqUo1a9zU/6jOZLmr/jspvb3zZg5RTw7Lg9lETWR8s2iu+QCnwMdg SX1iFmmtnHhtA2jsgEAwr3eVB/dMFiB/OayOAkoCQda7ahiEjMUV4XAWy6MYpegG5gDKk+vrX9S B5ZXc1aSbPaG9i2vimE+w93KUaPKGYcBGdZN1vACUeI/iVARnTq+Nncy8azFkbdHAj+CAfP5ery mq1oeRh9RwsPvkHnFJnFQXvhPYrxNToOr31zs8NhWsMOLF6+GzL5pGwnobq2Xh8RD9p9nAdCtNa kltbD2xQfnQS25N4BoJ8kzvgMbFmyMipALV6CmeHVKK2qd/FZWlYcld0eE8NYoVtjFeuQoMCq7B GYfu6aMpmsUl1CeOwxG/uKOkOFusMD8AUGNHUNbxTOVdUDxcuE3HtqVo= X-Received: by 2002:a05:600c:c4a4:b0:485:3e00:944a with SMTP id 5b1f17b1804b1-4854f583e77mr36424035e9.9.1773312667365; Thu, 12 Mar 2026 03:51:07 -0700 (PDT) X-Received: by 2002:a05:600c:c4a4:b0:485:3e00:944a with SMTP id 5b1f17b1804b1-4854f583e77mr36423755e9.9.1773312666862; Thu, 12 Mar 2026 03:51:06 -0700 (PDT) Received: from [192.168.88.32] ([216.128.11.95]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4854b5e912fsm311838795e9.2.2026.03.12.03.51.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 Mar 2026 03:51:06 -0700 (PDT) Message-ID: Date: Thu, 12 Mar 2026 11:51:04 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [net-next,v7,05/10] net: phy: Represent PHY-less SFP modules with phy_port To: maxime.chevallier@bootlin.com Cc: o.rempel@pengutronix.de, herve.codina@bootlin.com, andrew@lunn.ch, edumazet@google.com, linux-kernel@vger.kernel.org, hkallweit1@gmail.com, netdev@vger.kernel.org, kuba@kernel.org, romain.gantois@bootlin.com, f.fainelli@gmail.com, christophe.leroy@csgroup.eu, kory.maincent@bootlin.com, mwojtas@chromium.org, dimitri.fedrau@liebherr.com, nicveronese@gmail.com, horms@kernel.org, daniel@makrotopia.org, kabel@kernel.org, linux@armlinux.org.uk, vladimir.oltean@nxp.com, davem@davemloft.net, thomas.petazzoni@bootlin.com, bjorn@kernel.org References: <20260309152747.702373-6-maxime.chevallier@bootlin.com> <20260312104733.87688-1-pabeni@redhat.com> Content-Language: en-US From: Paolo Abeni In-Reply-To: <20260312104733.87688-1-pabeni@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 3/12/26 11:47 AM, Paolo Abeni wrote: > This is an AI-generated review of your patch. The human sending this > email has considered the AI review valid, or at least plausible. > > For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html > --- > net: phy: Represent PHY-less SFP modules with phy_port > > Now that the SFP bus infrastructure notifies when PHY-less modules are > connected, we can create a phy_port to represent it. Instead of letting > the SFP subsystem handle that, the Bus' upstream is in charge of > maintaining that phy_port and register it to the topology, as the > upstream (in this case a phy device) is directly interacting with the > underlying net_device. > > Add a phy_caps helper to get the achievable modes on this module based > on what the phy_port representing the bus supports. > >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index 4b06644866b2..faa58a780477 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c > > [ ... ] > >> @@ -1483,6 +1483,8 @@ static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy) >> struct phy_device *phydev = upstream; >> struct net_device *dev = phydev->attached_dev; >> >> + phydev->has_sfp_mod_phy = true; >> + >> if (dev) >> return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev); > > If phy_link_topo_add_phy() fails, does has_sfp_mod_phy get cleaned up > correctly? > > Looking at the SFP framework, when phy_link_topo_add_phy() fails here, > sfp_sm_probe_phy() in sfp.c will free the PHY but won't call > disconnect_phy (because sfp->mod_phy was not set). This leaves > has_sfp_mod_phy stuck as true. > > On a subsequent SFP module insertion, phy_sfp_module_start() checks > !phydev->has_sfp_mod_phy: > > drivers/net/phy/phy_device.c:phy_sfp_module_start() { > if (!phydev->has_sfp_mod_phy) > return phy_add_sfp_mod_port(phydev); > } > > Seeing has_sfp_mod_phy is true (stale), it skips creating the mod_port > for a PHY-less module. The module then won't be represented in the link > topology. > > Would it be better to either set has_sfp_mod_phy after > phy_link_topo_add_phy() succeeds, or clear it in an error path? Out of sheer ignorance IDK if phy_sfp_module_start() could really happen in practice after a phy_sfp_connect_phy() failure, but the inconsistency is likely worthy a cleanup? Leaving the series alive in PW, in case the phy crew disagree or a follow-up is preferred to a repost. /P