Skip to contentSkip to conversation

Intel(R) MEI Driver

RFC PATCH In review Oren Weil 4 participants 6 messages Feb 10, 2011 – Feb 11, 2011 Char and misc drivers

lore
Share & tools
lore

Oren Weil

author
#0 lore raw

RFC PATCH 1/13 Intel(R) MEI Driver

drivers/char/Kconfig +11 -0
index b7980a8..0a54b26 100644
@@ -1144,5 +1144,16 @@ config RAMOOPS
 	  This enables panic and oops messages to be logged to a circular
 	  buffer in RAM where it can be read back at some later point.
 
+config INTEL_MEI
+	tristate "Intel(R) Management Engine Interface (Intel(R) MEI)"
+	default m
+	depends on EXPERIMENTAL && X86
+	help
+	  If you have an Intel(R) ME chip in your system 
+	  say Yes to enable the Intel(R) Managment Engine interface.
+	  for more informaiton see
+	  <http://software.intel.com/en-us/manageability/>
+
+
 endmenu
 

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

#1 lore raw

drivers/char/Kconfig config RAMOOPS · line 1149

Quoted 11 lines · On 10 Feb 2011 01:54:57 -0800 Oren Weil wrote:
drivers/char/Kconfig +3 -0
index b7980a8..0a54b26 100644
@@ -1144,5 +1144,16 @@ config RAMOOPS
 	  This enables panic and oops messages to be logged to a circular
 	  buffer in RAM where it can be read back at some later point.
 
+config INTEL_MEI
+	tristate "Intel(R) Management Engine Interface (Intel(R) MEI)"
+	default m

Please don't enable random kconfig options.

drivers/char/Kconfig config RAMOOPS · line 1158

Quoted 9 lines
drivers/char/Kconfig +8 -0
+	depends on EXPERIMENTAL && X86
+	help
+	  If you have an Intel(R) ME chip in your system 
+	  say Yes to enable the Intel(R) Managment Engine interface.
+	  for more informaiton see
+	  <http://software.intel.com/en-us/manageability/>
+
+
 endmenu

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

#2 lore raw
In reply to Oren Weil

drivers/char/Kconfig

Quoted 1 line · Oren Weil
drivers/char/Kconfig +0 -0

Where is the description?

Where is the signed-off-by line?

drivers/char/Kconfig config RAMOOPS · line 1148

Quoted 9 lines

index b7980a8..0a54b26 100644

drivers/char/Kconfig +2 -0
@@ -1144,5 +1144,16 @@ config RAMOOPS
 	  This enables panic and oops messages to be logged to a circular
 	  buffer in RAM where it can be read back at some later point.
 
+config INTEL_MEI
+	tristate "Intel(R) Management Engine Interface (Intel(R) MEI)"

What's with the (R) symbols everywhere? The Linux kernel is not a place for this.

drivers/char/Kconfig config RAMOOPS · line 1149

Quoted 1 line
drivers/char/Kconfig +1 -0
+	default m

No, this is not a valid default.

drivers/char/Kconfig config RAMOOPS · line 1152

Quoted 3 lines
drivers/char/Kconfig +3 -0
+	depends on EXPERIMENTAL && X86
+	help
+	  If you have an Intel(R) ME chip in your system 

Trailing whitespace, which shows that you didn't take the simple time to run your patches through scripts/checkpatch.pl, so why should we take time to review them?

And you don't add a config option as your first patch, it does nothing at this point in time for any code that is existing in the kernel.

greg k-h

#3 lore raw

Hi

1) For the review purpose we wanted to split the patches into files, so the only way we found to do it is using "git diff", this is they there is not signed-off-by line
Maybe I should have adding a description for every file, I can send another mail with the descriptions, sorry for the mess.

2) "Intel(R) Management Engine Interface (Intel(R) MEI)" - is our official name.

3) we ran checkpatch.pl on every file, this file my by skipped by mistake.

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, February 10, 2011 20:01
To: Weil, Oren jer
Cc: [email protected]; [email protected]; [email protected]; Woodhouse, David
Subject: Re: [RFC PATCH 01/13] Intel(R) MEI Driver

drivers/char/Kconfig

Quoted 1 line · Oren Weil
drivers/char/Kconfig +0 -0

Where is the description?

Where is the signed-off-by line?

drivers/char/Kconfig config RAMOOPS · line 1148

Quoted 9 lines

index b7980a8..0a54b26 100644

drivers/char/Kconfig +2 -0
@@ -1144,5 +1144,16 @@ config RAMOOPS
 	  This enables panic and oops messages to be logged to a circular
 	  buffer in RAM where it can be read back at some later point.
 
+config INTEL_MEI
+	tristate "Intel(R) Management Engine Interface (Intel(R) MEI)"

What's with the (R) symbols everywhere? The Linux kernel is not a place for this.

drivers/char/Kconfig config RAMOOPS · line 1149

Quoted 1 line
drivers/char/Kconfig +1 -0
+	default m

No, this is not a valid default.

drivers/char/Kconfig config RAMOOPS · line 1152

Quoted 3 lines
drivers/char/Kconfig +3 -0
+	depends on EXPERIMENTAL && X86
+	help
+	  If you have an Intel(R) ME chip in your system

Trailing whitespace, which shows that you didn't take the simple time to run your patches through scripts/checkpatch.pl, so why should we take time to review them?

And you don't add a config option as your first patch, it does nothing at this point in time for any code that is existing in the kernel.

greg k-h

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

#4 lore raw
Quoted 4 lines · Oren jer

1) For the review purpose we wanted to split the patches into files, so the only way we found to do it is using "git diff", this is they there is not signed-off-by line

So you type it manually, when you're setting an appropriate subject and typing your coherent explanation of what's in each patch, and how it fits together.

Signature · 1 line
dwmw2
#5 lore raw
In reply to Weil, Oren jer
Quoted 7 lines · Oren jer

Hi

1) For the review purpose we wanted to split the patches into files, so the only way we found to do it is using "git diff", this is they there is not signed-off-by line
Maybe I should have adding a description for every file, I can send another mail with the descriptions, sorry for the mess.

Again, please read the documentation on how to properly send patches. We write it for a reason, not to just be ignored.

Actually, if you do ignore it, it's easy for us to ignore you, and somehow I doubt you want that.

Quoted 2 lines

2) "Intel(R) Management Engine Interface (Intel(R) MEI)" - is our official name.

That's nice, but it's not anything that kernel developers want to see all the time. Drop the (R) please, it's pointless within the kernel sources.

Quoted 1 line

3) we ran checkpatch.pl on every file, this file my by skipped by mistake.

It wasn't the only file you skipped, I saw lots of coding style violations in the other files, so I really don't believe you.

good luck,

greg k-h