lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4.4] net: phy: Add driver for Motorcomm yt8521 gigabit ethernet phy
On Tue, 23 Aug 2022 16:14:16 +0200 Andrew Lunn wrote:
> On Mon, Aug 22, 2022 at 08:21:47PM -0700, Jakub Kicinski wrote:
> > On Wed, 17 Aug 2022 19:25:54 +0800 Frank wrote:
> > > +static int yt8521_fiber_config_aneg(struct phy_device *phydev)
> > > +{
> > > + int err, changed;
> > > + u16 adv;
> > > +
> > > + if (phydev->autoneg != AUTONEG_ENABLE)
> > > + return yt8521_fiber_setup_forced(phydev);
> > > +
> > > + err = ytphy_modify_ext_with_lock(phydev, YTPHY_MISC_CONFIG_REG,
> > > + YTPHY_MCR_FIBER_SPEED_MASK,
> > > + YTPHY_MCR_FIBER_1000BX);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + /* enable Fiber auto sensing */
> > > + err = ytphy_modify_ext_with_lock(phydev, YT8521_LINK_TIMER_CFG2_REG,
> > > + 0, YT8521_LTCR_EN_AUTOSEN);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + /* Setup fiber advertisement */
> > > + adv = ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
> > > + ADVERTISE_1000XPSE_ASYM;
> >
> > Is it okay to ignore phydev->advertising and always set the same mask?
>
> The user could of changed the pause settings, which are going to be
> ignored here. Also, you should not assume the MAC can actually do
> asymmetric pause, not all can. phydev->advertising will be set to only
> include what the MAC can actually do.
>
> The whole concept of having two line sides connected to one MAC and
> seeing which gets link first is unsupported in Linux. In theory, you
> want to be able to configure each line side differently. Maybe you
> want autoneg on copper, but fixed on fibre, asymmetric pause with
> fibre, but symmetric pause on copper, etc. Since there is only one
> instance of phydev here, you don't have anywhere to store two sets of
> configuration, nor any sort of kAPI to deal with two phydev structures
> etc. So the user experience is not so great.
>
> With the Marvell Switches which also have this capability, i actually
> ignore it, use the phy-mode it decide which should be used, either
> copper or fibre, and leave the other powered off so it can never get
> link. There is at least one Marvell PHY which does however support
> first up wins, so this behaviour is not new. I just don't recommend
> it.
>
> And it gets even more interesting when the SFP is actually copper. But
> since the integration with phylink is missing in this driver, that is
> not supported here.

Interesting. Just to confirm - regardless of the two-sided design..
-edness.. IIUC my question has merit and we need v5?

\
 
 \ /
  Last update: 2022-08-23 21:41    [W:0.066 / U:0.860 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site