[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-tegra
Subject:    Re: [PATCH v2 1/2] Input: tegra-kbc - add device tree bindings
From:       Dmitry Torokhov <dmitry.torokhov () gmail ! com>
Date:       2011-12-29 9:23:08
Message-ID: 20111229092308.GA3397 () core ! coreip ! homeip ! net
[Download RAW message or body]

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.

> Forcing everyone not to use them doesn't seem right.
> There have been a ton of patches throughout the kernel converting drivers
> to use devm_* and at the same time removing and simplifying error
> handling.
>

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]