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