Subject: Re: [PATCH v7 09/11] net: pch_gbe: Convert to mdiobus and phylib

From: Andrew Lunn <[email-masked]>
Date: Wed, 27 Jun 2018 19:51:44 +0200

> @@ -5,7 +5,8 @@
>  config PCH_GBE
>  	tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
>  	depends on PCI && (X86_32 || COMPILE_TEST)
> -	select MII
> +	select PHYLIB
> +	imply AT803X_PHY if X86_32
>  	select PTP_1588_CLOCK_PCH
>  	select NET_PTP_CLASSIFY

That is unusual. I don't think any other MAC driver does this.

If the AT803X driver is not available, it will fall back to the
generic PHY driver. That means RGMII delays will not get set
correctly, no interrupts, no wol, and no workaround for the 8030.

Are any of these relevant to your board?

> @@ -197,16 +151,14 @@ static void pch_gbe_get_regs(struct net_device *netdev,
>  	struct pch_gbe_hw *hw = &adapter->hw;
>  	struct pci_dev *pdev = adapter->pdev;
>  	u32 *regs_buff = p;
> -	u16 i, tmp;
> +	u16 i;
>  
>  	regs->version = 0x1000000 | (__u32)pdev->revision << 16 | pdev->device;
>  	for (i = 0; i < PCH_GBE_MAC_REGS_LEN; i++)
>  		*regs_buff++ = ioread32(&hw->reg->INT_ST + i);
>  	/* PHY register */
> -	for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++) {
> -		pch_gbe_phy_read_reg_miic(&adapter->hw, i, &tmp);
> -		*regs_buff++ = tmp;
> -	}
> +	for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++)
> +		*regs_buff++ = phy_read(adapter->phydev, i);
>  }

In general, that is not safe. Some PHYs have pages, and you have no
idea what page is currently selected. If you don't need it, i would
drop this. There are other ways to get access to phy registers, like
miitool, which should do a better job.

	 Andrew