Re: [PATCH 2/3] net: hisilicon: new hip04 MDIO driver

2014-03-20 3:53 GMT-07:00 Zhangfei Gao [off-list ref]:

Dear Florian

On Wed, Mar 19, 2014 at 1:28 AM, Florian Fainelli [off-list ref] wrote:

quoted

2014-03-18 1:40 GMT-07:00 Zhangfei Gao [off-list ref]:

quoted

quoted

+static int hip04_mdio_reset(struct mii_bus *bus)
+{
+       int temp, err, i;
+
+       for (i = 0; i < 2; i++) {
+               hip04_mdio_write(bus, i, 22, 0);
+               temp = hip04_mdio_read(bus, i, MII_BMCR);
+               temp |= BMCR_RESET;
+               err = hip04_mdio_write(bus, i, MII_BMCR, temp);
+               if (err < 0)
+                       return err;
+       }
+
+       mdelay(500);

This does not look correct, you should iterate over all possible PHYs:
PHY_MAX_ADDR instead of hardcoding the loop to 2.

OK, got it.
Use 2 is since only have 2 phy in the board, will use PHY_MAX_ADDR instead.

quoted

I think we might want to remove the mdio bus reset callback in general
as the PHY library should already take care of software resetting the
PHY to put it in a sane state, as well as waiting for the appropriate
delay before using, unlike here, where you do not poll for BMCR_RESET
to be cleared by the PHY.

Do you mean will move BMCR_RESET to common code, that's would be great.
The mdio_reset is added here to get phy_id, otherwise the phy_id can
not be read as well as detection.

Oh I see, thanks for mentioning that, this is not properly covered
here today as we need to get the PHY id before we assign it a
phy_device structure, let me cook a patch for this.
-- 
Florian