Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

On Mon, Sep 02, 2019 at 06:40:31PM +0200, Jacopo Mondi wrote:
> On Mon, Sep 02, 2019 at 02:38:40PM +0100, Rob Herring wrote:
> > On Thu, Aug 29, 2019 at 02:46:40PM +0200, Jacopo Mondi wrote:
> >> On Tue, Aug 27, 2019 at 03:21:26PM +0300, Laurent Pinchart wrote:
> >>> 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?
> >
> > For accelerometers and/or gyroscopes, we already have 'mount-matrix'.
> 
> Pardon my ignorance, but I could not find it documented. Some binding
> files refers to an iio/mount-matrix.txt file which I cannot find. Has
> it been removed? Anyway, some individual bindings report examples of
> mount matrices (ie
> Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt)
> 
> > That would replace 'rotation'. Maybe we can do something similar here?
> 
> I think 'rotation' simply expressed as degrees is fine here, our
> problem was to find a term that would make it possible to specify that
> rotation/location are applied to a 'device' mounted on a 'device'.
> Sakari suggested on irc to use 'system' in place of the second
> 'device' and that seems to work well to me.
> 
> Or were you suggesting to use a construct similar to mount-matrix for
> a purpose I didn't get :) ?

mount-matrix would allow exposing the rotation in a more generic way,
and if there are standard DT bindings, I think this would make sense,
especially given that we could have device with camera sensors not
mounted parallel to the device side.

-- 
Regards,

Laurent Pinchart