Hi, Laurent:
Yes, the driver should be written in sub device form finally, and the
sub device driver would be in the folder drivers/media/video/mfld_ci in future.
But the driver in folder /drivers/staging/mfld_ledflash is only for function
testing, so it can be used independently(if you write it in sub device form,
you cannot test it very conveniently ), that why the driver is not written in
subdevice form in this folder.
Thanks~
Jing
Regards~
-----Original Message-----
From: Laurent Pinchart [mailto:[email protected]]
Sent: Tuesday, November 09, 2010 4:18 PM
To: Tao, Jing
Cc: [email protected]; [email protected]
Subject: Re: [Meego-kernel] [PATCH] New functions added to the LED Driver
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
