Re: [PATCH 2/2] media: ov5640: add support of module orientation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari, Rob,

Find a new proposal below:

On 06/13/2018 10:24 AM, Sakari Ailus wrote:
> On Wed, Jun 13, 2018 at 08:10:02AM +0000, Hugues FRUCHET wrote:
>> Hi Rob, thanks for review,
>>
>> On 06/13/2018 12:06 AM, Rob Herring wrote:
>>> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>>>> Add support of module being physically mounted upside down.
>>>> In this case, mirror and flip are enabled to fix captured images
>>>> orientation.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxx>
>>>> ---
>>>>    .../devicetree/bindings/media/i2c/ov5640.txt       |  3 +++
>>>
>>> Please split bindings to separate patches.
>>
>> OK, will do in next patchset.
>>
>>>
>>>>    drivers/media/i2c/ov5640.c                         | 28 ++++++++++++++++++++--
>>>>    2 files changed, 29 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>>> index 8e36da0..f76eb7e 100644
>>>> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>>> @@ -13,6 +13,8 @@ Optional Properties:
>>>>    	       This is an active low signal to the OV5640.
>>>>    - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
>>>>    		   if any. This is an active high signal to the OV5640.
>>>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>>>> +	    and 180 (sensor mounted upside down).
>>>
>>> Didn't we just add this as a common property? If so, just reference the
>>> common definition. If not, it needs a common definition.
>>>
>>
>> A common definition has been introduced by Sakari, I'm reusing it, see:
>> https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg132517.html
>>
>> I would so propose:
>>   >> +- rotation: as defined in
>>   >> +	Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> Shouldn't the description still include the valid values? As far as I can
> tell, these are ultimately device specific albeit more or less the same for
> *this kind* of sensors.

Yes you're right, let's put both together:
+- rotation: as defined in
+	Documentation/devicetree/bindings/media/video-interfaces.txt,
+	valid values are 0 (sensor mounted upright) and 180 (sensor
+	mounted upside down).


> 
> The file already contains a reference to video-interfaces.txt.
> 
Yes but it was related to 'port' child node.

Best regards,
Hugues.




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux