| Message ID | 1432131015-22397-9-git-send-email-j.anaszewski@samsung.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
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.
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. >
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@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). 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.
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 -- 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
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?
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?
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.
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.
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 -- 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
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.
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 -- 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
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.
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.
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.
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>;