Hi,
On Thursday 04 November 2010 04:44:02 Laurent Pinchart wrote:
> On Thursday 28 October 2010 11:57:34 Tao, Jing wrote:
> > Thanks a lot for your kind remind, Greg. I repaired the problem using the
> > checkpatch.pl, please have a look:
> >
> > This is the patch for the LED Flash Driver, the follow new functions are
> > added: --LED Mode Selection
> > --Flash Time Adjust
> > --Flash Current Adjust
> > --Torch Current Adjust
> > --Indicator Current Adjust
> > --OSPM
> > --Run-time PM
> >
> > According to Arjan's comment:
> > --dev_dbg is removed from set_reg function
> > --some unnecessary prototypes were cleaned up
> >
> > According to Alan's comment:
> > --Check return value of i2c_read/i2c_write
>
> Thanks for the patch.
>
> [snip]
>
> > diff --git a/drivers/staging/mfld_ledflash/lm3555.h
> > b/drivers/staging/mfld_ledflash/lm3555.h new file mode 100644
> > index 0000000..0fc290d
> > --- /dev/null
> > +++ b/drivers/staging/mfld_ledflash/lm3555.h
>
> [snip]
>
> > +/* Field for ioctl */
> > +/* Flash strobe control */
> > +#define V4L2_CID_FLASH_STROBE _IOW('v', 1, int)
> > +/* Flash timeout setting */
> > +#define V4L2_CID_FLASH_TIMEOUT _IOW('v', 2, int)
> > +/* Flash intensity setting */
> > +#define V4L2_CID_FLASH_INTENSITY _IOW('v', 3, int)
> > +/* Torch intensity setting */
> > +#define V4L2_CID_TORCH_INTENSITY _IOW('v', 4, int)
> > +/* Indicator intensity setting */
> > +#define V4L2_CID_INDICATOR_INTENSITY _IOW('v', 5, int)
>
> You're defining ioctl values, why are they named like V4L2 controls ?
>
> [snip]
>
> > diff --git a/drivers/staging/mfld_ledflash/mfld_ledflash.c
> > b/drivers/staging/mfld_ledflash/mfld_ledflash.c index 624b8b1..3ba6bc1
> > 100644
> > --- a/drivers/staging/mfld_ledflash/mfld_ledflash.c
> > +++ b/drivers/staging/mfld_ledflash/mfld_ledflash.c
>
> [snip]
>
> > +static int mfld_ledflash_command(struct i2c_client *client,
> > + unsigned int cmd, void *arg)
> > +{
> > + char *input = arg;
> > + int ret = 0;
> > +
> > + switch (cmd) {
> > + case V4L2_CID_FLASH_STROBE:
> > + ret = mfld_ledflash_set_extstrobe(client, *input);
> > + break;
> > + case V4L2_CID_FLASH_TIMEOUT:
> > + ret = mfld_ledflash_set_flashtime(client, *input);
> > + break;
> > + case V4L2_CID_FLASH_INTENSITY:
> > + ret = mfld_ledflash_set_flashcurrent(client, *input);
> > + break;
> > + case V4L2_CID_TORCH_INTENSITY:
> > + ret = mfld_ledflash_set_assistlightcurrent(client, *input);
> > + break;
> > + case V4L2_CID_INDICATOR_INTENSITY:
> > + ret = mfld_ledflash_set_ic(client, *input);
> > + break;
> > + default:
> > + ret = EINVAL;
> > + break;
> > + }
>
> The right way to implement this is to make the flash driver a V4L2
> sub-device and use V4L2 subdev control operations.
>
> See drivers/media/video/adp1653.c in http://gitorious.org/maemo-
> multimedia/omap3isp-rx51 for a flash driver example.
Any update on this ?
--
Regards,
Laurent Pinchart
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel
