From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965401AbcALPHR (ORCPT ); Tue, 12 Jan 2016 10:07:17 -0500 Received: from mail-qk0-f177.google.com ([209.85.220.177]:33829 "EHLO mail-qk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965343AbcALPHN (ORCPT ); Tue, 12 Jan 2016 10:07:13 -0500 MIME-Version: 1.0 In-Reply-To: <5682E3F9.4040006@users.sourceforge.net> References: <566ABCD9.1060404@users.sourceforge.net> <5682E3F9.4040006@users.sourceforge.net> Date: Tue, 12 Jan 2016 16:07:12 +0100 Message-ID: Subject: Re: [PATCH] mmc-core: One check less in mmc_select_hs200() after error detection From: Ulf Hansson To: SF Markus Elfring Cc: linux-mmc , LKML , kernel-janitors@vger.kernel.org, Julia Lawall Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29 December 2015 at 20:50, SF Markus Elfring wrote: > From: Markus Elfring > Date: Tue, 29 Dec 2015 20:28:46 +0100 > > This issue was detected by using the Coccinelle software. > > Move the jump label directly before the desired log statement > so that the variable "err" will not be checked once more > after it was determined that a call of the function > "__mmc_set_signal_voltage" or "__mmc_switch" failed. > Use the identifier "report_failure" instead of the label "err". I understand the report, but this unfortunate not the proper solution. Instead, the "if (err)" check should be entirely removed from the err label. To do that, mmc_select_hs200() should pre validate whether 4 or 8 -bit bus is supported which then prevents starting to switch to hs200 unless really supported. Moreover it means mmc_select_bus_width() shall return 0 to indicate success instead of as now, returning a positive value or 0. Kind regards Uffe > > Signed-off-by: Markus Elfring > --- > drivers/mmc/core/mmc.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 549c56e..866f72b 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1256,7 +1256,7 @@ static int mmc_select_hs200(struct mmc_card *card) > > /* If fails try again during next card power cycle */ > if (err) > - goto err; > + goto report_failure; > > mmc_select_driver_type(card); > > @@ -1276,7 +1276,7 @@ static int mmc_select_hs200(struct mmc_card *card) > card->ext_csd.generic_cmd6_time, > true, send_status, true); > if (err) > - goto err; > + goto report_failure; > old_timing = host->ios.timing; > mmc_set_timing(host, MMC_TIMING_MMC_HS200); > if (!send_status) { > @@ -1289,10 +1289,11 @@ static int mmc_select_hs200(struct mmc_card *card) > mmc_set_timing(host, old_timing); > } > } > -err: > - if (err) > + if (err) { > +report_failure: > pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host), > __func__, err); > + } > return err; > } > > -- > 2.6.3 >