Re: [PATCH v3 4/8] pwm: lpss: Include headers we are direct user of

Uwe Kleine-König
2022-09-27T15:56:22Z

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

On Tue, Sep 27, 2022 at 06:26:28PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 27, 2022 at 05:10:53PM +0200, Uwe Kleine-König wrote:
> > On Tue, Sep 27, 2022 at 05:47:19PM +0300, Andy Shevchenko wrote:
> > > For the sake of integrity, include headers we are direct user of.
> > >
> > > While at it, add missed struct pwm_lpss_boardinfo one and replace
> > > device.h with a forward declaration. The latter improves compile
> > > time due to reducing overhead of device.h parsing with entire train
> > > of dependencies.
> >
> > Hm, I copied the cmdline for the compiler from a V=1 build and only run
> > the compiler on drivers/pwm/pwm-lpss-pci.c.
> >
> > With #include <device.h> I got:
> >
> > real 0m0.421s
> > user 0m0.354s
> > sys 0m0.066s
> >
> > With struct device; I got:
> >
> > real 0m0.431s
> > user 0m0.378s
> > sys 0m0.052s
> >
> > Are the numbers for you considerably different?
>
> Why Ingo created thousands of patches to do something similar? Because for
> a single user you won't see a big difference, but when amount of small pieces
> are gathered together, you definitely will.

My doubt is that for me the effect of using struct device over #include
<device.h> is even negative (looking at real and user). Is it sys which
counts in the end?

> > > +struct device;
>
> ...
>
> > > +struct pwm_lpss_boardinfo;
> >
> > Hmm, I wonder why there is no compiler warning without that declaration.
> > At least in my builds. Do you see a warning? IMHO it's better to fix
> > that be swapping the order of struct pwm_lpss_chip and struct
> > pwm_lpss_boardinfo.
>
> Have I told about warning?

No, it's just me who expected there would be a warning if a pointer to
struct pwm_lpss_boardinfo is used before struct pwm_lpss_boardinfo is
defined (or declared).

Anyhow, I stand by my opinion that swapping the order of struct
pwm_lpss_chip and struct pwm_lpss_boardinfo is a saner fix.

> It's a proper C programming style.
> You don't have a warning because all pointers are considered to be the same,
> but it is better style to explicitly point that out.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |

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