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

[Jules Maselbas <jmaselbas@kalray.eu>]

Hi Ahmad,

On Wed, Mar 23, 2022 at 12:13:42PM +0100, Ahmad Fatoum wrote:
> 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.
yes, you're right, I'll send a v3 later, giving some time for other
people to comment.

> 
> > +		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. 
I don't really know how often this will be printed, this is an open point.

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