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

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

 



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.

The file already contains a reference to video-interfaces.txt.

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux