On Tue, May 14, 2024 at 02:27:27PM +0200, Thomas Gessler wrote:
> The recent silicon revision 3 of the DP83869 has a different PHY ID
> which has to be added to the driver in order for the PHY to be detected.
> There appear to be no documented differences between the revisions,
> although there are some discussions in the TI forum about different
> behavior for some registers.
Do you have the datasheet? What does it say about the ID in registers
2 and 3? Often the lower nibble is the revision. Meaning there can be
16 revisions of a PHY.
> static struct phy_driver dp83869_driver[] = {
> - DP83869_PHY_DRIVER(DP83869_PHY_ID, "TI DP83869"),
> + DP83869_PHY_DRIVER(DP83869REV1_PHY_ID, "TI DP83869 Rev. 1"),
> + DP83869_PHY_DRIVER(DP83869REV3_PHY_ID, "TI DP83869 Rev. 3"),
> DP83869_PHY_DRIVER(DP83561_PHY_ID, "TI DP83561-SP"),
If you look at DP83869_PHY_DRIVER() it uses:
#define DP83869_PHY_DRIVER(_id, _name) \
{ \
PHY_ID_MATCH_MODEL(_id), \
As the name suggests, it matches on the model. The revision is
ignored. A mask is applied to ignore the lower nibble. So this change
looks pointless.
> static struct mdio_device_id __maybe_unused dp83869_tbl[] = {
> - { PHY_ID_MATCH_MODEL(DP83869_PHY_ID) },
> + { PHY_ID_MATCH_MODEL(DP83869REV1_PHY_ID) },
> + { PHY_ID_MATCH_MODEL(DP83869REV3_PHY_ID) },
> { PHY_ID_MATCH_MODEL(DP83561_PHY_ID) },
And this has the same issue. If you want the match to include the
revision, you need to use PHY_ID_MATCH_EXACT(). If the different
revisions are supposed to be compatible, a single PHY_ID_MATCH_MODEL()
is sufficient.
Andrew
---
pw-bot: cr
Hi Andrew,
On Tue, 14 May 2024, Andrew Lunn wrote:
> As the name suggests, it matches on the model. The revision is
> ignored. A mask is applied to ignore the lower nibble. So this change
> looks pointless.
Ah, I see. I did not realize that the match ignores the lower bits. I was
having trouble getting the driver to match when first experimenting with
the chip and thought this was part of the problem. As it turns out, it now
works without this patch.
Please disregard it.
Thomas