From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6205270918308036608 X-Received: by 10.182.58.105 with SMTP id p9mr81563obq.21.1444779543439; Tue, 13 Oct 2015 16:39:03 -0700 (PDT) X-BeenThere: outreachy-kernel@googlegroups.com Received: by 10.182.16.165 with SMTP id h5ls1564813obd.93.gmail; Tue, 13 Oct 2015 16:39:03 -0700 (PDT) X-Received: by 10.182.28.73 with SMTP id z9mr74475obg.45.1444779543172; Tue, 13 Oct 2015 16:39:03 -0700 (PDT) Return-Path: Received: from mail.linuxfoundation.org (mail.linuxfoundation.org. [140.211.169.12]) by gmr-mx.google.com with ESMTPS id pe1si547667pac.2.2015.10.13.16.39.03 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Oct 2015 16:39:03 -0700 (PDT) Received-SPF: pass (google.com: domain of gregkh@linuxfoundation.org designates 140.211.169.12 as permitted sender) client-ip=140.211.169.12; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of gregkh@linuxfoundation.org designates 140.211.169.12 as permitted sender) smtp.mailfrom=gregkh@linuxfoundation.org Received: from localhost (50-76-40-150-ip-static.hfc.comcastbusiness.net [50.76.40.150]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id B20F09D; Tue, 13 Oct 2015 23:39:02 +0000 (UTC) Date: Tue, 13 Oct 2015 16:39:02 -0700 From: Greg KH To: Shivani Bhardwaj Cc: outreachy-kernel Subject: Re: [Outreachy kernel] [PATCH] Staging: rts5208: sd: Remove redundant code Message-ID: <20151013233902.GA9458@kroah.com> References: <20151013230010.GA16506@ubuntu> <20151013231421.GA8240@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) On Wed, Oct 14, 2015 at 04:54:45AM +0530, Shivani Bhardwaj wrote: > On Wed, Oct 14, 2015 at 4:44 AM, Greg KH wrote: > > On Wed, Oct 14, 2015 at 04:30:10AM +0530, Shivani Bhardwaj wrote: > >> Change the order of statements and remove extra code. > >> > >> Signed-off-by: Shivani Bhardwaj > >> --- > >> drivers/staging/rts5208/sd.c | 11 ++++------- > >> 1 file changed, 4 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c > >> index d6c4982..4269be9 100644 > >> --- a/drivers/staging/rts5208/sd.c > >> +++ b/drivers/staging/rts5208/sd.c > >> @@ -2707,17 +2707,14 @@ RTY_SD_RST: > >> return STATUS_FAIL; > >> } > >> > >> + CLR_SD_HCXC(sd_card); > >> + > >> if (hi_cap_flow) { > >> if (rsp[1] & 0x40) > >> SET_SD_HCXC(sd_card); > >> - else > >> - CLR_SD_HCXC(sd_card); > >> - > >> - support_1v8 = false; > >> - } else { > >> - CLR_SD_HCXC(sd_card); > >> - support_1v8 = false; > >> } > >> + > >> + support_1v8 = false; > >> dev_dbg(rtsx_dev(chip), "support_1v8 = %d\n", support_1v8); > >> > >> if (support_1v8) { > > > > Are you sure you didn't change the logic of what is happening here? > > > > Why are you making this change? What prompted it? > > > > thanks, > > > > greg k-h > > Logic remains fine, I think. support_1v8 statement was common to if > and else so it is taken out. Also, it can be noted that programmer > wants to set sd_card value only if hi_cap_flow is not null and (rsp[1] > & 0x40) is true. For rest of the cases, programmer wants to clear the > sd_card. > I think I'm sure about the changes made. But now you always call CLR_SD_HCXC() where before there was one code path that did not make that call. Are you sure it is safe to call CLR_SD_HCXC() before SET_SD_HCXC()? thanks, greg k-h