Re: [PATCH v2 3/3] phy: core: Warn when phy_power_on is called before phy_init

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Jules,

On 23.03.22 12:07, Jules Maselbas wrote:
> A warning when the order of phy operation is mixed up by drivers,
> this is an atempt to make the phy usage more uniform across (usb)
> drivers.

Thanks for picking up this suggestion.

> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com>
> Cc: Minas Harutyunyan <hminas@synopsys.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> change in v2:
>   - no changes
> 
>  drivers/phy/phy-core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index cbdad65d2caa..0cb4da62577e 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -252,6 +252,9 @@ int phy_init(struct phy *phy)
>  		return ret;
>  	ret = 0; /* Override possible ret == -ENOTSUPP */
>  
> +	if (phy->power_count > phy->init_count)

This needs to be moved into the critical section below.

> +		dev_warn(&phy->dev, "phy_power_on was called before phy_init\n");

I am wondering how often would this be triggered for e.g. a PHY that's being
runtime suspended. But the warning being obnoxious is the point of the patch,
so perhaps it's ok to not make it into a dev_warn_once. 

> +
>  	mutex_lock(&phy->mutex);
>  	if (phy->init_count == 0 && phy->ops->init) {
>  		ret = phy->ops->init(phy);


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |