V4L2 API for flash devices and the adp1653 flash controller driver

This patch adds the driver for the adp1653 LED flash controller. This controller supports a high power led in flash and torch modes and an indicator light, sometimes also called privacy light.

The adp1653 is used on the Nokia N900.

drivers/media/video/adp1653.c new file +0 -0

new file mode 100644
index 0000000..1679707

drivers/media/video/adp1653.c new file +9 -0

+static int adp1653_get_fault(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int fault;
+	int rval;
+
+	fault = i2c_smbus_read_byte_data(client, ADP1653_REG_FAULT);
+	if (IS_ERR_VALUE(fault))
+		return fault;

drivers/media/video/adp1653.c new file +2 -0

+	return flash->fault;
+}

drivers/media/video/adp1653.c new file +17 -0

+		return flash->fault;
+
+	ctrl->cur.val = 0;
+
+	if (flash->fault & ADP1653_REG_FAULT_FLT_SCP)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OT)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_TMR)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OV)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
+
+	flash->fault = 0;
+
+	return 0;
+}

drivers/media/video/adp1653.c new file +14 -0

+}
+
+static int
+adp1653_init_device(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int rval;
+
+	/* Clear FAULT register by writing zero to OUT_SEL */
+	rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0);
+	if (rval < 0) {
+		dev_err(&client->dev, "failed writing fault register\n");
+		return -EIO;
+	}

drivers/media/video/adp1653.c new file +14 -0

+		return -EIO;
+	}
+
+	mutex_lock(&flash->ctrls.lock);
+	rval = adp1653_update_hw(flash);
+	mutex_unlock(&flash->ctrls.lock);
+	if (rval) {
+		dev_err(&client->dev,
+			"adp1653_update_hw failed at %s\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}

Hi Sakari,

Quoted 5 lines

This patch adds the driver for the adp1653 LED flash controller. This controller supports a high power led in flash and torch modes and an indicator light, sometimes also called privacy light.

The adp1653 is used on the Nokia N900.

[snip]

Quoted 5 lines

drivers/media/video/adp1653.c new file +0 -0

new file mode 100644
index 0000000..1679707

[snip]

Quoted 9 lines

drivers/media/video/adp1653.c new file +9 -0

+static int adp1653_get_fault(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int fault;
+	int rval;
+
+	fault = i2c_smbus_read_byte_data(client, ADP1653_REG_FAULT);
+	if (IS_ERR_VALUE(fault))
+		return fault;

drivers/media/video/adp1653.c line 67

Quoted 10 lines

drivers/media/video/adp1653.c new file +10 -0

+
+	flash->fault |= fault;
+
+	if (!flash->fault)
+		return 0;
+
+	/* Clear faults. */
+	rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0);
+	if (IS_ERR_VALUE(rval))
+		return rval;

Should the faults be cleared right away, instead of when the user reads the faults control ?

drivers/media/video/adp1653.c line 69

Quoted 1 line

drivers/media/video/adp1653.c new file +1 -0

+	flash->led_mode->val = V4L2_FLASH_LED_MODE_NONE;

Does the hardware switch back to "none" mode when a fault occurs ? The datasheet just states that "the ADP1653 is disabled". Does that mean temporarily disabled until the faults are cleared ? If so you should update the registers to turn the LED off.

Quoted 2 lines

drivers/media/video/adp1653.c new file +2 -0

+	return flash->fault;
+}

[snip]

drivers/media/video/adp1653.c line 154

Quoted 7 lines

drivers/media/video/adp1653.c new file +7 -0

+static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct adp1653_flash *flash =
+		container_of(ctrl->handler, struct adp1653_flash, ctrls);
+
+	adp1653_get_fault(flash);
+	if (IS_ERR_VALUE(flash->fault))

Shouldn't you check the adp1653_get_fault() return value instead ?

Quoted 17 lines

drivers/media/video/adp1653.c new file +17 -0

+		return flash->fault;
+
+	ctrl->cur.val = 0;
+
+	if (flash->fault & ADP1653_REG_FAULT_FLT_SCP)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OT)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_TMR)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OV)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
+
+	flash->fault = 0;
+
+	return 0;
+}

[snip]

drivers/media/video/adp1653.c line 267

Quoted 6 lines

drivers/media/video/adp1653.c new file +6 -0

+static int
+adp1653_registered(struct v4l2_subdev *subdev)
+{
+	struct adp1653_flash *flash = to_adp1653_flash(subdev);
+
+	return adp1653_init_controls(flash);

Can't this be moved to adp1653_probe() ? You could then get rid of the registered callback.

Quoted 18 lines

drivers/media/video/adp1653.c new file +18 -0

+}
+
+static int
+adp1653_init_device(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int rval;
+
+	/* Clear FAULT register by writing zero to OUT_SEL */
+	rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0);
+	if (rval < 0) {
+		dev_err(&client->dev, "failed writing fault register\n");
+		return -EIO;
+	}
+
+	/* Read FAULT register */
+	rval = i2c_smbus_read_byte_data(client, ADP1653_REG_FAULT);
+	if (rval < 0) {

drivers/media/video/adp1653.c line 291

Quoted 6 lines

drivers/media/video/adp1653.c new file +6 -0

+		dev_err(&client->dev, "failed reading fault register\n");
+		return -EIO;
+	}
+
+	if ((rval & 0x0f) != 0) {
+		dev_err(&client->dev, "device fault\n");

Same comment as last time :-)

