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

From: Pavel Machek <pavel@ucw.cz>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: ivo.g.dimitrov.75@gmail.com, sre@kernel.org,
	pali.rohar@gmail.com, linux-media@vger.kernel.org,
	galak@codeaurora.org, mchehab@osg.samsung.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor
Date: Sun, 20 Nov 2016 16:21:27 +0100	[thread overview]
Message-ID: <20161120152127.GC5189@amd> (raw)
In-Reply-To: <20161119232943.GF13965@valkosipuli.retiisi.org.uk>

[-- 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 --]
