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

[Jacopo Mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

Hi Rob,

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:
> > 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?
>
> 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 :) ?

Thanks
   j
>
> Rob
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]