Commit 5ae06327 authored by Marek Vasut's avatar Marek Vasut Committed by Jakub Kicinski

net: dsa: microchip: fix RGMII delay configuration on KSZ8765/KSZ8794/KSZ8795

The blamed commit has replaced a ksz_write8() call to address
REG_PORT_5_CTRL_6 (0x56) with a ksz_set_xmii() -> ksz_pwrite8() call to
regs[P_XMII_CTRL_1], which is also defined as 0x56 for ksz8795_regs[].

The trouble is that, when compared to ksz_write8(), ksz_pwrite8() also
adjusts the register offset with the port base address. So in reality,
ksz_pwrite8(offset=0x56) accesses register 0x56 + 0x50 = 0xa6, which in
this switch appears to be unmapped, and the RGMII delay configuration on
the CPU port does nothing.

So if the switch wasn't fine with the RGMII delay configuration done
through pin strapping and relied on Linux to apply a different one in
order to pass traffic, this is now broken.

Using the offset translation logic imposed by ksz_pwrite8(), the correct
value for regs[P_XMII_CTRL_1] should have been 0x6 on ksz8795_regs[], in
order to really end up accessing register 0x56.

Static code analysis shows that, despite there being multiple other
accesses to regs[P_XMII_CTRL_1] in this driver, the only code path that
is applicable to ksz8795_regs[] and ksz8_dev_ops is ksz_set_xmii().
Therefore, the problem is isolated to RGMII delays.

In its current form, ksz8795_regs[] contains the same value for
P_XMII_CTRL_0 and for P_XMII_CTRL_1, and this raises valid suspicions
that writes made by the driver to regs[P_XMII_CTRL_0] might overwrite
writes made to regs[P_XMII_CTRL_1] or vice versa.

Again, static analysis shows that the only accesses to P_XMII_CTRL_0
from the driver are made from code paths which are not reachable with
ksz8_dev_ops. So the accesses made by ksz_set_xmii() are safe for this
switch family.

[ vladimiroltean: rewrote commit message ]

Fixes: c476bede ("net: dsa: microchip: ksz8795: use common xmii function")
Signed-off-by: default avatarMarek Vasut <marex@denx.de>
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: default avatarArun Ramadoss <arun.ramadoss@microchip.com>
Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20230315231916.2998480-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 816b1e8c
...@@ -319,7 +319,7 @@ static const u16 ksz8795_regs[] = { ...@@ -319,7 +319,7 @@ static const u16 ksz8795_regs[] = {
[S_BROADCAST_CTRL] = 0x06, [S_BROADCAST_CTRL] = 0x06,
[S_MULTICAST_CTRL] = 0x04, [S_MULTICAST_CTRL] = 0x04,
[P_XMII_CTRL_0] = 0x06, [P_XMII_CTRL_0] = 0x06,
[P_XMII_CTRL_1] = 0x56, [P_XMII_CTRL_1] = 0x06,
}; };
static const u32 ksz8795_masks[] = { static const u32 ksz8795_masks[] = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment