Re: [PATCH v1 05/11] cpufreq: tegra20: Clean up included headers

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]

On Fri, May 18, 2018 at 11:05:58AM +0300, Dmitry Osipenko wrote:
> On 18.05.2018 04:58, Viresh Kumar wrote:
> > On 17-05-18, 21:00, Dmitry Osipenko wrote:
> >> Remove unused/unneeded headers and sort them in the alphabet order.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  drivers/cpufreq/tegra20-cpufreq.c | 11 ++---------
> >>  1 file changed, 2 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> >> index 69f033d297e1..61f00d1cba26 100644
> >> --- a/drivers/cpufreq/tegra20-cpufreq.c
> >> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> >> @@ -16,16 +16,9 @@
> >>   *
> >>   */
> >>  
> >> -#include <linux/kernel.h>
> >> -#include <linux/module.h>
> >> -#include <linux/types.h>
> >> -#include <linux/sched.h>
> >> -#include <linux/cpufreq.h>
> >> -#include <linux/delay.h>
> >> -#include <linux/init.h>
> >> -#include <linux/err.h>
> >>  #include <linux/clk.h>
> >> -#include <linux/io.h>
> >> +#include <linux/cpufreq.h>
> >> +#include <linux/module.h>
> > 
> > Ideally you should keep all the headers whose declarations your code
> > is using directly. It may happen that removing above still compiles
> > because cpufreq.h has included the headers indirectly for you. But
> > that will break the day cpufreq.h doesn't need those headers anymore.
> > 
> > So just make sure you aren't using any of them in your code. For
> > example you are using bool in your code and so you shouldn't remove
> > types.h ? Same for init.h as you are using __init and __exit.
> 
> The preference on includes seems to vary among maintainers. I've seen other
> opinion that encouraged to minimize included headers and only add the headers
> when compilation breaks.

I think this probably depends on the type of include. For headers in
include/linux the reasoning is that they can change often as part of
some rework and this has in the past caused unrelated files to break
builds because suddenly a file no longer has all required definitions
available.

It's slightly different for header files that have a narrower exposure
because the chances of breaking things randomly are much lower.

> I'll revisit this patch and keep init.h and others in v2 since you prefer that
> way. Thank you for the review.

With those changes, this is:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]