On Wed, Aug 26, 2020 at 06:45:33PM +0200, Krzysztof Kozlowski wrote: > On Wed, Aug 26, 2020 at 04:59:09PM +0200, Lukasz Stelmach wrote: > >> +#include <linux/of.h> > > >> +#endif > > >> +#include <linux/crc32.h> > > >> +#include <linux/etherdevice.h> > > >> +#include <linux/ethtool.h> > > >> +#include <linux/gpio/consumer.h> > > >> +#include <linux/init.h> > > >> +#include <linux/io.h> > > >> +#include <linux/kmod.h> > > >> +#include <linux/mii.h> > > >> +#include <linux/module.h> > > >> +#include <linux/netdevice.h> > > >> +#include <linux/platform_device.h> > > >> +#include <linux/sched.h> > > >> +#include <linux/spi/spi.h> > > >> +#include <linux/timer.h> > > >> +#include <linux/uaccess.h> > > >> +#include <linux/usb.h> > > >> +#include <linux/version.h> > > >> +#include <linux/workqueue.h> > > > > > > All of these should be removed except the headers used directly in this > > > header. > > > > > > > This is "private" header file included in all ax88796c_*.c files and > > these are headers required in them. It seems more conveninet to have > > them all listed in one place. What is the reason to do otherwise? > > Because: > 1. The header is included in other files (more than one) so each other > compilation unit will include all these headers, while not all of them > need. This has a performance penalty during preprocessing. > > 2. You will loose the track which headers are needed, which are not. We > tend to keep it local, which means each compilation unit includes stuff > it needs. This helps removing obsolete includes later. > > 3. Otherwise you could make one header, including all headers of Linux, > and then include this one header in each of C files. One to rule them > all. ... and I got one more: 4. Drivers sometimes get reused, extended or they parts got reused. If a header includes more stuff, it simply will pollute all other units trying to reuse it... making the re-usage difficult. This is less likely reason, I mean, quite imaginary for this particular driver. I don't expect pieces of this driver to be reused... but who knows. Many times in the past in the kernel there was a huge work rewriting headers in many files, because something was including something else and we wanted to decouple these things. Therefore following the pattern - include stuff you explicitly use - helps in every case. Best regards, Krzysztof
