All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: "Wu, Songjun" <Songjun.Wu@microchip.com>, nicolas.ferre@atmel.com
Cc: robh@kernel.org, laurent.pinchart@ideasonboard.com,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org, "Arnd Bergmann" <arnd@arndb.de>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Benoit Parrot" <bparrot@ti.com>,
	linux-kernel@vger.kernel.org,
	"Andrew-CT Chen" <andrew-ct.chen@mediatek.com>,
	"Sudip Mukherjee" <sudipm.mukherjee@gmail.com>,
	"Kamil Debski" <kamil@wypas.org>,
	"Tiffany Lin" <tiffany.lin@mediatek.com>,
	"Peter Griffin" <peter.griffin@linaro.org>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Mikhail Ulyanov" <mikhail.ulyanov@cogentembedded.com>,
	"Richard Röjfors" <richard@puffinpack.se>,
	"Hans Verkuil" <hans.verkuil@cisco.com>,
	"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
	"Simon Horman" <horms+renesas@verge.net.au>
Subject: Re: [PATCH v7 1/2] [media] atmel-isc: add the Image Sensor Controller code
Date: Tue, 2 Aug 2016 09:32:44 +0200	[thread overview]
Message-ID: <cdb406b8-bb1e-9a78-e07c-f5df3dbcfe34@xs4all.nl> (raw)
In-Reply-To: <b32b2346-cc11-521e-c2f8-6d9e951c7a16@microchip.com>



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.

Regards,

	Hans

WARNING: multiple messages have this Message-ID (diff)
From: hverkuil@xs4all.nl (Hans Verkuil)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 1/2] [media] atmel-isc: add the Image Sensor Controller code
Date: Tue, 2 Aug 2016 09:32:44 +0200	[thread overview]
Message-ID: <cdb406b8-bb1e-9a78-e07c-f5df3dbcfe34@xs4all.nl> (raw)
In-Reply-To: <b32b2346-cc11-521e-c2f8-6d9e951c7a16@microchip.com>



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.

Regards,

	Hans

  reply	other threads:[~2016-08-02  7:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-29  7:54 [PATCH v7 0/2] [media] atmel-isc: add driver for Atmel ISC Songjun Wu
2016-07-29  7:54 ` Songjun Wu
2016-07-29  7:54 ` Songjun Wu
2016-07-29  7:54 ` [PATCH v7 1/2] [media] atmel-isc: add the Image Sensor Controller code Songjun Wu
2016-07-29  7:54   ` Songjun Wu
2016-08-01  9:47   ` Hans Verkuil
2016-08-01  9:47     ` Hans Verkuil
2016-08-02  6:20     ` Wu, Songjun
2016-08-02  6:20       ` Wu, Songjun
2016-08-02  7:32       ` Hans Verkuil [this message]
2016-08-02  7:32         ` Hans Verkuil
2016-08-02  7:48         ` Wu, Songjun
2016-08-02  7:48           ` Wu, Songjun
2016-07-29  7:54 ` [PATCH v7 2/2] [media] atmel-isc: DT binding for Image Sensor Controller driver Songjun Wu
2016-07-29  7:54   ` Songjun Wu
2016-07-29  7:54   ` Songjun Wu
2016-07-29 21:44   ` Rob Herring
2016-07-29 21:44     ` Rob Herring
2016-07-29 21:44     ` Rob Herring
2016-08-01  1:23     ` Wu, Songjun
2016-08-01  1:23       ` Wu, Songjun
2016-08-01  1:23       ` Wu, Songjun
2016-08-01  1:57       ` Rob Herring
2016-08-01  1:57         ` Rob Herring
2016-08-01  1:57         ` Rob Herring
2016-08-01  1:57         ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cdb406b8-bb1e-9a78-e07c-f5df3dbcfe34@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=Songjun.Wu@microchip.com \
    --cc=andrew-ct.chen@mediatek.com \
    --cc=arnd@arndb.de \
    --cc=bparrot@ti.com \
    --cc=geert@linux-m68k.org \
    --cc=hans.verkuil@cisco.com \
    --cc=horms+renesas@verge.net.au \
    --cc=kamil@wypas.org \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mikhail.ulyanov@cogentembedded.com \
    --cc=nicolas.ferre@atmel.com \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=peter.griffin@linaro.org \
    --cc=richard@puffinpack.se \
    --cc=robh@kernel.org \
    --cc=sudipm.mukherjee@gmail.com \
    --cc=tiffany.lin@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.