Hi HeungJun, On 03/16/2011 02:38 PM, Kim, Heungjun wrote: > Add I2C/V4L2 subdev driver for M-5MOLS camera sensor with integrated > image signal processor. > > Signed-off-by: Heungjun Kim<riverful.kim@xxxxxxxxxxx> > Signed-off-by: Sylwester Nawrocki<s.nawrocki@xxxxxxxxxxx> > Signed-off-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx> > --- > > Hi Hans and everyone, > > This is sixth version of M-5MOLS 8 Mega Pixel camera sensor. And, if you see Would be good to indicate the version in the subject too. > previous version, you can find at: > http://www.spinics.net/lists/linux-media/msg29350.html > > This driver patch is fixed several times, and the important issues is almost > corrected. And, I hope that this is the last version one merged for 2.6.39. > I look forward to be reviewed one more time. > > The summary of this version's feature is belows: > > 1. Add focus control > : I've suggest menu type focus control, but I agreed this version is > not yet the level accepted. So, I did not use focus control which > I suggest. > The M-5MOLS focus routine takes some time to execute. But, the user > application calling v4l2 control, should not hanged while streaming > using q/dqbuf. So, I use workqueue. I want to discuss the focus > subject on mailnglist next time. > > 2. Add irq routine > : M-5MOLS can issues using GPIO pin, and I insert the basic routine > of irq. This version handles only the Auto focus interrupt source. > It shows only lens focusing status, don't any action now. > > 3. Speed-up whole I2C operation > : I've tested several times for decreasing the stabilization time > while I2C communication, and I have find proper time. Of course, > it's more faster than previous version. That sounds good. Do you think the delays before I2C read/write could be avoided in some (if not all) cases by using some status registers polling? > > 4. Let streamon() be called once at the streamming > : It might be an issue, videobuf2 framework calls streamon when > qbuf() for enqueueing. It means, the driver's streamon() function No, that's not really the case. At last videobuf2 buf_queue op might be called in response to VIDIOC_STREAMON. Certainly there must be some bug in the host driver if subdev's s_stream is being called repeatedly. > might be callled continuously if there is no proper handling in the > subdev driver, and low the framerate by adding unneeded I2C operation. > The M-5MOLS sensor needs command one time for streaming. If commands > once, this sensor streams continuously, and this version handles it. > > 5. Update FW > : It's a little tricky. Originally, the v4l2 frame provide load_fw(), > but, there is the occasion which should do in openning the videonode, > and it's the same occasion with us. So, if it's not wrong or it makes > any problem, we hope to insert m5mols_update_fw() with weak attribute. > And, I'm sorry that the fw updating code is confidential. unserstand > by favor, plz. > > And, as always, this driver is tested on s5pc210 board using s5p-fimc driver. > > I'll wait for reviewing. > > Thanks and Regards, > Heungjun Kim > Samsung Electronics DMC R&D Center > > drivers/media/video/Kconfig | 2 + > drivers/media/video/Makefile | 1 + > drivers/media/video/m5mols/Kconfig | 5 + > drivers/media/video/m5mols/Makefile | 3 + > drivers/media/video/m5mols/m5mols.h | 251 ++++++ > drivers/media/video/m5mols/m5mols_controls.c | 213 +++++ > drivers/media/video/m5mols/m5mols_core.c | 1062 ++++++++++++++++++++++++++ > drivers/media/video/m5mols/m5mols_reg.h | 218 ++++++ > include/media/m5mols.h | 35 + > 9 files changed, 1790 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/video/m5mols/Kconfig > create mode 100644 drivers/media/video/m5mols/Makefile > create mode 100644 drivers/media/video/m5mols/m5mols.h > create mode 100644 drivers/media/video/m5mols/m5mols_controls.c > create mode 100644 drivers/media/video/m5mols/m5mols_core.c > create mode 100644 drivers/media/video/m5mols/m5mols_reg.h > create mode 100644 include/media/m5mols.h > ... > diff --git a/drivers/media/video/m5mols/m5mols_controls.c b/drivers/media/video/m5mols/m5mols_controls.c > new file mode 100644 > index 0000000..784b764 > --- /dev/null > +++ b/drivers/media/video/m5mols/m5mols_controls.c > @@ -0,0 +1,213 @@ > +/* > + * Controls for M-5MOLS 8M Pixel camera sensor with ISP > + * > + * Copyright (C) 2011 Samsung Electronics Co., Ltd > + * Author: HeungJun Kim, riverful.kim@xxxxxxxxxxx > + * > + * Copyright (C) 2009 Samsung Electronics Co., Ltd > + * Author: Dongsoo Nathaniel Kim, dongsoo45.kim@xxxxxxxxxxx > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include<linux/i2c.h> > +#include<linux/delay.h> > +#include<linux/videodev2.h> > +#include<media/v4l2-ctrls.h> > + > +#include "m5mols.h" > +#include "m5mols_reg.h" > + > +/* The externing camera control functions */ > +int m5mols_lock_ae(struct m5mols_info *info, bool lock) > +{ > + struct v4l2_subdev *sd =&info->sd; > + > + return i2c_w8_ae(sd, CAT3_AE_LOCK, !!(info->lock_ae = lock)); Shouldn't be info->lock_ae assigned a new value only in case I2C write succeeds? > +} > + > +int m5mols_lock_awb(struct m5mols_info *info, bool lock) > +{ > + struct v4l2_subdev *sd =&info->sd; > + > + info->lock_awb = lock; > + > + return i2c_w8_wb(sd, CAT6_AWB_LOCK, !!(info->lock_awb = lock)); Ditto. > +/* > + * m5mols_sensor_armboot() - booting M-5MOLS internal ARM core-controller. > + * > + * It makes to ready M-5MOLS for I2C& MIPI interface. After it's powered up, > + * it activates if it gets armboot command for I2C interface. After getting > + * cmd, it must wait about least 520ms referenced by M-5MOLS datasheet. > + */ > +static int m5mols_sensor_armboot(struct v4l2_subdev *sd) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(sd); > + u32 reg; > + int ret; > + > + /* ARM(M-5MOLS core) booting */ > + ret = i2c_w8_flash(sd, CATF_CAM_START, true); > + if (ret< 0) > + return ret; > + > + msleep(520); Don't you consider using a waitqueue and a relevant interrupt generated by the ISP when it has completed booting? This would allow to decrease the delay to an optimal minimum. -- Regards, Sylwester Nawrocki -- 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
