Re: [PATCH v3 1/3] media: i2c: cvs: Add driver of Intel Computer Vision Sensing Controller(CVS)

["Vadillo, Miguel" <miguel.vadillo@intel.com>]

Hi Mehdi,

On 5/22/26 1:39 AM, Mehdi Djait wrote:
> Hi Miguel,
> 
> Thank you for the patch.
> 
> On Thu, May 21, 2026 at 03:23:57PM -0700, Miguel Vadillo wrote:
>> Add driver for Intel Computer Vision Sensing (CVS) devices found on
> 
> [..]
> 
>> +fail_i2c:
>> +	ret = cvs_csi_init(ctx, dev, i2c);
>> +	if (ret) {
>> +		dev_err_probe(dev, ret, "CSI init failed\n");
>> +		goto err_put_ipu;
>> +	}
>> +
>> +	dev_set_drvdata(dev, ctx);
>> +	pm_runtime_set_autosuspend_delay(dev, 1000);
>> +	pm_runtime_use_autosuspend(dev);
>> +	pm_runtime_enable(dev);
>> +	pm_runtime_idle(dev);
>> +
>> +	/*
>> +	 * Create a PM runtime device link with IPU as consumer and CVS as
>> +	 * supplier. When the IPU runtime-resumes to start streaming, the PM
>> +	 * framework automatically resumes CVS first, triggering
>> +	 * cvs_runtime_resume() which hands CSI-2 link ownership to the host.
>> +	 */
>> +	ctx->ipu_link = device_link_add(&ipu->dev, dev,
>> +					DL_FLAG_PM_RUNTIME |
>> +					DL_FLAG_RPM_ACTIVE |
>> +					DL_FLAG_STATELESS);
>> +	put_device(&ipu->dev);
>> +	if (!ctx->ipu_link) {
>> +		dev_err(dev, "IPU device link failed\n");
>> +		ret = -ENODEV;
>> +		goto err_csi_remove;
>> +	}
>> +
>> +	if (has_acpi_companion(dev))
>> +		acpi_dev_clear_dependencies(ACPI_COMPANION(dev));
>> +
>> +	return 0;
>> +
>> +err_csi_remove:
>> +	if (ctx->ipu_link)
>> +		device_link_del(ctx->ipu_link);
>> +	cvs_csi_remove(ctx);
>> +	pm_runtime_dont_use_autosuspend(dev);
>> +	pm_runtime_disable(dev);
>> +	pm_runtime_set_suspended(dev);
>> +	return ret;
> 
> this return ret seems wrong here.

Are you referring to just the space before return?
@@ -1318,10 +1318,12 @@ static int cvs_core_probe(struct device *dev, 
struct i2c_client *i2c)
         pm_runtime_dont_use_autosuspend(dev);
         pm_runtime_disable(dev);
         pm_runtime_set_suspended(dev);
+
         return ret;

  err_put_ipu:
         put_device(&ipu->dev);
+
         return ret;
  }

put_device(ipu) was already done after the ipu_link so no need to 
waterfall all the way down. Please clarify if I am misunderstanding your 
comment.

--
regards,
Miguel

> 
>> +
>> +err_put_ipu:
>> +	put_device(&ipu->dev);
>> +	return ret;
>> +}