Hi Claudiu,
First of all, thanks Biju for checking the code and bringing up
this topic.
On Wed, Jun 26, 2024 at 09:30:52AM GMT, claudiu beznea wrote:
> >> On 25.06.2024 18:53, Biju Das wrote:
...
> >>>> static inline void riic_writeb(struct riic_dev *riic, u8 val, u8 offset) {
> >>>> writeb(val, riic->base + riic->info->regs[offset]); @@ -133,10
> >>>> +135,14 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> >>>> struct riic_dev *riic = i2c_get_adapdata(adap);
> >>>> struct device *dev = adap->dev.parent;
> >>>> unsigned long time_left;
> >>>> - int i;
> >>>> + int i, ret;
> >>>> u8 start_bit;
> >>>>
> >>>> - pm_runtime_get_sync(dev);
> >>>> + ret = pm_runtime_resume_and_get(dev);
> >>>> + if (ret) {
> >>>> + dev_err(dev, riic_rpm_err_msg);
> >>>
> >>> As at the moment we don't know how to reproduce this error condition
> >>> Can we use WARN_ON_ONCE() instead to catch detailed error condition here??
> >>
> >> [1] states "So, naturally, use of WARN_ON() is also now discouraged much of the time". I've go with
> >> dev_err() or something similar.
> >
> > WARN_ON_ONCE() should be ok I guess as people are using for printing this info only once??
>
> Ok, I'm leaving this to I2C maintainers.
>
> Andi, Wolfram,
>
> Would you prefer having WARN_ON_ONCE() instead of dev_err() for potential
> failures of pm_runtime_resume_and_get()?
I prefer dev_err. WARN_ON should be used for some serious
failures in the code.
E.g. memory corruption, like:
a = 5;
WARN_ON(a != 5);
but the system might still work even with such errors (otherwise
there is BUG_ON()).
Besides, WARN_ON() prints some information that are not really
useful to understand why the system didn't resume. For example
you don't need the stack trace for power management failures, but
you need it for code tracing code bugs.
Andi
