All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: mei: Organize the initialization state machine.
@ 2011-09-21 13:45 Oren Weil
  2011-09-21 13:45 ` [PATCH 2/2] staging: mei: clean the TODO file from done tasks Oren Weil
  0 siblings, 1 reply; 14+ messages in thread
From: Oren Weil @ 2011-09-21 13:45 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, tomas.winkler, Oren Weil

moving the final state, clearing of the client maps and
updating of mei state out from mei_host_client_properties function.

Acked-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
---
 drivers/staging/mei/init.c      |   30 ++++++++----------------------
 drivers/staging/mei/interrupt.c |   34 +++++++++++++++++++++++++++++++++-
 drivers/staging/mei/mei_dev.h   |    2 +-
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/mei/init.c b/drivers/staging/mei/init.c
index cb0ebbe..8bf3479 100644
--- a/drivers/staging/mei/init.c
+++ b/drivers/staging/mei/init.c
@@ -470,9 +470,12 @@ void mei_allocate_me_clients_storage(struct mei_device *dev)
  *
  * @dev: the device structure
  *
- * returns none.
+ * returns:
+ * 	< 0 - Error.
+ *  = 0 - no more clients.
+ *  = 1 - still have clients to send properties request.
  */
-void mei_host_client_properties(struct mei_device *dev)
+int mei_host_client_properties(struct mei_device *dev)
 {
 	struct mei_msg_hdr *mei_header;
 	struct hbm_props_request *host_cli_req;
@@ -504,32 +507,15 @@ void mei_host_client_properties(struct mei_device *dev)
 			dev->mei_state = MEI_RESETING;
 			dev_dbg(&dev->pdev->dev, "write send enumeration request message to FW fail.\n");
 			mei_reset(dev, 1);
-			return;
+			return -EIO;
 		}
 
 		dev->init_clients_timer = INIT_CLIENTS_TIMEOUT;
 		dev->me_client_index = b;
-		return;
+		return 1;
 	}
 
-
-	/*
-	 * Clear Map for indicating now ME clients
-	 * with associated host client
-	 */
-	bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX);
-	dev->open_handle_count = 0;
-	bitmap_set(dev->host_clients_map, 0, 3);
-	dev->mei_state = MEI_ENABLED;
-
-	/* if wd initialization fails, initialization the AMTHI client,
-	 * otherwise the AMTHI client will be initialized after the WD client connect response
-	 * will be received
-	 */
-	if (mei_wd_host_init(dev))
-		mei_host_init_iamthif(dev);
-
-	return;
+	return 0;
 }
 
 /**
diff --git a/drivers/staging/mei/interrupt.c b/drivers/staging/mei/interrupt.c
index d1da3aa..882d106 100644
--- a/drivers/staging/mei/interrupt.c
+++ b/drivers/staging/mei/interrupt.c
@@ -653,6 +653,7 @@ static void mei_irq_thread_read_bus_message(struct mei_device *dev,
 	struct hbm_host_enum_response *enum_res;
 	struct hbm_client_disconnect_request *disconnect_req;
 	struct hbm_host_stop_request *host_stop_req;
+	int res;
 
 	unsigned char *buffer;
 
@@ -746,7 +747,38 @@ static void mei_irq_thread_read_bus_message(struct mei_device *dev,
 					MEI_CLIENT_PROPERTIES_MESSAGE) {
 				dev->me_client_index++;
 				dev->me_client_presentation_num++;
-				mei_host_client_properties(dev);
+
+				/** Send Client Propeties request **/
+				res = mei_host_client_properties(dev);
+				if (res < 0) {
+					dev_dbg(&dev->pdev->dev, "mei_host_client_properties() failed");
+					return;
+				} else if (!res) {
+					/*
+					 * No more clients to send to.
+					 * Clear Map for indicating now ME clients
+					 * with associated host client
+					 */
+					bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX);
+					dev->open_handle_count = 0;
+
+					/*
+					 * Reserving the first three client IDs
+					 * Client Id 0 - Reserved for MEI Bus Message communications
+					 * Client Id 1 - Reserved for Watchdog
+					 * Client ID 2 - Reserved for AMTHI
+					 */
+					bitmap_set(dev->host_clients_map, 0, 3);
+					dev->mei_state = MEI_ENABLED;
+
+					/* if wd initialization fails, initialization the AMTHI client,
+					 * otherwise the AMTHI client will be initialized after the WD client connect response
+					 * will be received
+					 */
+					if (mei_wd_host_init(dev))
+						mei_host_init_iamthif(dev);
+				}
+
 			} else {
 				dev_dbg(&dev->pdev->dev, "reset due to received host client properties response bus message");
 				mei_reset(dev, 1);
diff --git a/drivers/staging/mei/mei_dev.h b/drivers/staging/mei/mei_dev.h
index 6487be1..af4b1af 100644
--- a/drivers/staging/mei/mei_dev.h
+++ b/drivers/staging/mei/mei_dev.h
@@ -329,7 +329,7 @@ static inline bool mei_cl_cmp_id(const struct mei_cl *cl1,
  */
 void mei_host_start_message(struct mei_device *dev);
 void mei_host_enum_clients_message(struct mei_device *dev);
-void mei_host_client_properties(struct mei_device *dev);
+int mei_host_client_properties(struct mei_device *dev);
 
 /*
  *  MEI interrupt functions prototype
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-21 13:45 [PATCH 1/2] staging: mei: Organize the initialization state machine Oren Weil
@ 2011-09-21 13:45 ` Oren Weil
  2011-09-22 16:29   ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Oren Weil @ 2011-09-21 13:45 UTC (permalink / raw)
  To: gregkh; +Cc: devel, linux-kernel, tomas.winkler, Oren Weil

