Re: [PATCH 06/12] net: phy: add phy_device_atomic_register helper

[Andrew Lunn]

> Currently we have one API which creates/allocates the 'struct
> phy_device' and intialize the state which is:
>    - phy_device_create()
> 
> This function requests a driver based on the phy_id/c45_ids. The ID have
> to come from somewhere if autodection is used. For autodetection case
>    - get_phy_device()
> 
> is called. This function try to access the phy without taken possible
> hardware dependencies into account. These dependecies can be reset-lines
> (in my case), clocks, supplies, ...
> 
> For taking fwnode (and possible dependencies) into account fwnode_mdio.c
> was written which provides two helpers:
>    - fwnode_mdiobus_register_phy()
>    - fwnode_mdiobus_phy_device_register().
> 
> The of_mdio.c and of_mdiobus_register_phy() is just a wrapper around
> fwnode_mdiobus_register_phy().

It seems to me that the real problem is that mdio_device_reset() takes
an mdio_device. mdiobus_register_gpiod() and mdiobus_register_reset()
also take an mdio_device. These are the functions you want to call
before calling of_mdiobus_register_phy() in __of_mdiobus_register() to
ensure the PHY is out of reset. But you don't have an mdio_device yet.

So i think a better solution is to refactor this code. Move the
resources into a structure of their own, and make that a member of
mdio_device. You can create a stack version of this resource structure
in __of_mdiobus_register(), parse DT to fill it out by calling
mdiobus_register_gpiod() and mdiobus_register_reset() taking this new
structure, take it out of reset by calling mdio_device_reset(), and
then call of_mdiobus_register_phy(). If a PHY is found, copy the
values in the resulting mdio_device. If not, release the resources.

Doing it like this means there is no API change.

      Andrew