Re: [PATCH v2] media: i2c: Add Omnivision OCV02C10 sensor driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Heimir,

On 16-Nov-24 3:42 PM, Heimir Thor Sverrisson wrote:
> Add a new driver for the Omnivision OCV02C10 camera sensor. This is based

I'm not sure where the "C" in OCV above and in the Subject (first
line of commit message) come from. Please do:

s/OCV02C10/OV02C10/ on the commit message.

> on the out of tree driver by Hao Yao <hao.yao@xxxxxxxxx> from:
> https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/ov02c10.c
> 
> This has been tested on a Dell XPS 9440 together with the IPU6 isys CSI
> driver and the libcamera software ISP code.
> 
> Tested-by: Heimir Thor Sverrisson <heimir.sverrisson@xxxxxxxxx>
> Signed-off-by: Heimir Thor Sverrisson <heimir.sverrisson@xxxxxxxxx>
> ---
>  drivers/media/i2c/Kconfig   |   10 +
>  drivers/media/i2c/Makefile  |    1 +
>  drivers/media/i2c/ov02c10.c | 1393 +++++++++++++++++++++++++++++++++++
>  3 files changed, 1404 insertions(+)
>  create mode 100644 drivers/media/i2c/ov02c10.c

<snip>

> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> new file mode 100644
> index 000000000000..86ae15b8475e
> --- /dev/null
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -0,0 +1,1393 @@

<snip>

> +static int ov02c10_read_mipi_lanes(struct ov02c10 *ov02c10, struct device *dev)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
> +	struct v4l2_fwnode_endpoint bus_cfg = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY
> +	};
> +	struct fwnode_handle *ep;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +

Since you are now getting the mipi-lanes from the fwnode, all
the ACPI stuff here is no longer necessary. Sorry I missed this
before when we were preparing the driver for submitting it
upstream off-list.

So you can drop everything starting here:

> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> +	union acpi_object *obj;
> +	acpi_status status;
> +	int ret;
> +
> +	if (!adev) {
> +		dev_info(&client->dev, "Not ACPI device\n");
> +		return -EBADF;
> +	}
> +	status = acpi_evaluate_object(adev->handle, "SSDB", NULL, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		dev_info(&client->dev, "ACPI fail: %d\n", -ENODEV);
> +		return -EBADF;
> +	}
> +
> +	obj = buffer.pointer;
> +	if (!obj) {
> +		dev_info(&client->dev, "Couldn't locate ACPI buffer\n");
> +		return -ENOMEM;
> +	}
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		dev_info(&client->dev, "Not an ACPI buffer\n");
> +		goto out_free_buff;
> +	}

Up to here, notice buffer / obj is never used below again except
for the kfree(buffer.pointer) which of course also can be dropped.

Regards,

Hans





