[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YuaSEpglXWbxQbAy@lunn.ch>
Date: Sun, 31 Jul 2022 16:30:42 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Frank <Frank.Sae@...or-comm.com>
Cc: Peter Geis <pgwipeout@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, yinghong.zhang@...or-comm.com,
fei.zhang@...or-comm.com, hua.sun@...or-comm.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
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@...or-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
Powered by blists - more mailing lists