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

[Greg KH]

On Thu, Sep 22, 2011 at 09:12:21PM +0300, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@xxxxxxxxx]
> > Sent: Thursday, September 22, 2011 7:30 PM
> > To: Weil, Oren jer
> > Cc: gregkh@xxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx; Winkler, Tomas; linux-
> > kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxx>
> > > Signed-off-by: Oren Weil <oren.jer.weil@xxxxxxxxx>
> > > ---
> > >  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/