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 linux-leds" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to