[RFC 0/3] Document bindings for camera modules and associated flash devices

[RFC 0/3] Document bindings for camera modules and associated flash devices

[Sakari Ailus]

Hi folks,

This RFC patchset documents properties commonly required by camera modules
and associated camera flash devices.

The camera module is essentially a package consisting of an image sensor,
a lens, possibly a voice coil to move the lens and a number of other
things that at least the drivers need not to know of. All the devices in a
camera module are declared separately in the system and as such the fact
that they come in a single package isn't generally very useful to driver
software.

I'm sending the set as RFC as there's no driver implementation, and a
dependency to the V4L2 async changes:

<URL:http://www.spinics.net/lists/linux-media/msg114915.html>

-- 
Regards,
Sakari

[RFC 1/3] dt: bindings: Add a binding for flash devices associated to a sensor

[Sakari Ailus]

Camera flash drivers (and LEDs) are separate from the sensor devices in
DT. In order to make an association between the two, provide the
association information to the software.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 9cd2a36..d6c62bc 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -67,6 +67,17 @@ are required in a relevant parent node:
 		    identifier, should be 1.
  - #size-cells    : should be zero.
 
+
+Optional properties
+-------------------
+
+- flash: An array of phandles that refer to the flash light sources
+  related to an image sensor. These could be e.g. LEDs. In case the LED
+  driver drives more than a single LED, then the phandles here refer to
+  the child nodes of the LED driver describing individual LEDs. Only
+  valid for device nodes that are related to an image sensor.
+
+
 Optional endpoint properties
 ----------------------------
 
-- 
2.7.4

Re: [RFC 1/3] dt: bindings: Add a binding for flash devices associated to a sensor

[Sebastian Reichel]

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

Hi Sakari,

On Tue, May 02, 2017 at 01:25:47PM +0300, Sakari Ailus wrote:
> Camera flash drivers (and LEDs) are separate from the sensor devices in
> DT. In order to make an association between the two, provide the
> association information to the software.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 9cd2a36..d6c62bc 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -67,6 +67,17 @@ are required in a relevant parent node:
>  		    identifier, should be 1.
>   - #size-cells    : should be zero.
>  
> +
> +Optional properties
> +-------------------
> +
> +- flash: An array of phandles that refer to the flash light sources
> +  related to an image sensor. These could be e.g. LEDs. In case the LED
> +  driver drives more than a single LED, then the phandles here refer to
> +  the child nodes of the LED driver describing individual LEDs. Only
> +  valid for device nodes that are related to an image sensor.

s/driver/controller/g - DT describes HW. Otherwise

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

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

Re: [RFC 1/3] dt: bindings: Add a binding for flash devices associated to a sensor

[Sakari Ailus]

Hi Sebastian,

Sebastian Reichel wrote:
> Hi Sakari,
>
> On Tue, May 02, 2017 at 01:25:47PM +0300, Sakari Ailus wrote:
>> Camera flash drivers (and LEDs) are separate from the sensor devices in
>> DT. In order to make an association between the two, provide the
>> association information to the software.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>  Documentation/devicetree/bindings/media/video-interfaces.txt | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> index 9cd2a36..d6c62bc 100644
>> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
>> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> @@ -67,6 +67,17 @@ are required in a relevant parent node:
>>  		    identifier, should be 1.
>>   - #size-cells    : should be zero.
>>
>> +
>> +Optional properties
>> +-------------------
>> +
>> +- flash: An array of phandles that refer to the flash light sources
>> +  related to an image sensor. These could be e.g. LEDs. In case the LED
>> +  driver drives more than a single LED, then the phandles here refer to
>> +  the child nodes of the LED driver describing individual LEDs. Only
>> +  valid for device nodes that are related to an image sensor.
>
> s/driver/controller/g - DT describes HW. Otherwise

Driver is hardware in this case. :-) The chip that acts as a current 
sink or source for the LED is the driver. E.g. the adp1653 documentation 
describes the chip as "Compact, High Efficiency, High Power, Flash/Torch 
LED Driver with Dual Interface".

