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.anaszewski@xxxxxxxxxxx> >>> Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> >>> Cc: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> >>> --- >>> .../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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html