* 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.