[PATCH] staging: greybus: Fix warning to limit chars per line

[PATCH] staging: greybus: Fix warning to limit chars per line

[Gaurav Dhingra]

Wrap comment to fix warning "prefer a maximum 75 chars per line"

Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
---
 drivers/staging/greybus/audio_codec.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
index a1d5440..01838d9 100644
--- a/drivers/staging/greybus/audio_codec.h
+++ b/drivers/staging/greybus/audio_codec.h
@@ -23,7 +23,9 @@ enum {
 	NUM_CODEC_DAIS,
 };
 
-/* device_type should be same as defined in audio.h (Android media layer) */
+/* device_type should be same as defined in audio.h
+ * (Android media layer)
+ */
 enum {
 	GBAUDIO_DEVICE_NONE                     = 0x0,
 	/* reserved bits */
-- 
1.9.1

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Gaurav Dhingra]

Hi,

Can anyone please review this patchset? Since this is my first patch 
set, so I felt it was okay to make change even in the comment, though 
the patch set fixes the warning. Please let me know in anycase your view 
on the patchset.


On Wednesday 04 April 2018 12:02 AM, Gaurav Dhingra wrote:
> Wrap comment to fix warning "prefer a maximum 75 chars per line"
>
> Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
> ---
>   drivers/staging/greybus/audio_codec.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
> index a1d5440..01838d9 100644
> --- a/drivers/staging/greybus/audio_codec.h
> +++ b/drivers/staging/greybus/audio_codec.h
> @@ -23,7 +23,9 @@ enum {
>   	NUM_CODEC_DAIS,
>   };
>   
> -/* device_type should be same as defined in audio.h (Android media layer) */
> +/* device_type should be same as defined in audio.h
> + * (Android media layer)
> + */
>   enum {
>   	GBAUDIO_DEVICE_NONE                     = 0x0,
>   	/* reserved bits */

-- 
Gaurav Dhingra
(sent from Thunderbird email client)

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Greg KH]

On Thu, Apr 05, 2018 at 12:47:19PM +0530, Gaurav Dhingra wrote:
> Hi,
> 
> Can anyone please review this patchset? Since this is my first patch set, so
> I felt it was okay to make change even in the comment, though the patch set
> fixes the warning. Please let me know in anycase your view on the patchset.

You sent a patch for a staging driver 24 hours ago.  Please be patient.
Normally patches get reviewed within 2 weeks.  Given that this is the
middle of the merge window, it will probably be at least 3 weeks before
I can get to them.

There is no rush, please relax.

greg k-h

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Mark Greer]

On Wed, Apr 04, 2018 at 12:02:46AM +0530, Gaurav Dhingra wrote:
> Wrap comment to fix warning "prefer a maximum 75 chars per line"
> 
> Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
> ---
>  drivers/staging/greybus/audio_codec.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
> index a1d5440..01838d9 100644
> --- a/drivers/staging/greybus/audio_codec.h
> +++ b/drivers/staging/greybus/audio_codec.h
> @@ -23,7 +23,9 @@ enum {
>  	NUM_CODEC_DAIS,
>  };
>  
> -/* device_type should be same as defined in audio.h (Android media layer) */
> +/* device_type should be same as defined in audio.h
> + * (Android media layer)
> + */
>  enum {
>  	GBAUDIO_DEVICE_NONE                     = 0x0,
>  	/* reserved bits */
> -- 
> 1.9.1

Hi Gaurav.

Thank you for the patch, it looks fine to me.

Reviewed-by: Mark Greer <mgreer@animalcreek.com>

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Viresh Kumar]