Acked-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
---
 drivers/staging/mei/TODO |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/mei/TODO b/drivers/staging/mei/TODO
index 3b6a667..7d9a13b 100644
--- a/drivers/staging/mei/TODO
+++ b/drivers/staging/mei/TODO
@@ -1,14 +1,4 @@
 TODO:
-	- Create in-kernel Client API. Examples of in-kernel clients are watchdog and AMTHI.
-	- ME Watchdog Driver to expose standard Linux watchdog interface
-	- Rewrite AMTHI to use in-kernel client interface
-	- Cleanup init and probe functions
-	- Review BUG/BUG_ON usage
-	- Cleanup and reorganize header files
-	- Rewrite client data structure
-	- Make state machine more readable
-	- Add mei.txt with driver explanation and it's driver
-	- Fix Kconfig
 	- Cleanup and split the timer function
 Upon Unstaging:
 	- move mei.h to include/linux/mei.h
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-21 13:45 ` [PATCH 2/2] staging: mei: clean the TODO file from done tasks Oren Weil
@ 2011-09-22 16:29   ` Greg KH
  2011-09-22 18:12     ` Winkler, Tomas
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 16:29 UTC (permalink / raw)
  To: Oren Weil; +Cc: gregkh, devel, tomas.winkler, linux-kernel

On Wed, Sep 21, 2011 at 04:45:31PM +0300, Oren Weil wrote:
> Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
> ---
>  drivers/staging/mei/TODO |   10 ----------
>  1 files changed, 0 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/mei/TODO b/drivers/staging/mei/TODO
> index 3b6a667..7d9a13b 100644
> --- a/drivers/staging/mei/TODO
> +++ b/drivers/staging/mei/TODO
> @@ -1,14 +1,4 @@
>  TODO:
> -	- Create in-kernel Client API. Examples of in-kernel clients are watchdog and AMTHI.

Did you really do this?

> -	- ME Watchdog Driver to expose standard Linux watchdog interface

And this?

> -	- Rewrite AMTHI to use in-kernel client interface

And this?

> -	- Cleanup init and probe functions
> -	- Review BUG/BUG_ON usage
> -	- Cleanup and reorganize header files
> -	- Rewrite client data structure

And this?

> -	- Make state machine more readable
> -	- Add mei.txt with driver explanation and it's driver

You really don't describe the kernel/user api here, that needs to be
well documented as it is a ABI you are creating and we need to know how
it works.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 16:29   ` Greg KH
@ 2011-09-22 18:12     ` Winkler, Tomas
  2011-09-22 18:31       ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Winkler, Tomas @ 2011-09-22 18:12 UTC (permalink / raw)
  To: Greg KH, Weil, Oren jer; +Cc: gregkh, devel, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 22, 2011 7:30 PM
> To: Weil, Oren jer
> Cc: gregkh@suse.de; devel@driverdev.osuosl.org; Winkler, Tomas; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> 
> On Wed, Sep 21, 2011 at 04:45:31PM +0300, Oren Weil wrote:
> > Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> > Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
> > ---
> >  drivers/staging/mei/TODO |   10 ----------
> >  1 files changed, 0 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/mei/TODO b/drivers/staging/mei/TODO index
> > 3b6a667..7d9a13b 100644
> > --- a/drivers/staging/mei/TODO
> > +++ b/drivers/staging/mei/TODO
> > @@ -1,14 +1,4 @@
> >  TODO:
> > -	- Create in-kernel Client API. Examples of in-kernel clients are
> watchdog and AMTHI.
> 
> Did you really do this?
We came to conclusion, this is not really necessary.
> 
> > -	- ME Watchdog Driver to expose standard Linux watchdog interface

Yes, this was submitted in the previous batch and it doesn't required really decupling as we previously estimated.
 
> And this?
> 
> > -	- Rewrite AMTHI to use in-kernel client interface
> 
> And this?
This would be only use case for creating API and in this case it would just bloat the driver for little benefit.

Since this le
> 
> > -	- Cleanup init and probe functions
> > -	- Review BUG/BUG_ON usage
> > -	- Cleanup and reorganize header files
> > -	- Rewrite client data structure
> 
> And this?

This was mostly done, you can judge if this is enough.

> 
> > -	- Make state machine more readable
> > -	- Add mei.txt with driver explanation and it's driver
> 
> You really don't describe the kernel/user api here, that needs to be well
> documented as it is a ABI you are creating and we need to know how it
> works.

The API is described in the mei.txt that is present in the driver folder. There was also lengthy discussion about it 
In LKML during our first try to submit the driver. If it is still not clear enough please let us know we would happy to fix it 
as any other gaps.

Thanks
Tomas
---------------------------------------------------------------------
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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 18:12     ` Winkler, Tomas
@ 2011-09-22 18:31       ` Greg KH
  2011-09-22 19:06         ` Winkler, Tomas
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 18:31 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Thu, Sep 22, 2011 at 09:12:21PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, September 22, 2011 7:30 PM
> > To: Weil, Oren jer
> > Cc: gregkh@suse.de; devel@driverdev.osuosl.org; Winkler, Tomas; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> > 
> > On Wed, Sep 21, 2011 at 04:45:31PM +0300, Oren Weil wrote:
> > > Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> > > Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
> > > ---
> > >  drivers/staging/mei/TODO |   10 ----------
> > >  1 files changed, 0 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/staging/mei/TODO b/drivers/staging/mei/TODO index
> > > 3b6a667..7d9a13b 100644
> > > --- a/drivers/staging/mei/TODO
> > > +++ b/drivers/staging/mei/TODO
> > > @@ -1,14 +1,4 @@
> > >  TODO:
> > > -	- Create in-kernel Client API. Examples of in-kernel clients are
> > watchdog and AMTHI.
> > 
> > Did you really do this?
> We came to conclusion, this is not really necessary.

