Messages in this thread | | | Date | Sun, 31 Jul 2022 16:30:42 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH v4] net: phy: Add driver for Motorcomm yt8521 gigabit ethernet |
| |
On Wed, Jul 27, 2022 at 03:08:27PM +0800, Frank wrote: > patch v4: > Hi Andrew,Jakub > Thanks very much and based on your comments we modified the patch v4 as below. > > We evaluated the Marvell 10G driver and at803x you suggested. The 2 drivers > implement SFP module attach/detach functions and we think these functions do > not help yt8521 to do UTP/Fiber register space arbitration which you may > concern in previous patch. > > Yt8521 can detect utp/fiber media link status automatically. For the case of > both media are connected, driver arbitrates the priority of the media (by > default, driver takes fiber as higher priority) and report the media status > to up layer(MAC). > > patch v3: > Hi Andrew > Thanks and based on your comments we modified the patch as below. > > > It is generally not that simple. Fibre, you probably want 1000BaseX, > > unless the fibre module is actually copper, and then you want > > SGMII. So you need something to talk to the fibre module and ask it > > what it is. That something is phylink. Phylink does not support both > > copper and fibre at the same time for one MAC. > > yes, you said it and for MAC, it does not support copper and Fiber at same time. > but from Physical Layer, you know, sometimes both Copper and Fiber cables are > connected. in this case, Phy driver should do arbitration and report to MAC > which media should be used and Link-up. > This is the reason that the driver has a "polling mode", and by default, also > this driver takes fiber as first priority which matches phy chip default behavior. > > > patch v2: > Hi Andrew, Russell King, Peter, > Thanks and based on your comments we modified the patch as below. > > > So there's only two possible pages that can be used in the extended > >register space? > > Yes,there is only two register space (utp and fiber). > > > > +/* Extended Register's Data Register */ > > > +#define YTPHY_PAGE_DATA 0x1F > > > > These are defined exactly the same way as below. Please reuse code > > where possible. > > Yes, code will be reuse, but "YT8511_PAGE" need to be rename like > "YTPHY_PAGE_DATA",as it is common register for yt phys. > > > patch v1: > Add a driver for the motorcomm yt8521 gigabit ethernet phy. We have verified > the driver on StarFive VisionFive development board, which is developed by > Shanghai StarFive Technology Co., Ltd.. On the board, yt8521 gigabit ethernet > phy works in utp mode, RGMII interface, supports 1000M/100M/10M speeds, and > wol(magic package). > > Signed-off-by: Frank <Frank.Sae@motor-comm.com> > --- > MAINTAINERS | 1 + > drivers/net/phy/Kconfig | 2 +- > drivers/net/phy/motorcomm.c | 1170 ++++++++++++++++++++++++++++++++++- > 3 files changed, 1170 insertions(+), 3 deletions(-)
This is not the correct way to format the commit message. All the text you have above will end up in the commit. Please put all discussion after the --- .
> + /* If it is reset, need to wait for the reset to complete */ > + if (set == BMCR_RESET) { > + while (max_cnt--) { > + /* unlock mdio bus during sleep */ > + phy_unlock_mdio_bus(phydev); > + usleep_range(1000, 1100); > + phy_lock_mdio_bus(phydev); > + > + ret = __phy_read(phydev, MII_BMCR); > + if (ret < 0) > + goto err_restore_page; > + > + if (!(ret & BMCR_RESET)) > + return phy_restore_page(phydev, old_page, 0); > + } > + if (max_cnt <= 0) > + ret = -ETIME;
ETIMEDOUT.
Andrew
| |