Re: [PATCH v2 11/11] media: i2c: Add WV517S lens actuator driver
From: Andy Shevchenko
Date: Thu Aug 27 2026 - 16:31:26 EST
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