Why?  Where was that discussion?

> > > -	- ME Watchdog Driver to expose standard Linux watchdog interface
> 
> Yes, this was submitted in the previous batch and it doesn't required
> really decupling as we previously estimated.

Ok, good.

> > And this?
> > 
> > > -	- Rewrite AMTHI to use in-kernel client interface
> > 
> > And this?
> This would be only use case for creating API and in this case it would
> just bloat the driver for little benefit.

Again, where was that discussion?

> Since this le

???

> > 
> > > -	- Cleanup init and probe functions
> > > -	- Review BUG/BUG_ON usage
> > > -	- Cleanup and reorganize header files
> > > -	- Rewrite client data structure
> > 
> > And this?
> 
> This was mostly done, you can judge if this is enough.

Ok, will look at this later.

> > > -	- Make state machine more readable
> > > -	- Add mei.txt with driver explanation and it's driver
> > 
> > You really don't describe the kernel/user api here, that needs to be well
> > documented as it is a ABI you are creating and we need to know how it
> > works.
> 
> The API is described in the mei.txt that is present in the driver folder.

Really?  I see no description of the ioctls and the structures used in
them, no any explaination of any of the sysfs files used in the driver
in that file.  Please fix this up.  The proper format for all sysfs
files is described in Documentation/ABI/README and the ioctls you can
also use the same format.

> There was also lengthy discussion about it In LKML during our first
> try to submit the driver. If it is still not clear enough please let
> us know we would happy to fix it as any other gaps.

It's not clear at all.  Where is the userspace tools that interact with
this driver that allows me to test that the code works properly?  That
would be a good first step as I do have access to this hardware around
here to test with.  Do any distros already have it packaged anywhere?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 18:31       ` Greg KH
@ 2011-09-22 19:06         ` Winkler, Tomas
  2011-09-22 19:38           ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Winkler, Tomas @ 2011-09-22 19:06 UTC (permalink / raw)
  To: Greg KH; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 22, 2011 9:32 PM
