Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752353AbbFZXKp (ORCPT ); Fri, 26 Jun 2015 19:10:45 -0400 Received: from mail-yk0-f175.google.com ([209.85.160.175]:36169 "EHLO mail-yk0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751717AbbFZXKg (ORCPT ); Fri, 26 Jun 2015 19:10:36 -0400 MIME-Version: 1.0 In-Reply-To: <558DAEAC.3000800@linux.vnet.ibm.com> References: <558DA29F.3020404@linux.vnet.ibm.com> <20150626191057.GA20142@kroah.com> <558DAEAC.3000800@linux.vnet.ibm.com> From: David Matlack Date: Fri, 26 Jun 2015 16:10:16 -0700 Message-ID: Subject: Re: [PATCH] staging:slicoss:slicoss.h remove volatile variables To: Vikul Gupta Cc: Greg Kroah-Hartman , "Cc: Lior Dotan" , Christopher Harrer , devel@driverdev.osuosl.org, "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2240 Lines: 52 Hi Vikul, welcome! See my comment below... On Fri, Jun 26, 2015 at 12:57 PM, Vikul Gupta wrote: > I am a high school student trying to become familiar with the opensource > process and linux kernel. This is my first submission to the mailing list. > > I fixed the slicoss sub-system. The TODO file asks to remove volatile > variables - also, checkpatch.pl warnings included volatile variables. > > I removed "volatile" from the variables /isr /and /linkstatus/ in the header > file, because they are not needed. The two variables are used in the > slicoss.c file, where /isr/ is used as function parameters, string outputs, > pointers, logic, and one assignment, while /linkstatus /is used as pointers, > logic, and one assignment. All but the assignments will not change these > variables, and the assignment does not warrant a volatile qualifier. It is not safe to simply drop volatile from these fields. For example, slic_card_init polls on isr waiting for the device to write to it. If you drop volatile, the compiler is within its rights to pull the read out of the loop. > > To make sure the changes were correct, I ran the files with checkpatch.pl > again, test built it, and rebooted it. > > Signed-off-by: Vikul Gupta > > diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h > index 3a5aa88..f19f86a 100644 > --- a/drivers/staging/slicoss/slic.h > +++ b/drivers/staging/slicoss/slic.h > @@ -357,8 +357,8 @@ struct base_driver { > }; > > struct slic_shmem { > - volatile u32 isr; > - volatile u32 linkstatus; > + u32 isr; > + u32 linkstatus; > volatile struct slic_stats inicstats; > }; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/