Re: [PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-04-30 Thread Jacek Anaszewski

Hi Sakari and Sylwester,

On 04/03/2015 12:53 PM, Sylwester Nawrocki wrote:

Hello,

On 25/03/15 09:52, Jacek Anaszewski wrote:

On 03/25/2015 02:06 AM, Sakari Ailus wrote:

On Fri, Mar 20, 2015 at 04:03:29PM +0100, Jacek Anaszewski wrote:

This patch adds a description of 'flashes' property
to the samsung-fimc.txt.

Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com
---
   .../devicetree/bindings/media/samsung-fimc.txt |8 
   1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..cb0e263 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera port 
A must be activated
   and the port B deactivated and for the state active-b it should be the 
other
   way around.

+Optional properties:
+
+- flashes - Array of phandles to the flash LEDs that can be controlled by the
+   sub-devices contained in this media device. Flash LED is
+   represented by a child node of a flash LED device


This should be in
Documentation/devicetree/bindings/media/video-interfaces.txt.


This file documents DT nodes starting from the level one below
the camera node aggregating all the devices belonging to the media
device (I am referring to the 'camera' node from the file
arch/arm/boot/dts/exynos4412-trats2.dts). Should 'leds' property be
put there, the aggregating node would have to be described there at
first. I am wondering whether video-interfaces is a suitable place
for documenting illuminators, though.


Should flash devices be associated with sensors somehow rather than ISPs?
That's how they commonly are arranged, however that doesn't limit placing
them in silly places.

I'm not necessarily saying the flashes-property should be present in
sensor's DT nodes, but it'd be good to be able to make the association if
it's there.


IMHO 'flashes' is a misleading name, these are simply high brightness LEDs
which can work as camera flash or auxiliary light for camcording, in context of
a camera device.

The led DT nodes which the entries of above flashes property is pointing to
have a text label, which presumably could be used to associate a LED device
with an image sensor. That said, I think we should allow a property as above
'flashes' be placed in aggregate camera node and also in sensor device node.
I think it should be left to the bridge/ISP binding to choose one option or
the other.


I agree.


For now I would propose to rename the flashes property to samsung,leds or
leds and leave it in camera node.


How about flash-leds?


I know of a SoC, which drives the flash from its on-chip ISP. The GPIO
connected to the flash controller's external strobe pin can be
configured so that the signal is routed to it from the ISP or from
CPU (for software strobe mode).

I think that Sylwester could say more in this subject.



+   (see Documentation/devicetree/bindings/leds/common.txt).
+
   The 'camera' node must include at least one 'fimc' child node.


@@ -166,6 +173,7 @@ Example:
clock-output-names = cam_a_clkout, cam_b_clkout;
pinctrl-names = default;
pinctrl-0 = cam_port_a_clk_active;
+   flashes = camera_flash, system_torch;
status = okay;
#address-cells = 1;
#size-cells = 1;


There will be other kind of devices that have somewhat similar relationship.
They just haven't been defined yet. Lens controllers or EEPROM for instance.
The two are an integral part of a module, something which is not modelled in
DT in any way, but perhaps should be.


Indeed, I'd say it belongs to a particular image sensor (camera module) binding
to describe each its physical subdevices, i.e. if a pointer to lens or EEPROM
is needed in the main module DT node.


Do you suggest using more generic name than 'flashes'?






--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-04-03 Thread Sylwester Nawrocki
Hello,

On 25/03/15 09:52, Jacek Anaszewski wrote:
 On 03/25/2015 02:06 AM, Sakari Ailus wrote:
 On Fri, Mar 20, 2015 at 04:03:29PM +0100, Jacek Anaszewski wrote:
 This patch adds a description of 'flashes' property
 to the samsung-fimc.txt.

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Sylwester Nawrocki s.nawro...@samsung.com
 ---
   .../devicetree/bindings/media/samsung-fimc.txt |8 
   1 file changed, 8 insertions(+)

 diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
 b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 index 922d6f8..cb0e263 100644
 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
 +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 @@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera 
 port A must be activated
   and the port B deactivated and for the state active-b it should be the 
 other
   way around.

 +Optional properties:
 +
 +- flashes - Array of phandles to the flash LEDs that can be controlled by 
 the
 +   sub-devices contained in this media device. Flash LED is
 +   represented by a child node of a flash LED device

 This should be in
 Documentation/devicetree/bindings/media/video-interfaces.txt.

 Should flash devices be associated with sensors somehow rather than ISPs?
 That's how they commonly are arranged, however that doesn't limit placing
 them in silly places.

 I'm not necessarily saying the flashes-property should be present in
 sensor's DT nodes, but it'd be good to be able to make the association if
 it's there.

IMHO 'flashes' is a misleading name, these are simply high brightness LEDs
which can work as camera flash or auxiliary light for camcording, in context of
a camera device.

The led DT nodes which the entries of above flashes property is pointing to
have a text label, which presumably could be used to associate a LED device
with an image sensor. That said, I think we should allow a property as above
'flashes' be placed in aggregate camera node and also in sensor device node.
I think it should be left to the bridge/ISP binding to choose one option or
the other.

For now I would propose to rename the flashes property to samsung,leds or
leds and leave it in camera node.

 I know of a SoC, which drives the flash from its on-chip ISP. The GPIO
 connected to the flash controller's external strobe pin can be
 configured so that the signal is routed to it from the ISP or from
 CPU (for software strobe mode).
 
 I think that Sylwester could say more in this subject.
 
 
 +   (see Documentation/devicetree/bindings/leds/common.txt).
 +
   The 'camera' node must include at least one 'fimc' child node.


 @@ -166,6 +173,7 @@ Example:
 clock-output-names = cam_a_clkout, cam_b_clkout;
 pinctrl-names = default;
 pinctrl-0 = cam_port_a_clk_active;
 +   flashes = camera_flash, system_torch;
 status = okay;
 #address-cells = 1;
 #size-cells = 1;

 There will be other kind of devices that have somewhat similar relationship.
 They just haven't been defined yet. Lens controllers or EEPROM for instance.
 The two are an integral part of a module, something which is not modelled in
 DT in any way, but perhaps should be.

Indeed, I'd say it belongs to a particular image sensor (camera module) binding
to describe each its physical subdevices, i.e. if a pointer to lens or EEPROM
is needed in the main module DT node.

 Do you suggest using more generic name than 'flashes'?


-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-04-03 Thread Sakari Ailus
Hi Jacek,

On Wed, Mar 25, 2015 at 09:52:02AM +0100, Jacek Anaszewski wrote:
 Hi Sakari,
 
 On 03/25/2015 02:06 AM, Sakari Ailus wrote:
 Hi Jacek,
 
 On Fri, Mar 20, 2015 at 04:03:29PM +0100, Jacek Anaszewski wrote:
 This patch adds a description of 'flashes' property
 to the samsung-fimc.txt.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Sylwester Nawrocki s.nawro...@samsung.com
 ---
   .../devicetree/bindings/media/samsung-fimc.txt |8 
   1 file changed, 8 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
 b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 index 922d6f8..cb0e263 100644
 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
 +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 @@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera 
 port A must be activated
   and the port B deactivated and for the state active-b it should be the 
  other
   way around.
 
 +Optional properties:
 +
 +- flashes - Array of phandles to the flash LEDs that can be controlled by 
 the
 +   sub-devices contained in this media device. Flash LED is
 +   represented by a child node of a flash LED device
 
 This should be in
 Documentation/devicetree/bindings/media/video-interfaces.txt.
 
 Should flash devices be associated with sensors somehow rather than ISPs?
 That's how they commonly are arranged, however that doesn't limit placing
 them in silly places.
 
 I'm not necessarily saying the flashes-property should be present in
 sensor's DT nodes, but it'd be good to be able to make the association if
 it's there.
 
 I know of a SoC, which drives the flash from its on-chip ISP. The GPIO
 connected to the flash controller's external strobe pin can be
 configured so that the signal is routed to it from the ISP or from
 CPU (for software strobe mode).
 
 I think that Sylwester could say more in this subject.

Yeah, some ISPs can also generate strobe signals. Otherwise this would be
the job of the sensor, sadly many module vendors ignore the sensor strobe
signal routing even when it's available. The sensor is by far in the best
position to trigger the flash since it has the best readout timing
information.

Currently we have no way to express this, I think what's first needed is the
data in the DT. Probably the flash driver and the driver that provides the
strobe signal should both be aware of this. User space interface wise the
strobe source control could be used --- we'd just need new menu entries.

 
 
 +   (see Documentation/devicetree/bindings/leds/common.txt).
 +
   The 'camera' node must include at least one 'fimc' child node.
 
 
 @@ -166,6 +173,7 @@ Example:
 clock-output-names = cam_a_clkout, cam_b_clkout;
 pinctrl-names = default;
 pinctrl-0 = cam_port_a_clk_active;
 +   flashes = camera_flash, system_torch;
 status = okay;
 #address-cells = 1;
 #size-cells = 1;
 
 There will be other kind of devices that have somewhat similar relationship.
 They just haven't been defined yet. Lens controllers or EEPROM for instance.
 The two are an integral part of a module, something which is not modelled in
 DT in any way, but perhaps should be.
 
 Do you suggest using more generic name than 'flashes'?

So far I don't have a better suggestion for the name of the property.
However, it'd be good to be able to associate the flash to a sensor. I
wonder how wrong would it be to put the flashes property to the port node...

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-03-25 Thread Jacek Anaszewski

Hi Sakari,

On 03/25/2015 02:06 AM, Sakari Ailus wrote:

Hi Jacek,

On Fri, Mar 20, 2015 at 04:03:29PM +0100, Jacek Anaszewski wrote:

This patch adds a description of 'flashes' property
to the samsung-fimc.txt.

Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com
---
  .../devicetree/bindings/media/samsung-fimc.txt |8 
  1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..cb0e263 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera port 
A must be activated
  and the port B deactivated and for the state active-b it should be the other
  way around.

+Optional properties:
+
+- flashes - Array of phandles to the flash LEDs that can be controlled by the
+   sub-devices contained in this media device. Flash LED is
+   represented by a child node of a flash LED device


This should be in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Should flash devices be associated with sensors somehow rather than ISPs?
That's how they commonly are arranged, however that doesn't limit placing
them in silly places.

I'm not necessarily saying the flashes-property should be present in
sensor's DT nodes, but it'd be good to be able to make the association if
it's there.


I know of a SoC, which drives the flash from its on-chip ISP. The GPIO
connected to the flash controller's external strobe pin can be
configured so that the signal is routed to it from the ISP or from
CPU (for software strobe mode).

I think that Sylwester could say more in this subject.



+   (see Documentation/devicetree/bindings/leds/common.txt).
+
  The 'camera' node must include at least one 'fimc' child node.


@@ -166,6 +173,7 @@ Example:
clock-output-names = cam_a_clkout, cam_b_clkout;
pinctrl-names = default;
pinctrl-0 = cam_port_a_clk_active;
+   flashes = camera_flash, system_torch;
status = okay;
#address-cells = 1;
#size-cells = 1;


There will be other kind of devices that have somewhat similar relationship.
They just haven't been defined yet. Lens controllers or EEPROM for instance.
The two are an integral part of a module, something which is not modelled in
DT in any way, but perhaps should be.


Do you suggest using more generic name than 'flashes'?

--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-03-24 Thread Sakari Ailus
Hi Jacek,

On Fri, Mar 20, 2015 at 04:03:29PM +0100, Jacek Anaszewski wrote:
 This patch adds a description of 'flashes' property
 to the samsung-fimc.txt.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Sylwester Nawrocki s.nawro...@samsung.com
 ---
  .../devicetree/bindings/media/samsung-fimc.txt |8 
  1 file changed, 8 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
 b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 index 922d6f8..cb0e263 100644
 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
 +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
 @@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera 
 port A must be activated
  and the port B deactivated and for the state active-b it should be the 
 other
  way around.
  
 +Optional properties:
 +
 +- flashes - Array of phandles to the flash LEDs that can be controlled by the
 + sub-devices contained in this media device. Flash LED is
 + represented by a child node of a flash LED device

This should be in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Should flash devices be associated with sensors somehow rather than ISPs?
That's how they commonly are arranged, however that doesn't limit placing
them in silly places.

I'm not necessarily saying the flashes-property should be present in
sensor's DT nodes, but it'd be good to be able to make the association if
it's there.

 + (see Documentation/devicetree/bindings/leds/common.txt).
 +
  The 'camera' node must include at least one 'fimc' child node.
  
  
 @@ -166,6 +173,7 @@ Example:
   clock-output-names = cam_a_clkout, cam_b_clkout;
   pinctrl-names = default;
   pinctrl-0 = cam_port_a_clk_active;
 + flashes = camera_flash, system_torch;
   status = okay;
   #address-cells = 1;
   #size-cells = 1;

There will be other kind of devices that have somewhat similar relationship.
They just haven't been defined yet. Lens controllers or EEPROM for instance.
The two are an integral part of a module, something which is not modelled in
DT in any way, but perhaps should be.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1 09/11] DT: Add documentation for exynos4-is 'flashes' property

2015-03-20 Thread Jacek Anaszewski
This patch adds a description of 'flashes' property
to the samsung-fimc.txt.

Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com
---
 .../devicetree/bindings/media/samsung-fimc.txt |8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt 
b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 922d6f8..cb0e263 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -40,6 +40,13 @@ should be inactive. For the active-a state the camera port 
A must be activated
 and the port B deactivated and for the state active-b it should be the other
 way around.
 
+Optional properties:
+
+- flashes - Array of phandles to the flash LEDs that can be controlled by the
+   sub-devices contained in this media device. Flash LED is
+   represented by a child node of a flash LED device
+   (see Documentation/devicetree/bindings/leds/common.txt).
+
 The 'camera' node must include at least one 'fimc' child node.
 
 
@@ -166,6 +173,7 @@ Example:
clock-output-names = cam_a_clkout, cam_b_clkout;
pinctrl-names = default;
pinctrl-0 = cam_port_a_clk_active;
+   flashes = camera_flash, system_torch;
status = okay;
#address-cells = 1;
#size-cells = 1;
-- 
1.7.9.5

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