On Mon, Nov 26, 2018 at 2:50 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > On 11/25/2018 05:55 PM, Sakari Ailus wrote: > > Hi Matt, > > > > On Sat, Nov 24, 2018 at 02:03:23PM -0800, Matt Ranostay wrote: > >> Not all future supported video chips will always have power management > >> support, and so it is important to check before calling set_power() is > >> defined. > >> > >> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > >> Cc: Hans Verkuil <hansverk@xxxxxxxxx> > >> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> > >> Cc: Akinobu Mita <akinobu.mita@xxxxxxxxx> > >> Signed-off-by: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> > >> --- > >> > >> Changes from v2: > >> - split out from mlx90640 patch series > >> - added to Cc list > >> > >> Changes from v1: > >> - none > >> > >> drivers/media/i2c/video-i2c.c | 22 +++++++++++++++++----- > >> 1 file changed, 17 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c > >> index b6ebb8d53e90..01dcf179f203 100644 > >> --- a/drivers/media/i2c/video-i2c.c > >> +++ b/drivers/media/i2c/video-i2c.c > >> @@ -736,9 +736,11 @@ static int video_i2c_probe(struct i2c_client *client, > >> video_set_drvdata(&data->vdev, data); > >> i2c_set_clientdata(client, data); > >> > >> - ret = data->chip->set_power(data, true); > >> - if (ret) > >> - goto error_unregister_device; > >> + if (data->chip->set_power) { > >> + ret = data->chip->set_power(data, true); > >> + if (ret) > >> + goto error_unregister_device; > >> + } > > > > How about adding a macro to call the op if it's set? It could be used to > > call other ops when they're set, and ignore them when they're not. Just an > > idea. See e.g. v4l2_subdev_call() in include/media/v4l2-subdev.h . > > Matt, is this something you want to do? If so, then I'll wait for a v4. > Probably wouldn't get around to doing it for a bit (on travel for the next few weeks). If you could review/accept v3 for now that would be great. - Matt > Regards, > > Hans > > > > >> > >> pm_runtime_get_noresume(&client->dev); > >> pm_runtime_set_active(&client->dev); > >> @@ -767,7 +769,9 @@ static int video_i2c_probe(struct i2c_client *client, > >> pm_runtime_disable(&client->dev); > >> pm_runtime_set_suspended(&client->dev); > >> pm_runtime_put_noidle(&client->dev); > >> - data->chip->set_power(data, false); > >> + > >> + if (data->chip->set_power) > >> + data->chip->set_power(data, false); > >> > >> error_unregister_device: > >> v4l2_device_unregister(v4l2_dev); > >> @@ -791,7 +795,9 @@ static int video_i2c_remove(struct i2c_client *client) > >> pm_runtime_disable(&client->dev); > >> pm_runtime_set_suspended(&client->dev); > >> pm_runtime_put_noidle(&client->dev); > >> - data->chip->set_power(data, false); > >> + > >> + if (data->chip->set_power) > >> + data->chip->set_power(data, false); > >> > >> video_unregister_device(&data->vdev); > >> > >> @@ -804,6 +810,9 @@ static int video_i2c_pm_runtime_suspend(struct device *dev) > >> { > >> struct video_i2c_data *data = i2c_get_clientdata(to_i2c_client(dev)); > >> > >> + if (!data->chip->set_power) > >> + return 0; > >> + > >> return data->chip->set_power(data, false); > >> } > >> > >> @@ -811,6 +820,9 @@ static int video_i2c_pm_runtime_resume(struct device *dev) > >> { > >> struct video_i2c_data *data = i2c_get_clientdata(to_i2c_client(dev)); > >> > >> + if (!data->chip->set_power) > >> + return 0; > >> + > >> return data->chip->set_power(data, true); > >> } > >> > >> -- > >> 2.17.1 > >> > > >