misc: mei: fixes for 3.5

misc: mei: fixes for 3.5

[Tomas Winkler]

Hi Greg,
please consider this batch for queuing into 3.5
1. patch 2/4 bug in probe: visible only after merging with the trunk
2. patch 3/4 fixes kernel oops that was revealed when probe failed
3. patch 4/4 is regression fix and should be merged also to stable 3.4
4. patch 1/4 is a minor documentation fix 

 

[PATCH 1/4 V2] mei: mei.txt: minor grammar fixes
[PATCH 2/4] misc: mei: set IRQF_ONESHOT for msi
[PATCH 3/4] misc: mei: unregister misc device in pci_remove
[PATCH 4/4] misc: mei: fix stalled read

[PATCH 1/4 V2] mei: mei.txt: minor grammar fixes

[Tomas Winkler]

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: just resend
 Documentation/misc-devices/mei/mei.txt |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/misc-devices/mei/mei.txt b/Documentation/misc-devices/mei/mei.txt
index 2785697..6ec7029 100644
--- a/Documentation/misc-devices/mei/mei.txt
+++ b/Documentation/misc-devices/mei/mei.txt
@@ -50,25 +50,25 @@ Intel MEI Driver
 The driver exposes a misc device called /dev/mei.
 
 An application maintains communication with an Intel ME feature while
-/dev/mei is open. The binding to a specific features is performed by calling
+/dev/mei is open. The binding to a specific feature is performed by calling
 MEI_CONNECT_CLIENT_IOCTL, which passes the desired UUID.
 The number of instances of an Intel ME feature that can be opened
 at the same time depends on the Intel ME feature, but most of the
 features allow only a single instance.
 
 The Intel AMT Host Interface (Intel AMTHI) feature supports multiple
-simultaneous user applications. Therefore, the Intel MEI driver handles
-this internally by maintaining request queues for the applications.
+simultaneous user connected applications. The Intel MEI driver
+handles this internally by maintaining request queues for the applications.
 
-The driver is oblivious to data that is passed between firmware feature
+The driver is transparent to data that are passed between firmware feature
 and host application.
 
 Because some of the Intel ME features can change the system
 configuration, the driver by default allows only a privileged
 user to access it.
 
-A code snippet for an application communicating with
-Intel AMTHI client:
+A code snippet for an application communicating with Intel AMTHI client:
+
 	struct mei_connect_client_data data;
 	fd = open(MEI_DEVICE);
 
@@ -185,7 +185,7 @@ The Intel AMT Watchdog is composed of two parts:
 	2) Intel MEI driver - connects to the watchdog feature, configures the
 	   watchdog and sends the heartbeats.
 
-The Intel MEI driver uses the kernel watchdog to configure the Intel AMT
+The Intel MEI driver uses the kernel watchdog API to configure the Intel AMT
 Watchdog and to send heartbeats to it. The default timeout of the
 watchdog is 120 seconds.
 
-- 
1.7.4.4

[PATCH 2/4] misc: mei: set IRQF_ONESHOT for msi request_threaded_irq

[Tomas Winkler]

when the default irq quick handler is used then IRQF_ONESHOT must be set
otherwise the request fails and following error is displayed:

genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq ...

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index c703332..88e5953 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -982,7 +982,7 @@ static int __devinit mei_probe(struct pci_dev *pdev,
 		err = request_threaded_irq(pdev->irq,
 			NULL,
 			mei_interrupt_thread_handler,
-			0, mei_driver_name, dev);
+			IRQF_ONESHOT, mei_driver_name, dev);
 	else
 		err = request_threaded_irq(pdev->irq,
 			mei_interrupt_quick_handler,
-- 
1.7.4.4

[PATCH 3/4] misc: mei: unregister misc device in pci_remove function

[Tomas Winkler]

Since the misc device is registered only in the pci probe function
it has to be also unregistered in the counterpart pci remove function
and not in the module exit function.
In case of probe failure the driver was oopsing in module exit function.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 88e5953..a5a17e7 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -1101,6 +1101,8 @@ static void __devexit mei_remove(struct pci_dev *pdev)
 
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
+
+	misc_deregister(&mei_misc_device);
 }
 #ifdef CONFIG_PM
 static int mei_pci_suspend(struct device *device)
