2017-11-23 02:40:35

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/4] rk3399_dmc: Fix line continuation format

On Thu, 2017-11-23 at 11:35 +0900, Chanwoo Choi wrote:
> On 2017년 11월 23일 11:29, Joe Perches wrote:
> > On Thu, 2017-11-23 at 11:23 +0900, Chanwoo Choi wrote:
> > > On 2017년 11월 23일 11:18, Joe Perches wrote:
> > > > never break user-visible strings such as
> > > > printk messages, because that breaks the ability to grep for them
> > >
> > > So, I suggested "Or, we better to modify the error message within 80 char.".
> >
> > Run checkpatch on your suggestion.
> > You will get a "split_string" warning.
>
> I knew about this. I don't like to hurt the readability
> in order to fix the warning with the improper way.
>
> If you want to fix it, I suggested that you better to modify
> the error message within 80 char. Or I prefer the Myungjoo's opinion.

Again, I don't really care how it's fixed,
but the 3 tabs in the middle of the message
are stupid.

Please fix it.


From 1584822582348868065@xxx Thu Nov 23 02:36:32 +0000 2017
X-GM-THRID: 1584243050242866698
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-23 01:22:25

by MyungJoo Ham

[permalink] [raw]
Subject: RE: Re: [PATCH 1/4] rk3399_dmc: Fix line continuation format

> On Wed, 2017-11-22 at 14:13 +0900, Chanwoo Choi wrote:
> > On 2017년 11월 17일 00:27, Joe Perches wrote:
> > > Line continuations with excess spacing causes unexpected output.
> > >
> > > Signed-off-by: Joe Perches <[email protected]>
> > > ---
> > > drivers/devfreq/rk3399_dmc.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> > > index 5dfbfa3cc878..0938c97d46f0 100644
> > > --- a/drivers/devfreq/rk3399_dmc.c
> > > +++ b/drivers/devfreq/rk3399_dmc.c
> > > @@ -146,8 +146,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> > >
> > > /* If get the incorrect rate, set voltage to old value. */
> > > if (dmcfreq->rate != target_rate) {
> > > - dev_err(dev, "Get wrong ddr frequency, Request frequency %lu,\
> > > - Current frequency %lu\n", target_rate, dmcfreq->rate);
> > > + dev_err(dev, "Get wrong ddr frequency, Request frequency %lu, Current frequency %lu\n",
> > > + target_rate, dmcfreq->rate);
> >
> > IMO, I don't like over 80 char in the one line.
>
> Fix it as you chose, but the code I proposed
> is what is preferred by CodingStyle.
>
> The current code is unintentional.
>
> Right now there are 3 tabs between "Request frequency"
> and "Current frequency" in the output.

Chanwoo, this is not a simple coding style issue.
I'm seeing these unintentional tabs as well.

If you want to keep it 80 cols with strings (which is not mandatory for strings in double quotes),
We'd better do:

- dev_err(dev, "Get wrong ddr frequency, Request frequency %lu,\
- Current frequency %lu\n", target_rate, dmcfreq->rate);
+ dev_err(dev, "Get wrong ddr frequency, Request frequency %lu,"
+ " Current frequency %lu\n", target_rate, dmcfreq->rate);

Cheers,
MyungJoo

>
> > > regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
> > > dmcfreq->volt);
> > > goto out;
> > >
> >
> >
>


From 1584762560222120335@xxx Wed Nov 22 10:42:31 +0000 2017
X-GM-THRID: 1584243050242866698
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread