Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

Pavel Machek
2016-11-20T15:21:30Z

[-- Attachment #1: Type: text/plain, Size: 5801 bytes --]

Hi!

> > +static void et8ek8_reglist_to_mbus(const struct et8ek8_reglist *reglist,
> > + struct v4l2_mbus_framefmt *fmt)
> > +{
> > + fmt->width = reglist->mode.window_width;
> > + fmt->height = reglist->mode.window_height;
> > +
> > + if (reglist->mode.pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
>
> The driver doesn't really need to deal with pixel formats. Could you use
> media bus formats instead, and rename the fields accordingly?
>
> The reason why it did use pixel formats was that (V4L2) media bus formats
> did not exist when the driver was written. :-)

Makes sense...

Something like this? [untested, will test complete changes.]

Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 0301e81..eb131b2 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -395,11 +395,7 @@ static void et8ek8_reglist_to_mbus(const struct et8ek8_reglist *reglist,
{
fmt->width = reglist->mode.window_width;
fmt->height = reglist->mode.window_height;
-
- if (reglist->mode.pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
- fmt->code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8;
- else
- fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+ fmt->code = reglist->mode.bus_format;
}

static struct et8ek8_reglist *et8ek8_reglist_find_mode_fmt(
@@ -538,7 +534,7 @@ static int et8ek8_reglist_import(struct i2c_client *client,
__func__,
list->type,
list->mode.window_width, list->mode.window_height,
- list->mode.pixel_format,
+ list->mode.bus_format,
list->mode.timeperframe.numerator,
list->mode.timeperframe.denominator,
(void *)meta->reglist[nlists].ptr);
@@ -967,21 +963,18 @@ static int et8ek8_enum_mbus_code(struct v4l2_subdev *subdev,
continue;

for (i = 0; i < npixelformat; i++) {
- if (pixelformat[i] == mode->pixel_format)
+ if (pixelformat[i] == mode->bus_format)
break;
}
if (i != npixelformat)
continue;

if (code->index == npixelformat) {
- if (mode->pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
- code->code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8;
- else
- code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+ code->code = mode->bus_format;
return 0;
}

- pixelformat[npixelformat] = mode->pixel_format;
+ pixelformat[npixelformat] = mode->bus_format;
npixelformat++;
}

diff --git a/drivers/media/i2c/et8ek8/et8ek8_mode.c b/drivers/media/i2c/et8ek8/et8ek8_mode.c
index 956fc60..12998d8 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_mode.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_mode.c
@@ -59,7 +59,7 @@ static struct et8ek8_reglist mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
},
.max_exp = 2012,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -160,7 +160,7 @@ static struct et8ek8_reglist mode1_16vga_2592x1968_13_12fps_dpcm10_8 = {
},
.max_exp = 2012,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10DPCM8,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
.sensitivity = 65536
},
.regs = {
@@ -216,7 +216,7 @@ static struct et8ek8_reglist mode3_4vga_1296x984_29_99fps_dpcm10_8 = {
},
.max_exp = 1004,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10DPCM8,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
.sensitivity = 65536
},
.regs = {
@@ -272,7 +272,7 @@ static struct et8ek8_reglist mode4_svga_864x656_29_88fps = {
},
.max_exp = 668,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -328,7 +328,7 @@ static struct et8ek8_reglist mode5_vga_648x492_29_93fps = {
},
.max_exp = 500,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -384,7 +384,7 @@ static struct et8ek8_reglist mode2_16vga_2592x1968_3_99fps = {
},
.max_exp = 6092,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -439,7 +439,7 @@ static struct et8ek8_reglist mode_648x492_5fps = {
},
.max_exp = 500,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -495,7 +495,7 @@ static struct et8ek8_reglist mode3_4vga_1296x984_5fps = {
},
.max_exp = 2996,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -551,7 +551,7 @@ static struct et8ek8_reglist mode_4vga_1296x984_25fps_dpcm10_8 = {
},
.max_exp = 1052,
/* .max_gain = 0, */
- .pixel_format = V4L2_PIX_FMT_SGRBG10DPCM8,
+ .bus_format = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
.sensitivity = 65536
},
.regs = {
diff --git a/drivers/media/i2c/et8ek8/et8ek8_reg.h b/drivers/media/i2c/et8ek8/et8ek8_reg.h
index 9970bff..64a8fb7 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_reg.h
+++ b/drivers/media/i2c/et8ek8/et8ek8_reg.h
@@ -48,7 +48,7 @@ struct et8ek8_mode {
u32 ext_clock; /* in Hz */
struct v4l2_fract timeperframe;
u32 max_exp; /* Maximum exposure value */
- u32 pixel_format; /* V4L2_PIX_FMT_xxx */
+ u32 bus_format; /* MEDIA_BUS_FMT_ */
u32 sensitivity; /* 16.16 fixed point */
};

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]