On Fri, Apr 6, 2018 at 3:49 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Wed, Apr 04, 2018 at 12:02:46AM +0530, Gaurav Dhingra wrote:
>> Wrap comment to fix warning "prefer a maximum 75 chars per line"
>>
>> Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
>> ---
>>  drivers/staging/greybus/audio_codec.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
>> index a1d5440..01838d9 100644
>> --- a/drivers/staging/greybus/audio_codec.h
>> +++ b/drivers/staging/greybus/audio_codec.h
>> @@ -23,7 +23,9 @@ enum {
>>       NUM_CODEC_DAIS,
>>  };
>>
>> -/* device_type should be same as defined in audio.h (Android media layer) */
>> +/* device_type should be same as defined in audio.h

This isn't the right way to write a multi-line comment. It should be like:

/*
 * XXXX
 * XXXX
 */

>> + * (Android media layer)
>> + */
>>  enum {
>>       GBAUDIO_DEVICE_NONE                     = 0x0,
>>       /* reserved bits */
>> --
>> 1.9.1
>
> Hi Gaurav.
>
> Thank you for the patch, it looks fine to me.
>
> Reviewed-by: Mark Greer <mgreer@animalcreek.com>

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Gaurav Dhingra]

Hi,

Thanks for reviewing the patch.


On Friday 06 April 2018 10:52 AM, Viresh Kumar wrote:
> On Fri, Apr 6, 2018 at 3:49 AM, Mark Greer <mgreer@animalcreek.com> wrote:
>> On Wed, Apr 04, 2018 at 12:02:46AM +0530, Gaurav Dhingra wrote:
>>> Wrap comment to fix warning "prefer a maximum 75 chars per line"
>>>
>>> Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
>>> ---
>>>   drivers/staging/greybus/audio_codec.h | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
>>> index a1d5440..01838d9 100644
>>> --- a/drivers/staging/greybus/audio_codec.h
>>> +++ b/drivers/staging/greybus/audio_codec.h
>>> @@ -23,7 +23,9 @@ enum {
>>>        NUM_CODEC_DAIS,
>>>   };
>>>
>>> -/* device_type should be same as defined in audio.h (Android media layer) */
>>> +/* device_type should be same as defined in audio.h
> This isn't the right way to write a multi-line comment. It should be like:
>
> /*
>   * XXXX
>   * XXXX
>   */
I sent in an updated patchset. Though I forgot to add 
viresh.kumar@linaro.org to "To" in mail. I tried to follow instructions 
described on https://kernelnewbies.org/FirstKernelPatch for updating my 
patch. Do you think I followed the instructions correctly? I was 
thinking may be I need to update the already sent patch by adding *new 
commit* to my already existing commit on that git branch, but instead I 
tried to do what I understood from the website I mentioned above.
>>> + * (Android media layer)
>>> + */
>>>   enum {
>>>        GBAUDIO_DEVICE_NONE                     = 0x0,
>>>        /* reserved bits */
>>> --
>>> 1.9.1
>> Hi Gaurav.
>>
>> Thank you for the patch, it looks fine to me.
>>
>> Reviewed-by: Mark Greer <mgreer@animalcreek.com>

-- 
Gaurav Dhingra
(sent from Thunderbird email client)

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Viresh Kumar]

On 06-04-18, 15:47, Gaurav Dhingra wrote:
> I sent in an updated patchset. Though I forgot to add
> viresh.kumar@linaro.org to "To" in mail.

That's fine, but you still haven't sent it to all the relevant people.
You should have used the get_maintainer script (present in kernel
source) for that.

$ scripts/get_maintainer.pl drivers/staging/greybus/audio_codec.h

Vaibhav Agarwal <vaibhav.sr@gmail.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
Mark Greer <mgreer@animalcreek.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
Johan Hovold <johan@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
Alex Elder <elder@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (maintainer:GREYBUS SUBSYSTEM)
greybus-dev@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
devel@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
linux-kernel@vger.kernel.org (open list)

> I tried to follow instructions
> described on https://kernelnewbies.org/FirstKernelPatch for updating my
> patch. Do you think I followed the instructions correctly?

Mostly yes, you did it fine.

