Laurent Pinchart wrote: > Hi Sakari, Hi Laurent, Thanks for the comments! > On Monday 28 March 2011 14:55:40 Sakari Ailus wrote: > > [snip] > >> V4L2_CID_FLASH_STROBE_MODE (menu; LED) >> >> Use hardware or software strobe. If hardware strobe is selected, the >> flash controller is a slave in the system where the sensor produces the >> strobe signal to the flash. >> >> In this case the flash controller setup is limited to programming strobe >> timeout and power (LED flash) and the sensor controls the timing and >> length of the strobe. >> >> enum v4l2_flash_strobe_mode { >> V4L2_FLASH_STROBE_MODE_SOFTWARE, >> V4L2_FLASH_STROBE_MODE_EXT_STROBE, >> }; > > [snip] > >> V4L2_CID_FLASH_LED_MODE (menu; LED) >> >> enum v4l2_flash_led_mode { >> V4L2_FLASH_LED_MODE_FLASH = 1, >> V4L2_FLASH_LED_MODE_TORCH, >> }; > > Thinking about this some more, shouldn't we combine the two controls ? They > are basically used to configure how the flash LED is controlled: manually > (torch mode), automatically by the flash controller (software strobe mode) or > automatically by an external component (external strobe mode). That's a good question. The adp1653 supports also additional control (not implemented in the driver, though) that affect hardware strobe length. Based on register setting, the led will be on after strobe either until the timeout expires, or until the strobe signal is high. Should this be also part of the same control, or a different one? Even without this, we'd have: V4L2_FLASH_MODE_OFF V4L2_FLASH_MODE_TORCH V4L2_FLASH_MODE_SOFTWARE_STROBE V4L2_FLASH_MODE_EXTERNAL_STROBE Additionally, this might be V4L2_FLASH_MODE_EXTERNAL_STROBE_EDGE It's true that these are mutually exclusive. I think this is about whether we want to specify the operation of the flash explicitly here or allow extending the interface later on when new hardware is available by adding new controls. There are upsides and downsides in each approach. There could be additional differentiating factors to the functionalty later on, like the torch/video light differentiation that some hardware does --- who knows based on what? I perhaps wouldn't combine the controls. What do you think? -- Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html