[{"id":3687577,"web_url":"http://patchwork.ozlabs.org/comment/3687577/","msgid":"<20260507080006.D0E1FC2BCC9@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-07T08:00:06","subject":"Re: [PATCH v8 phy-next 24/31] net: renesas: rswitch: include PHY\n provider header","submitter":{"id":93311,"url":"http://patchwork.ozlabs.org/api/people/93311/","name":null,"email":"sashiko-bot@kernel.org"},"content":"Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:\n- [High] Pre-existing data races and flawed power/initialization state tracking (API abuse)\n--\n\ncommit 2d0a78469a71c1f0b6d4b9cc19ca73b858d63917\nAuthor: Vladimir Oltean \n\nnet: renesas: rswitch: include PHY provider header\n\nAs a PHY consumer driver, the Renesas rswitch dereferences internal fields\nof struct phy, which is being made an opaque pointer. This commit works\naround the new boundary by explicitly including the internal PHY provider\nheader to keep the driver working as before.\n\nLink: https://lore.kernel.org/linux-phy/20260211194541.cdmibrpfn6ej6e74@skbuf/\n\n> diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c\n> index 6fe964816322a..132be5f15073c 100644\n> --- a/drivers/net/ethernet/renesas/rswitch_main.c\n> +++ b/drivers/net/ethernet/renesas/rswitch_main.c\n> @@ -27,6 +27,7 @@\n> #include \n> #include \n> \n> +#include \"../../../phy/phy-provider.h\" /* FIXME */\n\nThis isn't a new regression, but while the commit message notes this driver\nneeds a complete rethink, actively preserving this behavior by including the\nprovider header bypasses the boundary and leaves a race condition and logic\nerror intact.\n\nCould the driver track its own power and initialization states locally instead\nof relying on the internal global state of the generic PHY framework?\n\nIn rswitch_adjust_link(), the driver reads rdev->serdes->power_count\nwithout holding phy->mutex:\n\nrswitch_main.c:rswitch_adjust_link() {\n...\n\t\tif (phydev->link)\n\t\t\tphy_power_on(rdev->serdes);\n\t\telse if (rdev->serdes->power_count)\n\t\t\tphy_power_off(rdev->serdes);\n...\n}\n\nCan this lockless access create a TOCTOU (Time-Of-Check to Time-Of-Use)\ndata race?\n\nFurthermore, because multiple ports can share the same PHY in a QSGMII setup,\ndoes using the global power_count to deduce this port's state cause logic\nerrors?\n\nIf Port A's phy_power_on() fails, but Port B subsequently turns on the PHY,\nPort A will read power_count > 0. When Port A's link goes down, won't it\nincorrectly call phy_power_off(), stealing Port B's power vote and turning\noff the shared PHY while it is still in use by Port B?\n\nSimilarly, in renesas_eth_sw_suspend(), init_count is read without a lock:\n\nrswitch_main.c:renesas_eth_sw_suspend() {\n...\n\t\tif (priv->rdev[i]->serdes->init_count)\n\t\t\tphy_exit(priv->rdev[i]->serdes);\n...\n}\n\nWould it be safer to add local boolean flags (e.g., bool serdes_power_on or\nbool serdes_init_done) directly to struct rswitch_device to track successful\ncalls?\n\n> #include \"rswitch.h\"\n> #include \"rswitch_l2.h\"\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=PzZtqLKc;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-54057-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"PzZtqLKc\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4gB4S024wCz1yCg\n\tfor ; Thu, 07 May 2026 18:00:32 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 2C619302D10C\n\tfor ; Thu, 7 May 2026 08:00:21 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id C671737F8AD;\n\tThu, 7 May 2026 08:00:09 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 3735E37FF5C\n\tfor ; Thu, 7 May 2026 08:00:08 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id D0E1FC2BCC9;\n\tThu, 7 May 2026 08:00:06 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778140808; cv=none;\n b=ec1P95CXj5nZ5CBNVa+l0a2PgqiluzoKwh3QdkudHFj+weMs4ma7WWdmpGIy1pdo/B5gwcheXA2cBoMjB2MNNLD4pAMn4NTJWqW9NiRUIFTqzh6NrQbsk3xUjY0KC0XdRvlx+SeecSGjhikudr6DYwEOWy6k3SQvBXG0TuxOtdk=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778140808; c=relaxed/simple;\n\tbh=nDljSIuX+8VTht2wTJZtuUhYWCcLJNbeMRhwunn0HSs=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=F6UurdYepSaCbe1s9gjfuQjs8o+M4uK8asmVmL+nGcgmCciqrg1jcmuZyPzJD6+C6UlSA2ZeyCfM4X2QK6zZie6f8DCaj+djGZ1EVjfcw5CneQHtzCtzzEnOd7dmye08S+OYF0HDlUqudkzbCC5iwctI4gbLdmbcsWe0mKgAg2E=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=PzZtqLKc; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1778140807;\n\tbh=nDljSIuX+8VTht2wTJZtuUhYWCcLJNbeMRhwunn0HSs=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=PzZtqLKcS/5cUOKEGRsm28WurwksTaUja1+NZf5zw6kVbqBIf4N8PIuzKjJspA1DT\n\t n6HxxsfoZT0Kfx1XFSXqRZUWTHHGOWWNyyCHtpH/bgndNjhmhT1d99pidnp6pnEHT/\n\t DpmTF/2i2UPqocHeu2fi8y51o9uAeNifhU30YRqqCaNgM9kjqaRXIVVj/FVv8Ss9ke\n\t tLNYje26U7ElA7xDOC6oW5VCnuzapmb9G1gHsAAFTo9oFWxzS8oLHFSeOcL+aRnsDp\n\t BOw1pyLHUS6q15M+Y5y2z4LCnycDlV9/ARMGkyAdZholqINuVABis4O2uUtUeOpLUs\n\t K7jwWDv1i4vRg==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH v8 phy-next 24/31] net: renesas: rswitch: include PHY\n provider header","Reply-To":"sashiko@lists.linux.dev","To":"\"Vladimir Oltean\" ","Cc":"wsa+renesas@sang-engineering.com, linux-pci@vger.kernel.org","In-Reply-To":"<20260505100523.1922388-25-vladimir.oltean@nxp.com>","References":"<20260505100523.1922388-25-vladimir.oltean@nxp.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Thu, 07 May 2026 08:00:06 +0000","Message-Id":"<20260507080006.D0E1FC2BCC9@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]