On Thu, Nov 01, 2012 at 09:15:10PM +0100, Thierry Reding wrote: > On Wed, Oct 31, 2012 at 10:28:02AM +0100, Steffen Trumtrar wrote: > [...] > > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt > [...] > > @@ -0,0 +1,139 @@ > > +display-timings bindings > > +================== > > + > > +display-timings-node > > +------------ > > Maybe extend the underline to the length of the section and subsection > titles respectively? > > > +struct display_timing > > +=================== > > Same here. > > > +config OF_DISPLAY_TIMINGS > > + def_bool y > > + depends on DISPLAY_TIMING > > Maybe this should be called OF_DISPLAY_TIMING to match DISPLAY_TIMING, > or rename DISPLAY_TIMING to DISPLAY_TIMINGS for the sake of consistency? > Yes, to all three above. > > +/** > > + * of_get_display_timing_list - parse all display_timing entries from a device_node > > + * @np: device_node with the subnodes > > + **/ > > +struct display_timings *of_get_display_timing_list(struct device_node *np) > > Perhaps this would better be named of_get_display_timings() to match the > return type? > Hm, I'm not really sure about that. I found it to error prone, to have a function of_get_display_timing and of_get_display_timings. That's why I chose of_get_display_timing_list. But you are correct, that it doesn't match the return value. Maybe I should just make the first function static and change the name as you suggested. > > + disp = kzalloc(sizeof(*disp), GFP_KERNEL); > > Shouldn't you be checking this for allocation failures? > > > + disp->timings = kzalloc(sizeof(struct display_timing *)*disp->num_timings, > > + GFP_KERNEL); > > Same here. > Yes, to both. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
