Hello Russell,
> From: Russell King, Sent: Tuesday, January 3, 2023 6:54 PM
>
> On Mon, Dec 26, 2022 at 04:14:23PM +0900, Yoshihiro Shimoda wrote:
> > Set phy_dev->host_interfaces by pl->link_interface in
> > phylink_fwnode_phy_connect() for a non-sfp PHY.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > ---
> > drivers/net/phy/phylink.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 09cc65c0da93..1958d6cc9ef9 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -1809,6 +1809,7 @@ int phylink_fwnode_phy_connect(struct phylink *pl,
> > pl->link_interface = phy_dev->interface;
> > pl->link_config.interface = pl->link_interface;
> > }
> > + __set_bit(pl->link_interface, phy_dev->host_interfaces);
>
> This is probably going to break Macchiatobin platforms, since we
> declare that the link mode there is 10GBASE-R, we'll end up with
> host_interfaces containing just this mode. This will cause the
> 88x3310 driver to select a rate matching interface mode, which the
> mvpp2 MAC can't support.
>
> If we want to fill host_interfaces in, then it needs to be filled in
> properly - and by that I mean with all the host interface modes that
> can be electrically supported - otherwise platforms will break.
>
> So, sorry, but NAK on this change.
Thank you for your review! I understood why NAK on this change:
---
static int mv3310_select_mactype(unsigned long *interfaces)
{
...
else if (test_bit(PHY_INTERFACE_MODE_SGMII, interfaces) &&
test_bit(PHY_INTERFACE_MODE_10GBASER, interfaces))
return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER;
...
else if (test_bit(PHY_INTERFACE_MODE_10GBASER, interfaces))
return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH;
...
---
- On this change, since the interfaces is set to PHY_INTERFACE_MODE_10GBASER only,
this function will return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH.
- Without this change, since the host_interfaces value is zero, the mv3310_select_mactype()
will not called.
I'll investigate phylink and marvell10 codes again.
Best regards,
Yoshihiro Shimoda