> +
> +	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> +	if (!ep)
> +		return -ENXIO;
> +
> +	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> +	fwnode_handle_put(ep);
> +	if (ret)
> +		return ret;
> +
> +	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 2 &&
> +	    bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
> +		dev_err(dev, "number of CSI2 data lanes %d is not supported",
> +			bus_cfg.bus.mipi_csi2.num_data_lanes);
> +		ret = -EINVAL;
> +		goto out_free_buff;
> +	}
> +	ov02c10->mipi_lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
> +	ret = 0;
> +out_free_buff:
> +	kfree(buffer.pointer);
> +	return ret;
> +}
> +
> +static int ov02c10_identify_module(struct ov02c10 *ov02c10)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
> +	u64 chip_id;
> +	u32 ret = 0;
> +
> +	ov02c10->regmap = devm_cci_regmap_init_i2c(client, 16);
> +	cci_read(ov02c10->regmap, OV02C10_REG_CHIP_ID, &chip_id, &ret);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (chip_id != OV02C10_CHIP_ID) {
> +		dev_err(&client->dev, "chip id mismatch: %x!=%llx",
> +			OV02C10_CHIP_ID, chip_id);
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ov02c10_check_hwcfg(struct device *dev)
> +{
> +	struct v4l2_fwnode_endpoint bus_cfg = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY
> +	};
> +	struct fwnode_handle *ep;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +	unsigned int i, j;
> +	int ret;
> +	u32 ext_clk;
> +
> +	if (!fwnode)
> +		return -ENXIO;
> +
> +	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> +	if (!ep)
> +		return -EPROBE_DEFER;
> +
> +	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> +				       &ext_clk);
> +	if (ret) {
> +		dev_err(dev, "can't get clock frequency");
> +		return ret;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> +	fwnode_handle_put(ep);
> +	if (ret)
> +		return ret;
> +
> +	if (!bus_cfg.nr_of_link_frequencies) {
> +		dev_err(dev, "no link frequencies defined");
> +		ret = -EINVAL;
> +		goto out_err;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(link_freq_menu_items); i++) {
> +		for (j = 0; j < bus_cfg.nr_of_link_frequencies; j++) {
> +			if (link_freq_menu_items[i] ==
> +				bus_cfg.link_frequencies[j])
> +				break;
> +		}
> +
> +		if (j == bus_cfg.nr_of_link_frequencies) {
> +			dev_err(dev, "no link frequency %lld supported",
> +				link_freq_menu_items[i]);
> +			ret = -EINVAL;
> +			goto out_err;
> +		}
> +	}
> +
> +out_err:
> +	v4l2_fwnode_endpoint_free(&bus_cfg);
> +
> +	return ret;
> +}
> +
> +static void ov02c10_remove(struct i2c_client *client)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +	struct ov02c10 *ov02c10 = to_ov02c10(sd);
> +
> +	v4l2_async_unregister_subdev(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
> +	pm_runtime_disable(&client->dev);
> +	mutex_destroy(&ov02c10->mutex);
> +}
> +
> +static int ov02c10_probe(struct i2c_client *client)
> +{
> +	struct ov02c10 *ov02c10;
> +	int ret = 0;
> +
> +	/* Check HW config */
> +	ret = ov02c10_check_hwcfg(&client->dev);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to check hwcfg: %d", ret);
> +		return ret;
> +	}
> +
> +	ov02c10 = devm_kzalloc(&client->dev, sizeof(*ov02c10), GFP_KERNEL);
> +	if (!ov02c10)
> +		return -ENOMEM;
> +
> +	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
> +	ov02c10_get_pm_resources(&client->dev);
> +
> +	ret = ov02c10_power_on(&client->dev);
> +	if (ret) {
> +		dev_err_probe(&client->dev, ret, "failed to power on\n");
> +		return ret;
> +	}
> +
> +	ret = ov02c10_identify_module(ov02c10);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to find sensor: %d", ret);
> +		goto probe_error_ret;
> +	}
> +
> +	ret = ov02c10_read_mipi_lanes(ov02c10, &client->dev);
> +	if (ret)
> +		goto probe_error_ret;
> +
> +	mutex_init(&ov02c10->mutex);
> +	ov02c10->cur_mode = &supported_modes[0];
> +	if (ov02c10->mipi_lanes == 2)
> +		ov02c10->cur_mode = &supported_modes[1];
> +	ret = ov02c10_init_controls(ov02c10);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to init controls: %d", ret);
> +		goto probe_error_v4l2_ctrl_handler_free;
> +	}
> +
> +	ov02c10->sd.internal_ops = &ov02c10_internal_ops;
> +	ov02c10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	ov02c10->sd.entity.ops = &ov02c10_subdev_entity_ops;
> +	ov02c10->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +	ov02c10->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	ret = media_entity_pads_init(&ov02c10->sd.entity, 1, &ov02c10->pad);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to init entity pads: %d", ret);
> +		goto probe_error_v4l2_ctrl_handler_free;
> +	}
> +
> +	ret = v4l2_async_register_subdev_sensor(&ov02c10->sd);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to register V4L2 subdev: %d",
> +			ret);
> +		goto probe_error_media_entity_cleanup;
> +	}
> +
> +	/*
> +	 * Device is already turned on by i2c-core with ACPI domain PM.
> +	 * Enable runtime PM and turn off the device.
> +	 */
> +	pm_runtime_set_active(&client->dev);
> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_idle(&client->dev);
> +
> +	return 0;
> +
> +probe_error_media_entity_cleanup:
> +	media_entity_cleanup(&ov02c10->sd.entity);
> +
> +probe_error_v4l2_ctrl_handler_free:
> +	v4l2_ctrl_handler_free(ov02c10->sd.ctrl_handler);
> +	mutex_destroy(&ov02c10->mutex);
> +
> +probe_error_ret:
> +	ov02c10_power_off(&client->dev);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops ov02c10_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(ov02c10_suspend, ov02c10_resume)
> +	SET_RUNTIME_PM_OPS(ov02c10_power_off, ov02c10_power_on, NULL)
> +};
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id ov02c10_acpi_ids[] = {
> +	{"OVTI02C1"},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, ov02c10_acpi_ids);
> +#endif
> +
> +static struct i2c_driver ov02c10_i2c_driver = {
> +	.driver = {
> +		.name = "ov02c10",
> +		.pm = &ov02c10_pm_ops,
> +		.acpi_match_table = ACPI_PTR(ov02c10_acpi_ids),
> +	},
> +	.probe = ov02c10_probe,
> +	.remove = ov02c10_remove,
> +};
> +
> +module_i2c_driver(ov02c10_i2c_driver);
> +
> +MODULE_AUTHOR("Hao Yao <hao.yao@xxxxxxxxx>");
> +MODULE_DESCRIPTION("OmniVision OV02C10 sensor driver");
> +MODULE_LICENSE("GPL");





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux