From: Dan Carpenter <dan.carpenter@oracle.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Rafael Wysocki <rjw@rjwysocki.net>,
nm@ti.com, sboyd@codeaurora.org, linaro-kernel@lists.linaro.org,
linux-pm@vger.kernel.org, khilman@linaro.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Len Brown <len.brown@intel.com>,
open list <linux-kernel@vger.kernel.org>,
Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure
Date: Wed, 12 Aug 2015 11:11:39 +0300 [thread overview]
Message-ID: <20150812081113.GC32040@mwanda> (raw)
In-Reply-To: <20150812064309.GL32049@linux>
On Wed, Aug 12, 2015 at 12:13:09PM +0530, Viresh Kumar wrote:
> > If the first call to _opp_add_static_v2() fails we call
> > of_free_opp_table() and you say that triggers a WARN().
>
> No it doesn't.
>
> So, coming back to the point you made about freeing table on !count,
> because there were no nodes present in the DT opp table, we have never
> tried to add any OPPs. And so there is no need to call
> of_free_opp_table() in that case.
>
> Do you still think the current code is wrong ?
If it doesn't WARN() then it's not buggy, but it's still ugly. We
should not call of_free_opp_table() because we *tried* to add an OPP, we
should only call it if we *succeeded*.
The way the code is written and from your emails I was afraid that if
you tried to call _opp_add_static_v2() and it fails then it leaves
artifacts lying around that need to be cleaned up by the caller. This
would be the ugliest scenario. But I looked at _opp_add_static_v2()
and looks fine. It cleans up properly on failure. We only need to
clean up if it succeeds.
regards,
dan carpenter
next prev parent reply other threads:[~2015-08-12 8:12 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-11 10:34 [PATCH V2 0/6] PM / OPP: Add debugfs support Viresh Kumar
2015-08-11 10:34 ` [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
2015-08-11 14:43 ` Dan Carpenter
2015-08-11 14:59 ` Viresh Kumar
2015-08-11 17:11 ` Dan Carpenter
2015-08-12 6:43 ` Viresh Kumar
2015-08-12 8:11 ` Dan Carpenter [this message]
2015-08-12 8:23 ` Viresh Kumar
2015-08-12 9:03 ` Dan Carpenter
2015-08-12 10:10 ` Viresh Kumar
2015-08-12 10:52 ` Dan Carpenter
2015-08-11 10:34 ` [PATCH V2 2/6] PM / OPP: reuse of_parse_phandle() Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
2015-08-11 10:34 ` [PATCH V2 3/6] PM / OPP: Prefix exported opp routines with dev_pm_opp_ Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
2015-08-11 10:34 ` [PATCH V2 4/6] PM / OPP: Move opp core to its own directory Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
2015-08-11 10:34 ` [PATCH V2 5/6] PM / OPP: Move cpu specific code to opp/cpu.c Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
2015-08-11 10:34 ` [PATCH V2 6/6] PM / OPP: Add debugfs support Viresh Kumar
2015-08-11 10:34 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150812081113.GC32040@mwanda \
--to=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=khilman@linaro.org \
--cc=len.brown@intel.com \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=nm@ti.com \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
--cc=sboyd@codeaurora.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.