[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6b50b9996faffe9fbddb54ed6a407b037a8509ff.camel@redhat.com>
Date: Thu, 27 Oct 2022 11:23:09 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Frank <Frank.Sae@...or-comm.com>, Peter Geis <pgwipeout@...il.com>,
Andrew Lunn <andrew@...n.ch>,
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>
Cc: 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 net-next v8.1] net: phy: Add driver for Motorcomm yt8521
gigabit ethernet phy
Hello,
On Tue, 2022-10-25 at 14:45 +0800, Frank wrote:
[...]
> +/**
> + * yt8521_read_status_paged() - determines the speed and duplex of one page
> + * @phydev: a pointer to a &struct phy_device
> + * @page: The reg page(YT8521_RSSR_FIBER_SPACE/YT8521_RSSR_UTP_SPACE) to
> + * operate.
> + *
> + * returns 1 (utp or fiber link),0 (no link) or negative errno code
> + */
> +static int yt8521_read_status_paged(struct phy_device *phydev, int page)
> +{
> + int fiber_latch_val;
> + int fiber_curr_val;
> + int old_page;
> + int ret = 0;
> + int status;
> + int link;
> +
> + linkmode_zero(phydev->lp_advertising);
> + phydev->duplex = DUPLEX_UNKNOWN;
> + phydev->speed = SPEED_UNKNOWN;
> + phydev->asym_pause = 0;
> + phydev->pause = 0;
> +
> + /* YT8521 has two reg space (utp/fiber) for linkup with utp/fiber
> + * respectively. but for utp/fiber combo mode, reg space should be
> + * arbitrated based on media priority. by default, utp takes
> + * priority. reg space should be properly set before read
> + * YTPHY_SPECIFIC_STATUS_REG.
> + */
> +
> + page &= YT8521_RSSR_SPACE_MASK;
> + old_page = phy_select_page(phydev, page);
> + if (old_page < 0)
> + goto err_restore_page;
> +
> + /* Read YTPHY_SPECIFIC_STATUS_REG, which indicates the speed and duplex
> + * of the PHY is actually using.
> + */
> + ret = __phy_read(phydev, YTPHY_SPECIFIC_STATUS_REG);
> + if (ret < 0)
> + goto err_restore_page;
> +
> + status = ret;
> + link = !!(status & YTPHY_SSR_LINK);
> +
> + /* When PHY is in fiber mode, speed transferred from 1000Mbps to
> + * 100Mbps,there is not link down from YTPHY_SPECIFIC_STATUS_REG, so
> + * we need check MII_BMSR to identify such case.
> + */
> + if (page == YT8521_RSSR_FIBER_SPACE) {
> + ret = __phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + goto err_restore_page;
> +
> + fiber_latch_val = ret;
> + ret = __phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + goto err_restore_page;
> +
> + fiber_curr_val = ret;
> + if (link && fiber_latch_val != fiber_curr_val) {
> + link = 0;
> + phydev_info(phydev,
> + "%s, fiber link down detect, latch = %04x, curr = %04x\n",
> + __func__, fiber_latch_val, fiber_curr_val);
> + }
> + } else {
> + /* Read autonegotiation status */
> + ret = __phy_read(phydev, MII_BMSR);
> + if (ret < 0)
> + return ret;
You need to restore the page even on this error path.
[...]
> +/**
> + * yt8521_modify_bmcr_paged - bits modify a PHY's BMCR register of one page
> + * @phydev: the phy_device struct
> + * @page: The reg page(YT8521_RSSR_FIBER_SPACE/YT8521_RSSR_UTP_SPACE) to operate
> + * @mask: bit mask of bits to clear
> + * @set: bit mask of bits to set
> + *
> + * NOTE: Convenience function which allows a PHY's BMCR register to be
> + * modified as new register value = (old register value & ~mask) | set.
> + * YT8521 has two space (utp/fiber) and three mode (utp/fiber/poll), each space
> + * has MII_BMCR. poll mode combines utp and faber,so need do both.
> + * If it is reset, it will wait for completion.
> + *
> + * returns 0 or negative errno code
> + */
> +static int yt8521_modify_bmcr_paged(struct phy_device *phydev, int page,
> + u16 mask, u16 set)
> +{
> + int max_cnt = 500; /* the max wait time of reset ~ 500 ms */
> + int old_page;
> + int ret = 0;
> +
> + old_page = phy_select_page(phydev, page & YT8521_RSSR_SPACE_MASK);
> + if (old_page < 0)
> + goto err_restore_page;
> +
> + ret = __phy_modify(phydev, MII_BMCR, mask, set);
> + if (ret < 0)
> + goto err_restore_page;
> +
> + /* If it is reset, need to wait for the reset to complete */
> + if (set == BMCR_RESET) {
> + while (max_cnt--) {
> + usleep_range(1000, 1100);
> + 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)
The above check is not needed, the loop can terminate only when such
condition is true.
Cheers,
Paolo
Powered by blists - more mailing lists