Quoted 40 lines · Laurent Pinchart

Quoted 5 lines · On Tuesday 17 May 2011 17:14:04 Sakari Ailus wrote:

This patch adds the driver for the adp1653 LED flash controller. This controller supports a high power led in flash and torch modes and an indicator light, sometimes also called privacy light.

The adp1653 is used on the Nokia N900.

[snip]

Quoted 5 lines

(quoted diff) +0 -0

b/drivers/media/video/adp1653.c new file mode 100644 index 0000000..1679707

drivers/media/video/adp1653.c new file +0 -0

[snip]

Quoted 9 lines

drivers/media/video/adp1653.c new file +9 -0

+static int adp1653_get_fault(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int fault;
+	int rval;
+
+	fault = i2c_smbus_read_byte_data(client, ADP1653_REG_FAULT);
+	if (IS_ERR_VALUE(fault))
+		return fault;

drivers/media/video/adp1653.c line 67

Quoted 10 lines

drivers/media/video/adp1653.c new file +10 -0

+
+	flash->fault |= fault;
+
+	if (!flash->fault)
+		return 0;
+
+	/* Clear faults. */
+	rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0);
+	if (IS_ERR_VALUE(rval))
+		return rval;

Should the faults be cleared right away, instead of when the user reads the faults control ?

Quoted 6 lines

drivers/media/video/adp1653.c line 69

Quoted 1 line

drivers/media/video/adp1653.c new file +1 -0

+	flash->led_mode->val = V4L2_FLASH_LED_MODE_NONE;

Does the hardware switch back to "none" mode when a fault occurs ? The datasheet just states that "the ADP1653 is disabled". Does that mean temporarily disabled until the faults are cleared ? If so you should update the registers to turn the LED off.

My understanding is that this is temporary until the faults are cleared. However, this is difficult to find out since the faults don't just occur that easily.

OUT_SEL register controls the current to LEDs so this turned everything off. The indicator should still remain on, I guess, since it's not affected by the faults, except possibly the over-temperature one.

Quoted 5 lines

Quoted 2 lines

drivers/media/video/adp1653.c new file +2 -0

+	return flash->fault;
+}

[snip]

Quoted 9 lines

drivers/media/video/adp1653.c line 154

Quoted 7 lines

drivers/media/video/adp1653.c new file +7 -0

+static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct adp1653_flash *flash =
+		container_of(ctrl->handler, struct adp1653_flash, ctrls);
+
+	adp1653_get_fault(flash);
+	if (IS_ERR_VALUE(flash->fault))

Shouldn't you check the adp1653_get_fault() return value instead ?

Yes. Good catch. I've tried to simulate this code a bit but as always, error handling tends not to be one of the best parts of the driver, especially those errors that generally do not happen. :-I

Quoted 20 lines

Quoted 17 lines

drivers/media/video/adp1653.c new file +17 -0

+		return flash->fault;
+
+	ctrl->cur.val = 0;
+
+	if (flash->fault & ADP1653_REG_FAULT_FLT_SCP)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OT)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_TMR)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT;
+	if (flash->fault & ADP1653_REG_FAULT_FLT_OV)
+		ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
+
+	flash->fault = 0;
+
+	return 0;
+}

[snip]

Quoted 9 lines

drivers/media/video/adp1653.c line 267

Quoted 6 lines

drivers/media/video/adp1653.c new file +6 -0

+static int
+adp1653_registered(struct v4l2_subdev *subdev)
+{
+	struct adp1653_flash *flash = to_adp1653_flash(subdev);
+
+	return adp1653_init_controls(flash);

Can't this be moved to adp1653_probe() ? You could then get rid of the registered callback.

Good point. I'll look into this.

Quoted 18 lines

Quoted 18 lines

drivers/media/video/adp1653.c new file +18 -0

+}
+
+static int
+adp1653_init_device(struct adp1653_flash *flash)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev);
+	int rval;
+
+	/* Clear FAULT register by writing zero to OUT_SEL */
+	rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0);
+	if (rval < 0) {
+		dev_err(&client->dev, "failed writing fault register\n");
+		return -EIO;
+	}
+
+	/* Read FAULT register */
+	rval = i2c_smbus_read_byte_data(client, ADP1653_REG_FAULT);
+	if (rval < 0) {

Quoted 8 lines

drivers/media/video/adp1653.c line 291

Quoted 6 lines

drivers/media/video/adp1653.c new file +6 -0

+		dev_err(&client->dev, "failed reading fault register\n");
+		return -EIO;
+	}
+
+	if ((rval & 0x0f) != 0) {
+		dev_err(&client->dev, "device fault\n");

Same comment as last time :-)

Ouch. I'll try to get the fix done for the next time. :-)