Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx
--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux