* [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
@ 2024-07-02 23:44 Daniel Golle
2024-07-03 1:07 ` Andrew Lunn
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Daniel Golle @ 2024-07-02 23:44 UTC (permalink / raw)
To: Arınç ÜNAL, Daniel Golle, DENG Qingfang, Sean Wang,
Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Landen Chao, Frank Wunderlich,
Rob Herring, Krzysztof Kozlowski, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek, regressions
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 imporant also to not break compatibility with older Device Trees
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 to 0x1f.
Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
Only tested on BPi-R3 (with various deliberately broken DT) for now!
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/
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..b5049ec2d84d 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
+ * 8~14 -> 7
+ * 16~22 -> 15
+ * 24~30 -> 23
+ */
+return ((((phy_addr - MT7530_NUM_PORTS) & ~MT7530_NUM_PORTS) % PHY_MAX_ADDR) +
+ 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;
+
+ 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;
+
+ dev_warn(&mdiodev->dev, FW_WARN
+ "impossible switch MDIO address in device tree, assuming %d\n",
+ corrected_addr);
+
+ /* 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-02 23:44 [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
@ 2024-07-03 1:07 ` Andrew Lunn
2024-07-03 7:03 ` Arınç ÜNAL
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2024-07-03 1:07 UTC (permalink / raw)
To: Daniel Golle
Cc: Arınç ÜNAL, DENG Qingfang, Sean Wang,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Landen Chao, Frank Wunderlich,
Rob Herring, Krzysztof Kozlowski, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek, regressions
On Wed, Jul 03, 2024 at 12:44:28AM +0100, Daniel Golle wrote:
> 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 imporant also to not break compatibility with older Device Trees
> 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 to 0x1f.
>
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Fun code. Not seen anything like this before.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-02 23:44 [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-07-03 1:07 ` Andrew Lunn
@ 2024-07-03 7:03 ` Arınç ÜNAL
2024-07-04 17:21 ` Vladimir Oltean
2024-07-04 2:13 ` Jakub Kicinski
2024-07-04 14:46 ` Florian Fainelli
3 siblings, 1 reply; 11+ messages in thread
From: Arınç ÜNAL @ 2024-07-03 7:03 UTC (permalink / raw)
To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Landen Chao, Frank Wunderlich,
Rob Herring, Krzysztof Kozlowski, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek, regressions
On 03/07/2024 02:44, Daniel Golle wrote:
> 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 imporant also to not break compatibility with older Device Trees
> 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 to 0x1f.
>
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> Only tested on BPi-R3 (with various deliberately broken DT) for now!
>
> 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/
Works on standalone MT7530, MT7621's MCM MT7530, and MT7531. From MT7621's
MCM MT7530:
[ 1.357287] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
[ 1.364065] mt7530-mdio mdio-bus:00: [Firmware Warn]: impossible switch MDIO address in device tree, assuming 31
[ 1.374303] mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -14
[...]
[ 1.448370] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
[ 1.477676] mt7530-mdio mdio-bus:1f: configuring for fixed/rgmii link mode
[ 1.485687] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
[ 1.493480] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode
[ 1.502680] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
[ 1.513620] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
[ 1.519671] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
[ 1.533072] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
[ 1.545042] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
[ 1.557031] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
I'm not fond of the use of the non-standard term, MDIO address, instead of
"PHY Address" as described in 22.2.4.5.5 of IEEE Std 802.3-2022.
Regardless:
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Reviewed-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Arınç
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-02 23:44 [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-07-03 1:07 ` Andrew Lunn
2024-07-03 7:03 ` Arınç ÜNAL
@ 2024-07-04 2:13 ` Jakub Kicinski
2024-07-04 10:48 ` Daniel Golle
2024-07-04 14:46 ` Florian Fainelli
3 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2024-07-04 2:13 UTC (permalink / raw)
To: Daniel Golle
Cc: Arınç ÜNAL, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On Wed, 3 Jul 2024 00:44:28 +0100 Daniel Golle wrote:
> + /* The corrected address is calculated as stated below:
> + * 0~6 -> 31
> + * 8~14 -> 7
> + * 16~22 -> 15
> + * 24~30 -> 23
> + */
> +return ((((phy_addr - MT7530_NUM_PORTS) & ~MT7530_NUM_PORTS) % PHY_MAX_ADDR) +
> + MT7530_NUM_PORTS) & (PHY_MAX_ADDR - 1);
nit: the return statement lacks indentation
but also based on the comment, isn't it:
return (round_down(phy_addr, MT7530_NUM_PORTS + 1) - 1) & (PHY_MAX_ADDR - 1);
?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-04 2:13 ` Jakub Kicinski
@ 2024-07-04 10:48 ` Daniel Golle
2024-07-04 14:16 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Golle @ 2024-07-04 10:48 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Arınç ÜNAL, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On Wed, Jul 03, 2024 at 07:13:08PM -0700, Jakub Kicinski wrote:
> On Wed, 3 Jul 2024 00:44:28 +0100 Daniel Golle wrote:
> > + /* The corrected address is calculated as stated below:
> > + * 0~6 -> 31
> > + * 8~14 -> 7
> > + * 16~22 -> 15
> > + * 24~30 -> 23
> > + */
> > +return ((((phy_addr - MT7530_NUM_PORTS) & ~MT7530_NUM_PORTS) % PHY_MAX_ADDR) +
> > + MT7530_NUM_PORTS) & (PHY_MAX_ADDR - 1);
>
> nit: the return statement lacks indentation
Yes, lacks an additional space to match the level of the first open parentheses.
I'll fix that in the next round.
>
> but also based on the comment, isn't it:
>
> return (round_down(phy_addr, MT7530_NUM_PORTS + 1) - 1) & (PHY_MAX_ADDR - 1);
The original, more complicated statement covers also the correct addresses,
ie. 31 -> 31, 7 -> 7, 15 -> 15, 23 -> 23. However, the function is never
called if the address is deemed correct, so that doesn't actually matter.
It's kinda difficult to decide whether it is more important to return
correct results also for values never used with the current code, or
have a slightly more readable and shorter function but with expectations
regarding the input values given by the caller.
Opinions?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-04 10:48 ` Daniel Golle
@ 2024-07-04 14:16 ` Jakub Kicinski
0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2024-07-04 14:16 UTC (permalink / raw)
To: Daniel Golle
Cc: Arınç ÜNAL, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On Thu, 4 Jul 2024 11:48:42 +0100 Daniel Golle wrote:
> > > +return ((((phy_addr - MT7530_NUM_PORTS) & ~MT7530_NUM_PORTS) % PHY_MAX_ADDR) +
> > > + MT7530_NUM_PORTS) & (PHY_MAX_ADDR - 1);
> >
> > nit: the return statement lacks indentation
>
> Yes, lacks an additional space to match the level of the first open parentheses.
> I'll fix that in the next round.
To be clear I meant the line with "return", not the continuation line
starting with MT7530_NUM_PORTS
> > but also based on the comment, isn't it:
> >
> > return (round_down(phy_addr, MT7530_NUM_PORTS + 1) - 1) & (PHY_MAX_ADDR - 1);
>
> The original, more complicated statement covers also the correct addresses,
> ie. 31 -> 31, 7 -> 7, 15 -> 15, 23 -> 23. However, the function is never
> called if the address is deemed correct, so that doesn't actually matter.
>
> It's kinda difficult to decide whether it is more important to return
> correct results also for values never used with the current code, or
> have a slightly more readable and shorter function but with expectations
> regarding the input values given by the caller.
>
> Opinions?
No strong opinion, but I do think "% PHY_MAX_ADDR" is superfluous, no?
The masking at the end with "& (PHY_MAX_ADDR - 1)" will take care of
truncation.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-02 23:44 [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
` (2 preceding siblings ...)
2024-07-04 2:13 ` Jakub Kicinski
@ 2024-07-04 14:46 ` Florian Fainelli
2024-07-04 15:15 ` Daniel Golle
3 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2024-07-04 14:46 UTC (permalink / raw)
To: Daniel Golle, Arınç ÜNAL, DENG Qingfang, Sean Wang,
Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Landen Chao, Frank Wunderlich,
Rob Herring, Krzysztof Kozlowski, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek, regressions
On 7/3/2024 12:44 AM, Daniel Golle wrote:
> 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 imporant also to not break compatibility with older Device Trees
> 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 to 0x1f.
>
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> Only tested on BPi-R3 (with various deliberately broken DT) for now!
This seems like a whole lot of code just to auto-magically fix an issue
that could be caught with a warning. I appreciate that most of these
devices might be headless, and therefore having some attempt at getting
functional networking goes a long way into allowing users to correct
their mistakes.
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-04 14:46 ` Florian Fainelli
@ 2024-07-04 15:15 ` Daniel Golle
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Golle @ 2024-07-04 15:15 UTC (permalink / raw)
To: Florian Fainelli
Cc: Arınç ÜNAL, DENG Qingfang, Sean Wang, Andrew Lunn,
Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On Thu, Jul 04, 2024 at 04:46:29PM +0200, Florian Fainelli wrote:
> On 7/3/2024 12:44 AM, Daniel Golle wrote:
> > [...]
> > This is imporant also to not break compatibility with older Device Trees
> > 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 to 0x1f.
> >
> > Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > Only tested on BPi-R3 (with various deliberately broken DT) for now!
>
> This seems like a whole lot of code just to auto-magically fix an issue that
> could be caught with a warning. I appreciate that most of these devices
> might be headless, and therefore having some attempt at getting functional
> networking goes a long way into allowing users to correct their mistakes.
My initial motivation was to preserve compatibility with existing broken
device trees.
I then had given up on it because nobody seemed to care, but have
resumed and completed the patch now that reverting the change taking the
address in DT into account became the only alternative.
In OpenWrt all device trees with broken switch MDIO address have been
fixed long ago... (and we always ship the DT along with the kernel, so
DT backward-compatibility doesn't play a role there)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-03 7:03 ` Arınç ÜNAL
@ 2024-07-04 17:21 ` Vladimir Oltean
2024-07-04 18:19 ` Arınç ÜNAL
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2024-07-04 17:21 UTC (permalink / raw)
To: Arınç ÜNAL
Cc: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On Wed, Jul 03, 2024 at 10:03:09AM +0300, Arınç ÜNAL wrote:
> Works on standalone MT7530, MT7621's MCM MT7530, and MT7531. From MT7621's
> MCM MT7530:
>
> [ 1.357287] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
Why is the device corresponding to the first print located at address 1f,
then 0?
> [ 1.364065] mt7530-mdio mdio-bus:00: [Firmware Warn]: impossible switch MDIO address in device tree, assuming 31
> [ 1.374303] mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -14
> [...]
> [ 1.448370] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
> [ 1.477676] mt7530-mdio mdio-bus:1f: configuring for fixed/rgmii link mode
> [ 1.485687] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
> [ 1.493480] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode
> [ 1.502680] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
> [ 1.513620] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
> [ 1.519671] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
> [ 1.533072] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
> [ 1.545042] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
> [ 1.557031] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-04 17:21 ` Vladimir Oltean
@ 2024-07-04 18:19 ` Arınç ÜNAL
2024-07-04 18:52 ` Arınç ÜNAL
0 siblings, 1 reply; 11+ messages in thread
From: Arınç ÜNAL @ 2024-07-04 18:19 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On 04/07/2024 20:21, Vladimir Oltean wrote:
> On Wed, Jul 03, 2024 at 10:03:09AM +0300, Arınç ÜNAL wrote:
>> Works on standalone MT7530, MT7621's MCM MT7530, and MT7531. From MT7621's
>> MCM MT7530:
>>
>> [ 1.357287] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
>
> Why is the device corresponding to the first print located at address 1f,
> then 0?
Because mdio_device_register() in mt7530_reregister() runs with new_mdiodev
after dev_warn() runs with mdiodev.
>
>> [ 1.364065] mt7530-mdio mdio-bus:00: [Firmware Warn]: impossible switch MDIO address in device tree, assuming 31
>> [ 1.374303] mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -14
>> [...]
>> [ 1.448370] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
>> [ 1.477676] mt7530-mdio mdio-bus:1f: configuring for fixed/rgmii link mode
>> [ 1.485687] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
>> [ 1.493480] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode
>> [ 1.502680] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
>> [ 1.513620] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
>> [ 1.519671] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
>> [ 1.533072] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
>> [ 1.545042] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
>> [ 1.557031] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
Arınç
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-07-04 18:19 ` Arınç ÜNAL
@ 2024-07-04 18:52 ` Arınç ÜNAL
0 siblings, 0 replies; 11+ messages in thread
From: Arınç ÜNAL @ 2024-07-04 18:52 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
Landen Chao, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
regressions
On 04/07/2024 21:19, Arınç ÜNAL wrote:
> On 04/07/2024 20:21, Vladimir Oltean wrote:
>> On Wed, Jul 03, 2024 at 10:03:09AM +0300, Arınç ÜNAL wrote:
>>> Works on standalone MT7530, MT7621's MCM MT7530, and MT7531. From MT7621's
>>> MCM MT7530:
>>>
>>> [ 1.357287] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
>>
>> Why is the device corresponding to the first print located at address 1f,
>> then 0?
>
> Because mdio_device_register() in mt7530_reregister() runs with new_mdiodev
> after dev_warn() runs with mdiodev.
I meant before. mdio_device_register() with the new_mdiodev argument is
called before dev_warn() is called with the mdiodev argument.
>
>>
>>> [ 1.364065] mt7530-mdio mdio-bus:00: [Firmware Warn]: impossible switch MDIO address in device tree, assuming 31
>>> [ 1.374303] mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -14
>>> [...]
>>> [ 1.448370] mt7530-mdio mdio-bus:1f: MT7530 adapts as multi-chip module
>>> [ 1.477676] mt7530-mdio mdio-bus:1f: configuring for fixed/rgmii link mode
>>> [ 1.485687] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
>>> [ 1.493480] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode
>>> [ 1.502680] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=17)
>>> [ 1.513620] mt7530-mdio mdio-bus:1f: Link is Up - 1Gbps/Full - flow control rx/tx
>>> [ 1.519671] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=18)
>>> [ 1.533072] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=19)
>>> [ 1.545042] mt7530-mdio mdio-bus:1f lan4 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=20)
>>> [ 1.557031] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=21)
>
> Arınç
Arınç
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-07-04 18:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-02 23:44 [PATCH net v3] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-07-03 1:07 ` Andrew Lunn
2024-07-03 7:03 ` Arınç ÜNAL
2024-07-04 17:21 ` Vladimir Oltean
2024-07-04 18:19 ` Arınç ÜNAL
2024-07-04 18:52 ` Arınç ÜNAL
2024-07-04 2:13 ` Jakub Kicinski
2024-07-04 10:48 ` Daniel Golle
2024-07-04 14:16 ` Jakub Kicinski
2024-07-04 14:46 ` Florian Fainelli
2024-07-04 15:15 ` Daniel Golle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).