@@ -1216,7 +1218,6 @@ module_init(mei_init_module);
  */
 static void __exit mei_exit_module(void)
 {
-	misc_deregister(&mei_misc_device);
 	pci_unregister_driver(&mei_driver);
 
 	pr_debug("unloaded successfully.\n");
-- 
1.7.4.4

[PATCH 4/4] misc: mei: fix stalled read

[Tomas Winkler]

This bug caused severe connectivity issue in the LMS application
(LMS is described in  Documentation/misc-devices/mei/mei.txt)

The bug was introduced in patch:
commit 1ccb7b6249f9bc50678e2a383084ed0a34cc9239
staging/mei: propagate error codes up in the write flow

The patch has reverted the return value logic of some fo function but
the conditional in _mei_irq_thread_read function was not swapped
making read always entering the error path

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/interrupt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 93936f1..23f5463 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -835,7 +835,7 @@ static int _mei_irq_thread_read(struct mei_device *dev,	s32 *slots,
 			struct mei_cl *cl,
 			struct mei_io_list *cmpl_list)
 {
-	if ((*slots * sizeof(u32)) >= (sizeof(struct mei_msg_hdr) +
+	if ((*slots * sizeof(u32)) < (sizeof(struct mei_msg_hdr) +
 			sizeof(struct hbm_flow_control))) {
 		/* return the cancel routine */
 		list_del(&cb_pos->cb_list);
-- 
1.7.4.4

Re: misc: mei: fixes for 3.5

[Greg KH]

On Tue, May 29, 2012 at 04:39:08PM +0300, Tomas Winkler wrote:
> Hi Greg,
> please consider this batch for queuing into 3.5
> 1. patch 2/4 bug in probe: visible only after merging with the trunk
> 2. patch 3/4 fixes kernel oops that was revealed when probe failed
> 3. patch 4/4 is regression fix and should be merged also to stable 3.4
> 4. patch 1/4 is a minor documentation fix 

Grammer fixes are not ok for 3.5 at this point in time, sorry.  I'll
queue up the other 3 though.

greg k-h

RE: misc: mei: fixes for 3.5

[Winkler, Tomas]

> > Hi Greg,
> > please consider this batch for queuing into 3.5 1. patch 2/4 bug in
> > probe: visible only after merging with the trunk 2. patch 3/4 fixes
> > kernel oops that was revealed when probe failed 3. patch 4/4 is
> > regression fix and should be merged also to stable 3.4 4. patch 1/4 is
> > a minor documentation fix
> 
> Grammer fixes are not ok for 3.5 at this point in time, sorry.  I'll queue up the
> other 3 though.

Thanks reasonable. 

Please consider also  this two:

This one is one more fix connected to the same issue as the 3 patches. (From Samuel Oritz) 
http://marc.info/?l=linux-kernel&m=133940653731271&w=2

This one is just adding a flag to WATCHDOG devices that was only possible after merging the watchdog tree into 3.5-rc1. 
This one is not must so I leave it for you to decide. 
http://marc.info/?l=linux-kernel&m=133940653831272&w=2


Thanks
Tomas

Re: misc: mei: fixes for 3.5

[Greg KH]

On Wed, Jun 13, 2012 at 09:09:14PM +0000, Winkler, Tomas wrote:
> 
> 
> > > Hi Greg,
> > > please consider this batch for queuing into 3.5 1. patch 2/4 bug in
> > > probe: visible only after merging with the trunk 2. patch 3/4 fixes
> > > kernel oops that was revealed when probe failed 3. patch 4/4 is
> > > regression fix and should be merged also to stable 3.4 4. patch 1/4 is
> > > a minor documentation fix
> > 
> > Grammer fixes are not ok for 3.5 at this point in time, sorry.  I'll queue up the
> > other 3 though.
> 
> Thanks reasonable. 
> 
> Please consider also  this two:
> 
> This one is one more fix connected to the same issue as the 3 patches. (From Samuel Oritz) 
> http://marc.info/?l=linux-kernel&m=133940653731271&w=2
> 
> This one is just adding a flag to WATCHDOG devices that was only possible after merging the watchdog tree into 3.5-rc1. 
> This one is not must so I leave it for you to decide. 
> http://marc.info/?l=linux-kernel&m=133940653831272&w=2

Yes, they are still in my queue, I'll get to them...

greg k-h

Re: misc: mei: fixes for 3.5

[Greg KH]

On Wed, Jun 13, 2012 at 02:45:09PM -0700, Greg KH wrote:
> On Wed, Jun 13, 2012 at 09:09:14PM +0000, Winkler, Tomas wrote:
> > 
> > 
> > > > Hi Greg,
> > > > please consider this batch for queuing into 3.5 1. patch 2/4 bug in
> > > > probe: visible only after merging with the trunk 2. patch 3/4 fixes
> > > > kernel oops that was revealed when probe failed 3. patch 4/4 is
> > > > regression fix and should be merged also to stable 3.4 4. patch 1/4 is
> > > > a minor documentation fix
> > > 
> > > Grammer fixes are not ok for 3.5 at this point in time, sorry.  I'll queue up the
> > > other 3 though.
> > 
> > Thanks reasonable. 
> > 
> > Please consider also  this two:
> > 
> > This one is one more fix connected to the same issue as the 3 patches. (From Samuel Oritz) 
> > http://marc.info/?l=linux-kernel&m=133940653731271&w=2
> > 
> > This one is just adding a flag to WATCHDOG devices that was only possible after merging the watchdog tree into 3.5-rc1. 
> > This one is not must so I leave it for you to decide. 
> > http://marc.info/?l=linux-kernel&m=133940653831272&w=2
> 
> Yes, they are still in my queue, I'll get to them...

Now applied.