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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html