On 11-08-22 07:48 PM, Ryan Mallon wrote: > On 23/08/11 02:40, H Hartley Sweeten wrote: >> On Sunday, August 21, 2011 5:31 PM, Ryan Mallon wrote: >>> On 22/08/11 10:06, Axel Lin wrote: >>>> 2011/8/22 Ryan Mallon<rmallon@xxxxxxxxx>: >>>>> On 22/08/11 09:41, Ryan Mallon wrote: >>>>>> On 22/08/11 00:39, Axel Lin wrote: >>>>>>> ep93xx-fb.c uses interfaces from linux/module.h, >>>>>>> so it should include that file. This patch fixes below build errors. >>>>>> What actually changed to make these files broken? Did some other header >>>>>> previously include module.h for us? How many other drivers are broken? >>>>>> >>>>>> Anyway, the change is okay. >>>>>> >>>>>> Acked-by: Ryan Mallon<rmallon@xxxxxxxxx> >>>>> Actually, having a second look at this it does not look right. >>>>> >>>>> drivers/video/ep93xx-fb.c includes linux/platform.h (as its first include), >>>>> which includes linux/driver.h, which includes linux/module.h. For the record, this is exactly what we are trying to fix -- relying on implicit and non-obvious include chaining. If you are writing a modular driver, you should include module.h explicitly. >>>>> >>>>> Just tested on Linus' latest tree and both this driver and the ep93xx >>>>> backlight driver build fine. What kernel version are you using? >>>>> >>>>> ~Ryan >>>> hi Ryan, >>>> >>>> The patch is against linux-next tree. >>>> I got build error for ep93xx-fb.c and ep93xx_bl.c on linux-next tree. >>>> ( next-20110819 ) >>> Ok, I see now. The change which caused the breakage is fdb697c: >>> "include: replace linux/module.h with "struct module" wherever >>> possible". How many other drivers got broken now that device.h does not >>> include module.h? >> Probably a lot... Which is one of the reasons linux-next exists.. . ;-) > > Does anybody know how we can quickly determine which drivers are broken > short of doing an allyesconfig? I tried to do some quick tricks by > passing files which contain THIS_MODULE/MODULE_* through cpp, but I get > loads of errors in headers files because I'm missing some config > includes. Is there an easy way to get the kbuild arguments for the > current .config so I can pass them to cpp? > >> Actually, Paul Gortmaker caused this breakage with the commit. He should >> take a deeper look and see what it broke. From his commit: >> >> Most of the implicit dependencies on module.h being present by >> these headers pulling it in have been now weeded out, so we can >> finally make this change with hopefully minimal breakage. > > Quick glance: > > ryanm@kiwi:linux-2.6$ grep -lR "^MODULE_" drivers/ | xargs grep -L > "linux/module.h\|linux/moduleloader.h\|linux/miscdevice.h\|linux/regmap.h\|linux/irq.h" > | wc -l > 579 This can be misleading because subsystems may have a common header that includes module.h, and then all the C files include that common header. I think the e1000 network driver is one example. > > ryanm@kiwi:linux-2.6$ grep -lR "THIS_MODULE" drivers/ | xargs grep -L > "linux/module.h\|linux/moduleloader.h\|linux/miscdevice.h\|linux/regmap.h\|linux/irq.h\|linux/export.h\|acpi/platform/aclinux.h\|xen/xenbus.h" > | wc -l > 399 > > Not sure how many of those are really broken though since there may be a > few other ways to include module.h/export.h. I would also think there > would be a lot more build failure reports if all of those were broken :-). Exactly. I've done allyesconfig, allnoconfig, allmodconfig builds for all the common arch and even half a dozen or so uncommon arch. Plus in the case of things like arm and powerpc with board specific config files that are in arch/*/configs, I've built all those. So a couple of odd cases like this which don't get build coverage through any of the above was expected, and hence the value of being in linux-next for a while. Thanks all for the report, I'll make sure these files get the proper include header they need. Paul. > > ~Ryan > > > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
