[PATCH/RFC,8/8] DT: Add documentation for exynos4-is camera-flash property

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
 .../devicetree/bindings/media/samsung-fimc.txt     |    3 +++
 1 file changed, 3 insertions(+)

Hi Jacek,

On Thu, Mar 20, 2014 at 03:51:10PM +0100, Jacek Anaszewski wrote:
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> ---
>  .../devicetree/bindings/media/samsung-fimc.txt     |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> index 922d6f8..88f9287 100644
> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> @@ -108,6 +108,8 @@ Image sensor nodes
>  The sensor device nodes should be added to their control bus controller (e.g.
>  I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
>  using the common video interfaces bindings, defined in video-interfaces.txt.
> +If the sensor device has a led flash device associated with it then its phandle
> +should be assigned to the camera-flash property.
>  
>  Example:
>  
> @@ -125,6 +127,7 @@ Example:
>  			clock-frequency = <24000000>;
>  			clocks = <&camera 1>;
>  			clock-names = "mclk";
> +			camera-flash = <&led_flash>;
>  
>  			port {
>  				s5k6aa_ep: endpoint {

It's indeed an interesting idea to declare the flash controller in the
sensor's properties rather than those of the ISP. The obvious upside is that
this way it's easy to figure out which subdev group the flash controller
belongs to.

There are a few other things to consider as well:

- You can't have a flash without a sensor. I can't think of why this would
  be a real issue, though.

- Relations other than one-to-one become difficult. One flash but two
  cameras --- think of stereo cameras.

	- One camera and two flashes. I haven't seen any but I don't think
	  that's unthinkable.

- It's not very nice of the ISP driver to just go and parse the
  sensor's properties.

- As the property is FIMC specific, the sensor DT node now carries FIMC
  related information.

A generic solution would be preferrable as this is not a FIMC related
problem.

I have to admit that I can't think of a better solution right now than just
putting a list of the flash device phandles to the ISP device's DT node, and
then adding information on which sensor (numeric ID) the flash is related to
as an array. Better ideas would be welcome.

Hi Sakari,

On 03/24/2014 02:05 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Thu, Mar 20, 2014 at 03:51:10PM +0100, Jacek Anaszewski wrote:
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> Cc: Kumar Gala <galak@codeaurora.org>
>> ---
>>   .../devicetree/bindings/media/samsung-fimc.txt     |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> index 922d6f8..88f9287 100644
>> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> @@ -108,6 +108,8 @@ Image sensor nodes
>>   The sensor device nodes should be added to their control bus controller (e.g.
>>   I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
>>   using the common video interfaces bindings, defined in video-interfaces.txt.
>> +If the sensor device has a led flash device associated with it then its phandle
>> +should be assigned to the camera-flash property.
>>
>>   Example:
>>
>> @@ -125,6 +127,7 @@ Example:
>>   			clock-frequency = <24000000>;
>>   			clocks = <&camera 1>;
>>   			clock-names = "mclk";
>> +			camera-flash = <&led_flash>;
>>
>>   			port {
>>   				s5k6aa_ep: endpoint {
>
> It's indeed an interesting idea to declare the flash controller in the
> sensor's properties rather than those of the ISP. The obvious upside is that
> this way it's easy to figure out which subdev group the flash controller
> belongs to.
>
> There are a few other things to consider as well:
>
> - You can't have a flash without a sensor. I can't think of why this would
>    be a real issue, though.
>
> - Relations other than one-to-one become difficult. One flash but two
>    cameras --- think of stereo cameras.
>
> 	- One camera and two flashes. I haven't seen any but I don't think
> 	  that's unthinkable.
>
> - It's not very nice of the ISP driver to just go and parse the
>    sensor's properties.
>
> - As the property is FIMC specific, the sensor DT node now carries FIMC
>    related information.
>
> A generic solution would be preferrable as this is not a FIMC related
> problem.
>
> I have to admit that I can't think of a better solution right now than just
> putting a list of the flash device phandles to the ISP device's DT node, and
> then adding information on which sensor (numeric ID) the flash is related to
> as an array. Better ideas would be welcome.
>

One reason why the flash sub-dev is registered by the sensor is the
fact that a subdev has to be registered to make it available for use.
The second reason is that it is physically connected with the sensor on
the board via torchen/flashen traces. However it would be nice if the
flash could be available for use even if its parent sensor driver isn't 
probed.

There are also possible configurations where traces are routed through
multiplexers and in such cases the sensor-flash relation is not fixed.

I propose to introduce a "flash manager" which would maintain
all the available flashes. V4L2 Flash sub-devices could register
with it asynchronously. The flash manager could expose controls
for configuring flash-sensor relations. In specific case a flash
manager could be built upon a multiplexer device. This would suit
me very well as I am currently facing such a configuration
on another board. For time being I put gpios of a multiplexer
to the flash DT node, but it doesn't reflect board configuration,
where multiplexer is a separate device. I could try to implement
the flash manager and submit an RFC. What is your opinion, does
it make a sense?

Regards,
Jacek Anaszewski

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html