[PATCH v8 3/8] DT: aat1290: Document handling external strobe sources

[PATCH v8 3/8] DT: aat1290: Document handling external strobe sources

[Jacek Anaszewski]

This patch adds documentation for a pinctrl-names property.
The property, when present, is used for switching the source
of the strobe signal for the device.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/leds/leds-aat1290.txt      |   36 ++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-aat1290.txt b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
index ef88b9c..c05ed91 100644
--- a/Documentation/devicetree/bindings/leds/leds-aat1290.txt
+++ b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
@@ -2,7 +2,9 @@
 
 The device is controlled through two pins: FL_EN and EN_SET. The pins when,
 asserted high, enable flash strobe and movie mode (max 1/2 of flash current)
-respectively.
+respectively. In order to add a capability of selecting the strobe signal source
+(e.g. CPU or camera sensor) there is an additional switch required, independent
+of the flash chip. The switch is controlled with pin control.
 
 Required properties:
 
@@ -10,6 +12,13 @@ Required properties:
 - flen-gpios : Must be device tree identifier of the flash device FL_EN pin.
 - enset-gpios : Must be device tree identifier of the flash device EN_SET pin.
 
+Optional properties:
+- pinctrl-names : Must contain entries: "default", "host", "isp". Entries
+		"default" and "host" must refer to the same pin configuration
+		node, which sets the host as a strobe signal provider. Entry
+		"isp" must refer to the pin configuration node, which sets the
+		ISP as a strobe signal provider.
+
 A discrete LED element connected to the device must be represented by a child
 node - see Documentation/devicetree/bindings/leds/common.txt.
 
@@ -25,13 +34,22 @@ Required properties of the LED child node:
 Optional properties of the LED child node:
 - label : see Documentation/devicetree/bindings/leds/common.txt
 
