From: Dan Carpenter <dan.carpenter@oracle.com>
To: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Greg KH <gregkh@linuxfoundation.org>,
Frank Werner-Krippendorf <mail@kripp.ch>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Staging:rtl8712: Style - Removed inline block comment to fix "Statements should start on a tapstop"
Date: Wed, 8 Aug 2018 22:22:12 +0300 [thread overview]
Message-ID: <20180808192212.kboucbzcggchru22@mwanda> (raw)
In-Reply-To: <9b6d5aa1-dc6b-8d0a-78a1-b46d888db54f@lwfinger.net>
On Wed, Aug 08, 2018 at 10:59:26AM -0500, Larry Finger wrote:
> On 08/08/2018 04:22 AM, Greg KH wrote:
> > On Sun, Aug 05, 2018 at 09:41:06PM +0200, Frank Werner-Krippendorf wrote:
> > > Removed 5 inline comments "/*volatile*/" rtl87x_event.h, to fix
> > > a coding style issue "Statements should start on a tabstop"
> > > detected by checkpatch.pl script.
> > >
> > > Signed-off-by: Frank Werner-Krippendorf <mail@kripp.ch>
> > > ---
> > > drivers/staging/rtl8712/rtl871x_event.h | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8712/rtl871x_event.h b/drivers/staging/rtl8712/rtl871x_event.h
> > > index 5171379..2e59e8e 100644
> > > --- a/drivers/staging/rtl8712/rtl871x_event.h
> > > +++ b/drivers/staging/rtl8712/rtl871x_event.h
> > > @@ -90,13 +90,13 @@ struct event_node {
> > > unsigned char *node;
> > > unsigned char evt_code;
> > > unsigned short evt_sz;
> > > - /*volatile*/ int *caller_ff_tail;
> > > + int *caller_ff_tail;
> > > int caller_ff_sz;
> > > };
> > > struct c2hevent_queue {
> > > - /*volatile*/ int head;
> > > - /*volatile*/ int tail;
> >
> > Those "volatile" seem to imply that something is needed for these
> > variables. Please check to ensure that these are ok to remove in that
> > there is the proper locking for these types of things.
>
> Those in-line comments are there as a warning that these variables are
> special.
I assumed the comments meant that the locking was buggy...
I'm 100% serious. Normally that's what volatile means in a driver
context. Then I interpreted Greg's comments as meaning that warnings
are useful because they show us where the locking bugs are so we
shouldn't fix the warning until after we review the locking.
regards,
dan carpenter
next prev parent reply other threads:[~2018-08-08 19:22 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-05 19:41 Frank Werner-Krippendorf
2018-08-08 9:22 ` Greg KH
2018-08-08 15:59 ` Larry Finger
2018-08-08 19:22 ` Dan Carpenter [this message]
2018-08-08 19:25 ` Dan Carpenter
2018-08-08 19:34 ` Dan Carpenter
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=20180808192212.kboucbzcggchru22@mwanda \
--to=dan.carpenter@oracle.com \
--cc=Larry.Finger@lwfinger.net \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mail@kripp.ch \
/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
all inboxes | Powered by JetHome®