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

2018-04-06 Thread 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  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 
> >> ---
> >>  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:
> 
> /*
>  * 
>  * 
>  */

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

Mark
--

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

2018-04-06 Thread 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.ku...@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  (maintainer:GREYBUS AUDIO PROTOCOLS 
DRIVERS)
Mark Greer  (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
Johan Hovold  (maintainer:GREYBUS SUBSYSTEM)
Alex Elder  (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman  (maintainer:GREYBUS SUBSYSTEM)
greybus-...@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
de...@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

2018-04-06 Thread 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.ku...@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  (maintainer:GREYBUS AUDIO PROTOCOLS 
> DRIVERS)
> Mark Greer  (maintainer:GREYBUS AUDIO PROTOCOLS 
> DRIVERS)
> Johan Hovold  (maintainer:GREYBUS SUBSYSTEM)
> Alex Elder  (maintainer:GREYBUS SUBSYSTEM)
> Greg Kroah-Hartman  (maintainer:GREYBUS SUBSYSTEM)
> greybus-...@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
> de...@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

2018-04-06 Thread Viresh Kumar 
On 06-04-18, 15:47, Gaurav Dhingra wrote:
> I sent in an updated patchset. Though I forgot to add
> viresh.ku...@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  (maintainer:GREYBUS AUDIO PROTOCOLS 
DRIVERS)
Mark Greer  (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS)
Johan Hovold  (maintainer:GREYBUS SUBSYSTEM)
Alex Elder  (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman  (maintainer:GREYBUS SUBSYSTEM)
greybus-...@lists.linaro.org (moderated list:GREYBUS SUBSYSTEM)
de...@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

2018-04-06 Thread 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  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 
---
  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:

/*
  * 
  * 
  */
I sent in an updated patchset. Though I forgot to add 
viresh.ku...@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 


--
Gaurav Dhingra
(sent from Thunderbird email client)

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

2018-04-05 Thread Viresh Kumar 
On Fri, Apr 6, 2018 at 3:49 AM, Mark Greer  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 
>> ---
>>  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:

/*
 * 
 * 
 */

>> + * (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

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

2018-04-05 Thread 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 
> ---
>  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

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

2018-04-05 Thread 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

2018-04-05 Thread 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 
---
  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)