Re: [Outreachy kernel] Clean-up idea: IIO Headers

[Alison Schofield <amsfield22@gmail.com>]

On Sun, Feb 26, 2017 at 11:01:11AM -0800, Katie Dunne wrote:
> On Sat, Feb 25, 2017 at 10:23:53AM -0800, Alison Schofield wrote:
> > On Fri, Feb 24, 2017 at 09:27:52PM -0800, Katie Dunne wrote:
> > > On Fri, Feb 24, 2017 at 05:42:40PM -0800, Alison Schofield wrote:
> > > > Hi Outreachy Applicants,
> > > > 
> > > > Here is a clean-up task for staging/iio you might like to try.
> > > > 
> > > > Staging IIO Header file (re-)organization:
> > > > 
> > > > Guidelines to follow:
> > > > 
> > > > (Reference  https://kernelnewbies.org/KernelHeaders)
> > > > 
> > > > Three cases you may find:
> > > > 1) If it's a single source file, its header info should be in that source file.
> > > >    *** Do this!
> > > > 
> > > > 2) Interfaces between source files in a single module can have a header file
> > > >    in same dir as the source files.
> > > >    *** Leave this alone.
> > > > 
> > > > 3) Interfaces shared between modules go in a header file in include/linux/iio
> > > >    *** Leave this alone. We will do this when the driver is moving out of
> > > > 	staging 
> > > > 
> > > > Here's an example patchset:
> > > > https://groups.google.com/forum/#!searchin/outreachy-kernel/header$20file$20ad5933%7Csort:relevance/outreachy-kernel/THYpgGyOgsI/iz8lrqwfDAAJ
> > > > 
> > > > Note that when you pull the header file in, there's a customary order to it
> > > > that the drivers in staging may not be following.  In the example one above,
> > > > I moved the #defines to be before the chip structure definitions.  So, you'll
> > > > do best if you look at similar drivers in the same directory upstream, to
> > > > see how they order their header information.  Always good to look at a few,
> > > > because one may not be the perfect role model.  
> > > 
> > > Hi Alison,
> > > 
> > > I noticed in looking at the iio/accel driver upstream, there are no comments
> > > around register definitions. In the staging driver, all of the header
> > > files I've looked at have a comment for each definition. Are comments
> > > something that should be pushed forward? Even though the register names are
> > > descriptive, it seems wrong to take out comments.
> > > 
> > > Best,
> > > Katie
> > >
> > Thanks for pointing that out Katie.
> > You are right.  Let's not remove any comments in the reorg.  Caveat -
> > if they are poorly aligned/spelled/formatted as you move them, then
> > you have another patch in your set to correct that!
> > 
> > alisons
> > 
> I also notice you deleted an unnecessary include (asm/div64). How do you
> find unused includes? What I have found online is that no tools work
> perfectly, and the best solutions are scripts that remove one include 
> at time and compile at each step, which sounds terrible.
> 
> Katie

Katie,

My recollection is that I recognized it as unneeded and then 
did one compile at a time to try any others I questioned.

I don't know of a tool or script to share.

BTW - unneeded #includes aren't always a mistake in the original
driver.  I believe that some header files have evolved to include
other header files, and so over time the #include list for a
particular driver can have a redundancy.  Sorry - that's all so
vague...again, someone else might shed further light on it.

BTW BTW - i think we're OK with these IIO drivers, but I imagine
you could also have ARCH build dependencies.  Something to watch
out for and also something that gets caught by 0 day build testers
who will grab your patch and build it.

alisons 

> 
> > > > 
> > > > It is customary to alphabetize the #includes, and to separate the kernel
> > > > and IIO subsystem #includes with a blank line.  So, although we wouldn't
> > > > send in a patch just to alphabetize #includes, while you are moving them
> > > > to the .c file, and while you are checking that they are actually needed,
> > > > sure - go ahead and alphabetize them.  (Example patch didn't alphabetize,
> > > > you'll do better ;)
> > > >  
> > > > To verify the patch(set), I'd suggest applying it to a clean tree and
> > > > rebuilding.  
> > > > 
> > > > Questions?  Just ask on this thread.
> > > > 
> > > > alisons
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170225014231.GA3395%40d830.WORKGROUP.
> > > > For more options, visit https://groups.google.com/d/optout.