On Tue, Jun 27, 2023 at 07:56:41PM +0200, Hans de Goede wrote:
> In most cases when a VCM is used there is a single integrated module
> with the sensor + VCM + lens. This means that the sensor and VCM often
> share regulators and possibly also something like a powerdown pin.
>
> In the ACPI tables this is modelled as a single ACPI device with
> multiple I2cSerialBus resources.
>
> On atomisp devices the regulators and clks are modelled as ACPI
> power-resources, which are controlled by the (ACPI) power state
> of the sensor. So the sensor must be in D0 power state for the VCM
> to work.
>
> To make this work add a device-link with DL_FLAG_PM_RUNTIME flag
> so that the sensor will automatically be runtime-resumed whenever
> the VCM is runtime-resumed.
>
> This requires the probing of the VCM and thus the creation of the VCM
> I2C-client to be delayed till after the sensor driver has bound.
>
> Move the instantiation of the VCM I2C-client to the v4l2_async_notifier
> bound op, so that it is done after the sensor driver has bound; and
> add code to add the device-link.
>
> This fixes the problem with the shared ACPI power-resources on atomisp2
> and this avoids the need for VCM related workarounds on IPU3.
>
> E.g. until now the dw9719 driver needed to get and control a Vsio
> (V sensor IO) regulator since that needs to be enabled to enable I2C
> pass-through on the PMIC on the sensor module. So the driver was
> controlling this regulator even though the actual dw9719 chip has no
> Vsio pin / power-plane.
>
> This also removes the need for intel_cio2_bridge_init() to return
> -EPROBE_DEFER since the VCM is now instantiated later.
...
> +struct intel_cio2_bridge_instantiate_vcm_work_data {
Hmm... A bit long name :-)
> + struct work_struct work;
> + struct device *sensor;
> char name[16];
> + struct i2c_board_info board_info;
> +};
...
> + if (IS_ERR(vcm_client)) {
> + dev_err(work->sensor, "Error instantiating VCM client: %ld\n",
or even %pe
> + PTR_ERR(vcm_client));
...
> +out:
out_put_pm: ?
> + pm_runtime_put(work->sensor);
> + put_device(work->sensor);
> + if (put_fwnode)
> + fwnode_handle_put(work->board_info.fwnode);
> + kfree(work);
...
> +int intel_cio2_bridge_instantiate_vcm(struct device *sensor)
> +{
> + struct intel_cio2_bridge_instantiate_vcm_work_data *work;
> + struct acpi_device *adev = ACPI_COMPANION(sensor);
> + struct fwnode_handle *vcm_fwnode;
> + struct i2c_client *vcm_client;
> + char *sep;
I would rather split assignment, so below won't look a bit puzzling.
struct acpi_device *adev;
...
adev = ACPI_COMPANION(sensor);
> + if (!adev)
> + return 0;
> +
> + vcm_fwnode = fwnode_find_reference(dev_fwnode(sensor), "lens-focus", 0);
> + if (IS_ERR(vcm_fwnode))
> + return 0;
> +
> + /* When reloading modules the client will already exist */
> + vcm_client = i2c_find_device_by_fwnode(vcm_fwnode);
> + if (vcm_client) {
> + fwnode_handle_put(vcm_fwnode);
> + put_device(&vcm_client->dev);
> + return 0;
> + }
> +
> + work = kzalloc(sizeof(*work), GFP_KERNEL);
> + if (!work) {
> + fwnode_handle_put(vcm_fwnode);
> + return -ENOMEM;
> + }
> +
> + INIT_WORK(&work->work, intel_cio2_bridge_instantiate_vcm_work);
> + work->sensor = get_device(sensor);
> + snprintf(work->name, sizeof(work->name), "%s-VCM",
> + acpi_dev_name(adev));
> + work->board_info.dev_name = work->name;
> + work->board_info.fwnode = vcm_fwnode;
> + strscpy(work->board_info.type, fwnode_get_name(vcm_fwnode),
> + I2C_NAME_SIZE);
> + /* Strip "-<link>" postfix */
> + sep = strchrnul(work->board_info.type, '-');
> + *sep = 0;
> +
> + queue_work(system_long_wq, &work->work);
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
