Hi Philipp, Thank you for the patch. On Monday 26 May 2014 16:03:05 Philipp Zabel wrote: > From the looks of it, mt9v022 and mt9v032 are very similar, > as are mt9v024 and mt9v034. With minimal changes it is possible > to support mt9v02[24] with the same driver. > > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > --- > drivers/media/i2c/mt9v032.c | 56 ++++++++++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c > index 052e754..617b77f 100644 > --- a/drivers/media/i2c/mt9v032.c > +++ b/drivers/media/i2c/mt9v032.c > @@ -1,5 +1,5 @@ > /* > - * Driver for MT9V032 CMOS Image Sensor from Micron > + * Driver for MT9V022, MT9V024, MT9V032, and MT9V034 CMOS Image Sensors > * > * Copyright (C) 2010, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > * > @@ -68,6 +68,7 @@ > #define MT9V032_CHIP_CONTROL_MASTER_MODE (1 << 3) > #define MT9V032_CHIP_CONTROL_DOUT_ENABLE (1 << 7) > #define MT9V032_CHIP_CONTROL_SEQUENTIAL (1 << 8) > +#define MT9V024_CHIP_CONTROL_DEF_PIX_CORRECT (1 << 9) > #define MT9V032_SHUTTER_WIDTH1 0x08 > #define MT9V032_SHUTTER_WIDTH2 0x09 > #define MT9V032_SHUTTER_WIDTH_CONTROL 0x0a > @@ -133,8 +134,12 @@ > #define MT9V032_THERMAL_INFO 0xc1 > > enum mt9v032_model { > - MT9V032_MODEL_V032_COLOR, > - MT9V032_MODEL_V032_MONO, > + MT9V032_MODEL_V022_COLOR, /* MT9V022IX7ATC */ > + MT9V032_MODEL_V022_MONO, /* MT9V022IX7ATM */ > + MT9V032_MODEL_V024_COLOR, /* MT9V024IA7XTC */ > + MT9V032_MODEL_V024_MONO, /* MT9V024IA7XTM */ > + MT9V032_MODEL_V032_COLOR, /* MT9V032C12STM */ > + MT9V032_MODEL_V032_MONO, /* MT9V032C12STC */ > MT9V032_MODEL_V034_COLOR, > MT9V032_MODEL_V034_MONO, > }; > @@ -160,14 +165,14 @@ struct mt9v032_model_info { > }; > > static const struct mt9v032_model_version mt9v032_versions[] = { > - { MT9V032_CHIP_ID_REV1, "MT9V032 rev1/2" }, > - { MT9V032_CHIP_ID_REV3, "MT9V032 rev3" }, > - { MT9V034_CHIP_ID_REV1, "MT9V034 rev1" }, > + { MT9V032_CHIP_ID_REV1, "MT9V022/MT9V032 rev1/2" }, > + { MT9V032_CHIP_ID_REV3, "MT9V022/MT9V032 rev3" }, > + { MT9V034_CHIP_ID_REV1, "MT9V024/MT9V034 rev1" }, > }; > > static const struct mt9v032_model_data mt9v032_model_data[] = { > { > - /* MT9V032 revisions 1/2/3 */ > + /* MT9V022, MT9V032 revisions 1/2/3 */ > .min_row_time = 660, > .min_hblank = MT9V032_HORIZONTAL_BLANKING_MIN, > .min_vblank = MT9V032_VERTICAL_BLANKING_MIN, > @@ -176,7 +181,7 @@ static const struct mt9v032_model_data > mt9v032_model_data[] = { .max_shutter = MT9V032_TOTAL_SHUTTER_WIDTH_MAX, > .pclk_reg = MT9V032_PIXEL_CLOCK, > }, { > - /* MT9V034 */ > + /* MT9V024, MT9V034 */ > .min_row_time = 690, > .min_hblank = MT9V034_HORIZONTAL_BLANKING_MIN, > .min_vblank = MT9V034_VERTICAL_BLANKING_MIN, > @@ -188,6 +193,22 @@ static const struct mt9v032_model_data > mt9v032_model_data[] = { }; > > static const struct mt9v032_model_info mt9v032_models[] = { > + [MT9V032_MODEL_V022_COLOR] = { > + .data = &mt9v032_model_data[0], > + .color = true, > + }, > + [MT9V032_MODEL_V022_MONO] = { > + .data = &mt9v032_model_data[0], > + .color = false, > + }, > + [MT9V032_MODEL_V024_COLOR] = { > + .data = &mt9v032_model_data[1], > + .color = true, > + }, > + [MT9V032_MODEL_V024_MONO] = { > + .data = &mt9v032_model_data[1], > + .color = false, > + }, > [MT9V032_MODEL_V032_COLOR] = { > .data = &mt9v032_model_data[0], > .color = true, > @@ -415,7 +436,6 @@ static int mt9v032_s_stream(struct v4l2_subdev *subdev, > int enable) struct i2c_client *client = v4l2_get_subdevdata(subdev); > struct mt9v032 *mt9v032 = to_mt9v032(subdev); > struct v4l2_rect *crop = &mt9v032->crop; > - unsigned int read_mode; > unsigned int hbin; > unsigned int vbin; > int ret; > @@ -426,11 +446,9 @@ static int mt9v032_s_stream(struct v4l2_subdev *subdev, > int enable) /* Configure the window size and row/column bin */ > hbin = fls(mt9v032->hratio) - 1; > vbin = fls(mt9v032->vratio) - 1; > - read_mode = mt9v032_read(client, MT9V032_READ_MODE); > - read_mode &= ~0xff; /* bits 0x300 are reserved, on MT9V024 */ > - read_mode |= hbin << MT9V032_READ_MODE_COLUMN_BIN_SHIFT | > - vbin << MT9V032_READ_MODE_ROW_BIN_SHIFT; > - ret = mt9v032_write(client, MT9V032_READ_MODE, read_mode); > + ret = mt9v032_write(client, MT9V032_READ_MODE, > + hbin << MT9V032_READ_MODE_COLUMN_BIN_SHIFT | > + vbin << MT9V032_READ_MODE_ROW_BIN_SHIFT); Doesn't this change completely reverse one of your previous patches ? Is it needed, or is it included here by mistake ? > if (ret < 0) > return ret; > > @@ -902,6 +920,12 @@ static int mt9v032_probe(struct i2c_client *client, > mt9v032->pdata = pdata; > mt9v032->model = (const void *)did->driver_data; > > + /* Keep defective pixel correction enabled on MT9V024 */ > + ret = mt9v032_read(client, MT9V032_CHIP_CONTROL); > + if (ret < 0) > + return ret; > + mt9v032->chip_control = ret & MT9V024_CHIP_CONTROL_DEF_PIX_CORRECT; On the MT9V034 bit 9 is marked as reserved according to revision A of the datasheet, defaults to 1 after power-up and must be written to 0 for proper operation. This could thus break operation on the MT9V034. I don't have a copy of the MT9V022 register reference manual to check what happens on that chip though. > + > v4l2_ctrl_handler_init(&mt9v032->ctrls, 10); > > v4l2_ctrl_new_std(&mt9v032->ctrls, &mt9v032_ctrl_ops, > @@ -1015,6 +1039,10 @@ static int mt9v032_remove(struct i2c_client *client) > } > > static const struct i2c_device_id mt9v032_id[] = { > + { "mt9v022", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V022_COLOR] }, > + { "mt9v022m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V022_MONO] }, > + { "mt9v024", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V024_COLOR] }, > + { "mt9v024m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V024_MONO] }, > { "mt9v032", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V032_COLOR] }, > { "mt9v032m", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V032_MONO] }, > { "mt9v034", (kernel_ulong_t)&mt9v032_models[MT9V032_MODEL_V034_COLOR] }, -- Regards, Laurent Pinchart -- 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