Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
Hi Mauro, On Thu, Sep 12, 2019 at 09:51:47AM -0300, Mauro Carvalho Chehab wrote: > Em Tue, 27 Aug 2019 15:21:26 +0300 > Laurent Pinchart escreveu: > > > 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 > > > --- > > > .../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. > > Yeah, "location" doesn't sound a good name for me neither. > I think Laurent referred to the fact that as we changed the property name to 'location' it is not a good idea to refer to 'camera sensor' as it could refer to flash leds too (and potentially other devices). In v3 I have s/camera sensor/device s/device/system How would you name the property? > > > > 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. > > > > > + 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. > > Hmm... > > Besides the point that Laurent and Rob already commented, just those 3 options > doesn't seem good enough. I was reading a public article yesterday about a new > device (Samsung Galaxy Fold[1]) with comes with 6 cameras, being 3 at back, > 1 at front, when the device is opened, and 1 camera that could be either at > the > back or at the front, depending if the device is opened or not. > Pavel linked a similar device in a previous reply: https://www.samsung.com/global/galaxy/galaxy-a80/ I understand the above options might not get enough in future, but right now they cover the 99,9% of devices in the market, and those phones are far from being mainline supported I presume. > Btw, on a device with multiple cameras at the same side, it would > make sense to also be able to uniquely identify the location of each > sensor somehow. > I think the location property should not identify devices, but just report where they are installed. Does having two cameras reporting the same mounting location bother you? > There are also some other new devices with a front motorized slider camera > that sits hidden inside the phone, until when someone uses it. > So it's an HIDDEN_FRONT_CAMERA ? :) Thanks j > Thanks, > Mauro signature.asc Description: PGP signature
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
Em Tue, 27 Aug 2019 15:21:26 +0300 Laurent Pinchart escreveu: > 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 > > --- > > .../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. Yeah, "location" doesn't sound a good name for me neither. > > 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. > > > + 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. Hmm... Besides the point that Laurent and Rob already commented, just those 3 options doesn't seem good enough. I was reading a public article yesterday about a new device (Samsung Galaxy Fold[1]) with comes with 6 cameras, being 3 at back, 1 at front, when the device is opened, and 1 camera that could be either at the back or at the front, depending if the device is opened or not. Btw, on a device with multiple cameras at the same side, it would make sense to also be able to uniquely identify the location of each sensor somehow. There are also some other new devices with a front motorized slider camera that sits hidden inside the phone, until when someone uses it. Thanks, Mauro
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
Hi Jacopo, On Mon, Sep 02, 2019 at 09:48:30PM +0200, Jacopo Mondi wrote: > On Mon, Sep 02, 2019 at 07:49:37PM +0300, Laurent Pinchart wrote: > > 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 > >> --- > >> .../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. > > After reading this entry you pointed me to > https://lore.kernel.org/patchwork/patch/1044786/ > I understand why Rob suggested that, but it seems a bit far fetched to > me... The devices that could make use of 'mount-matrix', such as the > ones IIO deal with, sample data in a 3D space, and to compensate their > mounting displacement a rotation matrix is required. > > Do you see use cases for compensating mounting displacement in a 3-d > space for a camera ? I'm not saying it's not possible, but seems -a > bit- far fetched. 360° cameras come to mind, they can use multiple sensors mounted with different angles. > If we consider cameras as always mounted parallel to the device used > to take picturea, the correction you could apply to the possible mounting > displacements are just rotations along the origin (or the z axis you want > to consider a 3 dimensional space) and requiring to supply a 2-dimensions > rotation matrix (with only a few combinations of 0, 1 and -1, if we only > accept multiples of 90 degrees as we agreed) it seems a bit an overkill. > > True, we could also handle image flipping, not just rotation with a rotation > matrix but an image sensor mounted flipped upside down kind of defeat its > purpose, doesn't it ? (it would point to the ins
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
Hi again, On Mon, Sep 02, 2019 at 07:49:37PM +0300, Laurent Pinchart wrote: > 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 > > --- > > .../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. > After reading this entry you pointed me to https://lore.kernel.org/patchwork/patch/1044786/ I understand why Rob suggested that, but it seems a bit far fetched to me... The devices that could make use of 'mount-matrix', such as the ones IIO deal with, sample data in a 3D space, and to compensate their mounting displacement a rotation matrix is required. Do you see use cases for compensating mounting displacement in a 3-d space for a camera ? I'm not saying it's not possible, but seems -a bit- far fetched. If we consider cameras as always mounted parallel to the device used to take picturea, the correction you could apply to the possible mounting displacements are just rotations along the origin (or the z axis you want to consider a 3 dimensional space) and requiring to supply a 2-dimensions rotation matrix (with only a few combinations of 0, 1 and -1, if we only accept multiples of 90 degrees as we agreed) it seems a bit an overkill. True, we could also handle image flipping, not just rotation with a rotation matrix but an image sensor mounted flipped upside down kind of defeat its purpose, doesn't it ? (it would point to the inside of the device :) When we'll have devices with movable sensors like the one Pavel linked, rotation matrices could be used to report the current camera position maybe, but to represent a mounting displacem
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
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 > --- > .../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
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
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 > > > > --- > > > > .../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 > signature.asc Description: PGP signature
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
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 > > > --- > > > .../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'. That would replace 'rotation'. Maybe we can do something similar here? Rob
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
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 > > --- > > .../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 signature.asc Description: PGP signature
Re: [PATCH v2 01/10] media: dt-bindings: Document 'location' property
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 > --- > .../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. > + 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." > > Optional endpoint properties > -- Regards, Laurent Pinchart
