On Mon, Jul 17, 2023 at 1:44 AM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Fri, Jul 14, 2023 at 11:48:50AM -0600, Rob Herring wrote: > > The DT of_device.h and of_platform.h date back to the separate > > of_platform_bus_type before it as merged into the regular platform bus. > > As part of that merge prepping Arm DT support 13 years ago, they > > "temporarily" include each other. They also include platform_device.h > > and of.h. As a result, there's a pretty much random mix of those include > > files used throughout the tree. In order to detangle these headers and > > replace the implicit includes with struct declarations, users need to > > explicitly include the correct includes. > > so the eventual goal here is to prepare for: > > - drop #include <linux/of_device.h> from include/linux/of_platform.h > - drop #include <linux/of.h> from include/linux/of_device.h > - drop #include <linux/of_platform.h> from include/linux/of_device.h > - drop #include <linux/platform_device.h> from include/linux/of_device.h > - drop #include <linux/platform_device.h> from include/linux/of_platform.h Yes. > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx> > > --- > > drivers/pwm/core.c | 1 + > > drivers/pwm/pwm-apple.c | 1 + > > drivers/pwm/pwm-atmel-hlcdc.c | 1 + > > drivers/pwm/pwm-atmel-tcb.c | 3 +-- > > drivers/pwm/pwm-atmel.c | 1 - > > drivers/pwm/pwm-berlin.c | 1 + > > drivers/pwm/pwm-cros-ec.c | 1 + > > drivers/pwm/pwm-fsl-ftm.c | 3 +-- > > drivers/pwm/pwm-hibvt.c | 2 +- > > drivers/pwm/pwm-imx1.c | 1 - > > drivers/pwm/pwm-jz4740.c | 2 +- > > drivers/pwm/pwm-lp3943.c | 1 + > > drivers/pwm/pwm-lpc18xx-sct.c | 1 + > > drivers/pwm/pwm-mediatek.c | 1 - > > drivers/pwm/pwm-meson.c | 1 - > > drivers/pwm/pwm-microchip-core.c | 2 +- > > drivers/pwm/pwm-mtk-disp.c | 1 - > > drivers/pwm/pwm-pxa.c | 1 + > > drivers/pwm/pwm-sifive.c | 1 + > > drivers/pwm/pwm-sl28cpld.c | 1 + > > drivers/pwm/pwm-sprd.c | 1 + > > drivers/pwm/pwm-sun4i.c | 1 - > > drivers/pwm/pwm-sunplus.c | 1 + > > drivers/pwm/pwm-tegra.c | 1 - > > drivers/pwm/pwm-tiecap.c | 2 +- > > drivers/pwm/pwm-tiehrpwm.c | 2 +- > > drivers/pwm/pwm-visconti.c | 2 +- > > drivers/pwm/pwm-vt8500.c | 5 +---- > > 28 files changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > index 3dacceaef4a9..d37617c60eae 100644 > > --- a/drivers/pwm/core.c > > +++ b/drivers/pwm/core.c > > @@ -8,6 +8,7 @@ > > > > #include <linux/acpi.h> > > #include <linux/module.h> > > +#include <linux/of.h> > > #include <linux/pwm.h> > > #include <linux/radix-tree.h> > > #include <linux/list.h> > > This file includes neither of_device.h nor of_platform.h and up to now > gets of.h via <linux/pwm.h>. Indeed. > What is your plan for <linux/pwm.h>'s include? I think it would only need > > struct of_phandle_args; Here's what I'm testing with: diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 04ae1d9073a7..5a59a7d53be8 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -4,8 +4,10 @@ #include <linux/err.h> #include <linux/mutex.h> -#include <linux/of.h> +struct device; +struct fwnode_handle; +struct of_phandle_args; struct pwm_chip; /** > > to replace that. (But that would need another patch like this one, as > then e.g. drivers/pwm/pwm-sl28cpld.c fails to compile because > device_property_read_u32() is undeclared. It would need to #include > <linux/property.h> which now it gets transitively via <linux/of.h>.) property.h is added in this patch, so it should be okay? > If <linux/pwm.h> is planed to continue #including <linux/of.h>, the > explicit include here isn't necessary (and probably elsewhere). I would like to drop including of.h, but probably not this cycle. Either way, I thought kernel best practice was to not rely on implicit includes. BTW, linux/i2c.h is another source of lots of implicit of.h includes. That one looks like we can't get rid of. > I don't care much either way, but maybe your quest would be a bit > simpler if you only touch files that include the two files you want to > modify? Yes, that's how it started. I kind of decided it wasn't worth trying to split things up by every separate reason explicit includes were not correct. > > *shrug*, this patch is still an improvement so: > > Acked-by: Uwe Kleine-Köng <u.kleine-koenig@xxxxxxxxxxxxxx> > > Another thing I wonder is: How did you identify the files that need > these includes. I guess you have a list of types for each header and > search for files that use any of the types but doesn't include the > respecitve header? I wonder if tracking this type -> header mapping in > machine readable form somewhere would be nice, to e.g. make checkpatch > warn if a file uses struct of_node but only gets it by chance? It's been less automated than I'd like. It's been a lot of grepping with a list of symbols the headers provide. For example, I get all the files including of_device.h and then get the ones with no symbols from of_device.h. And then do a manual review of what are the correct headers for the file. And then run thru builds and fix all the issues. Rob