> To: Winkler, Tomas
> Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> 
> On Thu, Sep 22, 2011 at 09:12:21PM +0300, Winkler, Tomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Thursday, September 22, 2011 7:30 PM
> > > To: Weil, Oren jer
> > > Cc: gregkh@suse.de; devel@driverdev.osuosl.org; Winkler, Tomas;
> > > linux- kernel@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done
> tasks.
> > >
> > > On Wed, Sep 21, 2011 at 04:45:31PM +0300, Oren Weil wrote:
> > > > Acked-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > Signed-off-by: Oren Weil <oren.jer.weil@intel.com>
> > > > ---
> > > >  drivers/staging/mei/TODO |   10 ----------
> > > >  1 files changed, 0 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/mei/TODO b/drivers/staging/mei/TODO
> > > > index 3b6a667..7d9a13b 100644
> > > > --- a/drivers/staging/mei/TODO
> > > > +++ b/drivers/staging/mei/TODO
> > > > @@ -1,14 +1,4 @@
> > > >  TODO:
> > > > -	- Create in-kernel Client API. Examples of in-kernel clients are
> > > watchdog and AMTHI.
> > >
> > > Did you really do this?
> > We came to conclusion, this is not really necessary.
> 
> Why?  Where was that discussion?

You're right about it  we did it only internally, yet we also produced this requirement internally. Not sure how to address it in this  stage.
> 
> > > > -	- ME Watchdog Driver to expose standard Linux watchdog interface
> >
> > Yes, this was submitted in the previous batch and it doesn't required
> > really decupling as we previously estimated.
> 
> Ok, good.
> 
> > > And this?
> > >
> > > > -	- Rewrite AMTHI to use in-kernel client interface
> > >
> > > And this?
> > This would be only use case for creating API and in this case it would
> > just bloat the driver for little benefit.
> 
> Again, where was that discussion?
> 
> > Since this le
> 
> ???
> 
> > >
> > > > -	- Cleanup init and probe functions
> > > > -	- Review BUG/BUG_ON usage
> > > > -	- Cleanup and reorganize header files
> > > > -	- Rewrite client data structure
> > >
> > > And this?
> >
> > This was mostly done, you can judge if this is enough.
> 
> Ok, will look at this later.
> 
> > > > -	- Make state machine more readable
> > > > -	- Add mei.txt with driver explanation and it's driver
> > >
> > > You really don't describe the kernel/user api here, that needs to be
> > > well documented as it is a ABI you are creating and we need to know
> > > how it works.
> >
> > The API is described in the mei.txt that is present in the driver folder.
> 
> Really?  I see no description of the ioctls and the structures used in them, no
> any explaination of any of the sysfs files used in the driver in that file.  Please
> fix this up.  The proper format for all sysfs files is described in
> Documentation/ABI/README and the ioctls you can also use the same
> format.

We don't really use sysfs anymore this is just legacy named wrappers for adding device class, we should fix that.
We will address the IOCTLs as well.

> > There was also lengthy discussion about it In LKML during our first
> > try to submit the driver. If it is still not clear enough please let
> > us know we would happy to fix it as any other gaps.
> 
> It's not clear at all.  Where is the userspace tools that interact with this driver
> that allows me to test that the code works properly?  That would be a good
> first step as I do have access to this hardware around here to test with.  Do
> any distros already have it packaged anywhere?

In the mei.txt file are links to  freely available APIs. The user space is usually developed by  third parties
yet you can get  AMT Configuration Utility from http://software.intel.com/. We will send the patch to mei.txt that add the link.

I personally don't working with distros, but I believe that SUSE has major deployment of iAMT (MEI).  Oren may have more info about it.

Thanks
Tomas



> thanks,
> 
> 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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 19:06         ` Winkler, Tomas
@ 2011-09-22 19:38           ` Greg KH
  2011-09-22 19:43             ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 19:38 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Thu, Sep 22, 2011 at 10:06:14PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, September 22, 2011 9:32 PM
> > To: Winkler, Tomas
> > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.

Please get a better email client.

Or at least learn to trim stuff that is not needed at all...

> > > > > --- a/drivers/staging/mei/TODO
> > > > > +++ b/drivers/staging/mei/TODO
> > > > > @@ -1,14 +1,4 @@
> > > > >  TODO:
> > > > > -	- Create in-kernel Client API. Examples of in-kernel clients are
> > > > watchdog and AMTHI.
> > > >
> > > > Did you really do this?
> > > We came to conclusion, this is not really necessary.
> > 
> > Why?  Where was that discussion?
> 
> You're right about it  we did it only internally, yet we also produced
> this requirement internally. Not sure how to address it in this
> stage.

