Skip to contentSkip to conversation

staging: greybus: Fix warning to limit chars per line

PATCH Superseded Gaurav Dhingra 4 participants 10 messages Apr 3, 2018 – Apr 6, 2018

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: greybus: Fix warning to limit chars per line

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

---
 drivers/staging/greybus/audio_codec.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +3 -1
index a1d5440..01838d9 100644
@@ -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 */
Signature · 1 line
1.9.1
#1 lore raw

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.

Quoted 22 lines · Gaurav Dhingra

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

---
  drivers/staging/greybus/audio_codec.h | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +3 -1
index a1d5440..01838d9 100644
@@ -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 */
Signature · 2 lines
Gaurav Dhingra
(sent from Thunderbird email client)
maintainer
#2 lore raw
Quoted 5 lines · 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.

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

maintainer Reviewed-by on v1 1/1
#3 lore raw
In reply to Gaurav Dhingra
Quoted 24 lines · Gaurav Dhingra

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

---
 drivers/staging/greybus/audio_codec.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +3 -1
index a1d5440..01838d9 100644
@@ -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 */
Signature · 1 line
1.9.1

Hi Gaurav.

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

#4 lore raw
Quoted 16 lines · Mark Greer
Quoted 15 lines · Gaurav Dhingra

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

---
 drivers/staging/greybus/audio_codec.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +0 -0
index a1d5440..01838d9 100644
@@ -23,7 +23,9 @@ enum {
      NUM_CODEC_DAIS,
 };
 
Quoted 2 lines

drivers/staging/greybus/audio_codec.h enum { · line 26

Quoted 2 lines
drivers/staging/greybus/audio_codec.h +1 -1
-/* 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
*/

Quoted 13 lines
Quoted 7 lines
drivers/staging/greybus/audio_codec.h +2 -1
+ * (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.

#5 lore raw

Hi,

Thanks for reviewing the patch.

Quoted 17 lines · Viresh Kumar
Quoted 16 lines · Mark Greer
Quoted 15 lines · Gaurav Dhingra

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

---
  drivers/staging/greybus/audio_codec.h | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +0 -0
index a1d5440..01838d9 100644
@@ -23,7 +23,9 @@ enum {
       NUM_CODEC_DAIS,
  };
 
Quoted 8 lines
Quoted 2 lines

drivers/staging/greybus/audio_codec.h enum { · line 26

Quoted 2 lines
drivers/staging/greybus/audio_codec.h +1 -1
-/* 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 [email protected] 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.

Quoted 12 lines
Quoted 12 lines
Quoted 7 lines
drivers/staging/greybus/audio_codec.h +2 -1
+ * (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.

Signature · 2 lines
Gaurav Dhingra
(sent from Thunderbird email client)
#6 lore raw
Quoted 2 lines · Gaurav Dhingra

I sent in an updated patchset. Though I forgot to add [email protected] 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 <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Mark Greer <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Johan Hovold <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Alex Elder <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman <[email protected]> (maintainer:GREYBUS SUBSYSTEM) [email protected] (moderated list:GREYBUS SUBSYSTEM) [email protected] (open list:STAGING SUBSYSTEM) [email protected] (open list)

Quoted 3 lines

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.

Quoted 4 lines

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 ;)

Signature · 1 line
viresh
#7 lore raw
Quoted 10 lines · Viresh Kumar
Quoted 2 lines · Gaurav Dhingra

I sent in an updated patchset. Though I forgot to add [email protected] 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
Quoted 8 lines

Vaibhav Agarwal <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Mark Greer <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Johan Hovold <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Alex Elder <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman <[email protected]> (maintainer:GREYBUS SUBSYSTEM) [email protected] (moderated list:GREYBUS SUBSYSTEM) [email protected] (open list:STAGING SUBSYSTEM) [email protected] (open list)

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

Signature · 1 line
viresh
#8 lore raw
Quoted 18 lines · Viresh Kumar
Quoted 9 lines · Viresh Kumar
Quoted 2 lines · Gaurav Dhingra

I sent in an updated patchset. Though I forgot to add [email protected] 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
Quoted 8 lines

Vaibhav Agarwal <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Mark Greer <[email protected]> (maintainer:GREYBUS AUDIO PROTOCOLS DRIVERS) Johan Hovold <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Alex Elder <[email protected]> (maintainer:GREYBUS SUBSYSTEM)
Greg Kroah-Hartman <[email protected]> (maintainer:GREYBUS SUBSYSTEM) [email protected] (moderated list:GREYBUS SUBSYSTEM) [email protected] (open list:STAGING SUBSYSTEM) [email protected] (open list)

Quoted 1 line

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.

Signature · 2 lines
Gaurav Dhingra
(sent from Thunderbird email client)
maintainer
#9 lore raw
In reply to Viresh Kumar
Quoted 22 lines · Viresh Kumar
Quoted 16 lines · Mark Greer
Quoted 15 lines · Gaurav Dhingra

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

---
 drivers/staging/greybus/audio_codec.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/greybus/audio_codec.h +0 -0
index a1d5440..01838d9 100644
@@ -23,7 +23,9 @@ enum {
      NUM_CODEC_DAIS,
 };
 
Quoted 2 lines

drivers/staging/greybus/audio_codec.h enum { · line 26

Quoted 2 lines
drivers/staging/greybus/audio_codec.h +1 -1
-/* 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:

Quoted 4 lines

/*
* XXXX
* XXXX
*/

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

Mark
--