Re: [PATCH v2 11/11] media: i2c: Add WV517S lens actuator driver

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Aug 27, 2026 at 08:17:56PM +0200, Maurizio Casciano wrote:
> The Lenovo Yoga Book YB1-X91 rear camera contains a WV517S voice-coil
> actuator. Add a V4L2 lens subdevice exposing the standard 10-bit
> FOCUS_ABSOLUTE control and the device ringing-control mode.
> 
> Use regmap for register access and tie control updates to runtime PM so
> the IPU bridge sensor link keeps shared power resources active. Propagate
> PM acquisition failures and restore the drive mode and controls after
> resume.
> 
> The register addresses and drive-mode value are derived from Intel's
> GPL-2.0 WV517 driver. Retain the Intel copyright notice and document the
> 2026 copyright for this V4L2/regmap implementation.

...

+ container_of.h
+ device.h // dev_get_drvdata()

> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>

+ types.h // true, false

> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>

...

> +static int wv517s_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct wv517s_device *wv517s = container_of(ctrl->handler,
> +						      struct wv517s_device,
> +						      ctrl_handler);

	struct wv517s_device *wv517s =
		container_of(ctrl->handler, struct wv517s_device, ctrl_handler);

> +	struct device *dev = wv517s->sd.dev;
> +	int pm_ret;
> +	int ret;
> +
> +	/* Runtime resume restores controls while the PM state is RPM_RESUMING. */
> +	pm_ret = pm_runtime_get_if_active(dev);
> +	if (!pm_ret && !wv517s->resuming)
> +		return 0;
> +	if (pm_ret < 0)
> +		return pm_ret;

> +	if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> +		ret = regmap_write(wv517s->regmap, WV517S_REG_FOCUS,
> +				   ctrl->val);
> +	else
> +		ret = -EINVAL;

The else branch can be done before even trying runtime PM.

> +	if (pm_ret > 0)
> +		pm_runtime_put(dev);
> +
> +	return ret;
> +}

...

> +static int wv517s_probe(struct i2c_client *client)
> +{

	struct device *dev = &client->dev;

> +	struct wv517s_device *wv517s;
> +	int ret;
> +
> +	wv517s = devm_kzalloc(&client->dev, sizeof(*wv517s), GFP_KERNEL);

	wv517s = devm_kzalloc(dev, sizeof(*wv517s), GFP_KERNEL);

and so on...

> +	if (!wv517s)
> +		return -ENOMEM;
> +
> +	wv517s->regmap = devm_regmap_init_i2c(client, &wv517s_regmap_config);
> +	if (IS_ERR(wv517s->regmap))
> +		return dev_err_probe(&client->dev, PTR_ERR(wv517s->regmap),
> +				     "failed to initialize regmap\n");
> +
> +	v4l2_i2c_subdev_init(&wv517s->sd, client, &wv517s_subdev_ops);
> +	wv517s->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	wv517s->sd.internal_ops = &wv517s_internal_ops;
> +	wv517s->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> +	v4l2_ctrl_handler_init(&wv517s->ctrl_handler, 1);
> +	v4l2_ctrl_new_std(&wv517s->ctrl_handler, &wv517s_ctrl_ops,
> +			  V4L2_CID_FOCUS_ABSOLUTE, 0,
> +			  WV517S_MAX_FOCUS_POSITION, 1,
> +			  WV517S_DEFAULT_FOCUS_POSITION);
> +	if (wv517s->ctrl_handler.error) {
> +		ret = wv517s->ctrl_handler.error;
> +		goto err_free_ctrl_handler;
> +	}
> +	wv517s->sd.ctrl_handler = &wv517s->ctrl_handler;
> +
> +	ret = media_entity_pads_init(&wv517s->sd.entity, 0, NULL);
> +	if (ret)
> +		goto err_free_ctrl_handler;
> +
> +	pm_runtime_set_active(&client->dev);
> +	pm_runtime_enable(&client->dev);
> +
> +	ret = wv517s_resume(&client->dev);
> +	if (ret)
> +		goto err_disable_pm;
> +
> +	ret = v4l2_async_register_subdev(&wv517s->sd);
> +	if (ret)
> +		goto err_disable_pm;
> +
> +	pm_runtime_idle(&client->dev);
> +
> +	return 0;
> +
> +err_disable_pm:
> +	pm_runtime_disable(&client->dev);
> +	media_entity_cleanup(&wv517s->sd.entity);
> +err_free_ctrl_handler:
> +	v4l2_ctrl_handler_free(&wv517s->ctrl_handler);
> +
> +	return ret;
> +}

...

> +static const struct i2c_device_id wv517s_id_table[] = {
> +	{ "wv517s" },

Use C99 initialisers.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, wv517s_id_table);

> +static DEFINE_RUNTIME_DEV_PM_OPS(wv517s_pm_ops, NULL, wv517s_resume, NULL);

Move it up, closer to the implementation of the resume callback.

-- 
With Best Regards,
Andy Shevchenko