| Message ID | 1305550839-16724-3-git-send-email-sakari.ailus@maxwell.research.nokia.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Sakari, Thanks for the patch. On Monday 16 May 2011 15:00:39 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. > > Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> > Signed-off-by: Tuukka Toivonen <tuukkat76@gmail.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: David Cohen <dacohen@gmail.com> [snip] > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c > new file mode 100644 > index 0000000..92ea38b > --- /dev/null > +++ b/drivers/media/video/adp1653.c [snip] > +/* Write values into ADP1653 registers. */ > +static int adp1653_update_hw(struct adp1653_flash *flash) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev); > + u8 out_sel; > + u8 config = 0; > + int rval; > + > + out_sel = flash->indicator_intensity->val > + << ADP1653_REG_OUT_SEL_ILED_SHIFT; > + > + switch (flash->led_mode->val) { > + case V4L2_FLASH_LED_MODE_NONE: > + break; > + case V4L2_FLASH_LED_MODE_FLASH: > + /* Flash mode, light on with strobe, duration from timer */ > + config = ADP1653_REG_CONFIG_TMR_CFG; > + config |= TIMEOUT_US_TO_CODE(flash->flash_timeout->val) > + << ADP1653_REG_CONFIG_TMR_SET_SHIFT; > + break; > + case V4L2_FLASH_LED_MODE_TORCH: > + /* Torch mode, light immediately on, duration indefinite */ > + out_sel |= flash->torch_intensity->val > + << ADP1653_REG_OUT_SEL_HPLED_SHIFT; > + break; > + } > + > + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, out_sel); out_sel can be used uninitialized here. > + if (rval < 0) > + return rval; > + > + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_CONFIG, config); > + if (rval < 0) > + return rval; > + > + return 0; > +} [snip] > +static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct adp1653_flash *flash = > + container_of(ctrl->handler, struct adp1653_flash, ctrls); > + 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; > + > + /* Clear faults. */ > + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0); > + if (IS_ERR_VALUE(rval)) > + return rval; If several applications read controls, only one of them will be notified of faults. Shouldn't clearing the fault be handled explicitly by writing to a control ? I know this changes the API :-) > + /* Restore configuration. */ > + rval = adp1653_update_hw(flash); > + if (IS_ERR_VALUE(rval)) > + return rval; Will that produce expected results ? For instance, if a fault was detected during flash strobe, won't it try to re-strobe the flash ? Shouldn't the user be required to explicitly re-strobe the flash or turn the torch (or indicator) on after a fault ? Once again this should be clarified in the API :-) > + ctrl->cur.val = 0; > + > + if (fault & ADP1653_REG_FAULT_FLT_SCP) > + ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT; > + if (fault & ADP1653_REG_FAULT_FLT_OT) > + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE; > + if (fault & ADP1653_REG_FAULT_FLT_TMR) > + ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT; > + if (fault & ADP1653_REG_FAULT_FLT_OV) > + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE; > + > + return 0; > +} [snip] > +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) { > + dev_err(&client->dev, "failed reading fault register\n"); > + return -EIO; > + } > + > + if ((rval & 0x0f) != 0) { > + dev_err(&client->dev, "device fault\n"); You could print the fault value, that could help debugging problems. > + 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; > +} [snip]
Laurent Pinchart wrote: > Hi Sakari, > > Thanks for the patch. Thanks for the comments! :-) > On Monday 16 May 2011 15:00:39 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. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> >> Signed-off-by: Tuukka Toivonen <tuukkat76@gmail.com> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: David Cohen <dacohen@gmail.com> > > [snip] > >> diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c >> new file mode 100644 >> index 0000000..92ea38b >> --- /dev/null >> +++ b/drivers/media/video/adp1653.c > > [snip] > >> +/* Write values into ADP1653 registers. */ >> +static int adp1653_update_hw(struct adp1653_flash *flash) >> +{ >> + struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev); >> + u8 out_sel; >> + u8 config = 0; >> + int rval; >> + >> + out_sel = flash->indicator_intensity->val >> + << ADP1653_REG_OUT_SEL_ILED_SHIFT; >> + >> + switch (flash->led_mode->val) { >> + case V4L2_FLASH_LED_MODE_NONE: >> + break; >> + case V4L2_FLASH_LED_MODE_FLASH: >> + /* Flash mode, light on with strobe, duration from timer */ >> + config = ADP1653_REG_CONFIG_TMR_CFG; >> + config |= TIMEOUT_US_TO_CODE(flash->flash_timeout->val) >> + << ADP1653_REG_CONFIG_TMR_SET_SHIFT; >> + break; >> + case V4L2_FLASH_LED_MODE_TORCH: >> + /* Torch mode, light immediately on, duration indefinite */ >> + out_sel |= flash->torch_intensity->val >> + << ADP1653_REG_OUT_SEL_HPLED_SHIFT; >> + break; >> + } >> + >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, out_sel); > > out_sel can be used uninitialized here. I don't think so. It's assigned a value in the beginning of the function. >> + if (rval < 0) >> + return rval; >> + >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_CONFIG, config); >> + if (rval < 0) >> + return rval; >> + >> + return 0; >> +} > > [snip] > >> +static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl) >> +{ >> + struct adp1653_flash *flash = >> + container_of(ctrl->handler, struct adp1653_flash, ctrls); >> + 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; >> + >> + /* Clear faults. */ >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0); >> + if (IS_ERR_VALUE(rval)) >> + return rval; > > If several applications read controls, only one of them will be notified of > faults. Shouldn't clearing the fault be handled explicitly by writing to a > control ? I know this changes the API :-) This is true. Although I can't imagine right now why two separate processes should be so interested in the faults but it is still entirely possible that someone does that since it's permitted by the interface. Having to write zero to faults to clear them isn't good either since it might mean missing faults that are triggered between reading and writing this control. Perhaps this would make sense as a file handle specific control? The control documentation says that the faults are cleared when the register is read, but the adp1653 also clears the faults whenever writing zero to out_sel which happens also in other circumstances, for example when changing mode from flash to torch when the torch intensity is zero, or when indicator intensity is zero in other modes. >> + /* Restore configuration. */ >> + rval = adp1653_update_hw(flash); >> + if (IS_ERR_VALUE(rval)) >> + return rval; > > Will that produce expected results ? For instance, if a fault was detected > during flash strobe, won't it try to re-strobe the flash ? Shouldn't the user No. Flash is strobed using adp1653_strobe(). > be required to explicitly re-strobe the flash or turn the torch (or indicator) > on after a fault ? Once again this should be clarified in the API :-) The mode won't be changed from the flash but to strobe again, the user has to push V4L2_CID_FLASH_STROBE again. The adp1653 doesn't have any torch (as such) or indicator faults; some other chips do have indicator faults at least. Using the torch mode might trigger faults, too, since it's the same LED; just the power isn't that high. >> + ctrl->cur.val = 0; >> + >> + if (fault & ADP1653_REG_FAULT_FLT_SCP) >> + ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT; >> + if (fault & ADP1653_REG_FAULT_FLT_OT) >> + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE; >> + if (fault & ADP1653_REG_FAULT_FLT_TMR) >> + ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT; >> + if (fault & ADP1653_REG_FAULT_FLT_OV) >> + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE; >> + >> + return 0; >> +} > > [snip] > >> +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) { >> + dev_err(&client->dev, "failed reading fault register\n"); >> + return -EIO; >> + } >> + >> + if ((rval & 0x0f) != 0) { >> + dev_err(&client->dev, "device fault\n"); > > You could print the fault value, that could help debugging problems. I'll change that. >> + 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; >> +} > > [snip] > Regards,
Hi Sakari, On Tuesday 17 May 2011 07:38:38 Sakari Ailus wrote: > Laurent Pinchart wrote: > > On Monday 16 May 2011 15:00:39 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. > >> > >> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> > >> Signed-off-by: Tuukka Toivonen <tuukkat76@gmail.com> > >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Signed-off-by: David Cohen <dacohen@gmail.com> > > > > [snip] > > > >> diff --git a/drivers/media/video/adp1653.c > >> b/drivers/media/video/adp1653.c new file mode 100644 > >> index 0000000..92ea38b > >> --- /dev/null > >> +++ b/drivers/media/video/adp1653.c > > > > [snip] > > > >> +/* Write values into ADP1653 registers. */ > >> +static int adp1653_update_hw(struct adp1653_flash *flash) > >> +{ > >> + struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev); > >> + u8 out_sel; > >> + u8 config = 0; > >> + int rval; > >> + > >> + out_sel = flash->indicator_intensity->val > >> + << ADP1653_REG_OUT_SEL_ILED_SHIFT; > >> + > >> + switch (flash->led_mode->val) { > >> + case V4L2_FLASH_LED_MODE_NONE: > >> + break; > >> + case V4L2_FLASH_LED_MODE_FLASH: > >> + /* Flash mode, light on with strobe, duration from timer */ > >> + config = ADP1653_REG_CONFIG_TMR_CFG; > >> + config |= TIMEOUT_US_TO_CODE(flash->flash_timeout->val) > >> + << ADP1653_REG_CONFIG_TMR_SET_SHIFT; > >> + break; > >> + case V4L2_FLASH_LED_MODE_TORCH: > >> + /* Torch mode, light immediately on, duration indefinite */ > >> + out_sel |= flash->torch_intensity->val > >> + << ADP1653_REG_OUT_SEL_HPLED_SHIFT; > >> + break; > >> + } > >> + > >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, > >> out_sel); > > > > out_sel can be used uninitialized here. > > I don't think so. It's assigned a value in the beginning of the function. My bad, I missed that. Sorry. > >> + if (rval < 0) > >> + return rval; > >> + > >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_CONFIG, config); > >> + if (rval < 0) > >> + return rval; > >> + > >> + return 0; > >> +} > > > > [snip] > > > >> +static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl) > >> +{ > >> + struct adp1653_flash *flash = > >> + container_of(ctrl->handler, struct adp1653_flash, ctrls); > >> + 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; > >> + > >> + /* Clear faults. */ > >> + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0); > >> + if (IS_ERR_VALUE(rval)) > >> + return rval; > > > > If several applications read controls, only one of them will be notified > > of faults. Shouldn't clearing the fault be handled explicitly by writing > > to a control ? I know this changes the API :-) > > This is true. > > Although I can't imagine right now why two separate processes should be > so interested in the faults but it is still entirely possible that > someone does that since it's permitted by the interface. > > Having to write zero to faults to clear them isn't good either since it > might mean missing faults that are triggered between reading and writing > this control. > > Perhaps this would make sense as a file handle specific control? Good question. Control events will help I guess, maybe that's the solution. > The control documentation says that the faults are cleared when the > register is read, but the adp1653 also clears the faults whenever > writing zero to out_sel which happens also in other circumstances, for > example when changing mode from flash to torch when the torch intensity > is zero, or when indicator intensity is zero in other modes. > > >> + /* Restore configuration. */ > >> + rval = adp1653_update_hw(flash); > >> + if (IS_ERR_VALUE(rval)) > >> + return rval; > > > > Will that produce expected results ? For instance, if a fault was > > detected during flash strobe, won't it try to re-strobe the flash ? > > Shouldn't the user > > No. Flash is strobed using adp1653_strobe(). > > > be required to explicitly re-strobe the flash or turn the torch (or > > indicator) on after a fault ? Once again this should be clarified in the > > API :-) > > The mode won't be changed from the flash but to strobe again, the user > has to push V4L2_CID_FLASH_STROBE again. > > The adp1653 doesn't have any torch (as such) or indicator faults; some > other chips do have indicator faults at least. Using the torch mode > might trigger faults, too, since it's the same LED; just the power isn't > that high. When an over-current fault is detected, shouldn't the mode be set back to none ? If we just clear the fault and reprogram the registers, the torch will be turned back on, and the fault will likely happen again.
Laurent Pinchart wrote: > Hi Sakari, Hi Laurent, > On Tuesday 17 May 2011 07:38:38 Sakari Ailus wrote: [clip] >>> >>> If several applications read controls, only one of them will be notified >>> of faults. Shouldn't clearing the fault be handled explicitly by writing >>> to a control ? I know this changes the API :-) >> >> This is true. >> >> Although I can't imagine right now why two separate processes should be >> so interested in the faults but it is still entirely possible that >> someone does that since it's permitted by the interface. >> >> Having to write zero to faults to clear them isn't good either since it >> might mean missing faults that are triggered between reading and writing >> this control. >> >> Perhaps this would make sense as a file handle specific control? > > Good question. Control events will help I guess, maybe that's the solution. They would help, yes, but in the case of N900 we don't have that luxury since there's no interrupt line from the adp1653 to OMAP. So if one user would read the control, the others would get notified. Yes, that would work, too. The use case is mostly theoretical and that's actually best the hardware can offer, so I think this is good. No special arrangements needed then. So reading the value will reset the faults? >> The control documentation says that the faults are cleared when the >> register is read, but the adp1653 also clears the faults whenever >> writing zero to out_sel which happens also in other circumstances, for >> example when changing mode from flash to torch when the torch intensity >> is zero, or when indicator intensity is zero in other modes. >> >>>> + /* Restore configuration. */ >>>> + rval = adp1653_update_hw(flash); >>>> + if (IS_ERR_VALUE(rval)) >>>> + return rval; >>> >>> Will that produce expected results ? For instance, if a fault was >>> detected during flash strobe, won't it try to re-strobe the flash ? >>> Shouldn't the user >> >> No. Flash is strobed using adp1653_strobe(). >> >>> be required to explicitly re-strobe the flash or turn the torch (or >>> indicator) on after a fault ? Once again this should be clarified in the >>> API :-) >> >> The mode won't be changed from the flash but to strobe again, the user >> has to push V4L2_CID_FLASH_STROBE again. >> >> The adp1653 doesn't have any torch (as such) or indicator faults; some >> other chips do have indicator faults at least. Using the torch mode >> might trigger faults, too, since it's the same LED; just the power isn't >> that high. > > When an over-current fault is detected, shouldn't the mode be set back to none > ? If we just clear the fault and reprogram the registers, the torch will be > turned back on, and the fault will likely happen again. That's a good point. Over-temperature, over-voltage, and short circuit faults should change the LED mode to be set to none. This is likely chip independent behaviour but on the other hand, not all the chips implement all the faults. Regards,
Hi Sakari, On Tuesday 17 May 2011 12:47:25 Sakari Ailus wrote: > Laurent Pinchart wrote: > > On Tuesday 17 May 2011 07:38:38 Sakari Ailus wrote: > [clip] > > >>> If several applications read controls, only one of them will be > >>> notified of faults. Shouldn't clearing the fault be handled explicitly > >>> by writing to a control ? I know this changes the API :-) > >> > >> This is true. > >> > >> Although I can't imagine right now why two separate processes should be > >> so interested in the faults but it is still entirely possible that > >> someone does that since it's permitted by the interface. > >> > >> Having to write zero to faults to clear them isn't good either since it > >> might mean missing faults that are triggered between reading and writing > >> this control. > >> > >> Perhaps this would make sense as a file handle specific control? > > > > Good question. Control events will help I guess, maybe that's the > > solution. > > They would help, yes, but in the case of N900 we don't have that luxury > since there's no interrupt line from the adp1653 to OMAP. So if one user > would read the control, the others would get notified. Yes, that would > work, too. > > The use case is mostly theoretical and that's actually best the hardware > can offer, so I think this is good. No special arrangements needed then. > > So reading the value will reset the faults? OK, let's leave it that way, at least for now. > >> The control documentation says that the faults are cleared when the > >> register is read, but the adp1653 also clears the faults whenever > >> writing zero to out_sel which happens also in other circumstances, for > >> example when changing mode from flash to torch when the torch intensity > >> is zero, or when indicator intensity is zero in other modes. > >> > >>>> + /* Restore configuration. */ > >>>> + rval = adp1653_update_hw(flash); > >>>> + if (IS_ERR_VALUE(rval)) > >>>> + return rval; > >>> > >>> Will that produce expected results ? For instance, if a fault was > >>> detected during flash strobe, won't it try to re-strobe the flash ? > >>> Shouldn't the user > >> > >> No. Flash is strobed using adp1653_strobe(). > >> > >>> be required to explicitly re-strobe the flash or turn the torch (or > >>> indicator) on after a fault ? Once again this should be clarified in > >>> the API :-) > >> > >> The mode won't be changed from the flash but to strobe again, the user > >> has to push V4L2_CID_FLASH_STROBE again. > >> > >> The adp1653 doesn't have any torch (as such) or indicator faults; some > >> other chips do have indicator faults at least. Using the torch mode > >> might trigger faults, too, since it's the same LED; just the power isn't > >> that high. > > > > When an over-current fault is detected, shouldn't the mode be set back to > > none ? If we just clear the fault and reprogram the registers, the torch > > will be turned back on, and the fault will likely happen again. > > That's a good point. > > Over-temperature, over-voltage, and short circuit faults should change the > LED mode to be set to none. This is likely chip independent behaviour but on > the other hand, not all the chips implement all the faults. If the chip doesn't implement faults handling there little we can do. For chips that do implement faults handling, let's switch the LED mode to none when a fault is detected.
Hi Sakari, On Monday 16 May 2011 15:00:39 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. > > Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> > Signed-off-by: Tuukka Toivonen <tuukkat76@gmail.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: David Cohen <dacohen@gmail.com> [snip] > + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, > + V4L2_CID_FLASH_FAULT, 0, V4L2_FLASH_FAULT_OVER_VOLTAGE > + | V4L2_FLASH_FAULT_OVER_TEMPERATURE > + | V4L2_FLASH_FAULT_SHORT_CIRCUIT, 0, 0); You need to mark the fault control as volatile.
Laurent Pinchart wrote: > Hi Sakari, > > On Monday 16 May 2011 15:00:39 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. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> >> Signed-off-by: Tuukka Toivonen <tuukkat76@gmail.com> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: David Cohen <dacohen@gmail.com> > > [snip] > >> + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, >> + V4L2_CID_FLASH_FAULT, 0, V4L2_FLASH_FAULT_OVER_VOLTAGE >> + | V4L2_FLASH_FAULT_OVER_TEMPERATURE >> + | V4L2_FLASH_FAULT_SHORT_CIRCUIT, 0, 0); > > You need to mark the fault control as volatile. Thanks for catching this! I'll fix it for the pull req. Cheers,
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 00f51dd..c004dbb 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -344,6 +344,13 @@ config VIDEO_TCM825X This is a driver for the Toshiba TCM825x VGA camera sensor. It is used for example in Nokia N800. +config VIDEO_ADP1653 + tristate "ADP1653 flash support" + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER + ---help--- + This is a driver for the ADP1653 flash controller. It is used for + example in Nokia N900. + config VIDEO_SAA7110 tristate "Philips SAA7110 video decoder" depends on VIDEO_V4L2 && I2C diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index ace5d8b..abdf021 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -131,6 +131,8 @@ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/ +obj-$(CONFIG_VIDEO_ADP1653) += adp1653.o + obj-$(CONFIG_USB_ZR364XX) += zr364xx.o obj-$(CONFIG_USB_STKWEBCAM) += stkwebcam.o diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c new file mode 100644 index 0000000..92ea38b --- /dev/null +++ b/drivers/media/video/adp1653.c @@ -0,0 +1,453 @@ +/* + * drivers/media/video/adp1653.c + * + * Copyright (C) 2008--2011 Nokia Corporation + * + * Contact: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> + * + * Contributors: + * Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> + * Tuukka Toivonen <tuukkat76@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + * TODO: + * - fault interrupt handling + * - faster strobe (use i/o pin instead of i2c) + * - should ensure that the pin is in some sane state even if not used + * - power doesn't need to be ON if all lights are off + * + */ + +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/slab.h> +#include <linux/version.h> +#include <media/adp1653.h> +#include <media/v4l2-device.h> + +#define TIMEOUT_MAX 820000 +#define TIMEOUT_STEP 54600 +#define TIMEOUT_MIN (TIMEOUT_MAX - ADP1653_REG_CONFIG_TMR_SET_MAX \ + * TIMEOUT_STEP) +#define TIMEOUT_US_TO_CODE(t) ((TIMEOUT_MAX + (TIMEOUT_STEP / 2) - (t)) \ + / TIMEOUT_STEP) +#define TIMEOUT_CODE_TO_US(c) (TIMEOUT_MAX - (c) * TIMEOUT_STEP) + +/* Write values into ADP1653 registers. */ +static int adp1653_update_hw(struct adp1653_flash *flash) +{ + struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev); + u8 out_sel; + u8 config = 0; + int rval; + + out_sel = flash->indicator_intensity->val + << ADP1653_REG_OUT_SEL_ILED_SHIFT; + + switch (flash->led_mode->val) { + case V4L2_FLASH_LED_MODE_NONE: + break; + case V4L2_FLASH_LED_MODE_FLASH: + /* Flash mode, light on with strobe, duration from timer */ + config = ADP1653_REG_CONFIG_TMR_CFG; + config |= TIMEOUT_US_TO_CODE(flash->flash_timeout->val) + << ADP1653_REG_CONFIG_TMR_SET_SHIFT; + break; + case V4L2_FLASH_LED_MODE_TORCH: + /* Torch mode, light immediately on, duration indefinite */ + out_sel |= flash->torch_intensity->val + << ADP1653_REG_OUT_SEL_HPLED_SHIFT; + break; + } + + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, out_sel); + if (rval < 0) + return rval; + + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_CONFIG, config); + if (rval < 0) + return rval; + + return 0; +} + +static int adp1653_strobe(struct adp1653_flash *flash, int enable) +{ + struct i2c_client *client = v4l2_get_subdevdata(&flash->subdev); + u8 out_sel = flash->indicator_intensity->val + << ADP1653_REG_OUT_SEL_ILED_SHIFT; + int rval; + + if (flash->led_mode->val != V4L2_FLASH_LED_MODE_FLASH) + return -EBUSY; + + if (!enable) + return i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, + out_sel); + + out_sel |= flash->flash_intensity->val + << ADP1653_REG_OUT_SEL_HPLED_SHIFT; + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, out_sel); + if (rval) + return rval; + + /* Software strobe using i2c */ + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_SW_STROBE, + ADP1653_REG_SW_STROBE_SW_STROBE); + if (rval) + return rval; + return i2c_smbus_write_byte_data(client, ADP1653_REG_SW_STROBE, 0); +} + +/* -------------------------------------------------------------------------- + * V4L2 controls + */ + +static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl) +{ + struct adp1653_flash *flash = + container_of(ctrl->handler, struct adp1653_flash, ctrls); + 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; + + /* Clear faults. */ + rval = i2c_smbus_write_byte_data(client, ADP1653_REG_OUT_SEL, 0); + if (IS_ERR_VALUE(rval)) + return rval; + + /* Restore configuration. */ + rval = adp1653_update_hw(flash); + if (IS_ERR_VALUE(rval)) + return rval; + + ctrl->cur.val = 0; + + if (fault & ADP1653_REG_FAULT_FLT_SCP) + ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT; + if (fault & ADP1653_REG_FAULT_FLT_OT) + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE; + if (fault & ADP1653_REG_FAULT_FLT_TMR) + ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT; + if (fault & ADP1653_REG_FAULT_FLT_OV) + ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE; + + return 0; +} + +static int adp1653_set_ctrl(struct v4l2_ctrl *ctrl) +{ + struct adp1653_flash *flash = + container_of(ctrl->handler, struct adp1653_flash, ctrls); + + switch (ctrl->id) { + case V4L2_CID_FLASH_STROBE: + return adp1653_strobe(flash, 1); + case V4L2_CID_FLASH_STROBE_STOP: + return adp1653_strobe(flash, 0); + } + + return adp1653_update_hw(flash); +} + +static const struct v4l2_ctrl_ops adp1653_ctrl_ops = { + .g_volatile_ctrl = adp1653_get_ctrl, + .s_ctrl = adp1653_set_ctrl, +}; + +static int adp1653_init_controls(struct adp1653_flash *flash) +{ + v4l2_ctrl_handler_init(&flash->ctrls, 9); + + flash->led_mode = + v4l2_ctrl_new_std_menu(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_LED_MODE, + V4L2_FLASH_LED_MODE_TORCH, ~0x7, 0); + v4l2_ctrl_new_std_menu(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_STROBE_SOURCE, + V4L2_FLASH_STROBE_SOURCE_SOFTWARE, ~0x1, 0); + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_STROBE, 0, 0, 0, 0); + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_STROBE_STOP, 0, 0, 0, 0); + flash->flash_timeout = + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_TIMEOUT, TIMEOUT_MIN, + flash->platform_data->max_flash_timeout, + TIMEOUT_STEP, + flash->platform_data->max_flash_timeout); + flash->flash_intensity = + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_INTENSITY, + ADP1653_FLASH_INTENSITY_MIN, + flash->platform_data->max_flash_intensity, + 1, flash->platform_data->max_flash_intensity); + flash->torch_intensity = + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_TORCH_INTENSITY, + ADP1653_TORCH_INTENSITY_MIN, + flash->platform_data->max_torch_intensity, + 1, flash->platform_data->max_torch_intensity); + flash->indicator_intensity = + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_INDICATOR_INTENSITY, + ADP1653_INDICATOR_INTENSITY_MIN, + flash->platform_data->max_indicator_intensity, + 1, ADP1653_INDICATOR_INTENSITY_MIN); + v4l2_ctrl_new_std(&flash->ctrls, &adp1653_ctrl_ops, + V4L2_CID_FLASH_FAULT, 0, V4L2_FLASH_FAULT_OVER_VOLTAGE + | V4L2_FLASH_FAULT_OVER_TEMPERATURE + | V4L2_FLASH_FAULT_SHORT_CIRCUIT, 0, 0); + + if (flash->ctrls.error) + return flash->ctrls.error; + + flash->subdev.ctrl_handler = &flash->ctrls; + return 0; +} + +/* -------------------------------------------------------------------------- + * V4L2 subdev operations + */ + +static int +adp1653_registered(struct v4l2_subdev *subdev) +{ + struct adp1653_flash *flash = to_adp1653_flash(subdev); + + return adp1653_init_controls(flash); +} + +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) { + dev_err(&client->dev, "failed reading fault register\n"); + return -EIO; + } + + if ((rval & 0x0f) != 0) { + dev_err(&client->dev, "device fault\n"); + 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; +} + +static int +__adp1653_set_power(struct adp1653_flash *flash, int on) +{ + int ret; + + ret = flash->platform_data->power(&flash->subdev, on); + if (ret < 0) + return ret; + + if (!on) + return 0; + + ret = adp1653_init_device(flash); + if (ret < 0) + flash->platform_data->power(&flash->subdev, 0); + + return ret; +} + +static int +adp1653_set_power(struct v4l2_subdev *subdev, int on) +{ + struct adp1653_flash *flash = to_adp1653_flash(subdev); + int ret = 0; + + mutex_lock(&flash->power_lock); + + /* If the power count is modified from 0 to != 0 or from != 0 to 0, + * update the power state. + */ + if (flash->power_count == !on) { + ret = __adp1653_set_power(flash, !!on); + if (ret < 0) + goto done; + } + + /* Update the power count. */ + flash->power_count += on ? 1 : -1; + WARN_ON(flash->power_count < 0); + +done: + mutex_unlock(&flash->power_lock); + return ret; +} + +static int adp1653_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + return adp1653_set_power(sd, 1); +} + +static int adp1653_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + return adp1653_set_power(sd, 0); +} + +static const struct v4l2_subdev_core_ops adp1653_core_ops = { + .s_power = adp1653_set_power, +}; + +static const struct v4l2_subdev_ops adp1653_ops = { + .core = &adp1653_core_ops, +}; + +static const struct v4l2_subdev_internal_ops adp1653_internal_ops = { + .registered = adp1653_registered, + .open = adp1653_open, + .close = adp1653_close, +}; + +/* -------------------------------------------------------------------------- + * I2C driver + */ +#ifdef CONFIG_PM + +static int adp1653_suspend(struct i2c_client *client, pm_message_t mesg) +{ + struct v4l2_subdev *subdev = i2c_get_clientdata(client); + struct adp1653_flash *flash = to_adp1653_flash(subdev); + + if (!flash->power_count) + return 0; + + return __adp1653_set_power(flash, 0); +} + +static int adp1653_resume(struct i2c_client *client) +{ + struct v4l2_subdev *subdev = i2c_get_clientdata(client); + struct adp1653_flash *flash = to_adp1653_flash(subdev); + + if (!flash->power_count) + return 0; + + return __adp1653_set_power(flash, 1); +} + +#else + +#define adp1653_suspend NULL +#define adp1653_resume NULL + +#endif /* CONFIG_PM */ + +static int adp1653_probe(struct i2c_client *client, + const struct i2c_device_id *devid) +{ + struct adp1653_flash *flash; + int ret; + + flash = kzalloc(sizeof(*flash), GFP_KERNEL); + if (flash == NULL) + return -ENOMEM; + + flash->platform_data = client->dev.platform_data; + + mutex_init(&flash->power_lock); + + v4l2_i2c_subdev_init(&flash->subdev, client, &adp1653_ops); + flash->subdev.internal_ops = &adp1653_internal_ops; + flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + + ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0); + if (ret < 0) + kfree(flash); + + return ret; +} + +static int __exit adp1653_remove(struct i2c_client *client) +{ + struct v4l2_subdev *subdev = i2c_get_clientdata(client); + struct adp1653_flash *flash = to_adp1653_flash(subdev); + + v4l2_device_unregister_subdev(&flash->subdev); + v4l2_ctrl_handler_free(&flash->ctrls); + media_entity_cleanup(&flash->subdev.entity); + kfree(flash); + return 0; +} + +static const struct i2c_device_id adp1653_id_table[] = { + { ADP1653_NAME, 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, adp1653_id_table); + +static struct i2c_driver adp1653_i2c_driver = { + .driver = { + .name = ADP1653_NAME, + }, + .probe = adp1653_probe, + .remove = __exit_p(adp1653_remove), + .suspend = adp1653_suspend, + .resume = adp1653_resume, + .id_table = adp1653_id_table, +}; + +static int __init adp1653_init(void) +{ + int rval; + + rval = i2c_add_driver(&adp1653_i2c_driver); + if (rval) + printk(KERN_ALERT "%s: failed at i2c_add_driver\n", __func__); + + return rval; +} + +static void __exit adp1653_exit(void) +{ + i2c_del_driver(&adp1653_i2c_driver); +} + +module_init(adp1653_init); +module_exit(adp1653_exit); + +MODULE_AUTHOR("Sakari Ailus <sakari.ailus@nokia.com>"); +MODULE_DESCRIPTION("Analog Devices ADP1653 LED flash driver"); +MODULE_LICENSE("GPL"); diff --git a/include/media/adp1653.h b/include/media/adp1653.h new file mode 100644 index 0000000..1da9964 --- /dev/null +++ b/include/media/adp1653.h @@ -0,0 +1,94 @@ +/* + * include/media/adp1653.h + * + * Copyright (C) 2008--2011 Nokia Corporation + * + * Contact: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> + * + * Contributors: + * Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> + * Tuukka Toivonen <tuukkat76@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#ifndef ADP1653_H +#define ADP1653_H + +#include <linux/i2c.h> +#include <linux/mutex.h> +#include <linux/videodev2.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-subdev.h> + +#define ADP1653_NAME "adp1653" +#define ADP1653_I2C_ADDR (0x60 >> 1) + +/* Register definitions */ +#define ADP1653_REG_OUT_SEL 0x00 +#define ADP1653_REG_OUT_SEL_HPLED_MAX 0x1f +#define ADP1653_REG_OUT_SEL_HPLED_SHIFT 3 +#define ADP1653_REG_OUT_SEL_ILED_MAX 0x07 +#define ADP1653_REG_OUT_SEL_ILED_SHIFT 0 + +#define ADP1653_REG_CONFIG 0x01 +#define ADP1653_REG_CONFIG_TMR_CFG (1 << 4) +#define ADP1653_REG_CONFIG_TMR_SET_MAX 0x0f +#define ADP1653_REG_CONFIG_TMR_SET_SHIFT 0 + +#define ADP1653_REG_SW_STROBE 0x02 +#define ADP1653_REG_SW_STROBE_SW_STROBE (1 << 0) + +#define ADP1653_REG_FAULT 0x03 +#define ADP1653_REG_FAULT_FLT_SCP (1 << 3) +#define ADP1653_REG_FAULT_FLT_OT (1 << 2) +#define ADP1653_REG_FAULT_FLT_TMR (1 << 1) +#define ADP1653_REG_FAULT_FLT_OV (1 << 0) + +#define ADP1653_INDICATOR_INTENSITY_MIN 0 +#define ADP1653_INDICATOR_INTENSITY_MAX ADP1653_REG_OUT_SEL_ILED_MAX +#define ADP1653_TORCH_INTENSITY_MIN 0 +#define ADP1653_TORCH_INTENSITY_MAX 11 +#define ADP1653_FLASH_INTENSITY_MIN 12 +#define ADP1653_FLASH_INTENSITY_MAX ADP1653_REG_OUT_SEL_HPLED_MAX + +struct adp1653_platform_data { + int (*power)(struct v4l2_subdev *sd, int on); + + u32 max_flash_timeout; /* flash light timeout in us */ + u32 max_flash_intensity; /* led intensity, flash mode */ + u32 max_torch_intensity; /* led intensity, torch mode */ + u32 max_indicator_intensity; /* indicator led intensity */ +}; + +#define to_adp1653_flash(sd) container_of(sd, struct adp1653_flash, subdev) + +struct adp1653_flash { + struct v4l2_subdev subdev; + struct adp1653_platform_data *platform_data; + + struct v4l2_ctrl_handler ctrls; + struct v4l2_ctrl *led_mode; + struct v4l2_ctrl *flash_timeout; + struct v4l2_ctrl *flash_intensity; + struct v4l2_ctrl *torch_intensity; + struct v4l2_ctrl *indicator_intensity; + + struct mutex power_lock; + int power_count; +}; + +#endif /* ADP1653_H */