On Thu, Dec 29, 2011 at 01:23:08AM -0800, Dmitry Torokhov wrote: > On Wed, Dec 28, 2011 at 10:00:06PM -0800, Stephen Warren wrote: > > Dmitry Torokhov wrote at Tuesday, December 27, 2011 11:49 PM: > > > On Tue, Dec 27, 2011 at 10:19:29PM -0800, Olof Johansson wrote: > > > > This adds a simple device tree binding to the tegra keyboard controller. > > ... > > > > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > > > > > > Error handling is missing. I also dislike devm_* facilities as it causes > > > inconsistencies in the way we handle releasing of resources: some of > > > them will be released automatically while others need t be released > > > manually. I prefer having consistent model. > > > > I have to say that I also disagree here. Weren't the devm_* function > > specifically added to allow people not to write all the error-handling > > code themselves. > > I guess they were. Unfortunately they also introduce inconsistency in > the way resources are tracked and freed since some of them support > devm_* facilities while others don't. I might be OK with drivers that > use one model for all resources but this patch was mixing the two. > > devm_* facilities are also not free; you are saving on error handling > code but pay with memory footprint required to implement tracking. FWIW, I agree with Olof and Stephen. The devm_* functions are a good thing and I strongly encourage driver authors to use them. The cost in memory footprint is pretty negligible, but making it easier for driver authors to get things right is huge. g. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
