Re: [PATCH] media: Add t4ka3 camera sensor driver

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

 



Le 02/10/2024 à 11:30, Kate Hsuan a écrit :
Add the t4ka3 driver from:
https://github.com/kitakar5525/surface3-atomisp-cameras.git

With many cleanups / changes (almost a full rewrite) to make it suitable
for upstream:

* Remove the VCM and VCM-OTP support, the mainline kernel models VCMs and
   calibration data eeproms as separate v4l2-subdev-s.

* Remove the integration-factor t4ka3_get_intg_factor() support and IOCTL,
   this provided info to userspace through an atomisp private IOCTL.

* Turn atomisp specific exposure/gain IOCTL into standard v4l2 controls.

* Use normal ACPI power-management in combination with runtime-pm support
   instead of atomisp specific GMIN power-management code.

* Turn into a standard V4L2 sensor driver using
   v4l2_async_register_subdev_sensor().

* Add vblank, hblank, and link-freq controls; drop get_frame_interval().

* Use CCI register helpers.

* Calculate values for modes instead of using fixed register-value lists,
   allowing arbritrary modes.

* Add get_selection() and set_selection() support

* Add a CSI2 bus configuration check

This been tested on a Xiaomi Mipad2 tablet which has a T4KA3 sensor with
DW9761 VCM as back sensor.

Co-developed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Kate Hsuan <hpa@xxxxxxxxxx>
---

Hi,

a few comments, should it help.

+static int t4ka3_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct t4ka3_data *sensor = to_t4ka3_sensor(sd);
+	int ret;
+
+	mutex_lock(&sensor->lock);
+
+	if (sensor->streaming == enable) {
+		dev_warn(sensor->dev, "Stream already %s\n", enable ? "started" : "stopped");
+		goto error_unlock;
+	}
+
+	if (enable) {
+		ret = pm_runtime_get_sync(sensor->sd.dev);
+		if (ret) {
+			dev_err(sensor->dev, "power-up err.\n");
+			goto error_unlock;
+		}
+
+		cci_multi_reg_write(sensor->regmap, t4ka3_init_config,
+				    ARRAY_SIZE(t4ka3_init_config), &ret);
+		/* enable group hold */
+		cci_write(sensor->regmap, T4KA3_REG_PARAM_HOLD, 1, &ret);
+		cci_multi_reg_write(sensor->regmap, t4ka3_pre_mode_set_regs,
+				    ARRAY_SIZE(t4ka3_pre_mode_set_regs), &ret);
+		if (ret)
+			goto error_powerdown;
+
+		ret = t4ka3_set_mode(sensor);
+		if (ret)
+			goto error_powerdown;
+
+		ret = cci_multi_reg_write(sensor->regmap, t4ka3_post_mode_set_regs,
+					  ARRAY_SIZE(t4ka3_post_mode_set_regs), NULL);
+		if (ret)
+			goto error_powerdown;
+
+		/* Restore value of all ctrls */
+		ret = __v4l2_ctrl_handler_setup(&sensor->ctrls.handler);
+		if (ret)
+			goto error_powerdown;
+
+		/* disable group hold */
+		cci_write(sensor->regmap, T4KA3_REG_PARAM_HOLD, 0, &ret);
+		cci_write(sensor->regmap, T4KA3_REG_STREAM, 1, &ret);
+		if (ret)
+			goto error_powerdown;
+
+		sensor->streaming = 1;
+	} else {
+		ret = cci_write(sensor->regmap, T4KA3_REG_STREAM, 0, NULL);
+		if (ret)
+			goto error_powerdown;
+
+		ret = pm_runtime_put(sensor->sd.dev);
+		if (ret)
+			goto error_unlock;
+
+		sensor->streaming = 0;
+	}
+
+	mutex_unlock(&sensor->lock);
+	return ret;
+
+error_powerdown:
+	ret = pm_runtime_put(sensor->sd.dev);

I think that the "ret = " should be removed here.

+error_unlock:
+	mutex_unlock(&sensor->lock);
+	return ret;
+}

...

+static int t4ka3_probe(struct i2c_client *client)
+{
+	struct t4ka3_data *sensor;
+	int ret;
+
+	/* allocate sensor device & init sub device */
+	sensor = devm_kzalloc(&client->dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->dev = &client->dev;
+
+	ret = t4ka3_check_hwcfg(sensor);
+	if (ret)
+		return ret;
+
+	mutex_init(&sensor->lock);
+
+	sensor->link_freq[0] = T4KA3_LINK_FREQ;
+	sensor->mode.crop = t4ka3_default_crop;
+	t4ka3_fill_format(sensor, &sensor->mode.fmt, T4KA3_ACTIVE_WIDTH, T4KA3_ACTIVE_HEIGHT);
+	t4ka3_calc_mode(sensor);
+
+	v4l2_i2c_subdev_init(&(sensor->sd), client, &t4ka3_ops);
+	sensor->sd.internal_ops = &t4ka3_internal_ops;
+
+	sensor->powerdown_gpio = devm_gpiod_get(&client->dev, "powerdown",
+						GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->powerdown_gpio))
+		return dev_err_probe(&client->dev, PTR_ERR(sensor->powerdown_gpio),
+				     "getting powerdown GPIO\n");
+
+	sensor->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->reset_gpio))
+		return dev_err_probe(&client->dev, PTR_ERR(sensor->reset_gpio),
+				     "getting reset GPIO\n");
+
+	pm_runtime_set_suspended(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_set_autosuspend_delay(&client->dev, 1000);
+	pm_runtime_use_autosuspend(&client->dev);
+
+	sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(sensor->regmap))
+		return PTR_ERR(sensor->regmap);

I thing this should goto err_pm_runtime;

+
+	ret = t4ka3_s_config(&sensor->sd);
+	if (ret)
+		goto err_pm_runtime;
+
+	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+	sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+	ret = t4ka3_init_controls(sensor);
+	if (ret)
+		goto err_controls;
+
+	ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
+	if (ret)
+		goto err_controls;
+
+	ret = v4l2_async_register_subdev_sensor(&sensor->sd);
+	if (ret)
+		goto err_media_entity;
+
+	return 0;
+
+err_media_entity:
+	media_entity_cleanup(&sensor->sd.entity);
+err_controls:
+	v4l2_ctrl_handler_free(&sensor->ctrls.handler);
+err_pm_runtime:
+	pm_runtime_disable(&client->dev);
+	return ret;
+}
+
+static struct acpi_device_id t4ka3_acpi_match[] = {
+	{ "XMCC0003" },
+	{},

No need for ending comma after terminator.

+};

...

CJ




[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