Re: [PATCH 06/12] PTI header file.

[J Freyensee <james_p_freyensee@linux.intel.com>]

On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> On Tue, Feb 08, 2011 at 11:34:51AM -0800, james_p_freyensee@linux.intel.com wrote:
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This adds PTI header information for the PTI project.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > ---
> >  include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++
> 
> Why does this have to be in include/linux/?  What other files use it?
> 
> >  1 files changed, 38 insertions(+), 0 deletions(-)
> >  create mode 100644 include/linux/pti.h
> > 
> > diff --git a/include/linux/pti.h b/include/linux/pti.h
> > new file mode 100644
> > index 0000000..4b1c9f6
> > --- /dev/null
> > +++ b/include/linux/pti.h
> > @@ -0,0 +1,38 @@
> > +/*
> > + *  Copyright (C) Intel 2010
> > + *  Ken Mills <ken.k.mills@intel.com>
> 
> That's nice, where is Ken's signed-off-by?  We need that please.
> 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> > + * USA
> 
> This paragraph is not needed, please remove it unless you are willing to
> keep up with the FSF's office moves for the next 40+ years.
> 
> Same goes for this paragraph on other files in this patchset, please
> remove it.
> 
> > + *
> > + */
> > +
> > +#ifndef PTI_H_
> > +#define PTI_H_
> > +
> > +/* basic structure used as a write address to the PTI HW */
> > +struct masterchannel {
> 
> Pretty generic structure name, please prefix it with "pti_"
> 

no problem, I'll fix all these.

> > +	u8 master;
> > +	u8 channel;
> > +};
> > +
> > +/* the following functions are defined in misc/pti.c */
> > +void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);
> 
> "cp"?
> 

All of the questions concerning these variables are described in pti.c.
Should have the function header comments been copied/moved here??

> > +struct masterchannel *mipi_request_masterchannel(u8 kerneluser);
> 
> kerneluser?
> 
> > +void mipi_release_masterchannel(struct masterchannel *mc);
> 
> "Mipi"?  What's that?

'mipi' is the standard body for the PTI standard.  It's an acronymn that
does not stand for anything other than the Organization controlling the
standard: http://www.mipi.org/about-mipi/frequently-asked-questions#q2

> 
> > +
> > +#define APERTURE_14 0x3800000
> > +#define APERTURE_LEN 0x400000

> What are these for?
> 

Write address and write length for the PTI but I can add that.

Thanks,
jay

> > +
> > +#endif /*PTI_H_*/
> > -- 
> > 1.6.6.1