From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965369AbbHKO7o (ORCPT ); Tue, 11 Aug 2015 10:59:44 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:35019 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965214AbbHKO7n (ORCPT ); Tue, 11 Aug 2015 10:59:43 -0400 Date: Tue, 11 Aug 2015 20:29:38 +0530 From: Viresh Kumar To: Dan Carpenter Cc: Rafael Wysocki , nm@ti.com, sboyd@codeaurora.org, linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, khilman@linaro.org, Greg Kroah-Hartman , Len Brown , open list , Pavel Machek Subject: Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure Message-ID: <20150811145938.GA32049@linux> References: <334a9052264630b9157fa9bfc3d4efe945054c34.1439288881.git.viresh.kumar@linaro.org> <20150811144345.GN5180@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150811144345.GN5180@mwanda> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11-08-15, 17:43, Dan Carpenter wrote: > > @@ -1323,28 +1323,29 @@ static int _of_init_opp_table_v2(struct device *dev, > > if (ret) { > > dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, > > ret); > > - break; > > + goto free_table; > > } > > } > > > > /* There should be one of more OPP defined */ > > - if (WARN_ON(!count)) > > + if (WARN_ON(!count)) { > > + ret = -ENOENT; > > goto put_opp_np; > > + } The purpose of 'count' here is to see if the dtb contained any OPP nodes or not. i.e. if we ever entered the body of for_each_available_child_of_node() or not.. Its different than, "if we were able to add any OPPs"; > This is weird to me, because we are going backwards. What happens if > we goto free_table without adding anything? It will WARN() today. > I suspect it's fine, but if > it's a bug then this code still has problems. I don't think we have a bug here, we never added anything and so don't need to free it. > What about if we only increment count when _opp_add_static_v2() > succeeds That's not what we want. -- viresh