On Tuesday, November 22, 2016 8:31:42 PM CET Laurent Pinchart wrote:
> > > @@ -2915,7 +2906,11 @@ static int smiapp_probe(struct i2c_client *client,
> > >
> > > pm_runtime_enable(&client->dev);
> > >
> > > +#ifdef CONFIG_PM
> > > rval = pm_runtime_get_sync(&client->dev);
> > > +#else
> > > + rval = smiapp_power_on(&client->dev);
> > > +#endif
> > >
> > > if (rval < 0) {
> > > rval = -ENODEV;
> > > goto out_power_off;
> >
> > I would suggest writing this as
> >
> > if (IS_ENABLED(CONFIG_PM))
> > rval = pm_runtime_get_sync(&client->dev);
> > else
> > rval = smiapp_power_on(&client->dev);
> >
> > though that is a purely cosmetic change.
>
> Are all drivers really supposed to code this kind of construct ? Shouldn't
> this be handled in the PM core ? A very naive approach would be to call
> .runtime_resume() and .runtime_suspend() from the non-CONFIG_PM versions of
> pm_runtime_enable() and pm_runtime_disable() respectively. I assume that would
> break things, but can't we implement something similar to that that wouldn't
> require all drivers to open-code it ?
I know nothing about the details of how the suspend/resume code should
do this, I was just commenting on the syntax above, preferring an
IS_ENABLED() check over an #ifdef.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
