Re: [PATCH net] net: phy: mdio_device: leave PHY reset deasserted on unregister

[Leonardo Mörlein]

> I'm wondering if we are fixing the wrong thing here.

In general, I see two options to fix this. Either we fix the
tear down or we fix the setup of the device.

Let's look at the current state of the setup code:

Assuming, the reset is asserted when the PHY setup takes place,
it fails because of_mdiobus_phy_device_register() fails with
ENODEV.

This happens the following way:
- of_mdiobus_phy_device_register() -> ENODEV
  - fwnode_mdiobus_register_phy() -> ENODEV
    - get_phy_device() -> ENODEV
      - Asks for the device on the bus.
      - (The phy could only answer if reset is deasserted here, 
        but get_phy_device() currently does not ensure this.)
      - Returns ENODEV since there is no PHY present on the bus.
     - —— early return here: ENODEV ——
     - If the early return wouldn’t happen, the code would continue
       calling phy_device_register().
     - phy_device_register():
       - Handling the reset gpio would start here.
       - mdiobus_register_device()
         - Asserts reset.
       - Deasserts reset.
       - Things would go their proper way...

Currently, it seems that this case only occurs when the kernel
leaves the reset asserted. The bootloader initially leaves the
reset deasserted, at least for my current hardware.

Now, we have two options:
a) Deassert the reset in get_phy_device() (or somewhere around it)
   before asking for the device on the bus.
b) Deassert the reset in the tear down of the device.

My current patch implements option b). 

> Rather than take it out of reset, should it not be put into reset? 

If we go for option a), we could do this. But then, I do not see why
we would care at all how we leave the reset?

> When the GPIO is unregistered, are we leaving it as an output?

My current understanding is yes, since mdio_device_unregister_reset()
only calls gpiod_put().

Cheers,
Leo