From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932221AbdJWBRN (ORCPT ); Sun, 22 Oct 2017 21:17:13 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:13945 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932171AbdJWBRK (ORCPT ); Sun, 22 Oct 2017 21:17:10 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout3.samsung.com 20171023011707epoutp031d59a84187039c8c033be25d044ed110~wDiUYcDOo0649706497epoutp03M X-AuditID: b6c32a36-786ac9c000001039-4e-59ed43137385 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="utf-8" Message-id: <59ED4313.701@samsung.com> Date: Mon, 23 Oct 2017 10:17:07 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: SF Markus Elfring , linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-samsung-soc@vger.kernel.org, Krzysztof Kozlowski , Kukjin Kim , MyungJoo Ham , Kyungmin Park Cc: LKML , kernel-janitors@vger.kernel.org Subject: Re: [PATCH] devfreq/exynos-bus: Use common error handling code in exynos_bus_target() In-reply-to: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> X-Brightmail-Tracker: H4sIAAAAAAAAA01Sa0gUYRTt25mdHaWtr1XzYmHbQJKCm7PpNlZGpcRKBVYGWsQ66OQuuQ9m dqOsSCUypfdzUaMoyzJJ0wrbfORqDyoT7J2kiVFaWZS9oJB2HKL+nXs5595zD5cmdDupCNrm cAuig89jqGDyanu0ITYkeTgz7lVzKNdVUkVwV15M4fYNvCe4rq46DddZ9EHD1Q88UXMPfRUU N7KnA3HerhYV11N4jloYbK6vLqHMDZXbzXsvVyNz0d1itXmkPjJNvUaYbxX4HEHUC45sZ47N kZvELF1lSbYkmOLYWDaRm8PoHbxdSGJSlqXFLrHlBTwx+o18nifQSuMliZm1YL7o9LgFvdUp uZOYtSxrNLBxcwxGo9EQP3vdXGNCgJIlWC81jFCue5M2Nb8VC1C/thQF0YDj4VvrBaIUBdM6 3IjgyLMijVL8QHD41xDxl/W6aoiSsQ7XIah6Ey1jLZ4EPw/1kqWIpgk8DTq6N8htAkfD4NeD pDKnF0HTo91qhR8FNw5eVMmYxDPggH+UlDGFY6B18NnY/Il4Ojz+OYBkHIYz4NqJ72OGQrFP BZ+HTqqVDWYo6PeNmQvBWVB++xEhmwjCqXDtTrLMB9xHgX/oPqkckAK3Tv/WKDgE3t2+rJH5 gKdA980khV+MoKGil1SKXQj6zheqFcFsuFtaqFIWT4CP3+RrZLEWdu3UKRQzFFa2UgpeBOXt PlIJqwJByW92P4os+y+vsn95lf2X10lEVKPJgkuy5woS6zIaJN4ueRy5hmynvR6N/WCMqRGd erDMjzCNmPHacexwpk7Nb5Q22/0IaIIJ1VZwgZY2h9+cL4hOi+jJEyQ/SgjEfYCICMt2Bj7a 4baw8Ylx8SZj4LkSWZYJ14bVPsnQ4VzeLWwQBJcg/tWp6KCIAnSdb+yuSVlsct47vX9L5rF3 LsTO/ZLeEzn8JaTkU/vjUc5W3JC/fl5LJ+uuOX5mb51pHNHoOaFtEp8ubluiWr3D2xGR3jJ1 K3e0rbnYp/JuO1tum1H78kdMb/fn9MGj6sPe5/qZ3uWjBhT+9OKe1BX591MnMDUTWyxLV0Yb OqParAwpWXk2hhAl/g+boqlMmQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrBLMWRmVeSWpSXmKPExsVy+t9jQV0h57eRBquuWlmc71zObLH1lrRF /+PXzBbnz29gtzjb9IbdYtPja6wWl3fNYbP43HuE0WLG+X1MFrcbV7A5cHlsWtXJ5rF5Sb1H 35ZVjB5Np9pZPT5vkgtgjeKySUnNySxLLdK3S+DK2Lj5M1vBacGKvc+LGhgf8nYxcnJICJhI PFn+kq2LkYtDSGAdo8SEl6sYQRK8AoISPybfY+li5OBgFpCXOHIpG8JUl5gyJRei/AGjRM/n ucwQ5WoSByatYwKxWQRUJSYe+scCYrMJaEnsf3GDDcTmF1CUuPrjMSPIHFGBCInuE5Ugc0QE djFJXJpwFKyXWcBDouHhLrCZwgIJErOPX2GGWDaPUeJRwyWwezgFPCV2nnCewCgwC8mlsxAu nYVw6QJG5lWMkqkFxbnpucVGBYZ5qeV6xYm5xaV56XrJ+bmbGIHhv+2wVt8OxvtL4g8xCnAw KvHwMhi+jRRiTSwrrsw9xCjBwawkwjvHAijEm5JYWZValB9fVJqTWnyIUZqDRUmc93besUgh gfTEktTs1NSC1CKYLBMHp1QDo8zPM5frgxTN8sq8j5RJ3vh+svigO4OjUvxB0cYKhU0stp8O dqwMKknMdnJ1m3vzxpm1CTuUL9qEhX05d3viBjXh/SKn50+45c50/p2ylmmzzovY5b1i6Q+v 1fuXrGM2/Pfg/mcxX7mw51Nv+b3qf9q+PTrmnuJ/q4usV2bKPXX+7c1boPLzqxJLcUaioRZz UXEiAKyAkKF7AgAA X-CMS-MailID: 20171023011707epcas1p20bfefc16dbbfc16dffbd2e8cc1f81d5f X-Msg-Generator: CA CMS-TYPE: 101P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20171022134057epcas4p43e6f37b8994c28f6ef240536349811a8 X-RootMTR: 20171022134057epcas4p43e6f37b8994c28f6ef240536349811a8 References: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Markus, As I already commented on your other patch related to extcon-max14577, this patch might remove the redundant error message. But, it makes the code more complicated in side of readability. I prefer existing code. On 2017년 10월 22일 22:40, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 22 Oct 2017 15:33:06 +0200 > > Adjust jump targets so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- > drivers/devfreq/exynos-bus.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 49f68929e024..e38d456b2d7d 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -124,34 +124,33 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags) > > if (old_freq < new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > > ret = clk_set_rate(bus->clk, new_freq); > if (ret < 0) { > dev_err(dev, "failed to change clock of bus\n"); > clk_set_rate(bus->clk, old_freq); > - goto out; > + goto unlock; > } > > if (old_freq > new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > bus->curr_freq = new_freq; > > dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n", > old_freq, new_freq, clk_get_rate(bus->clk)); > -out: > +unlock: > mutex_unlock(&bus->lock); > - > return ret; > + > +report_failure: > + dev_err(bus->dev, "failed to set voltage\n"); > + goto unlock; > } > > static int exynos_bus_get_dev_status(struct device *dev, > -- Best Regards, Chanwoo Choi Samsung Electronics From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chanwoo Choi Date: Mon, 23 Oct 2017 01:17:07 +0000 Subject: Re: [PATCH] devfreq/exynos-bus: Use common error handling code in exynos_bus_target() Message-Id: <59ED4313.701@samsung.com> List-Id: References: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> In-Reply-To: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: linux-arm-kernel@lists.infradead.org Hi Markus, As I already commented on your other patch related to extcon-max14577, this patch might remove the redundant error message. But, it makes the code more complicated in side of readability. I prefer existing code. On 2017년 10월 22일 22:40, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 22 Oct 2017 15:33:06 +0200 > > Adjust jump targets so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- > drivers/devfreq/exynos-bus.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 49f68929e024..e38d456b2d7d 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -124,34 +124,33 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags) > > if (old_freq < new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > > ret = clk_set_rate(bus->clk, new_freq); > if (ret < 0) { > dev_err(dev, "failed to change clock of bus\n"); > clk_set_rate(bus->clk, old_freq); > - goto out; > + goto unlock; > } > > if (old_freq > new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > bus->curr_freq = new_freq; > > dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n", > old_freq, new_freq, clk_get_rate(bus->clk)); > -out: > +unlock: > mutex_unlock(&bus->lock); > - > return ret; > + > +report_failure: > + dev_err(bus->dev, "failed to set voltage\n"); > + goto unlock; > } > > static int exynos_bus_get_dev_status(struct device *dev, > -- Best Regards, Chanwoo Choi Samsung Electronics From mboxrd@z Thu Jan 1 00:00:00 1970 From: cw00.choi@samsung.com (Chanwoo Choi) Date: Mon, 23 Oct 2017 10:17:07 +0900 Subject: [PATCH] devfreq/exynos-bus: Use common error handling code in exynos_bus_target() In-Reply-To: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> References: <4a407cd6-020c-1750-019a-6c0ceb40e245@users.sourceforge.net> Message-ID: <59ED4313.701@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Markus, As I already commented on your other patch related to extcon-max14577, this patch might remove the redundant error message. But, it makes the code more complicated in side of readability. I prefer existing code. On 2017? 10? 22? 22:40, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 22 Oct 2017 15:33:06 +0200 > > Adjust jump targets so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- > drivers/devfreq/exynos-bus.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 49f68929e024..e38d456b2d7d 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -124,34 +124,33 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags) > > if (old_freq < new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > > ret = clk_set_rate(bus->clk, new_freq); > if (ret < 0) { > dev_err(dev, "failed to change clock of bus\n"); > clk_set_rate(bus->clk, old_freq); > - goto out; > + goto unlock; > } > > if (old_freq > new_freq) { > ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol); > - if (ret < 0) { > - dev_err(bus->dev, "failed to set voltage\n"); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > } > bus->curr_freq = new_freq; > > dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n", > old_freq, new_freq, clk_get_rate(bus->clk)); > -out: > +unlock: > mutex_unlock(&bus->lock); > - > return ret; > + > +report_failure: > + dev_err(bus->dev, "failed to set voltage\n"); > + goto unlock; > } > > static int exynos_bus_get_dev_status(struct device *dev, > -- Best Regards, Chanwoo Choi Samsung Electronics