-Example (by Ct = 220nF, Rset = 160kohm):
+Example (by Ct = 220nF, Rset = 160kohm and exynos4412-trats2 board with
+a switch that allows for routing strobe signal either from the host or from
+the camera sensor):
+
+#include "exynos4412.dtsi"
 
 aat1290 {
 	compatible = "skyworks,aat1290";
 	flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
 	enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
 
+	pinctrl-names = "default", "host", "isp";
+	pinctrl-0 = <&camera_flash_host>;
+	pinctrl-1 = <&camera_flash_host>;
+	pinctrl-2 = <&camera_flash_isp>;
+
 	camera_flash: flash-led {
 		label = "aat1290-flash";
 		led-max-microamp = <520833>;
@@ -39,3 +57,17 @@ aat1290 {
 		flash-timeout-us = <1940000>;
 	};
 };
+
+&pinctrl_0 {
+	camera_flash_host: camera-flash-host {
+		samsung,pins = "gpj1-0";
+		samsung,pin-function = <1>;
+		samsung,pin-val = <0>;
+	};
+
+	camera_flash_isp: camera-flash-isp {
+		samsung,pins = "gpj1-0";
+		samsung,pin-function = <1>;
+		samsung,pin-val = <1>;
+	};
+};
-- 
1.7.9.5

[PATCH v8 6/8] DT: s5c73m3: Add documentation for samsung,flash-led property

[Jacek Anaszewski]

This patch adds a description of 'samsung,flash-led' property
to the samsung-s5c73m3.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-s5c73m3.txt  |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
index 2c85c45..8a413c0 100644
--- a/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
+++ b/Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
@@ -32,6 +32,10 @@ Optional properties:
 - clock-frequency   : the frequency at which the "cis_extclk" clock should be
 		      configured to operate, in Hz; if this property is not
 		      specified default 24 MHz value will be used.
+- samsung,flash-led : phandle to the flash LED associated with this sensor.
+		      Flash LED is represented by the child node of a flash LED
+		      device
+		      (see Documentation/devicetree/bindings/leds/common.txt).
 
 The common video interfaces bindings (see video-interfaces.txt) should be used
 to specify link from the S5C73M3 to an external image data receiver. The S5C73M3
@@ -78,6 +82,7 @@ i2c@138A000000 {
 		clock-names = "cis_extclk";
 		reset-gpios = <&gpf1 3 1>;
 		standby-gpios = <&gpm0 1 1>;
+		samsung,flash-led = <&camera1_flash>;
 		port {
 			s5c73m3_ep: endpoint {
 				remote-endpoint = <&csis0_ep>;
-- 
1.7.9.5

[PATCH v8 7/8] DT: s5k6a3: Add documentation for samsung,flash-led property

[Jacek Anaszewski]

This patch adds a description of 'samsung,flash-led' property
to the samsung-s5k6a3.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-s5k6a3.txt   |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
index cce01e8..25f2e82 100644
--- a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
+++ b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
@@ -22,6 +22,10 @@ Optional properties:
 - clock-frequency : the frequency at which the "extclk" clock should be
 		    configured to operate, in Hz; if this property is not
 		    specified default 24 MHz value will be used.
+- samsung,flash-led : phandle to the flash LED associated with this sensor.
+		      Flash LED is represented by the child node of a flash LED
+		      device
+		      (see Documentation/devicetree/bindings/leds/common.txt).
 
 The common video interfaces bindings (see video-interfaces.txt) should be
 used to specify link to the image data receiver. The S5K6A3(YX) device
-- 
1.7.9.5

[PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

This patch adds examples for samsung,flash-led property to the
samsung-fimc.txt.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..57edffa 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -126,6 +126,8 @@ Example:
 			clocks = <&camera 1>;
 			clock-names = "mclk";
 
+			samsung,flash-led = <&front_cam_flash>;
+
 			port {
 				s5k6aa_ep: endpoint {
 					remote-endpoint = <&fimc0_ep>;
@@ -147,6 +149,8 @@ Example:
 			clocks = <&camera 0>;
 			clock-names = "mclk";
 
+			samsung,flash-led = <&rear_cam_flash>;
+
 			port {
 				s5c73m3_1: endpoint {
 					data-lanes = <1 2 3 4>;
-- 
1.7.9.5

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Jacek,

On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
> This patch adds examples for samsung,flash-led property to the
> samsung-fimc.txt.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> index 922d6f8..57edffa 100644
> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> @@ -126,6 +126,8 @@ Example:
>  			clocks = <&camera 1>;
>  			clock-names = "mclk";
>  
> +			samsung,flash-led = <&front_cam_flash>;
> +
>  			port {
>  				s5k6aa_ep: endpoint {
>  					remote-endpoint = <&fimc0_ep>;
> @@ -147,6 +149,8 @@ Example:
>  			clocks = <&camera 0>;
>  			clock-names = "mclk";
>  
> +			samsung,flash-led = <&rear_cam_flash>;
> +
>  			port {
>  				s5c73m3_1: endpoint {
>  					data-lanes = <1 2 3 4>;

Oops. I missed this property would have ended to the sensor's DT node. I
don't think we should have properties here that are parsed by another
driver --- let's discuss this tomorrow.

There are two main options that I can think of --- either put the property
under the bridge (ISP) driver's device node as a temporary solution that
works on a few ISP drivers, or think how sensor modules should be modelled,
in which case we'd have some idea how lens device would be taken into
account.

Cc Sebastian.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

Hi Sakari,

On 05/21/2015 12:00 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
>> This patch adds examples for samsung,flash-led property to the
>> samsung-fimc.txt.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> index 922d6f8..57edffa 100644
>> --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
>> @@ -126,6 +126,8 @@ Example:
>>   			clocks = <&camera 1>;
>>   			clock-names = "mclk";
>>
>> +			samsung,flash-led = <&front_cam_flash>;
>> +
>>   			port {
>>   				s5k6aa_ep: endpoint {
>>   					remote-endpoint = <&fimc0_ep>;
>> @@ -147,6 +149,8 @@ Example:
>>   			clocks = <&camera 0>;
>>   			clock-names = "mclk";
>>
>> +			samsung,flash-led = <&rear_cam_flash>;
>> +
>>   			port {
>>   				s5c73m3_1: endpoint {
>>   					data-lanes = <1 2 3 4>;
>
> Oops. I missed this property would have ended to the sensor's DT node. I
> don't think we should have properties here that are parsed by another
> driver --- let's discuss this tomorrow.

exynos4-is driver already parses sensor nodes (at least their 'port'
sub-nodes).

> There are two main options that I can think of --- either put the property
> under the bridge (ISP) driver's device node as a temporary solution that
> works on a few ISP drivers, or think how sensor modules should be modelled,
> in which case we'd have some idea how lens device would be taken into
> account.
>
> Cc Sebastian.
>


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Jacek,

On Thu, May 21, 2015 at 11:10:49AM +0200, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 05/21/2015 12:00 AM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, May 20, 2015 at 04:10:15PM +0200, Jacek Anaszewski wrote:
> >>This patch adds examples for samsung,flash-led property to the
> >>samsung-fimc.txt.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Cc: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >>Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>---
> >>  .../devicetree/bindings/media/samsung-fimc.txt     |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>index 922d6f8..57edffa 100644
> >>--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
> >>@@ -126,6 +126,8 @@ Example:
> >>  			clocks = <&camera 1>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&front_cam_flash>;
> >>+
> >>  			port {
> >>  				s5k6aa_ep: endpoint {
> >>  					remote-endpoint = <&fimc0_ep>;
> >>@@ -147,6 +149,8 @@ Example:
> >>  			clocks = <&camera 0>;
> >>  			clock-names = "mclk";
> >>
> >>+			samsung,flash-led = <&rear_cam_flash>;
> >>+
> >>  			port {
> >>  				s5c73m3_1: endpoint {
> >>  					data-lanes = <1 2 3 4>;
> >
> >Oops. I missed this property would have ended to the sensor's DT node. I
> >don't think we should have properties here that are parsed by another
> >driver --- let's discuss this tomorrow.
> 
> exynos4-is driver already parses sensor nodes (at least their 'port'
> sub-nodes).

If you read the code and the comment, it looks like something that should be
done better but hasn't been done yet. :-) That's something we should avoid.
Also, flash devices are by far more common than external ISPs I presume.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

On 21/05/15 13:32, Sakari Ailus wrote:
>>>> @@ -147,6 +149,8 @@ Example:
>>>> > >>  			clocks = <&camera 0>;
>>>> > >>  			clock-names = "mclk";
>>>> > >>
>>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
>>>> > >>+
>>>> > >>  			port {
>>>> > >>  				s5c73m3_1: endpoint {
>>>> > >>  					data-lanes = <1 2 3 4>;
>>> > >
>>> > >Oops. I missed this property would have ended to the sensor's DT node. I
>>> > >don't think we should have properties here that are parsed by another
>>> > >driver --- let's discuss this tomorrow.
>> > 
>> > exynos4-is driver already parses sensor nodes (at least their 'port'
>> > sub-nodes).
>
> If you read the code and the comment, it looks like something that should be
> done better but hasn't been done yet. :-) That's something we should avoid.
> Also, flash devices are by far more common than external ISPs I presume.

Yes, especially let's not require any samsung specific properties in
other vendors' sensor bindings.

One way of modelling [flash led]/[image sensor] association I imagine
would be to put, e.g. 'flash-leds' property in the SoC camera host
interface/ISP DT node. This property would then contain pairs of phandles,
first to the led node and the second to the sensor node, e.g.

i2c_controller {
	...
	flash_xx@NN {
		...
		led_a {
			...		
		}
	};

	image_sensor_x@NN {
		...
	};
};

flash-leds = <&flash_xx &image_sensor_x>, <...>;

For the purpose of this patch set presumably just samsung specific
property name could be used (i.e. samsung,flash-leds).

--
Thanks,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> On 21/05/15 13:32, Sakari Ailus wrote:
> >>>> @@ -147,6 +149,8 @@ Example:
> >>>> > >>  			clocks = <&camera 0>;
> >>>> > >>  			clock-names = "mclk";
> >>>> > >>
> >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
> >>>> > >>+
> >>>> > >>  			port {
> >>>> > >>  				s5c73m3_1: endpoint {
> >>>> > >>  					data-lanes = <1 2 3 4>;
> >>> > >
> >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
> >>> > >don't think we should have properties here that are parsed by another
> >>> > >driver --- let's discuss this tomorrow.
> >> > 
> >> > exynos4-is driver already parses sensor nodes (at least their 'port'
> >> > sub-nodes).
> >
> > If you read the code and the comment, it looks like something that should be
> > done better but hasn't been done yet. :-) That's something we should avoid.
> > Also, flash devices are by far more common than external ISPs I presume.
> 
> Yes, especially let's not require any samsung specific properties in
> other vendors' sensor bindings.
> 
> One way of modelling [flash led]/[image sensor] association I imagine
> would be to put, e.g. 'flash-leds' property in the SoC camera host
> interface/ISP DT node. This property would then contain pairs of phandles,
> first to the led node and the second to the sensor node, e.g.
> 
> i2c_controller {
> 	...
> 	flash_xx@NN {
> 		...
> 		led_a {
> 			...		
> 		}
> 	};
> 
> 	image_sensor_x@NN {
> 		...
> 	};
> };
> 
> flash-leds = <&flash_xx &image_sensor_x>, <...>;

Maybe a stupid question, but how do you access this in a driver? I have to
admit I'm no DT expert.

> 
> For the purpose of this patch set presumably just samsung specific
> property name could be used (i.e. samsung,flash-leds).

I agree. I'll add similar support for the omap3isp driver in the near future
though. Let's see how the camera modules will get modelled, if they will,
and if this property still fits to the picture by that time, then we make it
more generic.

What do you think?

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

Hi Sakari,

On 21/05/15 16:20, Sakari Ailus wrote:
> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>> > On 21/05/15 13:32, Sakari Ailus wrote:
>>>>>> > >>>> @@ -147,6 +149,8 @@ Example:
>>>>>>>>> > >>>> > >>  			clocks = <&camera 0>;
>>>>>>>>> > >>>> > >>  			clock-names = "mclk";
>>>>>>>>> > >>>> > >>
>>>>>>>>> > >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
>>>>>>>>> > >>>> > >>+
>>>>>>>>> > >>>> > >>  			port {
>>>>>>>>> > >>>> > >>  				s5c73m3_1: endpoint {
>>>>>>>>> > >>>> > >>  					data-lanes = <1 2 3 4>;
>>>>>>> > >>> > >
>>>>>>> > >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
>>>>>>> > >>> > >don't think we should have properties here that are parsed by another
>>>>>>> > >>> > >driver --- let's discuss this tomorrow.
>>>>> > >> > 
>>>>> > >> > exynos4-is driver already parses sensor nodes (at least their 'port'
>>>>> > >> > sub-nodes).
>>> > >
>>> > > If you read the code and the comment, it looks like something that should be
>>> > > done better but hasn't been done yet. :-) That's something we should avoid.
>>> > > Also, flash devices are by far more common than external ISPs I presume.
>> > 
>> > Yes, especially let's not require any samsung specific properties in
>> > other vendors' sensor bindings.
>> > 
>> > One way of modelling [flash led]/[image sensor] association I imagine
>> > would be to put, e.g. 'flash-leds' property in the SoC camera host
>> > interface/ISP DT node. This property would then contain pairs of phandles,
>> > first to the led node and the second to the sensor node, e.g.
>> > 
>> > i2c_controller {
>> > 	...
>> > 	flash_xx@NN {
>> > 		...
>> > 		led_a {
>> > 			...		
>> > 		}
>> > 	};
>> > 
>> > 	image_sensor_x@NN {
>> > 		...
>> > 	};
>> > };
>> > 
>> > flash-leds = <&flash_xx &image_sensor_x>, <...>;
>
> Maybe a stupid question, but how do you access this in a driver? I have to
> admit I'm no DT expert.

You could get of_node pointers with of_parse_phandle() call and then
lookup related flash and sensor devices based on that.

>> > For the purpose of this patch set presumably just samsung specific
>> > property name could be used (i.e. samsung,flash-leds).
>
> I agree. I'll add similar support for the omap3isp driver in the near future
> though. Let's see how the camera modules will get modelled, if they will,
> and if this property still fits to the picture by that time, then we make it
> more generic.
> 
> What do you think?

I think we could do that, perhaps we could get some more opinions and
use generic name already in this series? I'm not sure what are exact
plans for this series, I guess it is targeted for 4.2?

-- 
Regards,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Thu, May 21, 2015 at 06:58:59PM +0200, Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> On 21/05/15 16:20, Sakari Ailus wrote:
> > On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >> > On 21/05/15 13:32, Sakari Ailus wrote:
> >>>>>> > >>>> @@ -147,6 +149,8 @@ Example:
> >>>>>>>>> > >>>> > >>  			clocks = <&camera 0>;
> >>>>>>>>> > >>>> > >>  			clock-names = "mclk";
> >>>>>>>>> > >>>> > >>
> >>>>>>>>> > >>>> > >>+			samsung,flash-led = <&rear_cam_flash>;
> >>>>>>>>> > >>>> > >>+
> >>>>>>>>> > >>>> > >>  			port {
> >>>>>>>>> > >>>> > >>  				s5c73m3_1: endpoint {
> >>>>>>>>> > >>>> > >>  					data-lanes = <1 2 3 4>;
> >>>>>>> > >>> > >
> >>>>>>> > >>> > >Oops. I missed this property would have ended to the sensor's DT node. I
> >>>>>>> > >>> > >don't think we should have properties here that are parsed by another
> >>>>>>> > >>> > >driver --- let's discuss this tomorrow.
> >>>>> > >> > 
> >>>>> > >> > exynos4-is driver already parses sensor nodes (at least their 'port'
> >>>>> > >> > sub-nodes).
> >>> > >
> >>> > > If you read the code and the comment, it looks like something that should be
> >>> > > done better but hasn't been done yet. :-) That's something we should avoid.
> >>> > > Also, flash devices are by far more common than external ISPs I presume.
> >> > 
> >> > Yes, especially let's not require any samsung specific properties in
> >> > other vendors' sensor bindings.
> >> > 
> >> > One way of modelling [flash led]/[image sensor] association I imagine
> >> > would be to put, e.g. 'flash-leds' property in the SoC camera host
> >> > interface/ISP DT node. This property would then contain pairs of phandles,
> >> > first to the led node and the second to the sensor node, e.g.
> >> > 
> >> > i2c_controller {
> >> > 	...
> >> > 	flash_xx@NN {
> >> > 		...
> >> > 		led_a {
> >> > 			...		
> >> > 		}
> >> > 	};
> >> > 
> >> > 	image_sensor_x@NN {
> >> > 		...
> >> > 	};
> >> > };
> >> > 
> >> > flash-leds = <&flash_xx &image_sensor_x>, <...>;
> >
> > Maybe a stupid question, but how do you access this in a driver? I have to
> > admit I'm no DT expert.
> 
> You could get of_node pointers with of_parse_phandle() call and then
> lookup related flash and sensor devices based on that.

Ack. Looks good to me.

> >> > For the purpose of this patch set presumably just samsung specific
> >> > property name could be used (i.e. samsung,flash-leds).
> >
> > I agree. I'll add similar support for the omap3isp driver in the near future
> > though. Let's see how the camera modules will get modelled, if they will,
> > and if this property still fits to the picture by that time, then we make it
> > more generic.
> > 
> > What do you think?
> 
> I think we could do that, perhaps we could get some more opinions and
> use generic name already in this series? I'm not sure what are exact
> plans for this series, I guess it is targeted for 4.2?

There have been very few opinions expressed besides yours, mine and Jacek's,
unfortunately. I'm also not very certain on the future-proofness of this
solution until we have better understanding of how modules would best be
expressed in DT.

v4.2 would be nice target for these, yes.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester and Jacek,

On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> flash-leds = <&flash_xx &image_sensor_x>, <...>;

One more matter to consider: xenon flash devices.

How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
this awhile, I'm ok with removing the vendor prefix as well.

Let me know what you think.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

Hi,

On 23/05/15 14:03, Sakari Ailus wrote:
> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> 
> One more matter to consider: xenon flash devices.
> 
> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> this awhile, I'm ok with removing the vendor prefix as well.
> 
> Let me know what you think.

I thought about it a bit more and I have some doubts about semantics
as above. I'm fine with 'camera-flashes' as far as name is concerned.

Perhaps we should put only phandles to leds or xenon flash devices
in the 'camera-flashes' property. I think it would be more future
proof in case there is more nodes needed to describe the camera flash
(or a camera module) than the above two. And phandles to corresponding
image sensor device nodes would be put in a separate property.

camera-flashes = <&flash_xx>, ...
camera-flash-masters = <&image_sensor_x>, ...

Then pairs at same index would describe a single flash, 0 would indicate
a null entry if needed.
Similarly we could create properties for other sub-devices of a camera
module, like lenses, etc.

--
Thanks,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

Hi,

On 05/25/2015 02:00 PM, Sylwester Nawrocki wrote:
> Hi,
>
> On 23/05/15 14:03, Sakari Ailus wrote:
>> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>
>> One more matter to consider: xenon flash devices.
>>
>> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>> this awhile, I'm ok with removing the vendor prefix as well.
>>
>> Let me know what you think.
>
> I thought about it a bit more and I have some doubts about semantics
> as above. I'm fine with 'camera-flashes' as far as name is concerned.
>
> Perhaps we should put only phandles to leds or xenon flash devices
> in the 'camera-flashes' property. I think it would be more future
> proof in case there is more nodes needed to describe the camera flash
> (or a camera module) than the above two. And phandles to corresponding
> image sensor device nodes would be put in a separate property.

Could you give examples of the cases you are thinking of?

> camera-flashes = <&flash_xx>, ...
> camera-flash-masters = <&image_sensor_x>, ...
>
> Then pairs at same index would describe a single flash, 0 would indicate
> a null entry if needed.

When it should be needed?

> Similarly we could create properties for other sub-devices of a camera
> module, like lenses, etc.


-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sylwester Nawrocki]

On 25/05/15 14:50, Jacek Anaszewski wrote:
>> On 23/05/15 14:03, Sakari Ailus wrote:
>>> >> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>>> >>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>> >>
>>> >> One more matter to consider: xenon flash devices.
>>> >>
>>> >> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>>> >> this awhile, I'm ok with removing the vendor prefix as well.
>>> >>
>>> >> Let me know what you think.
>> >
>> > I thought about it a bit more and I have some doubts about semantics
>> > as above. I'm fine with 'camera-flashes' as far as name is concerned.
>> >
>> > Perhaps we should put only phandles to leds or xenon flash devices
>> > in the 'camera-flashes' property. I think it would be more future
>> > proof in case there is more nodes needed to describe the camera flash
>> > (or a camera module) than the above two. And phandles to corresponding
>> > image sensor device nodes would be put in a separate property.
>
> Could you give examples of the cases you are thinking of?

I don't have any examples in mind ATM, I just wanted to point out
the above convention might not be flexible enough. Especially since
we already know there is more sub-devices within the camera module
than just flashes and image sensors.

>> > camera-flashes = <&flash_xx>, ...
>> > camera-flash-masters = <&image_sensor_x>, ...
>> >
>> > Then pairs at same index would describe a single flash, 0 would indicate
>> > a null entry if needed.
>
> When it should be needed?

Not sure if there is a real use case for null entries, it was just to
note we can skip any entry if needed - probably an irrelevant comment.
I could imagine 2 LEDs of which one is only triggered in software, so
it wouldn't have a 'camera-flash-masters' entry.

>> > Similarly we could create properties for other sub-devices of a camera
>> > module, like lenses, etc.

--
Regards,
Sylwester

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Jacek Anaszewski]

On 05/25/2015 04:28 PM, Sylwester Nawrocki wrote:
> On 25/05/15 14:50, Jacek Anaszewski wrote:
>>> On 23/05/15 14:03, Sakari Ailus wrote:
>>>>>> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
>>>>>>>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
>>>>>>
>>>>>> One more matter to consider: xenon flash devices.
>>>>>>
>>>>>> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
>>>>>> this awhile, I'm ok with removing the vendor prefix as well.
>>>>>>
>>>>>> Let me know what you think.
>>>>
>>>> I thought about it a bit more and I have some doubts about semantics
>>>> as above. I'm fine with 'camera-flashes' as far as name is concerned.
>>>>
>>>> Perhaps we should put only phandles to leds or xenon flash devices
>>>> in the 'camera-flashes' property. I think it would be more future
>>>> proof in case there is more nodes needed to describe the camera flash
>>>> (or a camera module) than the above two. And phandles to corresponding
>>>> image sensor device nodes would be put in a separate property.
>>
>> Could you give examples of the cases you are thinking of?
>
> I don't have any examples in mind ATM, I just wanted to point out
> the above convention might not be flexible enough. Especially since
> we already know there is more sub-devices within the camera module
> than just flashes and image sensors.
>
>>>> camera-flashes = <&flash_xx>, ...
>>>> camera-flash-masters = <&image_sensor_x>, ...
>>>>
>>>> Then pairs at same index would describe a single flash, 0 would indicate
>>>> a null entry if needed.
>>
>> When it should be needed?
>
> Not sure if there is a real use case for null entries, it was just to
> note we can skip any entry if needed - probably an irrelevant comment.
> I could imagine 2 LEDs of which one is only triggered in software, so
> it wouldn't have a 'camera-flash-masters' entry.
>
>>>> Similarly we could create properties for other sub-devices of a camera
>>>> module, like lenses, etc.

I have had the ninth version of the patch set ready to send today,
so in view of your doubts I made the property samsung specific
so as not to prevent us from going further while we will be
discussing the implementation of the generic property.
The 9th version of the patch set has been just sent.

-- 
Best Regards,
Jacek Anaszewski

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Mon, May 25, 2015 at 04:28:22PM +0200, Sylwester Nawrocki wrote:
> On 25/05/15 14:50, Jacek Anaszewski wrote:
> >> On 23/05/15 14:03, Sakari Ailus wrote:
> >>> >> On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >>>> >>> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> >>> >>
> >>> >> One more matter to consider: xenon flash devices.
> >>> >>
> >>> >> How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> >>> >> this awhile, I'm ok with removing the vendor prefix as well.
> >>> >>
> >>> >> Let me know what you think.
> >> >
> >> > I thought about it a bit more and I have some doubts about semantics
> >> > as above. I'm fine with 'camera-flashes' as far as name is concerned.
> >> >
> >> > Perhaps we should put only phandles to leds or xenon flash devices
> >> > in the 'camera-flashes' property. I think it would be more future
> >> > proof in case there is more nodes needed to describe the camera flash
> >> > (or a camera module) than the above two. And phandles to corresponding
> >> > image sensor device nodes would be put in a separate property.
> >
> > Could you give examples of the cases you are thinking of?
> 
> I don't have any examples in mind ATM, I just wanted to point out
> the above convention might not be flexible enough. Especially since
> we already know there is more sub-devices within the camera module
> than just flashes and image sensors.

I have to admit I've never seen a camera module with an integrated flash.
The lens is part of the module but typically flash is not. That doesn't say
there aren't such devices though.

-- 
Kind regards,

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

Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Sakari Ailus]

Hi Sylwester,

On Mon, May 25, 2015 at 02:00:33PM +0200, Sylwester Nawrocki wrote:
> Hi,
> 
> On 23/05/15 14:03, Sakari Ailus wrote:
> > On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote:
> >> flash-leds = <&flash_xx &image_sensor_x>, <...>;
> > 
> > One more matter to consider: xenon flash devices.
> > 
> > How about samsung,camera-flashes (and ti,camera-flashes)? After pondering
> > this awhile, I'm ok with removing the vendor prefix as well.
> > 
> > Let me know what you think.
> 
> I thought about it a bit more and I have some doubts about semantics
> as above. I'm fine with 'camera-flashes' as far as name is concerned.
> 
> Perhaps we should put only phandles to leds or xenon flash devices
> in the 'camera-flashes' property. I think it would be more future
> proof in case there is more nodes needed to describe the camera flash
> (or a camera module) than the above two. And phandles to corresponding
> image sensor device nodes would be put in a separate property.
> 
> camera-flashes = <&flash_xx>, ...
> camera-flash-masters = <&image_sensor_x>, ...
> 
> Then pairs at same index would describe a single flash, 0 would indicate
> a null entry if needed.
> Similarly we could create properties for other sub-devices of a camera
> module, like lenses, etc.

This arrangement would be advantageous compared to a single property when
adding modules (or lenses) to the equation, and probably more future proof
than "samsung,camera-flashes" / "ti,camera-flashes" I believe.

I'm also ok with keeping it as-is though.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html