On 6/22/24 21:12, Andrew Lunn wrote:
On Fri, Jun 21, 2024 at 01:26:33PM +0200, Kamil Horák (2N) wrote:
Implement single-pair BroadR-Reach modes on bcm5481x PHY by Broadcom.
Create set of functions alternative to IEEE 802.3 to handle
configuration of these modes on compatible Broadcom PHYs.
What i've not seen anywhere is a link between BroadR-Reach and LRE.
Maybe you could explain the relationship here in the commit message?
And maybe also how LDS fits in.
Tried to extend it a bit... LRE should be for "Long Reach Ethernet" but
Broadcom
only uses the acronym in the datasheets... LDS is "Long-Distance
Signaling", really screwed
term for a link auto-negotiation...
+int bcm_setup_master_slave(struct phy_device *phydev)
This is missing the lre in the name.
Fixed.
+static int bcm54811_read_abilities(struct phy_device *phydev)
+{
+ int i, val, err;
+ u8 brr_mode;
+
+ for (i = 0; i < ARRAY_SIZE(bcm54811_linkmodes); i++)
+ linkmode_clear_bit(bcm54811_linkmodes[i], phydev->supported);
I think that needs a comment since it is not clear what is going on
here. What set these bits in supported?
This is an equivalent of genphy_read_abilities for an IEEE PHY, that is,
it fills the phydev->supported bit array exactly
as genphy_read_abilities does. The genphy_read_abilities is even called
if the PHY is not in BRR mode.
+
+ err = bcm5481x_get_brrmode(phydev, &brr_mode);
+ if (err)
+ return err;
+
+ if (brr_mode) {
I would expect the DT property to be here somewhere. If the DT
property is present, set phydev->supported to only the BRR modes,
otherwise set it to the standard baseT modes. That should then allow
the core to do most of the validation. This is based on my
understanding the coupling hardware makes the two modes mutually
exclusive?
From my point of view relying on DT property only would imply to
validate the setting with what is read from the PHY on
all code locations where it is currently read by bcm5481x_get_brrmode.
This is because the PHY can be reset externally
(at least by power-cycling it) and after reset, it is in IEEE mode.
Thus, I chose to set the BRR mode on/off upon initialization
and then read the setting from the chip when necessary. The PHY can
also be reset by writing bit 15 to register 0
in both IEEE and BRR modes (LRECR/BMCR).
The device I am developing on has no option for IEEE interface but in
pure theory, kind of hardware plug-in would be
possible as I was told by our hardware team. However, not even the
evaluation kit for bcm54811 can be switched
between BRR and IEEE hardware without at least soldering and desoldering
some components on the PCB.
+ /* With BCM54811, BroadR-Reach implies no autoneg */
+ if (brr)
+ phydev->autoneg = 0;
So long as phydev->supported does not indicate autoneg, this should
not happen.
I also thought so but unfortunately our batch of bcm54811 indicates
possible autoneg in its status register
(LRESR_LDSABILITY) but refuses to negotiate. So this is rather a
preparation for bcm54810 full support. Unlike bcm54811,
the bcm54810 should be aneg-capable but I cannot verify it without the
hardware available. The information around
it is rather fuzzy, we were told by Broadcom tech support that the
54811 should autonegotiate as well but
the datasheets from the same source clearly indicates "no". Same for
the bcm54811 evaluation kit,
there is no autoneg option available (only 10/100Mbit and master/slave).
In conclusion, the idea was to support as much as possible but with
given hardware, only a subset could be verified
- that is bcm54811 10 or 100 Mbit on one pair and forced master /
slave selection. As for the other possibilities, I only
could test that the autoneg is probably not there or at least it does
not function with identical hardware on both ends.
We also have a BRR switch and some media converters (BRR/Ethernet) from
other manufacturer with bcm54811 to help
the development and all those are fixed setting only, no autoneg.
Andrew
Kamil
