On Thu, Nov 01, 2012 at 09:08:42PM +0100, Thierry Reding wrote: > On Wed, Oct 31, 2012 at 10:28:01AM +0100, Steffen Trumtrar wrote: > [...] > > +void timings_release(struct display_timings *disp) > > +{ > > + int i; > > + > > + for (i = 0; i < disp->num_timings; i++) > > + kfree(disp->timings[i]); > > +} > > + > > +void display_timings_release(struct display_timings *disp) > > +{ > > + timings_release(disp); > > + kfree(disp->timings); > > +} > > I'm not quite sure I understand how these are supposed to be used. The > only use-case where a struct display_timings is dynamically allocated is > for the OF helpers. In that case, wouldn't it be more useful to have a > function that frees the complete structure, including the struct > display_timings itself? Something like this, which has all of the above > rolled into one: > > void display_timings_free(struct display_timings *disp) > { > if (disp->timings) { > unsigned int i; > > for (i = 0; i < disp->num_timings; i++) > kfree(disp->timings[i]); > } > > kfree(disp->timings); > kfree(disp); > } > Well, you are right. They can be rolled into one function. The extra function call is useless and as it seems confusing. Regards, Steffen > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@xxxxxxxxxxxxxxxx > https://lists.ozlabs.org/listinfo/devicetree-discuss -- 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 | -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html