Once you listed this as an external TODO item, I would hope that you
would be willing to discuss it externally.  Internal discussions are not
how Linux kernel development works at all, which of course you know.

> > > > > -	- ME Watchdog Driver to expose standard Linux watchdog interface
> > >
> > > Yes, this was submitted in the previous batch and it doesn't required
> > > really decupling as we previously estimated.
> > 
> > Ok, good.
> > 
> > > > And this?
> > > >
> > > > > -	- Rewrite AMTHI to use in-kernel client interface
> > > >
> > > > And this?
> > > This would be only use case for creating API and in this case it would
> > > just bloat the driver for little benefit.
> > 
> > Again, where was that discussion?
> > 
> > > Since this le
> > 
> > ???

Again, you didn't finish this sentence.

> > > > > -	- Cleanup init and probe functions
> > > > > -	- Review BUG/BUG_ON usage
> > > > > -	- Cleanup and reorganize header files
> > > > > -	- Rewrite client data structure
> > > >
> > > > And this?
> > >
> > > This was mostly done, you can judge if this is enough.
> > 
> > Ok, will look at this later.
> > 
> > > > > -	- Make state machine more readable
> > > > > -	- Add mei.txt with driver explanation and it's driver
> > > >
> > > > You really don't describe the kernel/user api here, that needs to be
> > > > well documented as it is a ABI you are creating and we need to know
> > > > how it works.
> > >
> > > The API is described in the mei.txt that is present in the driver folder.
> > 
> > Really?  I see no description of the ioctls and the structures used in them, no
> > any explaination of any of the sysfs files used in the driver in that file.  Please
> > fix this up.  The proper format for all sysfs files is described in
> > Documentation/ABI/README and the ioctls you can also use the same
> > format.
> 
> We don't really use sysfs anymore this is just legacy named wrappers
> for adding device class, we should fix that.
> We will address the IOCTLs as well.

Please do.

> > > There was also lengthy discussion about it In LKML during our first
> > > try to submit the driver. If it is still not clear enough please let
> > > us know we would happy to fix it as any other gaps.
> > 
> > It's not clear at all.  Where is the userspace tools that interact with this driver
> > that allows me to test that the code works properly?  That would be a good
> > first step as I do have access to this hardware around here to test with.  Do
> > any distros already have it packaged anywhere?
> 
> In the mei.txt file are links to  freely available APIs.

Links grow stale.  Please document exactly the interface in the kernel
otherwise we have no idea os what happens here.

> The user space is usually developed by  third parties yet you can get
> AMT Configuration Utility from http://software.intel.com/.

Do you have a direct link to it?  I tried:
	http://search.intel.com/Default.aspx?q=amt+configuration+manager&submit=+&c=en_US&results=10&offset=0&categories=&filetypes=&q2=&q3=&q4=&method=edit#q=amt+configuration+manager&submit=+&c=en_US&results=10&categories=93581&method=edit&m=search

yet did not find any software to download.

> We will send the patch to mei.txt that add the link.

Please provide one that actually works :)

> I personally don't working with distros, but I believe that SUSE has
> major deployment of iAMT (MEI).  Oren may have more info about it.

Ah, this one:
	https://build.opensuse.org/package/show?package=intel-iamt&project=openSUSE%3A11.4
If so, that package has been dropped by SUSE as it seems to no longer be
maintained upstream.  Is this incorrect?

