| Message ID | 20220323110708.8254-4-jmaselbas@kalray.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Cleanup the call ordering of phy_init and phy_power_on | expand |
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);
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); > >
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) + dev_warn(&phy->dev, "phy_power_on was called before phy_init\n"); + mutex_lock(&phy->mutex); if (phy->init_count == 0 && phy->ops->init) { ret = phy->ops->init(phy);
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. 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(+)