| Message ID | 20211115070809.15529-5-sergio.paracuellos@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: mt7621: remove specific MIPS code from driver | expand |
Hi Sergio and Yanteng, Thank you for taking care of this! > MT7620 PCIe host controller driver can be built as a module but there is no > 'MODULE_LICENSE()' specified in code, causing a build error due to missing > license information. > > ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o > > Fix this by adding 'MODULE_LICENSE()' to the driver. > > Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver") > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > --- > drivers/pci/controller/pcie-mt7621.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > index 9cf541f5de9c..a120a61ede07 100644 > --- a/drivers/pci/controller/pcie-mt7621.c > +++ b/drivers/pci/controller/pcie-mt7621.c > @@ -561,3 +561,5 @@ static struct platform_driver mt7621_pci_driver = { > }, > }; > builtin_platform_driver(mt7621_pci_driver); > + > +MODULE_LICENSE("GPL v2"); A question here about the builtin_platform_driver() use in this driver, especially since it's set as tristate in Kconfig, thus I am not sure if using builtin_platform_driver() over module_platform_driver() is correct? Unless this is more because you need to reply on device_initcall() for the driver to properly initialise? Otherwise, Reviewed-by: Krzysztof Wilczyński <kw@linux.com> Krzysztof
On Mon, Nov 15, 2021 at 1:44 PM Krzysztof Wilczyński <kw@linux.com> wrote: > > MT7620 PCIe host controller driver can be built as a module but there is no > > 'MODULE_LICENSE()' specified in code, causing a build error due to missing > > license information. > > > > ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o > > > > Fix this by adding 'MODULE_LICENSE()' to the driver. > > > > Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver") > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > --- > > drivers/pci/controller/pcie-mt7621.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > > index 9cf541f5de9c..a120a61ede07 100644 > > --- a/drivers/pci/controller/pcie-mt7621.c > > +++ b/drivers/pci/controller/pcie-mt7621.c > > @@ -561,3 +561,5 @@ static struct platform_driver mt7621_pci_driver = { > > }, > > }; > > builtin_platform_driver(mt7621_pci_driver); > > + > > +MODULE_LICENSE("GPL v2"); > > A question here about the builtin_platform_driver() use in this driver, > especially since it's set as tristate in Kconfig, thus I am not sure if > using builtin_platform_driver() over module_platform_driver() is correct? > > Unless this is more because you need to reply on device_initcall() for the > driver to properly initialise? builtin_platform_driver() does the right thing for loadable modules that have no module-unload and are not intended to be removable. This is often use for PCI drivers, but after Rob reworked this code a while back, it should actually be possible to reliably remove and reload PCI host bridge drivers, and it would be good to eventually lift the restriction here as well. Arnd
Hi Arnd, On Mon, Nov 15, 2021 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Nov 15, 2021 at 1:44 PM Krzysztof Wilczyński <kw@linux.com> wrote: > > > MT7620 PCIe host controller driver can be built as a module but there is no > > > 'MODULE_LICENSE()' specified in code, causing a build error due to missing > > > license information. > > > > > > ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o > > > > > > Fix this by adding 'MODULE_LICENSE()' to the driver. > > > > > > Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver") > > > Signed-off-by: Yanteng Si <siyanteng@loongson.cn> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > > --- > > > drivers/pci/controller/pcie-mt7621.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > > > index 9cf541f5de9c..a120a61ede07 100644 > > > --- a/drivers/pci/controller/pcie-mt7621.c > > > +++ b/drivers/pci/controller/pcie-mt7621.c > > > @@ -561,3 +561,5 @@ static struct platform_driver mt7621_pci_driver = { > > > }, > > > }; > > > builtin_platform_driver(mt7621_pci_driver); > > > + > > > +MODULE_LICENSE("GPL v2"); > > > > A question here about the builtin_platform_driver() use in this driver, > > especially since it's set as tristate in Kconfig, thus I am not sure if > > using builtin_platform_driver() over module_platform_driver() is correct? > > > > Unless this is more because you need to reply on device_initcall() for the > > driver to properly initialise? > > builtin_platform_driver() does the right thing for loadable modules that > have no module-unload and are not intended to be removable. Yes, this is the current state of this controller driver and the reason for 'builtin_platform_driver()' being used. > > This is often use for PCI drivers, but after Rob reworked this code a while > back, it should actually be possible to reliably remove and reload PCI > host bridge drivers, and it would be good to eventually lift the restriction > here as well. I see. Thanks for letting me know. I will search for a way to accomplish this but that will be a different patch series. Best regards, Sergio Paracuellos > > Arnd
On Mon, Nov 15, 2021 at 2:51 PM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > On Mon, Nov 15, 2021 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Nov 15, 2021 at 1:44 PM Krzysztof Wilczyński <kw@linux.com> wrote: > > This is often use for PCI drivers, but after Rob reworked this code a while > > back, it should actually be possible to reliably remove and reload PCI > > host bridge drivers, and it would be good to eventually lift the restriction > > here as well. > > I see. Thanks for letting me know. I will search for a way to > accomplish this but that will be a different patch series. Right, that is what I meant. I don't think it will be difficult, but there is no point intermixing it with your current work. Arnd
Hi Arnd, [...] > > > builtin_platform_driver(mt7621_pci_driver); > > > + > > > +MODULE_LICENSE("GPL v2"); > > > > A question here about the builtin_platform_driver() use in this driver, > > especially since it's set as tristate in Kconfig, thus I am not sure if > > using builtin_platform_driver() over module_platform_driver() is correct? > > > > Unless this is more because you need to reply on device_initcall() for the > > driver to properly initialise? > > builtin_platform_driver() does the right thing for loadable modules that > have no module-unload and are not intended to be removable. > > This is often use for PCI drivers, but after Rob reworked this code a while > back, it should actually be possible to reliably remove and reload PCI > host bridge drivers, and it would be good to eventually lift the restriction > here as well. Thank you for letting me know. Much appreciated. I assumed in the past that with tristate in Kconfig the module_platform_driver() would be the preferred route. Krzysztof
Hello, [...] > ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o > > Fix this by adding 'MODULE_LICENSE()' to the driver. To add for posterity, should someone stumble upon this in the future. Lack of MODULE_LICENSE() used to be a warning, but that has been changed quite recently in the following commit: commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into error") Krzysztof
diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c index 9cf541f5de9c..a120a61ede07 100644 --- a/drivers/pci/controller/pcie-mt7621.c +++ b/drivers/pci/controller/pcie-mt7621.c @@ -561,3 +561,5 @@ static struct platform_driver mt7621_pci_driver = { }, }; builtin_platform_driver(mt7621_pci_driver); + +MODULE_LICENSE("GPL v2");
MT7621 PCIe host controller driver can be built as a module but there is no 'MODULE_LICENSE()' specified in code, causing a build error due to missing license information. ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o Fix this by adding 'MODULE_LICENSE()' to the driver. Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver") Signed-off-by: Yanteng Si <siyanteng@loongson.cn> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> --- drivers/pci/controller/pcie-mt7621.c | 2 ++ 1 file changed, 2 insertions(+)