| Message ID | 20130106215509.GD26928@lizard.sbx05280.losalca.wayport.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 08, 2013 at 07:15:26PM +0100, Andrew Lunn wrote: > GPLv2+ is not a valid license string. Replace it with one that is. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- Applied, thanks a lot! > drivers/power/reset/qnap-poweroff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c > index ca0b476..8af772b 100644 > --- a/drivers/power/reset/qnap-poweroff.c > +++ b/drivers/power/reset/qnap-poweroff.c > @@ -121,4 +121,4 @@ module_platform_driver(qnap_power_off_driver); > > MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>"); > MODULE_DESCRIPTION("QNAP Power off driver"); > -MODULE_LICENSE("GPLv2+"); > +MODULE_LICENSE("GPL v2"); > -- > 1.7.10.4
On Tue, Jan 08, 2013 at 07:15:26PM +0100, Andrew Lunn wrote: > GPLv2+ is not a valid license string. Replace it with one that is. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/power/reset/qnap-poweroff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c > index ca0b476..8af772b 100644 > --- a/drivers/power/reset/qnap-poweroff.c > +++ b/drivers/power/reset/qnap-poweroff.c > @@ -121,4 +121,4 @@ module_platform_driver(qnap_power_off_driver); > > MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>"); > MODULE_DESCRIPTION("QNAP Power off driver"); > -MODULE_LICENSE("GPLv2+"); > +MODULE_LICENSE("GPL v2"); This change is wrong. According to include/linux/module.h "GPL v2" means exactly that: version 2. As the file specifies v2 or later in the header you have to use "GPL" which means v2 or later. Best regards Uwe
On Sun, Jan 20, 2013 at 09:13:36PM +0100, Uwe Kleine-König wrote: > On Tue, Jan 08, 2013 at 07:15:26PM +0100, Andrew Lunn wrote: > > GPLv2+ is not a valid license string. Replace it with one that is. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/power/reset/qnap-poweroff.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c > > index ca0b476..8af772b 100644 > > --- a/drivers/power/reset/qnap-poweroff.c > > +++ b/drivers/power/reset/qnap-poweroff.c > > @@ -121,4 +121,4 @@ module_platform_driver(qnap_power_off_driver); > > > > MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>"); > > MODULE_DESCRIPTION("QNAP Power off driver"); > > -MODULE_LICENSE("GPLv2+"); > > +MODULE_LICENSE("GPL v2"); > This change is wrong. > > According to include/linux/module.h "GPL v2" means exactly that: version > 2. As the file specifies v2 or later in the header you have to use "GPL" > which means v2 or later. Does it even make sense to have the two separate things ("GPL v2" and "GPL")? Suppose there is a global change that modifies a bunch of drivers, some of them are GPLv2+. Now, the author of the global change is submitting it under "GPL v2 only" license, which, by definition, turns any GPLv2+ code into "GPL v2 only", right? So, changing from GPLv2+ to "GPL v2 only" is OK, but not the other way around. IANAL, tho. Anton p.s. Yes, in this particular driver it also makes sense to remove "or later" words from the header, just to be consistent.
Hello Anton, On Sun, Jan 20, 2013 at 12:47:29PM -0800, Anton Vorontsov wrote: > On Sun, Jan 20, 2013 at 09:13:36PM +0100, Uwe Kleine-König wrote: > > On Tue, Jan 08, 2013 at 07:15:26PM +0100, Andrew Lunn wrote: > > > GPLv2+ is not a valid license string. Replace it with one that is. > > > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > > --- > > > drivers/power/reset/qnap-poweroff.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c > > > index ca0b476..8af772b 100644 > > > --- a/drivers/power/reset/qnap-poweroff.c > > > +++ b/drivers/power/reset/qnap-poweroff.c > > > @@ -121,4 +121,4 @@ module_platform_driver(qnap_power_off_driver); > > > > > > MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>"); > > > MODULE_DESCRIPTION("QNAP Power off driver"); > > > -MODULE_LICENSE("GPLv2+"); > > > +MODULE_LICENSE("GPL v2"); > > This change is wrong. > > > > According to include/linux/module.h "GPL v2" means exactly that: version > > 2. As the file specifies v2 or later in the header you have to use "GPL" > > which means v2 or later. > > Does it even make sense to have the two separate things ("GPL v2" and > "GPL")? Yeah. If you had another OS project that uses GPL-4 you can just copy over a GPL-2+ driver to it, not an GPL-2 driver. So assuming the kernel will stay at GPL-2 forever it doesn't make any difference for the kernel. But other projects might benefit. (And if in the future someone might want to change the kernel to GPL-4, she only needs to contact the GPL-2 authors and can legally change GPL-2+ to GPL-4+.) > Suppose there is a global change that modifies a bunch of drivers, some of > them are GPLv2+. Now, the author of the global change is submitting it > under "GPL v2 only" license, which, by definition, turns any GPLv2+ code > into "GPL v2 only", right? See http://yarchive.net/comp/linux/dual_license_bsd_gpl.html for Linus' POV. It's an old mail, but I think it still applies. > So, changing from GPLv2+ to "GPL v2 only" is OK, but not the other way > around. OK as in (probably) legal. OK as in fair is questionable. > IANAL, tho. ditto. Best regards Uwe
diff --git a/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt b/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt index 0951952..9a599d2 100644 --- a/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt +++ b/Documentation/devicetree/bindings/power_supply/qnap-poweroff.txt @@ -11,4 +11,3 @@ Required Properties: - reg: Address and length of the register set for UART1 - clocks: tclk clock - diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c index ca0b476..cecb317 100644 --- a/drivers/power/reset/qnap-poweroff.c +++ b/drivers/power/reset/qnap-poweroff.c @@ -28,8 +28,7 @@ static void __iomem *base; static unsigned long tclk; -static void -qnap_power_off(void) +static void qnap_power_off(void) { /* 19200 baud divisor */ const unsigned divisor = ((tclk + (8 * 19200)) / (16 * 19200)); @@ -49,28 +48,25 @@ qnap_power_off(void) writel('A', UART1_REG(TX)); } -static int -qnap_power_off_probe(struct platform_device *pdev) +static int qnap_power_off_probe(struct platform_device *pdev) { struct resource *res; struct clk *clk; char symname[KSYM_NAME_LEN]; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res == NULL) { + if (!res) { dev_err(&pdev->dev, "Missing resource"); return -EINVAL; } - base = devm_ioremap(&pdev->dev, res->start, - resource_size(res)); - if (base == NULL) { + base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); + if (!base) { dev_err(&pdev->dev, "Unable to map resource"); return -EINVAL; } - /* We need to know tclk in order to calculate the UART - divisor */ + /* We need to know tclk in order to calculate the UART divisor */ clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(clk)) { dev_err(&pdev->dev, "Clk missing"); @@ -80,8 +76,8 @@ qnap_power_off_probe(struct platform_device *pdev) tclk = clk_get_rate(clk); /* Check that nothing else has already setup a handler */ - if (pm_power_off != NULL) { - lookup_symbol_name(pm_power_off, symname); + if (pm_power_off) { + lookup_symbol_name((ulong)pm_power_off, symname); dev_err(&pdev->dev, "pm_power_off already claimed %p %s", pm_power_off, symname); @@ -92,11 +88,9 @@ qnap_power_off_probe(struct platform_device *pdev) return 0; } -static int -qnap_power_off_remove(struct platform_device *pdev) +static int qnap_power_off_remove(struct platform_device *pdev) { pm_power_off = NULL; - return 0; } @@ -104,7 +98,6 @@ static const struct of_device_id qnap_power_off_of_match_table[] = { { .compatible = "qnap,power-off", }, {} }; - MODULE_DEVICE_TABLE(of, qnap_power_off_of_match_table); static struct platform_driver qnap_power_off_driver = { @@ -116,7 +109,6 @@ static struct platform_driver qnap_power_off_driver = { .of_match_table = of_match_ptr(qnap_power_off_of_match_table), }, }; - module_platform_driver(qnap_power_off_driver); MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch>");