Re: [PATCH 2/2] staging: mei: clean the TODO file from done tasks.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 22, 2011 at 10:06:14PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@xxxxxxxxx]
> > Sent: Thursday, September 22, 2011 9:32 PM
> > To: Winkler, Tomas
> > Cc: Weil, Oren jer; gregkh@xxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx
> > 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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux