From patchwork Sun Nov 20 15:21:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Machek X-Patchwork-Id: 38265 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c8TwG-0007TJ-I2; Sun, 20 Nov 2016 15:21:52 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-8) with esmtp id 1c8TwE-0001rG-k9; Sun, 20 Nov 2016 16:21:52 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753350AbcKTPVb (ORCPT + 1 other); Sun, 20 Nov 2016 10:21:31 -0500 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:57748 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752889AbcKTPVa (ORCPT ); Sun, 20 Nov 2016 10:21:30 -0500 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id 53B2C823C3; Sun, 20 Nov 2016 16:21:28 +0100 (CET) Date: Sun, 20 Nov 2016 16:21:27 +0100 From: Pavel Machek To: Sakari Ailus 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 Message-ID: <20161120152127.GC5189@amd> References: <20161023200355.GA5391@amd> <20161119232943.GF13965@valkosipuli.retiisi.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161119232943.GF13965@valkosipuli.retiisi.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.11.20.151516 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' BODY_PARA_IS_SENTENCE_URL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, KNOWN_FREEWEB_URI 0.05, MSGID_ADDED_BY_MTA 0.05, BODY_SIZE_6000_6999 0, BODY_SIZE_7000_LESS 0, INVALID_MSGID_NO_FQDN 0, IN_REP_TO 0, LEGITIMATE_NEGATE 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, REFERENCES 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __ATTACHMENT_SIZE_0_10K 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CD 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __FORWARDED_MSG 0, __HAS_ATTACHMENT 0, __HAS_ATTACHMENT1 0, __HAS_ATTACHMENT2 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __KNOWN_FREEWEB_URI2 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MIME_TEXT_P2 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __NO_HTML_TAG_RAW 0, __REFERENCES 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __TO_NAME 0, __TO_NAME_DIFF_FROM_ACC 0, __TO_REAL_NAMES 0, __URI_IN_BODY 0, __URI_NS , __URI_WITH_PATH 0, __USER_AGENT 0' 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 */ };