* Clean-up idea: IIO Headers
@ 2017-02-25 1:42 Alison Schofield
2017-02-25 5:27 ` [Outreachy kernel] " Katie Dunne
2017-02-27 17:56 ` Alison Schofield
0 siblings, 2 replies; 6+ messages in thread
From: Alison Schofield @ 2017-02-25 1:42 UTC (permalink / raw)
To: outreachy-kernel
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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Clean-up idea: IIO Headers
2017-02-25 1:42 Clean-up idea: IIO Headers Alison Schofield
@ 2017-02-25 5:27 ` Katie Dunne
2017-02-25 18:23 ` Alison Schofield
2017-02-27 17:56 ` Alison Schofield
1 sibling, 1 reply; 6+ messages in thread
From: Katie Dunne @ 2017-02-25 5:27 UTC (permalink / raw)
To: Alison Schofield; +Cc: outreachy-kernel
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
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Clean-up idea: IIO Headers
2017-02-25 5:27 ` [Outreachy kernel] " Katie Dunne
@ 2017-02-25 18:23 ` Alison Schofield
2017-02-26 19:01 ` Katie Dunne
0 siblings, 1 reply; 6+ messages in thread
From: Alison Schofield @ 2017-02-25 18:23 UTC (permalink / raw)
To: Katie Dunne; +Cc: outreachy-kernel
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
> >
> > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Clean-up idea: IIO Headers
2017-02-25 18:23 ` Alison Schofield
@ 2017-02-26 19:01 ` Katie Dunne
2017-02-26 21:06 ` Alison Schofield
0 siblings, 1 reply; 6+ messages in thread
From: Katie Dunne @ 2017-02-26 19:01 UTC (permalink / raw)
To: Alison Schofield; +Cc: outreachy-kernel
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
> > >
> > > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Clean-up idea: IIO Headers
2017-02-26 19:01 ` Katie Dunne
@ 2017-02-26 21:06 ` Alison Schofield
0 siblings, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2017-02-26 21:06 UTC (permalink / raw)
To: Katie Dunne; +Cc: outreachy-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Clean-up idea: IIO Headers
2017-02-25 1:42 Clean-up idea: IIO Headers Alison Schofield
2017-02-25 5:27 ` [Outreachy kernel] " Katie Dunne
@ 2017-02-27 17:56 ` Alison Schofield
1 sibling, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2017-02-27 17:56 UTC (permalink / raw)
To: outreachy-kernel; +Cc: lars
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!
Update - (thanks Lars)
The platform data struct needs to stay in the separate header file until
the driver is moved out of staging. If this is your case, you can move
the other contents but will need to leave the platform data in the .h
file. When the driver is moved out of staging the header will go into
the global include/linux/platform_data/ folder.
You can see examples of this in the outreachy mailing list already.
>
> 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.
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-27 17:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-25 1:42 Clean-up idea: IIO Headers Alison Schofield
2017-02-25 5:27 ` [Outreachy kernel] " Katie Dunne
2017-02-25 18:23 ` Alison Schofield
2017-02-26 19:01 ` Katie Dunne
2017-02-26 21:06 ` Alison Schofield
2017-02-27 17:56 ` Alison Schofield
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.