Re: [PATCH 11/12] media: intel-cio2-bridge: Add a runtime-pm device-link between VCM and sensor

[Hans de Goede <hdegoede@redhat.com>]

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.

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.

Regards,

Hans