It might be still possible to improve the wording. Software oriented 
folks are more likely to misunderstand the meaning of driver here, but 
controller might seem ambiguous for hardware oriented people.

How about:

- flash: An array of phandles that refer to the flash light sources
   related to an image sensor. These could be e.g. LEDs. In case the LED
   driver (current sink or source chip for the LED(s)) drives more than a
   single LED, then the phandles here refer to the child nodes of the LED
   driver describing individual LEDs. Only valid for device nodes that are
   related to an image sensor.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 1/3] dt: bindings: Add a binding for flash devices associated to a sensor

[Sebastian Reichel]

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

Hi,

On Fri, May 05, 2017 at 11:28:34AM +0300, Sakari Ailus wrote:
> Sebastian Reichel wrote:
> > On Tue, May 02, 2017 at 01:25:47PM +0300, Sakari Ailus wrote:
> > > +- flash: An array of phandles that refer to the flash light sources
> > > +  related to an image sensor. These could be e.g. LEDs. In case the LED
> > > +  driver drives more than a single LED, then the phandles here refer to
> > > +  the child nodes of the LED driver describing individual LEDs. Only
> > > +  valid for device nodes that are related to an image sensor.
> > 
> > s/driver/controller/g - DT describes HW. Otherwise
> 
> Driver is hardware in this case. :-) The chip that acts as a current sink or
> source for the LED is the driver. E.g. the adp1653 documentation describes
> the chip as "Compact, High Efficiency, High Power, Flash/Torch LED Driver
> with Dual Interface".
> 
> It might be still possible to improve the wording. Software oriented folks
> are more likely to misunderstand the meaning of driver here, but controller
> might seem ambiguous for hardware oriented people.
> 
> How about:
> 
> - flash: An array of phandles that refer to the flash light sources
>   related to an image sensor. These could be e.g. LEDs. In case the LED
>   driver (current sink or source chip for the LED(s)) drives more than a
>   single LED, then the phandles here refer to the child nodes of the LED
>   driver describing individual LEDs. Only valid for device nodes that are
>   related to an image sensor.

Maybe drop the last sentence? The requirement is already in the
first one. Otherwise:

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

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

[RFC 2/3] dt: bindings: Add lens-focus binding for image sensors

[Sakari Ailus]

The lens-focus property contains a phandle to the lens voice coil driver
that is associated to the sensor; typically both are contained in the same
camera module.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index d6c62bc..e52aefc 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -77,6 +77,9 @@ Optional properties
   the child nodes of the LED driver describing individual LEDs. Only
   valid for device nodes that are related to an image sensor.
 
+- lens-focus: A phandle to the node of the lens. Only valid for device
+  nodes that are related to an image sensor.
+
 
 Optional endpoint properties
 ----------------------------
-- 
2.7.4

Re: [RFC 2/3] dt: bindings: Add lens-focus binding for image sensors

[Sebastian Reichel]

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

Hi,

On Tue, May 02, 2017 at 01:25:48PM +0300, Sakari Ailus wrote:
> The lens-focus property contains a phandle to the lens voice coil driver
> that is associated to the sensor; typically both are contained in the same
> camera module.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index d6c62bc..e52aefc 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -77,6 +77,9 @@ Optional properties
>    the child nodes of the LED driver describing individual LEDs. Only
>    valid for device nodes that are related to an image sensor.
>  
> +- lens-focus: A phandle to the node of the lens. Only valid for device
> +  nodes that are related to an image sensor.

s/of the lens/of the focus lens controller/

Also I wonder about the second sentence. That sentence could
basically be added to every property, so can't we just drop it?

-- Sebastian

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

[RFC 3/3] dt: bindings: Add a binding for referencing EEPROM from camera sensors

[Sakari Ailus]

Many camera sensor devices contain EEPROM chips that describe the
properties of a given unit --- the data is specific to a given unit can
thus is not stored e.g. in user space or the driver.

Some sensors embed the EEPROM chip and it can be accessed through the
sensor's I²C interface. This property is to be used for devices where the
EEPROM chip is accessed through a different I²C address than the sensor.

