Hi Laurent,

On Tue, Aug 27, 2019 at 03:21:26PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Aug 27, 2019 at 11:23:27AM +0200, Jacopo Mondi wrote:
> > Add the 'location' device property, used to specify the camera device
> > mounting position. The property is particularly meaningful for mobile
> > devices with a well defined usage orientation.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  .../devicetree/bindings/media/video-interfaces.txt     | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > index f884ada0bffc..865f4142f432 100644
> > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > @@ -89,6 +89,16 @@ Optional properties
> >    but a number of degrees counter clockwise. Typical values are 0 and 180
> >    (upside down).
> >
> > +- location: The camera sensor mounting location, expressed as a position
> > +  relative to the usage orientation of the device the sensor is installed on.
>
> DT bindings being ABIs, we need to be precise and thorough there. One
> particular point that bothers me is that the property is named location,
> and its description refers to camera sensor mounting location.
>
> I see two options to fix this. One of them is to rename the property to
> camera-location, but that would limit its future usage for other types
> of devices. The other one is to document the property as applying to a
> "device" instead of a "camera sensor", and add one sentence stating that
> this property is valid for camera sensors only.
>
> This will require finding another name for the device that the device is
> mounted on though, as using device twice would be very confusing.

I had the same concern, but I cannot find another term to convey
this... suggestions?

>
> > +  Possible values are:
> > +  0 - Front. The image sensor is mounted on the front facing side of the device.
> > +  For mobile devices such as smartphones, tablets and laptops the front side is
> > +  the user facing side of the device.
> > +  1 - Back. The image sensor is mounted on the back side of the device, which is
> > +  defined as the opposite side of the front facing one.
> > +  2 - External. The image sensor is connected to the device by extension cables,
> > +  and can be freely moved, regardless of the device position.
>
> It could be something else than cables (wireless possibly ?). I would
> phrase this along the lines of "The device is not attached directly to
> the [device], or is attached in a way that allows it to move to
> different locations."

Agreed, once we find a good term for [device] :)

>
> >
> >  Optional endpoint properties
> >  ----------------------------
>
> --
> Regards,
>
> Laurent Pinchart