Hi Sylwester Thank you for the review. On 2 August 2012 21:11, Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx> wrote: > > Hi Sangwook, > > On 08/02/2012 03:42 PM, Sangwook Lee wrote: > > The following 2 patches add driver for S5K4ECGX sensor with embedded ISP SoC, > > and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor from Samsung > > > > Changes since v2: > > - added GPIO (reset/stby) and regulators > > - updated I2C read/write, based on s5k6aa datasheet > > - fixed set_fmt errors > > - reduced register tables a bit > > - removed vmalloc > > It looks like a great improvement, well done! Thanks! > > In the S5K4CAGX sensor datasheet, that can be found on the internet, > there is 0x0000...0x002E registers description, which look very much > same as in S5K6AAFX case and likely is also valid for S5K4CAGX. [snip] > > > What do you think about converting s5k4ecgx_img_regs arrays (it has > over 2700 entries) to a firmware file and adding some simple parser > to the driver ? Then we would have the problem solved in most part. > Thanks, fair enough. let me try this. > > Regarding various preview resolution set up, the difference in all > those s5k4ecgx_*_preview[] arrays is rather small, only register > values differ, e.g. for 640x480 and 720x480 there is only 8 different > entries: > Ok, let me reduce table size again. > > $ diff -a s5k4ec_640.txt s5k4ec_720.txt > 1c1 > < static const struct regval_list s5k4ecgx_640_preview[] = { > --- > > static const struct regval_list s5k4ecgx_720_preview[] = { > 3c3 > < { 0x70000252, 0x0780 }, > --- > > { 0x70000252, 0x06a8 }, > 5c5 [snip] > > < { 0x70000256, 0x000c }, > > { 0x700002a6, 0x02d [snip] > > Could you please try to implement a function that replaces those tables, > based s5k6aa_set_prev_config() and s5k6aa_set_output_framefmt() ? > I was thinking about this, but this seems to be is a bit time-consuming because I have to do this just due to lack of s5k4ecgx hardware information. let me try it later once this patch is accepted. Thanks Sangwook > Regards, > Sylwester -- 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