public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Vikul Gupta <guptav@linux.vnet.ibm.com>
Cc: "Cc: Lior Dotan" <liodot@gmail.com>,
	Christopher Harrer <charrer@alacritech.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging:slicoss:slicoss.h remove volatile variables
Date: Fri, 26 Jun 2015 13:09:13 -0700	[thread overview]
Message-ID: <20150626200913.GA7696@kroah.com> (raw)
In-Reply-To: <558DAEAC.3000800@linux.vnet.ibm.com>

On Fri, Jun 26, 2015 at 12:57:32PM -0700, 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.

Great, but this paragraph doesn't belong in here, as this whole thing
will end up in the changelog body.  You can put any comments /
discussion below the --- line and it will not show up in the changelog
when I apply the patch.

> I fixed the slicoss sub-system.

The whole subsystem, nice?  :)

Seriously, this sentance is not needed.

> 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

what's with the '/' usage?

> 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.
> 
> To make sure the changes were correct, I ran the files with checkpatch.pl
> again, test built it, and rebooted it.

can you test this code?  rebooting and not loading the driver isn't much
proof of much other than the code still builds (which is a good thing to
test.)  Also, this paragraph is not needed in the changelog either.

Care to fix this all up and resend?

thanks,

greg k-h

  reply	other threads:[~2015-06-26 20:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <558DA29F.3020404@linux.vnet.ibm.com>
     [not found] ` <20150626191057.GA20142@kroah.com>
2015-06-26 19:57   ` [PATCH] staging:slicoss:slicoss.h remove volatile variables Vikul Gupta
2015-06-26 20:09     ` Greg Kroah-Hartman [this message]
2015-06-26 23:10     ` David Matlack

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=20150626200913.GA7696@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=charrer@alacritech.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=guptav@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liodot@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox