From patchwork Wed Oct 4 09:19:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marek_Beh=C3=BAn?= X-Patchwork-Id: 13408530 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 2A2C710A09 for ; Wed, 4 Oct 2023 09:19:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7D6A3C433C9; Wed, 4 Oct 2023 09:19:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1696411150; bh=ARqqx35Yo1WsqWxuoBmHHsUb0Zf+p1XF8jxNeXqyJgA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uy0BRJiI3qDuayZh8Jmas81lwvfVU5pbmjOHE5UUQg3gsHgdmrf0WArNXna5UEDpu DHyYIyF/2cP5QdkbU8BWMb+IS5NH6zZtVIK0PdfmOQpW7jrXhvVEEWUqBCPMA2+jKu 23yEVsHN8Nsk/oNxAtJpCOi4QJdLycC+/PhM2iR+zR5hc7EGr43LuGR/ECrAU5NtwO At2IweyUbxMjd1Ge3BSI4PZm+Jew04YEsynpFmdjd0k3n8GsAk9L2dyAq2Be4n5KHy r45eX67uKE3Gl0q/k1mJr/ebm3ZgzS9U+q7kB/m7A0qEP8KE2OPOfMqTWHAPNcgVkM NScOFOjvCRhRQ== From: =?utf-8?q?Marek_Beh=C3=BAn?= To: Christian Marangi , "David S. Miller" , Paolo Abeni , netdev@vger.kernel.org Cc: =?utf-8?q?Marek_Beh=C3=BAn?= Subject: [PATCH net v2 1/2] net: dsa: qca8k: fix regmap bulk read/write methods on big endian systems Date: Wed, 4 Oct 2023 11:19:03 +0200 Message-ID: <20231004091904.16586-2-kabel@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231004091904.16586-1-kabel@kernel.org> References: <20231004091904.16586-1-kabel@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Commit c766e077d927 ("net: dsa: qca8k: convert to regmap read/write API") introduced bulk read/write methods to qca8k's regmap. The regmap bulk read/write methods get the register address in a buffer passed as a void pointer parameter (the same buffer contains also the read/written values). The register address occupies only as many bytes as it requires at the beginning of this buffer. For example if the .reg_bits member in regmap_config is 16 (as is the case for this driver), the register address occupies only the first 2 bytes in this buffer, so it can be cast to u16. But the original commit implementing these bulk read/write methods cast the buffer to u32: u32 reg = *(u32 *)reg_buf & U16_MAX; taking the first 4 bytes. This works on little endian systems where the first 2 bytes of the buffer correspond to the low 16-bits, but it obviously cannot work on big endian systems. Fix this by casting the beginning of the buffer to u16 as u32 reg = *(u16 *)reg_buf; Fixes: c766e077d927 ("net: dsa: qca8k: convert to regmap read/write API") Signed-off-by: Marek Behún Tested-by: Christian Marangi Reviewed-by: Christian Marangi --- drivers/net/dsa/qca/qca8k-8xxx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c index de1dc22cf683..d2df30640269 100644 --- a/drivers/net/dsa/qca/qca8k-8xxx.c +++ b/drivers/net/dsa/qca/qca8k-8xxx.c @@ -505,8 +505,8 @@ qca8k_bulk_read(void *ctx, const void *reg_buf, size_t reg_len, void *val_buf, size_t val_len) { int i, count = val_len / sizeof(u32), ret; - u32 reg = *(u32 *)reg_buf & U16_MAX; struct qca8k_priv *priv = ctx; + u32 reg = *(u16 *)reg_buf; if (priv->mgmt_master && !qca8k_read_eth(priv, reg, val_buf, val_len)) @@ -527,8 +527,8 @@ qca8k_bulk_gather_write(void *ctx, const void *reg_buf, size_t reg_len, const void *val_buf, size_t val_len) { int i, count = val_len / sizeof(u32), ret; - u32 reg = *(u32 *)reg_buf & U16_MAX; struct qca8k_priv *priv = ctx; + u32 reg = *(u16 *)reg_buf; u32 *val = (u32 *)val_buf; if (priv->mgmt_master && From patchwork Wed Oct 4 09:19:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marek_Beh=C3=BAn?= X-Patchwork-Id: 13408531 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 A45B710A01 for ; Wed, 4 Oct 2023 09:19:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0E086C433C8; Wed, 4 Oct 2023 09:19:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1696411152; bh=mp+1EXdp3qgYWtCDpptgNkw0RRPLLgHZjeZQSO6Wllc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VgdSrkTxIWDuKDLP9qMYbatOkPs926JTAozi8qc8MEqG009GRqFTsVJUXJKrsh/s7 ZNONhRBRtbYh13isQIeYs7exrn31rxd2xHlqoe8av+ynQ9SjNZqxJteU5ZaIrQFRtJ cltTNirFFC0RlXj1f51csVUc94KcTZ5Va1Iku8WitldCcEgRch2koCDHQfpXodEi3U 4sKjW+LvJExdZ792jloFbrWZRqXu2/hPQCv0FaxQnzu+Af/ynYiqAkqR2VbEfYokh/ ODliWoHOPIHdJ8wLC38c9y94pP+HH4vFatGXUMJ5UQnlSwqd8UcEr152iReXvYMy4W Im9L6ptJ1gc5w== From: =?utf-8?q?Marek_Beh=C3=BAn?= To: Christian Marangi , "David S. Miller" , Paolo Abeni , netdev@vger.kernel.org Cc: =?utf-8?q?Marek_Beh=C3=BAn?= Subject: [PATCH net v2 2/2] net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames Date: Wed, 4 Oct 2023 11:19:04 +0200 Message-ID: <20231004091904.16586-3-kabel@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231004091904.16586-1-kabel@kernel.org> References: <20231004091904.16586-1-kabel@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Besides the QCA8337 switch the Turris 1.x device has on it's MDIO bus also Micron ethernet PHY (dedicated to the WAN port). We've been experiencing a strange behavior of the WAN ethernet interface, wherein the WAN PHY started timing out the MDIO accesses, for example when the interface was brought down and then back up. Bisecting led to commit 2cd548566384 ("net: dsa: qca8k: add support for phy read/write with mgmt Ethernet"), which added support to access the QCA8337 switch's internal PHYs via management ethernet frames. Connecting the MDIO bus pins onto an oscilloscope, I was able to see that the MDIO bus was active whenever a request to read/write an internal PHY register was done via an management ethernet frame. My theory is that when the switch core always communicates with the internal PHYs via the MDIO bus, even when externally we request the access via ethernet. This MDIO bus is the same one via which the switch and internal PHYs are accessible to the board, and the board may have other devices connected on this bus. An ASCII illustration may give more insight: +---------+ +----| | | | WAN PHY | | +--| | | | +---------+ | | | | +----------------------------------+ | | | QCA8337 | MDC | | | +-------+ | ------o-+--|--------o------------o--| | | MDIO | | | | | PHY 1 |-|--to RJ45 --------o--|---o----+---------o--+--| | | | | | | | +-------+ | | +-------------+ | o--| | | | | MDIO MDC | | | | PHY 2 |-|--to RJ45 eth1 | | | o--+--| | | -----------|-|port0 | | | +-------+ | | | | | o--| | | | | switch core | | | | PHY 3 |-|--to RJ45 | +-------------+ o--+--| | | | | | +-------+ | | | o--| ... | | +----------------------------------+ When we send a request to read an internal PHY register via an ethernet management frame via eth1, the switch core receives the ethernet frame on port 0 and then communicates with the internal PHY via MDIO. At this time, other potential devices, such as the WAN PHY on Turris 1.x, cannot use the MDIO bus, since it may cause a bus conflict. Fix this issue by locking the MDIO bus even when we are accessing the PHY registers via ethernet management frames. Fixes: 2cd548566384 ("net: dsa: qca8k: add support for phy read/write with mgmt Ethernet") Signed-off-by: Marek Behún Reviewed-by: Christian Marangi --- drivers/net/dsa/qca/qca8k-8xxx.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c index d2df30640269..4ce68e655a63 100644 --- a/drivers/net/dsa/qca/qca8k-8xxx.c +++ b/drivers/net/dsa/qca/qca8k-8xxx.c @@ -666,6 +666,15 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, goto err_read_skb; } + /* It seems that accessing the switch's internal PHYs via management + * packets still uses the MDIO bus within the switch internally, and + * these accesses can conflict with external MDIO accesses to other + * devices on the MDIO bus. + * We therefore need to lock the MDIO bus onto which the switch is + * connected. + */ + mutex_lock(&priv->bus->mdio_lock); + /* Actually start the request: * 1. Send mdio master packet * 2. Busy Wait for mdio master command @@ -678,6 +687,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, mgmt_master = priv->mgmt_master; if (!mgmt_master) { mutex_unlock(&mgmt_eth_data->mutex); + mutex_unlock(&priv->bus->mdio_lock); ret = -EINVAL; goto err_mgmt_master; } @@ -765,6 +775,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, QCA8K_ETHERNET_TIMEOUT); mutex_unlock(&mgmt_eth_data->mutex); + mutex_unlock(&priv->bus->mdio_lock); return ret;