Re: [PATCH net v3] net: phy: dp83640: fix per-bus clock lifetime

[Andrew Lunn <andrew@lunn.ch>]

> +static void dp83640_clock_init(struct dp83640_clock *clock)
>  {
> -	INIT_LIST_HEAD(&clock->list);
> -	clock->bus = bus;
>  	mutex_init(&clock->extreg_lock);
>  	mutex_init(&clock->clock_lock);
>  	INIT_LIST_HEAD(&clock->phylist);
> +	clock->caps.pin_config = clock->pin_config;

Can this patch be split up a bit? It is not clear to my why this is
needed. If you have lots of small patches, each with good commit
messages, you can explain "Why?" this is needed.

>  	clock->caps.owner = THIS_MODULE;
>  	sprintf(clock->caps.name, "dp83640 timer");
>  	clock->caps.max_adj	= 1953124;
> @@ -977,14 +971,8 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
>  	clock->caps.settime64	= ptp_dp83640_settime;
>  	clock->caps.enable	= ptp_dp83640_enable;
>  	clock->caps.verify	= ptp_dp83640_verify;
> -	/*
> -	 * Convert the module param defaults into a dynamic pin configuration.
> -	 */
> +	/* Convert the module param defaults into the pin configuration. */
>  	dp83640_gpio_defaults(clock->caps.pin_config);

Why is this no longer dynamic?

> +	clock = phy_package_get_priv(phydev);
> +	/* Ensure other PHY probes wait for shared clock initialization. */
> +	mutex_lock(&phydev->mdio.bus->shared_lock);

Please add phy_package_lock() and phy_package_unlock().


    Andrew

---
pw-bot: cr