From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Cc: Wolfram Sang <wsa@kernel.org>,
linux-i2c <linux-i2c@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Ray Jui <rjui@broadcom.com>,
Scott Branden <sbranden@broadcom.com>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
Lori Hikichi <lori.hikichi@broadcom.com>,
Robert Richter <rrichter@marvell.com>,
Nishka Dasgupta <nishkadg.linux@gmail.com>,
linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH V1 2/2] i2c: iproc: add slave pec support
Date: Thu, 16 Jul 2020 21:26:24 +0300 [thread overview]
Message-ID: <20200716182624.GT3703480@smile.fi.intel.com> (raw)
In-Reply-To: <CAHO=5PEMou=a7Kqc=_ZJ8V9FQ=dHA0cJkkojdq91NBsO1Dp3TQ@mail.gmail.com>
On Thu, Jul 16, 2020 at 10:49:14PM +0530, Rayagonda Kokatanur wrote:
> On Thu, Jul 16, 2020 at 3:44 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jul 16, 2020 at 11:14 AM Rayagonda Kokatanur
> > <rayagonda.kokatanur@broadcom.com> wrote:
...
> > > -#define S_RX_PEC_ERR_SHIFT 29
> > > +#define S_RX_PEC_ERR_SHIFT 28
> > This needs to be explained in the commit message, in particular why
> > this change makes no regression.
>
> I didn't get what do you mean by "no regression", please elaborate.
The definition above has been changed. The point is you have to point out in
the commit message why it's okay and makes no regression.
For example, "..._SHIFT is changed to ... according to documentation. Since
there was no user of it no regression will be made." Provide proper text, b/c
I have no idea what is exactly the reason of the change and if it's indeed used
to have no users.
...
> > > + ret = bcm_iproc_smbus_check_slave_pec(iproc_i2c,
> > > + val);
> >
> > One line looks better.
>
> Yes, but to have 80 char per line, I have to do this.
We have more, but even if you stick with 80 the above is harder to get than if it is one line.
--
With Best Regards,
Andy Shevchenko
prev parent reply other threads:[~2020-07-16 18:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 8:08 [PATCH V1 0/2] add PEC support on slave side Rayagonda Kokatanur
2020-07-16 8:08 ` [PATCH V1 1/2] i2c: add PEC error event Rayagonda Kokatanur
2020-07-16 8:08 ` [PATCH V1 2/2] i2c: iproc: add slave pec support Rayagonda Kokatanur
2020-07-16 10:14 ` Andy Shevchenko
2020-07-16 17:19 ` Rayagonda Kokatanur
2020-07-16 18:26 ` Andy Shevchenko [this message]
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=20200716182624.GT3703480@smile.fi.intel.com \
--to=andy.shevchenko@gmail.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lori.hikichi@broadcom.com \
--cc=nishkadg.linux@gmail.com \
--cc=rayagonda.kokatanur@broadcom.com \
--cc=rjui@broadcom.com \
--cc=rrichter@marvell.com \
--cc=sbranden@broadcom.com \
--cc=wsa@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).