Care to point to the proper download location for the lms tarball?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 19:38           ` Greg KH
@ 2011-09-22 19:43             ` Greg KH
  2011-09-22 19:48               ` Winkler, Tomas
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 19:43 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Thu, Sep 22, 2011 at 12:38:05PM -0700, Greg KH wrote:
> > I personally don't working with distros, but I believe that SUSE has
> > major deployment of iAMT (MEI).  Oren may have more info about it.
> 
> Ah, this one:
> 	https://build.opensuse.org/package/show?package=intel-iamt&project=openSUSE%3A11.4
> If so, that package has been dropped by SUSE as it seems to no longer be
> maintained upstream.  Is this incorrect?
> 
> Care to point to the proper download location for the lms tarball?

No, sorry, I think that tools uses the old, obsolete, heci kernel
driver, and not this "mei" driver, right?

Do you have a pointer to any tools that use this interface for this
driver?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 19:43             ` Greg KH
@ 2011-09-22 19:48               ` Winkler, Tomas
  2011-09-22 19:54                 ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Winkler, Tomas @ 2011-09-22 19:48 UTC (permalink / raw)
  To: Greg KH; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 22, 2011 10:43 PM
> To: Winkler, Tomas
> Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> 
> On Thu, Sep 22, 2011 at 12:38:05PM -0700, Greg KH wrote:
> > > I personally don't working with distros, but I believe that SUSE has
> > > major deployment of iAMT (MEI).  Oren may have more info about it.
> >
> > Ah, this one:
> >
> > https://build.opensuse.org/package/show?package=intel-
> iamt&project=ope
> > nSUSE%3A11.4 If so, that package has been dropped by SUSE as it seems
> > to no longer be maintained upstream.  Is this incorrect?
> >
> > Care to point to the proper download location for the lms tarball?
> 
> No, sorry, I think that tools uses the old, obsolete, heci kernel driver, and not
> this "mei" driver, right?
> 
> Do you have a pointer to any tools that use this interface for this driver?

Please start here
http://software.intel.com/en-us/blogs/2011/06/01/new-updated-intelr-amt-linux-drivers/
There are no direct download links I can provide. I think this was packaged by SUSE and not by openSUSE, but we really need wait for Oren to answer that.

Tomas

> 
> thanks,
> 
> 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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 19:48               ` Winkler, Tomas
@ 2011-09-22 19:54                 ` Greg KH
  2011-09-22 20:11                   ` Winkler, Tomas
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 19:54 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Thu, Sep 22, 2011 at 10:48:52PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, September 22, 2011 10:43 PM
> > To: Winkler, Tomas
> > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> > 
> > On Thu, Sep 22, 2011 at 12:38:05PM -0700, Greg KH wrote:
> > > > I personally don't working with distros, but I believe that SUSE has
> > > > major deployment of iAMT (MEI).  Oren may have more info about it.
> > >
> > > Ah, this one:
> > >
> > > https://build.opensuse.org/package/show?package=intel-
> > iamt&project=ope
> > > nSUSE%3A11.4 If so, that package has been dropped by SUSE as it seems
> > > to no longer be maintained upstream.  Is this incorrect?
> > >
> > > Care to point to the proper download location for the lms tarball?
> > 
> > No, sorry, I think that tools uses the old, obsolete, heci kernel driver, and not
> > this "mei" driver, right?
> > 
> > Do you have a pointer to any tools that use this interface for this driver?
> 
> Please start here
> http://software.intel.com/en-us/blogs/2011/06/01/new-updated-intelr-amt-linux-drivers/

That's a pointer to the same driver that is already in the kernel tree,
right?  What am I missing here?

> There are no direct download links I can provide. I think this was
> packaged by SUSE and not by openSUSE, but we really need wait for Oren
> to answer that.

I searched the internal SUSE package database and didn't find anything
other than the above mentioned intel-iamt tool, which does not seem to
work with the mei driver (which makes sense as the mei driver is not in
any SUSE product that I can tell.)

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 19:54                 ` Greg KH
@ 2011-09-22 20:11                   ` Winkler, Tomas
  2011-09-22 20:18                     ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Winkler, Tomas @ 2011-09-22 20:11 UTC (permalink / raw)
  To: Greg KH; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 22, 2011 10:55 PM
> To: Winkler, Tomas
> Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> 
> On Thu, Sep 22, 2011 at 10:48:52PM +0300, Winkler, Tomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Thursday, September 22, 2011 10:43 PM
> > > To: Winkler, Tomas
> > > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org;
> > > linux- kernel@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done
> tasks.
> > >
> > > On Thu, Sep 22, 2011 at 12:38:05PM -0700, Greg KH wrote:
> > > > > I personally don't working with distros, but I believe that SUSE
> > > > > has major deployment of iAMT (MEI).  Oren may have more info
> about it.
> > > >
> > > > Ah, this one:
> > > >
> > > > https://build.opensuse.org/package/show?package=intel-
> > > iamt&project=ope
> > > > nSUSE%3A11.4 If so, that package has been dropped by SUSE as it
> > > > seems to no longer be maintained upstream.  Is this incorrect?
> > > >
> > > > Care to point to the proper download location for the lms tarball?
> > >
> > > No, sorry, I think that tools uses the old, obsolete, heci kernel
> > > driver, and not this "mei" driver, right?
> > >
> > > Do you have a pointer to any tools that use this interface for this driver?
> >
> > Please start here
> > http://software.intel.com/en-us/blogs/2011/06/01/new-updated-intelr-
> am
> > t-linux-drivers/
> 
> That's a pointer to the same driver that is already in the kernel tree, right?
> What am I missing here?

