From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756033AbcHBH5l (ORCPT ); Tue, 2 Aug 2016 03:57:41 -0400 Received: from exsmtp03.microchip.com ([198.175.253.49]:27283 "EHLO email.microchip.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751042AbcHBH5e (ORCPT ); Tue, 2 Aug 2016 03:57:34 -0400 Subject: Re: [PATCH v7 1/2] [media] atmel-isc: add the Image Sensor Controller code To: Hans Verkuil , References: <1469778856-24253-1-git-send-email-songjun.wu@microchip.com> <1469778856-24253-2-git-send-email-songjun.wu@microchip.com> CC: , , , , "Arnd Bergmann" , =?UTF-8?Q?Niklas_S=c3=83=c2=b6derlund?= , Benoit Parrot , , Andrew-CT Chen , Sudip Mukherjee , Kamil Debski , Tiffany Lin , Peter Griffin , Geert Uytterhoeven , Mikhail Ulyanov , =?UTF-8?Q?Richard_R=c3=b6jfors?= , Hans Verkuil , Laurent Pinchart , Simon Horman From: "Wu, Songjun" Organization: Microchip Message-ID: <300f7f9a-d3dd-1430-3edd-9ca682038ca8@microchip.com> Date: Tue, 2 Aug 2016 15:48:25 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/2/2016 15:32, Hans Verkuil wrote: > > > On 08/02/2016 08:20 AM, Wu, Songjun wrote: >>>> +static unsigned int sensor_preferred = 1; >>>> +module_param(sensor_preferred, uint, S_IRUGO|S_IWUSR); >>>> +MODULE_PARM_DESC(sensor_preferred, >>>> + "Sensor is preferred to output the specified format (1-on 0-off) default 1"); >>> >>> I have no idea what this means. Can you elaborate? Why would you want to set this to 0? >>> >> ISC can convert the raw format to the other format, e.g. YUYV. >> If we want to output YUYV format, there are two choices, one is the >> sensor output YUYV format, ISC bypass the data to the memory, the other >> is the sensor output raw format, ISC convert raw format to YUYV. >> >> So I provide a module parameter to user to select. >> I prefer to select the sensor to output the specified format, then I set >> this parameter to '1', not '0'. > > Does this only apply to YUYV? > > The reason I am hesitant about this option is that I am not convinced you need > it. The default (sensor preferred) makes sense and that's what other drivers > do as well. Unless you know of a real use-case where you want to set this to 0, > I would just drop this option. > > If there *is* a real use-case, then split off adding this module option into a > separate patch so we can discuss it more without blocking getting this driver > into mainline. I don't like the way this is done here. > This does not only apply to YUYV, ISC IP defines some formats that can be converted from raw format. In some scenarios, ISC can extend the formats, e.g. if the sensor does not support YUYV, but raw format, the ISC can output YUYV. OK, I will remove the related code. After this driver is accepted, I will submit another patch to add this feature. Thank you. > Regards, > > Hans > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Songjun.Wu@microchip.com (Wu, Songjun) Date: Tue, 2 Aug 2016 15:48:25 +0800 Subject: [PATCH v7 1/2] [media] atmel-isc: add the Image Sensor Controller code In-Reply-To: References: <1469778856-24253-1-git-send-email-songjun.wu@microchip.com> <1469778856-24253-2-git-send-email-songjun.wu@microchip.com> Message-ID: <300f7f9a-d3dd-1430-3edd-9ca682038ca8@microchip.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 8/2/2016 15:32, Hans Verkuil wrote: > > > On 08/02/2016 08:20 AM, Wu, Songjun wrote: >>>> +static unsigned int sensor_preferred = 1; >>>> +module_param(sensor_preferred, uint, S_IRUGO|S_IWUSR); >>>> +MODULE_PARM_DESC(sensor_preferred, >>>> + "Sensor is preferred to output the specified format (1-on 0-off) default 1"); >>> >>> I have no idea what this means. Can you elaborate? Why would you want to set this to 0? >>> >> ISC can convert the raw format to the other format, e.g. YUYV. >> If we want to output YUYV format, there are two choices, one is the >> sensor output YUYV format, ISC bypass the data to the memory, the other >> is the sensor output raw format, ISC convert raw format to YUYV. >> >> So I provide a module parameter to user to select. >> I prefer to select the sensor to output the specified format, then I set >> this parameter to '1', not '0'. > > Does this only apply to YUYV? > > The reason I am hesitant about this option is that I am not convinced you need > it. The default (sensor preferred) makes sense and that's what other drivers > do as well. Unless you know of a real use-case where you want to set this to 0, > I would just drop this option. > > If there *is* a real use-case, then split off adding this module option into a > separate patch so we can discuss it more without blocking getting this driver > into mainline. I don't like the way this is done here. > This does not only apply to YUYV, ISC IP defines some formats that can be converted from raw format. In some scenarios, ISC can extend the formats, e.g. if the sensor does not support YUYV, but raw format, the ISC can output YUYV. OK, I will remove the related code. After this driver is accepted, I will submit another patch to add this feature. Thank you. > Regards, > > Hans >