> I was thinking
> may be I need to update the already sent patch by adding *new commit* to my
> already existing commit on that git branch, but instead I tried to do what I
> understood from the website I mentioned above.

Sorry, I find it difficult to understand what you wrote ;)

-- 
viresh

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Viresh Kumar]

On 06-04-18, 15:55, Viresh Kumar wrote:
> On 06-04-18, 15:47, Gaurav Dhingra wrote:
> > I sent in an updated patchset. Though I forgot to add
> > viresh.kumar@linaro.org to "To" in mail.
> 
> That's fine, but you still haven't sent it to all the relevant people.
> You should have used the get_maintainer script (present in kernel
> source) for that.
> 
> $ scripts/get_maintainer.pl drivers/staging/greybus/audio_codec.h
> 
> Vaibhav Agarwal <vaibhav.sr@gmail.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
> Mark Greer <mgreer@animalcreek.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
> Johan Hovold <johan@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
> Alex Elder <elder@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (maintainer:GREYBUS SUBSYSTEM)
> greybus-dev@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
> devel@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
> linux-kernel@vger.kernel.org (open list)

Actually your cc list in V2 is fine, it wasn't correct in v1.

-- 
viresh

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Gaurav Dhingra]

On Friday 06 April 2018 03:56 PM, Viresh Kumar wrote:
> On 06-04-18, 15:55, Viresh Kumar wrote:
>> On 06-04-18, 15:47, Gaurav Dhingra wrote:
>>> I sent in an updated patchset. Though I forgot to add
>>> viresh.kumar@linaro.org to "To" in mail.
>> That's fine, but you still haven't sent it to all the relevant people.
>> You should have used the get_maintainer script (present in kernel
>> source) for that.
>>
>> $ scripts/get_maintainer.pl drivers/staging/greybus/audio_codec.h
>>
>> Vaibhav Agarwal <vaibhav.sr@gmail.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
>> Mark Greer <mgreer@animalcreek.com> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
>> Johan Hovold <johan@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
>> Alex Elder <elder@kernel.org> (maintainer:GREYBUS SUBSYSTEM)
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (maintainer:GREYBUS SUBSYSTEM)
>> greybus-dev@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
>> devel@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
>> linux-kernel@vger.kernel.org (open list)
> Actually your cc list in V2 is fine, it wasn't correct in v1.
Yes, I too think it is correct. In v1 I forgot to remove '--nol' from 
command shown on kernelnewbies/FirstKernelPatch webpage and that removed 
a few relevant mailing lists.

-- 
Gaurav Dhingra
(sent from Thunderbird email client)

Re: [PATCH] staging: greybus: Fix warning to limit chars per line

[Mark Greer]

On Fri, Apr 06, 2018 at 10:52:17AM +0530, Viresh Kumar wrote:
> On Fri, Apr 6, 2018 at 3:49 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> > On Wed, Apr 04, 2018 at 12:02:46AM +0530, Gaurav Dhingra wrote:
> >> Wrap comment to fix warning "prefer a maximum 75 chars per line"
> >>
> >> Signed-off-by: Gaurav Dhingra <gauravdhingra.gxyd@gmail.com>
> >> ---
> >>  drivers/staging/greybus/audio_codec.h | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/greybus/audio_codec.h b/drivers/staging/greybus/audio_codec.h
> >> index a1d5440..01838d9 100644
> >> --- a/drivers/staging/greybus/audio_codec.h
> >> +++ b/drivers/staging/greybus/audio_codec.h
> >> @@ -23,7 +23,9 @@ enum {
> >>       NUM_CODEC_DAIS,
> >>  };
> >>
> >> -/* device_type should be same as defined in audio.h (Android media layer) */
> >> +/* device_type should be same as defined in audio.h
> 
> This isn't the right way to write a multi-line comment. It should be like:
> 
> /*
>  * XXXX
>  * XXXX
>  */

Ugh, yeah, you're right.  I must have been sleeping.

Mark
--