Hi Daniel,
Thanks for the update. The driver is very nicely written in general btw. One
comment below.
On Tue, Jan 28, 2014 at 03:55:58PM +0900, Daniel Jeong wrote:
...
> +/*
> + * struct lm3646_flash
> + *
> + * @pdata: platform data
> + * @regmap: reg. map for i2c
> + * @lock: muxtex for serial access.
> + * @led_mode: V4L2 LED mode
> + * @ctrls_led: V4L2 contols
> + * @subdev_led: V4L2 subdev
> + */
> +struct lm3646_flash {
> + struct device *dev;
> + struct lm3646_platform_data *pdata;
> + struct regmap *regmap;
> +
> + enum v4l2_flash_led_mode led_mode;
> + struct v4l2_ctrl_handler ctrls_led;
> + struct v4l2_subdev subdev_led;
> +};
> +
> +#define to_lm3646_flash(_ctrl) \
> + container_of(_ctrl->handler, struct lm3646_flash, ctrls_led)
> +
> +/* enable mode control */
> +static int lm3646_mode_ctrl(struct lm3646_flash *flash)
> +{
> + int rval = -EINVAL;
> +
> + switch (flash->led_mode) {
> + case V4L2_FLASH_LED_MODE_NONE:
> + rval = regmap_update_bits(flash->regmap,
> + REG_ENABLE, MASK_ENABLE, MODE_SHDN);
> + break;
> + case V4L2_FLASH_LED_MODE_TORCH:
> + rval = regmap_update_bits(flash->regmap,
> + REG_ENABLE, MASK_ENABLE, MODE_TORCH);
> + break;
> + case V4L2_FLASH_LED_MODE_FLASH:
> + rval = regmap_update_bits(flash->regmap,
> + REG_ENABLE, MASK_ENABLE, MODE_FLASH);
> + break;
> + }
> + return rval;
> +}
> +
> +/* V4L2 controls */
> +static int lm3646_get_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct lm3646_flash *flash = to_lm3646_flash(ctrl);
> + int rval = -EINVAL;
> +
> + if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> + s32 fault = 0;
> + unsigned int reg_val;
> + rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> + if (rval < 0)
> + return rval;
> +
> + if (reg_val & FAULT_TIMEOUT)
> + fault |= V4L2_FLASH_FAULT_TIMEOUT;
> + if (reg_val & FAULT_SHORT_CIRCUIT)
> + fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> + if (reg_val & FAULT_UVLO)
> + fault |= V4L2_FLASH_FAULT_UVLO;
> + if (reg_val & FAULT_IVFM)
> + fault |= V4L2_FLASH_FAULT_IVFM;
> + if (reg_val & FAULT_OCP)
> + fault |= V4L2_FLASH_FAULT_OVER_CURRENT;
> + if (reg_val & FAULT_OVERTEMP)
> + fault |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
> + if (reg_val & FAULT_NTC_TRIP)
> + fault |= V4L2_FLASH_FAULT_NTC_TRIP;
> + if (reg_val & FAULT_OVP)
> + fault |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
> +
> + ctrl->cur.val = fault;
> + }
> +
> + return rval;
> +}
> +
> +static int lm3646_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct lm3646_flash *flash = to_lm3646_flash(ctrl);
> + u8 bval;
> + int rval = -EINVAL;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_FLASH_LED_MODE:
> + flash->led_mode = ctrl->val;
Do you need to keep led_mode in struct lm3646_flash? Could you access the
value in struct v4l2_ctrl directly? (See smiapp-core.c for an example.)
> + if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH)
> + rval = lm3646_mode_ctrl(flash);
> + break;
> +
> + case V4L2_CID_FLASH_STROBE_SOURCE:
> + rval = regmap_update_bits(flash->regmap,
> + REG_STROBE_SRC, MASK_STROBE_SRC,
> + (ctrl->val) << 7);
> + break;
> +
> + case V4L2_CID_FLASH_STROBE:
> + if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH)
> + return rval;
> + rval = lm3646_mode_ctrl(flash);
> + break;
> +
> + case V4L2_CID_FLASH_STROBE_STOP:
> + if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH)
> + return rval;
> + flash->led_mode = V4L2_FLASH_LED_MODE_NONE;
> + rval = lm3646_mode_ctrl(flash);
> + break;
> +
> + case V4L2_CID_FLASH_TIMEOUT:
> + bval = LM3646_FLASH_TOUT_ms_TO_REG(ctrl->val);
> + rval = regmap_update_bits(flash->regmap,
> + REG_FLASH_TOUT, MASK_FLASH_TOUT,
> + bval);
> + break;
> +
> + case V4L2_CID_FLASH_INTENSITY:
> + bval = LM3646_TOTAL_FLASH_BRT_uA_TO_REG(ctrl->val);
> + rval = regmap_update_bits(flash->regmap,
> + REG_FLASH_BR, MASK_FLASH_BR, bval);
> + break;
> +
> + case V4L2_CID_FLASH_TORCH_INTENSITY:
> + bval = LM3646_TOTAL_TORCH_BRT_uA_TO_REG(ctrl->val);
> + rval = regmap_update_bits(flash->regmap,
> + REG_TORCH_BR, MASK_TORCH_BR,
> + bval << 4);
> + break;
> + }
> +
> + return rval;
> +}
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
