| Message ID | 20161130225110.10759-1-hauke@hauke-m.de |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, 30 Nov 2016 23:51:10 +0100 Hauke Mehrtens <hauke@hauke-m.de> wrote: > The header file with the definition of MODULE_DEVICE_TABLE() was > missing, add include for linux/module.h to fix the problem in 4.9. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> Fixes: 024366750c2e ("mtd: nand: xway: convert to normal platform driver") Cc: <stable@vger.kernel.org> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> Brian, can you take this patch directly in the MTD tree with the additional Fixes and Cc tags? > --- > drivers/mtd/nand/xway_nand.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c > index 1f2948c..bcbc1c7 100644 > --- a/drivers/mtd/nand/xway_nand.c > +++ b/drivers/mtd/nand/xway_nand.c > @@ -8,6 +8,7 @@ > */ > > #include <linux/mtd/nand.h> > +#include <linux/module.h> > #include <linux/of_gpio.h> > #include <linux/of_platform.h> >
Hi Hauke, On Wed, 30 Nov 2016 23:51:10 +0100 Hauke Mehrtens <hauke@hauke-m.de> wrote: > The header file with the definition of MODULE_DEVICE_TABLE() was > missing, add include for linux/module.h to fix the problem in 4.9. I tried to enable this driver as a module, and the build failed because of a missing symbol (see the following patch). Now, if it's not supposed to be compiled as a module, then you should modify the Kconfig accordingly. Regards, Boris --->8--- diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 100644 --- a/arch/mips/lantiq/xway/sysctrl.c +++ b/arch/mips/lantiq/xway/sysctrl.c @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; static void __iomem *ltq_xbar_membase; void __iomem *ltq_cgu_membase; void __iomem *ltq_ebu_membase; +EXPORT_SYMBOL(ltq_ebu_membase); static u32 ifccr = CGU_IFCCR; static u32 pcicr = CGU_PCICR;
On 01/12/2016 13:36, Boris Brezillon wrote: > Hi Hauke, > > On Wed, 30 Nov 2016 23:51:10 +0100 > Hauke Mehrtens <hauke@hauke-m.de> wrote: > >> The header file with the definition of MODULE_DEVICE_TABLE() was >> missing, add include for linux/module.h to fix the problem in 4.9. > > I tried to enable this driver as a module, and the build failed because > of a missing symbol (see the following patch). > Now, if it's not supposed to be compiled as a module, then you should > modify the Kconfig accordingly. > > Regards, > > Boris > > --->8--- > diff --git a/arch/mips/lantiq/xway/sysctrl.c > b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 > 100644 --- a/arch/mips/lantiq/xway/sysctrl.c > +++ b/arch/mips/lantiq/xway/sysctrl.c > @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; > static void __iomem *ltq_xbar_membase; > void __iomem *ltq_cgu_membase; > void __iomem *ltq_ebu_membase; > +EXPORT_SYMBOL(ltq_ebu_membase); Hi, i would be against exporting the membase pointers as global symbols. i already find it annoying that cgu and ebu are not static John
On Thu, 1 Dec 2016 14:02:55 +0100 John Crispin <john@phrozen.org> wrote: > On 01/12/2016 13:36, Boris Brezillon wrote: > > Hi Hauke, > > > > On Wed, 30 Nov 2016 23:51:10 +0100 > > Hauke Mehrtens <hauke@hauke-m.de> wrote: > > > >> The header file with the definition of MODULE_DEVICE_TABLE() was > >> missing, add include for linux/module.h to fix the problem in 4.9. > > > > I tried to enable this driver as a module, and the build failed because > > of a missing symbol (see the following patch). > > Now, if it's not supposed to be compiled as a module, then you should > > modify the Kconfig accordingly. > > > > Regards, > > > > Boris > > > > --->8--- > > diff --git a/arch/mips/lantiq/xway/sysctrl.c > > b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 > > 100644 --- a/arch/mips/lantiq/xway/sysctrl.c > > +++ b/arch/mips/lantiq/xway/sysctrl.c > > @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; > > static void __iomem *ltq_xbar_membase; > > void __iomem *ltq_cgu_membase; > > void __iomem *ltq_ebu_membase; > > +EXPORT_SYMBOL(ltq_ebu_membase); > > Hi, > > i would be against exporting the membase pointers as global symbols. i > already find it annoying that cgu and ebu are not static Yep, that's also my opinion. Let's patch the Kconfig entry instead. BTW, if you don't compile the driver as a module, then you don't need the MODULE_XX() calls, and you can get rid of the module.h header.
On 01/12/2016 14:47, Boris Brezillon wrote: > On Thu, 1 Dec 2016 14:02:55 +0100 > John Crispin <john@phrozen.org> wrote: > >> On 01/12/2016 13:36, Boris Brezillon wrote: >>> Hi Hauke, >>> >>> On Wed, 30 Nov 2016 23:51:10 +0100 >>> Hauke Mehrtens <hauke@hauke-m.de> wrote: >>> >>>> The header file with the definition of MODULE_DEVICE_TABLE() was >>>> missing, add include for linux/module.h to fix the problem in 4.9. >>> >>> I tried to enable this driver as a module, and the build failed because >>> of a missing symbol (see the following patch). >>> Now, if it's not supposed to be compiled as a module, then you should >>> modify the Kconfig accordingly. >>> >>> Regards, >>> >>> Boris >>> >>> --->8--- >>> diff --git a/arch/mips/lantiq/xway/sysctrl.c >>> b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 >>> 100644 --- a/arch/mips/lantiq/xway/sysctrl.c >>> +++ b/arch/mips/lantiq/xway/sysctrl.c >>> @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; >>> static void __iomem *ltq_xbar_membase; >>> void __iomem *ltq_cgu_membase; >>> void __iomem *ltq_ebu_membase; >>> +EXPORT_SYMBOL(ltq_ebu_membase); >> >> Hi, >> >> i would be against exporting the membase pointers as global symbols. i >> already find it annoying that cgu and ebu are not static > > Yep, that's also my opinion. > > Let's patch the Kconfig entry instead. BTW, if you don't compile the > driver as a module, then you don't need the MODULE_XX() calls, and you > can get rid of the module.h header. > agreed. not sure how hauke tested his patch in the first place. John
On 12/01/2016 02:57 PM, John Crispin wrote: > > > On 01/12/2016 14:47, Boris Brezillon wrote: >> On Thu, 1 Dec 2016 14:02:55 +0100 >> John Crispin <john@phrozen.org> wrote: >> >>> On 01/12/2016 13:36, Boris Brezillon wrote: >>>> Hi Hauke, >>>> >>>> On Wed, 30 Nov 2016 23:51:10 +0100 >>>> Hauke Mehrtens <hauke@hauke-m.de> wrote: >>>> >>>>> The header file with the definition of MODULE_DEVICE_TABLE() was >>>>> missing, add include for linux/module.h to fix the problem in 4.9. >>>> >>>> I tried to enable this driver as a module, and the build failed because >>>> of a missing symbol (see the following patch). >>>> Now, if it's not supposed to be compiled as a module, then you should >>>> modify the Kconfig accordingly. >>>> >>>> Regards, >>>> >>>> Boris >>>> >>>> --->8--- >>>> diff --git a/arch/mips/lantiq/xway/sysctrl.c >>>> b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 >>>> 100644 --- a/arch/mips/lantiq/xway/sysctrl.c >>>> +++ b/arch/mips/lantiq/xway/sysctrl.c >>>> @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; >>>> static void __iomem *ltq_xbar_membase; >>>> void __iomem *ltq_cgu_membase; >>>> void __iomem *ltq_ebu_membase; >>>> +EXPORT_SYMBOL(ltq_ebu_membase); >>> >>> Hi, >>> >>> i would be against exporting the membase pointers as global symbols. i >>> already find it annoying that cgu and ebu are not static >> >> Yep, that's also my opinion. I also do not like this ebu integration. the ebu lock is also a problem. >> Let's patch the Kconfig entry instead. BTW, if you don't compile the >> driver as a module, then you don't need the MODULE_XX() calls, and you >> can get rid of the module.h header. >> > > agreed. not sure how hauke tested his patch in the first place. Sorry, I never tested this as a module, I only used it compiled into the kernel. I will send a patch which changes Kconfig. Hauke
On 04/12/2016 20:15, Hauke Mehrtens wrote:
> I also do not like this ebu integration. the ebu lock is also a problem.
yep, the lock is needed for pci to work. its really ugly i agree.
John
On 12/01/2016 01:36 PM, Boris Brezillon wrote: > Hi Hauke, > > On Wed, 30 Nov 2016 23:51:10 +0100 > Hauke Mehrtens <hauke@hauke-m.de> wrote: > >> The header file with the definition of MODULE_DEVICE_TABLE() was >> missing, add include for linux/module.h to fix the problem in 4.9. > > I tried to enable this driver as a module, and the build failed because > of a missing symbol (see the following patch). > Now, if it's not supposed to be compiled as a module, then you should > modify the Kconfig accordingly. > > Regards, > > Boris > > --->8--- > diff --git a/arch/mips/lantiq/xway/sysctrl.c > b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 > 100644 --- a/arch/mips/lantiq/xway/sysctrl.c > +++ b/arch/mips/lantiq/xway/sysctrl.c > @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; > static void __iomem *ltq_xbar_membase; > void __iomem *ltq_cgu_membase; > void __iomem *ltq_ebu_membase; > +EXPORT_SYMBOL(ltq_ebu_membase); > > static u32 ifccr = CGU_IFCCR; > static u32 pcicr = CGU_PCICR; > Also module_platform_driver() needs linux/module.h, because it contains module_init() and some more stuff. I would like to add the include for linux/module.h and deactivate module support. It looks like it was possible to select module building since it was added, but it should never have worked. Hauke
On Sun, 4 Dec 2016 23:40:56 +0100 Hauke Mehrtens <hauke@hauke-m.de> wrote: > On 12/01/2016 01:36 PM, Boris Brezillon wrote: > > Hi Hauke, > > > > On Wed, 30 Nov 2016 23:51:10 +0100 > > Hauke Mehrtens <hauke@hauke-m.de> wrote: > > > >> The header file with the definition of MODULE_DEVICE_TABLE() was > >> missing, add include for linux/module.h to fix the problem in 4.9. > > > > I tried to enable this driver as a module, and the build failed because > > of a missing symbol (see the following patch). > > Now, if it's not supposed to be compiled as a module, then you should > > modify the Kconfig accordingly. > > > > Regards, > > > > Boris > > > > --->8--- > > diff --git a/arch/mips/lantiq/xway/sysctrl.c > > b/arch/mips/lantiq/xway/sysctrl.c index 236193b5210b..29e753556597 > > 100644 --- a/arch/mips/lantiq/xway/sysctrl.c > > +++ b/arch/mips/lantiq/xway/sysctrl.c > > @@ -156,6 +156,7 @@ static void __iomem *pmu_membase; > > static void __iomem *ltq_xbar_membase; > > void __iomem *ltq_cgu_membase; > > void __iomem *ltq_ebu_membase; > > +EXPORT_SYMBOL(ltq_ebu_membase); > > > > static u32 ifccr = CGU_IFCCR; > > static u32 pcicr = CGU_PCICR; > > > > Also module_platform_driver() needs linux/module.h, because it contains > module_init() and some more stuff. You should use builtin_platform_driver() and not module_platform_driver(). > > I would like to add the include for linux/module.h and deactivate module > support. It looks like it was possible to select module building since > it was added, but it should never have worked. > > Hauke
diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c index 1f2948c..bcbc1c7 100644 --- a/drivers/mtd/nand/xway_nand.c +++ b/drivers/mtd/nand/xway_nand.c @@ -8,6 +8,7 @@ */ #include <linux/mtd/nand.h> +#include <linux/module.h> #include <linux/of_gpio.h> #include <linux/of_platform.h>
The header file with the definition of MODULE_DEVICE_TABLE() was missing, add include for linux/module.h to fix the problem in 4.9. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- drivers/mtd/nand/xway_nand.c | 1 + 1 file changed, 1 insertion(+)