Re: [PATCH net v4 4/4] net: phy: dp83640: fix per-bus clock lifetime

> -static int dp83640_probe(struct phy_device *phydev)
> +static void dp83640_phy_release(void *data)
>  {

...

> +	mutex_lock(&clock->clock_lock);
> +	if (dp83640 == clock->chosen) {
> +		ptp_clock_unregister(clock->ptp_clock);
> +		clock->ptp_clock = NULL;
> +		clock->chosen = NULL;
> +	} else {

Probe has:

> +	/* Ensure other PHY probes wait for shared clock initialization. */
> +	phy_package_lock(phydev);
> +	if (phy_package_probe_once(phydev))
> +		dp83640_clock_init(clock);
> +	phy_package_unlock(phydev);

It seems like a phy_package_release_once(phydev) would help keep probe
and release being symmetric. The problem is getting the semantics
correct. phy_package_probe_once() will be true for the first PHY
probed. You want phy_package_release_once() to be true when the last
PHY is removed from the package. It probably needs to look at
phydev->shared->refcnt. However that probably also requires using
phy_package_join() not devm_phy_package_join().

I then _think_ all the list manipulation can go away, and the driver
will look cleaner.

    Andrew

---
pw-bot: cr