There is LMS there but I guess this links is more updated, they call it driver but actually it is a user space daemon.

Anyhow this one should be the latest one http://software.intel.com/en-us/articles/download-the-latest-intel-amt-open-source-drivers/?wapkw=%28Linux+AMT%29
It also provides ACU.


 
> > There are no direct download links I can provide. I think this was
> > packaged by SUSE and not by openSUSE, but we really need wait for Oren
> > to answer that.
> 
> I searched the internal SUSE package database and didn't find anything other
> than the above mentioned intel-iamt tool, which does not seem to work with
> the mei driver (which makes sense as the mei driver is not in any SUSE
> product that I can tell.)

Once again, I give you the answer when I have it

Thanks
Tomas

---------------------------------------------------------------------
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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 20:11                   ` Winkler, Tomas
@ 2011-09-22 20:18                     ` Greg KH
  2011-09-22 21:10                       ` Winkler, Tomas
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2011-09-22 20:18 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Thu, Sep 22, 2011 at 11:11:38PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, September 22, 2011 10:55 PM
> > To: Winkler, Tomas
> > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.

Again, please fix your email client.

> > > Please start here
> > > http://software.intel.com/en-us/blogs/2011/06/01/new-updated-intelr-
> > am
> > > t-linux-drivers/
> > 
> > That's a pointer to the same driver that is already in the kernel tree, right?
> > What am I missing here?
> 
> There is LMS there but I guess this links is more updated, they call it driver but actually it is a user space daemon.
> 
> Anyhow this one should be the latest one http://software.intel.com/en-us/articles/download-the-latest-intel-amt-open-source-drivers/?wapkw=%28Linux+AMT%29
> It also provides ACU.

What is "ACU"?

Anyway, we want a good description of just what this driver is exporting
to userspace, and how it is doing it.  That's the important part here,
and is what we need to be able to properly review the code if you wish
to start the process to move out of drivers/staging/

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 20:18                     ` Greg KH
@ 2011-09-22 21:10                       ` Winkler, Tomas
  2011-09-22 21:26                         ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Winkler, Tomas @ 2011-09-22 21:10 UTC (permalink / raw)
  To: Greg KH; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, September 22, 2011 11:18 PM
> To: Winkler, Tomas
> Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> 
> On Thu, Sep 22, 2011 at 11:11:38PM +0300, Winkler, Tomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Thursday, September 22, 2011 10:55 PM
> > > To: Winkler, Tomas
> > > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org;
> > > linux- kernel@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done
> tasks.
> 
> Again, please fix your email client.
> 
> > > > Please start here
> > > > http://software.intel.com/en-us/blogs/2011/06/01/new-updated-intel
> > > > r-
> > > am
> > > > t-linux-drivers/
> > >
> > > That's a pointer to the same driver that is already in the kernel tree, right?
> > > What am I missing here?
> >
> > There is LMS there but I guess this links is more updated, they call it driver
> but actually it is a user space daemon.
> >
> > Anyhow this one should be the latest one
> > http://software.intel.com/en-us/articles/download-the-latest-intel-amt
> > -open-source-drivers/?wapkw=%28Linux+AMT%29
> > It also provides ACU.
> 
> What is "ACU"?
This is actually cli to work with ME,  all the docs can be found through that links.

I guess this is all quite complex, there is  Linux Enablement Guide on the link above it should be useful, 
yet we probably need to think to make something simple for reviewer also run the code in simple way...

> 
> Anyway, we want a good description of just what this driver is exporting to
> userspace, and how it is doing it.  That's the important part here, and is what
> we need to be able to properly review the code if you wish to start the
> process to move out of drivers/staging/

Yes I understand that and hoped we addressed that in mei.txt and patch0. 
Can you please comment directly on mei.txt what is not clear there or are you suggesting 
other form of documentation. We will also review it again and will address your ABI comments.

Briefly since this is all in mei.txt 

MEI provides nothing mere just a tunnel between user space and MEI firmware.
There are many features that MEI firmware can provide and each has its own rich API (we call it also protocol)
 The specific API documentation is available from link in the mei.txt (we need to fill the place holder actually)
The exceptions are Watchdog which is in kernel, talking to MEI firmware watchdog feature. Now it exposes standard
Watchdog Linux API, and AMTHI which just provides multiplexing between more than one user space application for AMTHI feature.

There is only one in/out ioctl  IOCTL_MEI_CONNECT_CLIENT, which after opening /dev/mei specifies which firmware feature we are going talk to.
Client is specified by UUID.  List of UUIDs is not part of the kernel API  (only the wd and amthi are visible in the code), this is up to the application to
to know what client it want to talk to it. 


Thanks
Tomas

---------------------------------------------------------------------
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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
  2011-09-22 21:10                       ` Winkler, Tomas