The intent is to later provide this information to the user space.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index e52aefc..9bd2005 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -80,6 +80,9 @@ Optional properties
 - lens-focus: A phandle to the node of the lens. Only valid for device
   nodes that are related to an image sensor.
 
+- eeprom: A phandle to the node of the related EEPROM. Only valid for
+  device nodes that are related to an image sensor.
+
 
 Optional endpoint properties
 ----------------------------
-- 
2.7.4

Re: [RFC 3/3] dt: bindings: Add a binding for referencing EEPROM from camera sensors

[Sebastian Reichel]

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

Hi,

On Tue, May 02, 2017 at 01:25:49PM +0300, Sakari Ailus wrote:
> Many camera sensor devices contain EEPROM chips that describe the
> properties of a given unit --- the data is specific to a given unit can
> thus is not stored e.g. in user space or the driver.
> 
> Some sensors embed the EEPROM chip and it can be accessed through the
> sensor's I²C interface. This property is to be used for devices where the
> EEPROM chip is accessed through a different I²C address than the sensor.
> 
> The intent is to later provide this information to the user space.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index e52aefc..9bd2005 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -80,6 +80,9 @@ Optional properties
>  - lens-focus: A phandle to the node of the lens. Only valid for device
>    nodes that are related to an image sensor.
>  
> +- eeprom: A phandle to the node of the related EEPROM. Only valid for
> +  device nodes that are related to an image sensor.

Here it's even more obvious, that the second sentence is redundant.
The requirement is already in the first sentence :) Instead it
should be mentioned, that this is to be used by devices not having
their own embedded eeprom. How about:

eeprom: A phandle to the node of the EEPROM describing the camera
sensor (i.e. device specific calibration data), in case it differs
from the sensor node.

-- Sebastian

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

Re: [RFC 3/3] dt: bindings: Add a binding for referencing EEPROM from camera sensors

[Sakari Ailus]

On Thu, May 04, 2017 at 04:38:04PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, May 02, 2017 at 01:25:49PM +0300, Sakari Ailus wrote:
> > Many camera sensor devices contain EEPROM chips that describe the
> > properties of a given unit --- the data is specific to a given unit can
> > thus is not stored e.g. in user space or the driver.
> > 
> > Some sensors embed the EEPROM chip and it can be accessed through the
> > sensor's I²C interface. This property is to be used for devices where the
> > EEPROM chip is accessed through a different I²C address than the sensor.
> > 
> > The intent is to later provide this information to the user space.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > index e52aefc..9bd2005 100644
> > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > @@ -80,6 +80,9 @@ Optional properties
> >  - lens-focus: A phandle to the node of the lens. Only valid for device
> >    nodes that are related to an image sensor.
> >  
> > +- eeprom: A phandle to the node of the related EEPROM. Only valid for
> > +  device nodes that are related to an image sensor.
> 
> Here it's even more obvious, that the second sentence is redundant.
> The requirement is already in the first sentence :) Instead it
> should be mentioned, that this is to be used by devices not having
> their own embedded eeprom. How about:
> 
> eeprom: A phandle to the node of the EEPROM describing the camera
> sensor (i.e. device specific calibration data), in case it differs
> from the sensor node.

Ack. I addressed the rest of the feedback, too, and posted RFC v2.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [RFC 0/3] Document bindings for camera modules and associated flash devices

[Pavel Machek]

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

Hi!

> This RFC patchset documents properties commonly required by camera modules
> and associated camera flash devices.
> 
> The camera module is essentially a package consisting of an image sensor,
> a lens, possibly a voice coil to move the lens and a number of other
> things that at least the drivers need not to know of. All the devices in a
> camera module are declared separately in the system and as such the fact
> that they come in a single package isn't generally very useful to driver
> software.
> 
> I'm sending the set as RFC as there's no driver implementation, and a
> dependency to the V4L2 async changes:
> 
> <URL:http://www.spinics.net/lists/linux-media/msg114915.html>

For the series,

Acked-by: Pavel Machek <pavel@ucw.cz>

In 3/3, I'd avoid using strange characters in the changelog. Just
write it as I2C... If someone ever tries to grep the changelogs, this
could bite them.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]