| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f485d1d4f7b34cc2ebf3d60030d1c67b4016af3c.1720107535.git.daniel@makrotopia.org>
Date: Fri, 5 Jul 2024 11:48:40 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: Arınç ÜNAL <arinc.unal@...nc9.com>,
Daniel Golle <daniel@...rotopia.org>,
DENG Qingfang <dqfext@...il.com>,
Sean Wang <sean.wang@...iatek.com>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Landen Chao <Landen.Chao@...iatek.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: [PATCH v5] net: dsa: mt7530: fix impossible MDIO address and issue
warning
The MDIO address of the MT7530 and MT7531 switch ICs can be configured
using bootstrap pins. However, there are only 4 possible options for the
switch itself: 7, 15, 23 and 31. As in MediaTek's SDK the address of the
switch is wrongly stated in the device tree as 0 (while in reality it is
31), warn the user about such broken device tree and make a good guess
what was actually intended.
This is necessary to not break compatibility with existing Device Trees
wrongly declaring the switch to be present at address 0 or 1, as with
commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY address of switch
from device tree") the address in device tree will be taken into
account, while before it was hard-coded in the driver to 0x1f
independently of the value in Device Tree.
Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
Signed-off-by: Daniel Golle <daniel@...rotopia.org>
Reviewed-by: Andrew Lunn <andrew@...n.ch>
Tested-by: Arınç ÜNAL <arinc.unal@...nc9.com>
Reviewed-by: Arınç ÜNAL <arinc.unal@...nc9.com>
---
Changes since v4 [4]:
- fix indentation
- bring back parentheses to satisfy GCC
- print warning before registering new device to avoid confusing
order of messages in kernel log
- improve commit message
Changes since v3 [3]:
- simplify calculation of correct address
Changes since v2 [2]:
- use macros instead of magic numbers
- introduce helper functions
- register new device on MDIO bus instead of messing with the address
and schedule delayed_work to unregister the "wrong" device.
This is a slightly different approach than suggested by Russell, but
imho makes things much easier than keeping the "wrong" device and
having to deal with keeping the removal of both devices linked.
- improve comments
Changes since v1 [1]:
- use FW_WARN as suggested.
- fix build on net tree which doesn't have 'mdiodev' as member of the
priv struct. Imho including this patch as fix makes sense to warn
users about broken firmware, even if the change introducing the
actual breakage is only present in net-next for now.
[1]: https://patchwork.kernel.org/project/netdevbpf/patch/e615351aefba25e990215845e4812e6cb8153b28.1714433716.git.daniel@makrotopia.org/
[2]: https://patchwork.kernel.org/project/netdevbpf/patch/11f5f127d0350e72569c36f9060b6e642dfaddbb.1714514208.git.daniel@makrotopia.org/
[3]: https://patchwork.kernel.org/project/netdevbpf/patch/7e3fed489c0bbca84a386b1077c61589030ff4ab.1719963228.git.daniel@makrotopia.org/
[4]: https://patchwork.kernel.org/project/netdevbpf/patch/1c378be54d0fb76117f6d72dadd4a43a9950f0dc.1720105125.git.daniel@makrotopia.org/
drivers/net/dsa/mt7530-mdio.c | 92 +++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 51df42ccdbe6..c666edec6b9e 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -11,6 +11,7 @@
#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/regulator/consumer.h>
+#include <linux/workqueue.h>
#include <net/dsa.h>
#include "mt7530.h"
@@ -136,6 +137,93 @@ static const struct of_device_id mt7530_of_match[] = {
};
MODULE_DEVICE_TABLE(of, mt7530_of_match);
+static int
+mt7530_correct_addr(int phy_addr)
+{
+ /* The corrected address is calculated as stated below:
+ * 0 ~ 6, 31 -> 31
+ * 7 ~ 14 -> 7
+ * 15 ~ 22 -> 15
+ * 23 ~ 30 -> 23
+ */
+ return (((phy_addr - MT7530_NUM_PORTS) & ~MT7530_NUM_PORTS) +
+ MT7530_NUM_PORTS) & (PHY_MAX_ADDR - 1);
+}
+
+static bool
+mt7530_is_invalid_addr(int phy_addr)
+{
+ /* Only MDIO bus addresses 7, 15, 23, and 31 are valid options,
+ * which all have the least significant three bits set. Check
+ * for this.
+ */
+ return (phy_addr & MT7530_NUM_PORTS) != MT7530_NUM_PORTS;
+}
+
+struct remove_impossible_priv {
+ struct delayed_work remove_impossible_work;
+ struct mdio_device *mdiodev;
+};
+
+static void
+mt7530_remove_impossible(struct work_struct *work)
+{
+ struct remove_impossible_priv *priv = container_of(work, struct remove_impossible_priv,
+ remove_impossible_work.work);
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ mdio_device_remove(mdiodev);
+ mdio_device_free(mdiodev);
+ kfree(priv);
+}
+
+static int
+mt7530_reregister(struct mdio_device *mdiodev)
+{
+ /* If the address in DT must be wrong, make a good guess about
+ * the most likely intention, issue a warning, register a new
+ * MDIO device at the correct address and schedule the removal
+ * of the device having an impossible address.
+ */
+ struct fwnode_handle *fwnode = dev_fwnode(&mdiodev->dev);
+ int corrected_addr = mt7530_correct_addr(mdiodev->addr);
+ struct remove_impossible_priv *rem_priv;
+ struct mdio_device *new_mdiodev;
+ int ret;
+
+ dev_warn(&mdiodev->dev, FW_WARN
+ "impossible switch MDIO address in device tree, assuming %d\n",
+ corrected_addr);
+
+ rem_priv = kmalloc(sizeof(*rem_priv), GFP_KERNEL);
+ if (!rem_priv)
+ return -ENOMEM;
+
+ new_mdiodev = mdio_device_create(mdiodev->bus, corrected_addr);
+ if (IS_ERR(new_mdiodev)) {
+ ret = PTR_ERR(new_mdiodev);
+ goto out_free_work;
+ }
+ device_set_node(&new_mdiodev->dev, fwnode);
+
+ ret = mdio_device_register(new_mdiodev);
+ if (WARN_ON(ret))
+ goto out_free_dev;
+
+ /* schedule impossible device for removal from mdio bus */
+ rem_priv->mdiodev = mdiodev;
+ INIT_DELAYED_WORK(&rem_priv->remove_impossible_work, mt7530_remove_impossible);
+ schedule_delayed_work(&rem_priv->remove_impossible_work, 0);
+
+ return -EFAULT;
+
+out_free_dev:
+ mdio_device_free(new_mdiodev);
+out_free_work:
+ kfree(rem_priv);
+ return ret;
+}
+
static int
mt7530_probe(struct mdio_device *mdiodev)
{
@@ -144,6 +232,10 @@ mt7530_probe(struct mdio_device *mdiodev)
struct device_node *dn;
int ret;
+ /* Check and if needed correct the MDIO address of the switch */
+ if (mt7530_is_invalid_addr(mdiodev->addr))
+ return mt7530_reregister(mdiodev);
+
dn = mdiodev->dev.of_node;
priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL);
--
2.45.2
Powered by blists - more mailing lists