* [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning
@ 2024-04-29 23:45 Daniel Golle
2024-04-30 4:58 ` Arınç ÜNAL
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Golle @ 2024-04-29 23:45 UTC (permalink / raw
To: Arınç ÜNAL, Felix Fietkau, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, 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, devicetree, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
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 (ie. only 3 and 4 can be configured, bit
0~2 are always 111). Practically all boards known as of today use the
default setting which is to have the switch respond to address 31, while
the built-in switch PHYs respond to address 0~4 in this case.
However, even in MediaTek's SDK the address of the switch is wrongly
stated in the device trees as 0 (while in reality it is 31), so warn the
user about such broken device tree and make a good guess what was
actually intended.
This is imporant 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. Doing so instead of assuming the switch is always at
address 31 which was previously hard-coded will obviously break things
for many existing downstream device trees as they contain the wrong
address (0) which previously didn't matter.
Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
drivers/net/dsa/mt7530-mdio.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index fa3ee85a99c1..119630fd9060 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -193,6 +193,19 @@ mt7530_probe(struct mdio_device *mdiodev)
return PTR_ERR(priv->io_pwr);
}
+ /* Only MDIO bus address 7, 15, 23 and 31 are valid options */
+ if (~(priv->mdiodev->addr & 0x7) & 0x7) {
+ /* If the address in DT must be wrong, make a good guess about
+ * the most likely intention, and issue a warning.
+ */
+ int correct_addr = ((((priv->mdiodev->addr - 7) & ~0x7) % 0x20) + 7) & 0x1f;
+
+ dev_warn(&mdiodev->dev,
+ "impossible switch MDIO address in device tree: %d, assuming %d\n",
+ priv->mdiodev->addr, correct_addr);
+ priv->mdiodev->addr = correct_addr;
+ }
+
regmap_config = devm_kzalloc(&mdiodev->dev, sizeof(*regmap_config),
GFP_KERNEL);
if (!regmap_config)
--
2.44.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-04-29 23:45 [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
@ 2024-04-30 4:58 ` Arınç ÜNAL
2024-04-30 13:21 ` Andrew Lunn
2024-04-30 20:08 ` Simon Horman
2 siblings, 0 replies; 4+ messages in thread
From: Arınç ÜNAL @ 2024-04-30 4:58 UTC (permalink / raw
To: Daniel Golle, Felix Fietkau, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, 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, devicetree, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On 4/30/24 02:45, 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 (ie. only 3 and 4 can be configured, bit
> 0~2 are always 111). Practically all boards known as of today use the
> default setting which is to have the switch respond to address 31, while
> the built-in switch PHYs respond to address 0~4 in this case.
I'm not aware of the MT7531 switch having bootstrap pins to configure the
PHY address. Are you sure this is the case?
Arınç
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-04-29 23:45 [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-04-30 4:58 ` Arınç ÜNAL
@ 2024-04-30 13:21 ` Andrew Lunn
2024-04-30 20:08 ` Simon Horman
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2024-04-30 13:21 UTC (permalink / raw
To: Daniel Golle
Cc: Arınç ÜNAL, Felix Fietkau, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, DENG Qingfang, Sean Wang,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Landen Chao, devicetree, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Tue, Apr 30, 2024 at 12:45:46AM +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 (ie. only 3 and 4 can be configured, bit
> 0~2 are always 111). Practically all boards known as of today use the
> default setting which is to have the switch respond to address 31, while
> the built-in switch PHYs respond to address 0~4 in this case.
>
> However, even in MediaTek's SDK the address of the switch is wrongly
> stated in the device trees as 0 (while in reality it is 31), so warn the
> user about such broken device tree and make a good guess what was
> actually intended.
>
> This is imporant 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. Doing so instead of assuming the switch is always at
> address 31 which was previously hard-coded will obviously break things
> for many existing downstream device trees as they contain the wrong
> address (0) which previously didn't matter.
>
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> drivers/net/dsa/mt7530-mdio.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
> index fa3ee85a99c1..119630fd9060 100644
> --- a/drivers/net/dsa/mt7530-mdio.c
> +++ b/drivers/net/dsa/mt7530-mdio.c
> @@ -193,6 +193,19 @@ mt7530_probe(struct mdio_device *mdiodev)
> return PTR_ERR(priv->io_pwr);
> }
>
> + /* Only MDIO bus address 7, 15, 23 and 31 are valid options */
> + if (~(priv->mdiodev->addr & 0x7) & 0x7) {
> + /* If the address in DT must be wrong, make a good guess about
> + * the most likely intention, and issue a warning.
> + */
> + int correct_addr = ((((priv->mdiodev->addr - 7) & ~0x7) % 0x20) + 7) & 0x1f;
> +
> + dev_warn(&mdiodev->dev,
> + "impossible switch MDIO address in device tree: %d, assuming %d\n",
> + priv->mdiodev->addr, correct_addr);
You could include FW_WARN in this, to indicate this is a firmware
issue. It is not used too much with DT, since it was originally
intended for ACPI issues, but a few ARM systems use it with DT.
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning
2024-04-29 23:45 [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-04-30 4:58 ` Arınç ÜNAL
2024-04-30 13:21 ` Andrew Lunn
@ 2024-04-30 20:08 ` Simon Horman
2 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2024-04-30 20:08 UTC (permalink / raw
To: Daniel Golle
Cc: Arınç ÜNAL, Felix Fietkau, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, 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, devicetree, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
On Tue, Apr 30, 2024 at 12:45:46AM +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 (ie. only 3 and 4 can be configured, bit
> 0~2 are always 111). Practically all boards known as of today use the
> default setting which is to have the switch respond to address 31, while
> the built-in switch PHYs respond to address 0~4 in this case.
>
> However, even in MediaTek's SDK the address of the switch is wrongly
> stated in the device trees as 0 (while in reality it is 31), so warn the
> user about such broken device tree and make a good guess what was
> actually intended.
>
> This is imporant 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. Doing so instead of assuming the switch is always at
> address 31 which was previously hard-coded will obviously break things
> for many existing downstream device trees as they contain the wrong
> address (0) which previously didn't matter.
>
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The cited commit is present in net-next but not net.
So I think this patch should target net-next.
...
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-30 20:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-29 23:45 [PATCH net] net: dsa: mt7530: fix impossible MDIO address and issue warning Daniel Golle
2024-04-30 4:58 ` Arınç ÜNAL
2024-04-30 13:21 ` Andrew Lunn
2024-04-30 20:08 ` Simon Horman
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 read-only IMAP folder(s) and NNTP newsgroup(s).