Hi Hans,
On Mon, Oct 30, 2023 at 09:58:09AM +0100, Hans de Goede wrote:
> Hi,
>
> On 10/30/23 09:55, Sakari Ailus wrote:
> > Hi Bingbu,
> >
> > On Mon, Oct 30, 2023 at 04:30:39PM +0800, Bingbu Cao wrote:
> >>> +static void intel_cio2_bridge_instantiate_vcm_work(struct work_struct *_work)
> >>> +{
> >>> + struct intel_cio2_bridge_instantiate_vcm_work_data *work =
> >>> + container_of(_work,
> >>> + struct intel_cio2_bridge_instantiate_vcm_work_data,
> >>> + work);
> >>> + struct acpi_device *adev = ACPI_COMPANION(work->sensor);
> >>> + struct i2c_client *vcm_client;
> >>> + bool put_fwnode = true;
> >>> + int ret;
> >>>
> >>> - snprintf(name, sizeof(name), "%s-VCM", acpi_dev_name(sensor->adev));
> >>> - board_info.dev_name = name;
> >>> - strscpy(board_info.type, sensor->vcm_type, ARRAY_SIZE(board_info.type));
> >>> - board_info.swnode = &sensor->swnodes[SWNODE_VCM];
> >>> -
> >>> - sensor->vcm_i2c_client =
> >>> - i2c_acpi_new_device_by_fwnode(acpi_fwnode_handle(sensor->adev),
> >>> - 1, &board_info);
> >>> - if (IS_ERR(sensor->vcm_i2c_client)) {
> >>> - dev_warn(&sensor->adev->dev, "Error instantiation VCM i2c-client: %ld\n",
> >>> - PTR_ERR(sensor->vcm_i2c_client));
> >>> - sensor->vcm_i2c_client = NULL;
> >>> + /*
> >>> + * The client may get probed before the device_link gets added below
> >>> + * make sure the sensor is powered-up during probe.
> >>> + */
> >>> + ret = pm_runtime_get_sync(work->sensor);
> >>> + if (ret < 0) {
> >>> + dev_err(work->sensor, "Error %d runtime-resuming sensor, cannot instantiate VCM\n",
> >>> + ret);
> >>> + goto out;
> >>> }
> >>
> >> One question here: how do we make sure that the runtime PM of the sensor
> >> is enabled during the .bound callback? Or is it a mandatory requirement
> >> of driver of such camera sensors?
> >
> > The sensor driver needs to enable runtime PM in probe, otherwise this won't
> > work. But I don't see why a driver wouldn't? Of course otherwise it
> > wouldn't be a hard requirement.
I meant to write "wasn't earlier a hard requirement". This could also be
documented, I can write a patch for this.
>
> Right. I believe that having runtime pm support is more or less a hard
> requirement for sensor drivers used on x86. This is also necessary in
> cases where the power on/off sequence is (partially) implemented in
> ACPI AML code in the _PS0 and _PS3 methods of the sensor's ACPI device.
>
> This happens on [atom]ISP2 based devices and AFAIK also on ChromeOS
> devices.
>
> So I think it is safe to assume that sensor drivers implement runtime
> pm support and in cases where the driver does not we need to fix the driver.
I agree.
--
Regards,
Sakari Ailus