@ 2011-09-22 21:26                         ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2011-09-22 21:26 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: Weil, Oren jer, gregkh, devel, linux-kernel

On Fri, Sep 23, 2011 at 12:10:53AM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, September 22, 2011 11:18 PM
> > To: Winkler, Tomas
> > Cc: Weil, Oren jer; gregkh@suse.de; devel@driverdev.osuosl.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.
> > 

Oh come on, I'm getting tired of this crap.

Again, learn to trim properly.  And wrap your lines correctly.  This
isn't rocket science, and while I do realize it is September, the month
is almost over so your excuses are about to run out.

> > > Anyhow this one should be the latest one
> > > http://software.intel.com/en-us/articles/download-the-latest-intel-amt
> > > -open-source-drivers/?wapkw=%28Linux+AMT%29
> > > It also provides ACU.
> > 
> > What is "ACU"?
> This is actually cli to work with ME,  all the docs can be found through that links.

"CLI"?

"Colluder of Linux Internals"?

"ME"?

"Millennium Edition"?

I can guess, but again, please spell it out, as I probably got it wrong.

> I guess this is all quite complex, there is  Linux Enablement Guide on
> the link above it should be useful, yet we probably need to think to
> make something simple for reviewer also run the code in simple way...

Yes you do.

> > Anyway, we want a good description of just what this driver is exporting to
> > userspace, and how it is doing it.  That's the important part here, and is what
> > we need to be able to properly review the code if you wish to start the
> > process to move out of drivers/staging/
> 
> Yes I understand that and hoped we addressed that in mei.txt and patch0. 

patch0?

What are you referring to here?

Again, mei.txt does not describe what the api is in any form, please be
explicit and see the links I pointed you to (Documentation/ABI/) for how
to do this properly for your sysfs files.

> Can you please comment directly on mei.txt what is not clear there or are you suggesting 
> other form of documentation. We will also review it again and will address your ABI comments.

I just did.

> Briefly since this is all in mei.txt 
> 
> MEI provides nothing mere just a tunnel between user space and MEI firmware.
> There are many features that MEI firmware can provide and each has its own rich API (we call it also protocol)

Where is the protocol documentated?

> The specific API documentation is available from link in the mei.txt
> (we need to fill the place holder actually)

That would help :)

> The exceptions are Watchdog which is in kernel, talking to MEI
> firmware watchdog feature. Now it exposes standard Watchdog Linux API,
> and AMTHI which just provides multiplexing between more than one user
> space application for AMTHI feature.
> 
> There is only one in/out ioctl  IOCTL_MEI_CONNECT_CLIENT, which after
> opening /dev/mei specifies which firmware feature we are going talk
> to.
> Client is specified by UUID.  List of UUIDs is not part of the kernel
> API  (only the wd and amthi are visible in the code), this is up to
> the application to to know what client it want to talk to it. 

So this is a pass-through to the hardware almost directly?  Again,
document the heck out of this so as to make it obvious as to what
exactly is going on here, as that is not the case today.

And again, we need a link to a working tool that we can test this with.

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-09-22 21:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-21 13:45 [PATCH 1/2] staging: mei: Organize the initialization state machine Oren Weil
2011-09-21 13:45 ` [PATCH 2/2] staging: mei: clean the TODO file from done tasks Oren Weil
2011-09-22 16:29   ` Greg KH
2011-09-22 18:12     ` Winkler, Tomas
2011-09-22 18:31       ` Greg KH
2011-09-22 19:06         ` Winkler, Tomas
2011-09-22 19:38           ` Greg KH
2011-09-22 19:43             ` Greg KH
2011-09-22 19:48               ` Winkler, Tomas
2011-09-22 19:54                 ` Greg KH
2011-09-22 20:11                   ` Winkler, Tomas
2011-09-22 20:18                     ` Greg KH
2011-09-22 21:10                       ` Winkler, Tomas
2011-09-22 21:26                         ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.