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.