From: Florian Fainelli <f.fainelli@gmail•com>
To: Vladimir Oltean <vladimir.oltean@nxp•com>, netdev@vger•kernel.org
Cc: Andrew Lunn <andrew@lunn•ch>,
"David S. Miller" <davem@davemloft•net>,
Eric Dumazet <edumazet@google•com>,
Jakub Kicinski <kuba@kernel•org>, Paolo Abeni <pabeni@redhat•com>,
Woojung Huh <woojung.huh@microchip•com>,
UNGLinuxDriver@microchip•com,
Arun Ramadoss <arun.ramadoss@microchip•com>,
Russell King <linux@armlinux•org.uk>, Marek Vasut <marex@denx•de>,
linux-kernel@vger•kernel.org
Subject: Re: [PATCH v2 net] net: dsa: microchip: fix RGMII delay configuration on KSZ8765/KSZ8794/KSZ8795
Date: Wed, 15 Mar 2023 16:23:57 -0700 [thread overview]
Message-ID: <e18a490e-02cd-ae2a-37ac-e6731e149aa3@gmail.com> (raw)
In-Reply-To: <20230315231916.2998480-1-vladimir.oltean@nxp.com>
On 3/15/23 16:19, Vladimir Oltean wrote:
> From: Marek Vasut <marex@denx•de>
>
> 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: c476bede4b0f ("net: dsa: microchip: ksz8795: use common xmii function")
> Signed-off-by: Marek Vasut <marex@denx•de>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp•com>
> Acked-by: Arun Ramadoss <arun.ramadoss@microchip•com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail•com>
--
Florian
next prev parent reply other threads:[~2023-03-15 23:24 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-15 23:19 [PATCH v2 net] net: dsa: microchip: fix RGMII delay configuration on KSZ8765/KSZ8794/KSZ8795 Vladimir Oltean
2023-03-15 23:23 ` Florian Fainelli [this message]
2023-03-17 4:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e18a490e-02cd-ae2a-37ac-e6731e149aa3@gmail.com \
--to=f.fainelli@gmail$(echo .)com \
--cc=UNGLinuxDriver@microchip$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=arun.ramadoss@microchip$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=marex@denx$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=vladimir.oltean@nxp$(echo .)com \
--cc=woojung.huh@microchip$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox