[PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Timur Tabi]

Export the global variable 'ppc_tb_freq', so that modules (like the Book-E
watchdog driver) can use it.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This export is necessary for the Book-E watchdog driver to be compiled as a
module.  Since ppc_proc_freq is already exported, I figured it's okay for
ppc_tb_freq to be exported as well.

 arch/powerpc/kernel/time.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 8533b3b..49aa130 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -163,6 +163,7 @@ static long timezone_offset;
 unsigned long ppc_proc_freq;
 EXPORT_SYMBOL(ppc_proc_freq);
 unsigned long ppc_tb_freq;
+EXPORT_SYMBOL(ppc_tb_freq);
 
 static DEFINE_PER_CPU(u64, last_jiffy);
 
-- 
1.7.2.3

[PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

[Timur Tabi]

Register the __init and __exit functions in the PowerPC e500 watchdog driver
as module entry/exit functions, and modify the Kconfig entry.

Add a .release method for the PowerPC e500 watchdog driver, so that the
watchdog is disabled when the driver is closed.

Loosely based on original code from Jiang Yutang <b14898@freescale.com>.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch requires:

	powerpc: export ppc_tb_freq so that modules can reference it

 drivers/watchdog/Kconfig     |    5 ++++-
 drivers/watchdog/booke_wdt.c |   39 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 24efd8e..d9cf5a9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -957,9 +957,12 @@ config PIKA_WDT
 	  the Warp platform.
 
 config BOOKE_WDT
-	bool "PowerPC Book-E Watchdog Timer"
+	tristate "PowerPC Book-E Watchdog Timer"
 	depends on BOOKE || 4xx
 	---help---
+	  Watchdog driver for PowerPC e500 chips, such as the Freescale
+	  MPC85xx SOCs.
+
 	  Please see Documentation/watchdog/watchdog-api.txt for
 	  more information.
 
diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
index 3d49671..a989998 100644
--- a/drivers/watchdog/booke_wdt.c
+++ b/drivers/watchdog/booke_wdt.c
@@ -4,7 +4,7 @@
  * Author: Matthew McClintock
  * Maintainer: Kumar Gala <galak@kernel.crashing.org>
  *
- * Copyright 2005, 2008 Freescale Semiconductor Inc.
+ * Copyright 2005, 2008, 2010 Freescale Semiconductor Inc.
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -114,6 +114,27 @@ static void __booke_wdt_enable(void *data)
 	mtspr(SPRN_TCR, val);
 }
 
+/**
+ * booke_wdt_disable - disable the watchdog on the given CPU
+ *
+ * This function is called on each CPU.  It disables the watchdog on that CPU.
+ *
+ * TCR[WRC] cannot be changed once it has been set to non-zero, but we can
+ * effectively disable the watchdog by setting its period to the maximum value.
+ */
+static void __booke_wdt_disable(void *data)
+{
+	u32 val;
+
+	val = mfspr(SPRN_TCR);
+	val &= ~(TCR_WIE | WDTP_MASK);
+	mtspr(SPRN_TCR, val);
+
+	/* clear status to make sure nothing is pending */
+	__booke_wdt_ping(NULL);
+
+}
+
 static ssize_t booke_wdt_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *ppos)
 {
@@ -193,12 +214,21 @@ static int booke_wdt_open(struct inode *inode, struct file *file)
 	return nonseekable_open(inode, file);
 }
 
+static int booke_wdt_release(struct inode *inode, struct file *file)
+{
+	on_each_cpu(__booke_wdt_disable, NULL, 0);
+	booke_wdt_enabled = 0;
+
+	return 0;
+}
+
 static const struct file_operations booke_wdt_fops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,
 	.write = booke_wdt_write,
 	.unlocked_ioctl = booke_wdt_ioctl,
 	.open = booke_wdt_open,
+	.release = booke_wdt_release,
 };
 
 static struct miscdevice booke_wdt_miscdev = {
@@ -237,4 +267,9 @@ static int __init booke_wdt_init(void)
 
 	return ret;
 }
-device_initcall(booke_wdt_init);
+
+module_init(booke_wdt_init);
+module_exit(booke_wdt_exit);
+
+MODULE_DESCRIPTION("PowerPC Book-E watchdog driver");
+MODULE_LICENSE("GPL");
-- 
1.7.2.3

Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

[Josh Boyer]

On Fri, Sep 17, 2010 at 6:53 PM, Timur Tabi <timur@freescale.com> wrote:
> Register the __init and __exit functions in the PowerPC e500 watchdog dri=
ver
> as module entry/exit functions, and modify the Kconfig entry.
>
> Add a .release method for the PowerPC e500 watchdog driver, so that the
> watchdog is disabled when the driver is closed.

This is used for more than just e500.

>
> Loosely based on original code from Jiang Yutang <b14898@freescale.com>.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> This patch requires:
>
> =A0 =A0 =A0 =A0powerpc: export ppc_tb_freq so that modules can reference =
it
>
> =A0drivers/watchdog/Kconfig =A0 =A0 | =A0 =A05 ++++-
> =A0drivers/watchdog/booke_wdt.c | =A0 39 ++++++++++++++++++++++++++++++++=
+++++--
> =A02 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 24efd8e..d9cf5a9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -957,9 +957,12 @@ config PIKA_WDT
> =A0 =A0 =A0 =A0 =A0the Warp platform.
>
> =A0config BOOKE_WDT
> - =A0 =A0 =A0 bool "PowerPC Book-E Watchdog Timer"
> + =A0 =A0 =A0 tristate "PowerPC Book-E Watchdog Timer"
> =A0 =A0 =A0 =A0depends on BOOKE || 4xx
> =A0 =A0 =A0 =A0---help---
> + =A0 =A0 =A0 =A0 Watchdog driver for PowerPC e500 chips, such as the Fre=
escale
> + =A0 =A0 =A0 =A0 MPC85xx SOCs.
> +

Again, used for more than e500.  That || 4xx in the depends statement
right above your addition isn't there for fun :).

josh

Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

[Timur Tabi]

On Fri, Sep 17, 2010 at 7:37 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>> =A0config BOOKE_WDT
>> - =A0 =A0 =A0 bool "PowerPC Book-E Watchdog Timer"
>> + =A0 =A0 =A0 tristate "PowerPC Book-E Watchdog Timer"
>> =A0 =A0 =A0 =A0depends on BOOKE || 4xx
>> =A0 =A0 =A0 =A0---help---
>> + =A0 =A0 =A0 =A0 Watchdog driver for PowerPC e500 chips, such as the Fr=
eescale
>> + =A0 =A0 =A0 =A0 MPC85xx SOCs.
>> +
>
> Again, used for more than e500. =A0That || 4xx in the depends statement
> right above your addition isn't there for fun :).

Does this mean that this comment which is already in the driver is
wrong?  It alludes that "Book-E" is synonymous with "e500".

/* If the kernel parameter wdt=3D1, the watchdog will be enabled at boot.
 * Also, the wdt_period sets the watchdog timer period timeout.
 * For E500 cpus the wdt_period sets which bit changing from 0->1 will
 * trigger a watchog timeout. This watchdog timeout will occur 3 times, the
 * first time nothing will happen, the second time a watchdog exception wil=
l
 * occur, and the final time the board will reset.
 */

#ifdef	CONFIG_FSL_BOOKE
#define WDT_PERIOD_DEFAULT 38	/* Ex. wdt_period=3D28 bus=3D333Mhz,reset=3D~=
40sec */
#else
#define WDT_PERIOD_DEFAULT 3	/* Refer to the PPC40x and PPC4xx manuals */
#endif				/* for timing information */

--=20
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

[Josh Boyer]

On Mon, Sep 20, 2010 at 10:51:37AM -0500, Timur Tabi wrote:
>On Fri, Sep 17, 2010 at 7:37 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>>>  config BOOKE_WDT
>>> -       bool "PowerPC Book-E Watchdog Timer"
>>> +       tristate "PowerPC Book-E Watchdog Timer"
>>>        depends on BOOKE || 4xx
>>>        ---help---
>>> +         Watchdog driver for PowerPC e500 chips, such as the Freescale
>>> +         MPC85xx SOCs.
>>> +
>>
>> Again, used for more than e500.  That || 4xx in the depends statement
>> right above your addition isn't there for fun :).
>
>Does this mean that this comment which is already in the driver is
>wrong?  It alludes that "Book-E" is synonymous with "e500".
>
>/* If the kernel parameter wdt=1, the watchdog will be enabled at boot.
> * Also, the wdt_period sets the watchdog timer period timeout.
> * For E500 cpus the wdt_period sets which bit changing from 0->1 will
> * trigger a watchog timeout. This watchdog timeout will occur 3 times, the
> * first time nothing will happen, the second time a watchdog exception will
> * occur, and the final time the board will reset.
> */

I guess I don't see what you mean.  It talks about e500 but it doesn't
say "Book-E" anywhere in that comment.  (Though I'm pretty sure that
comment does apply to 4xx.)

>
>#ifdef	CONFIG_FSL_BOOKE
>#define WDT_PERIOD_DEFAULT 38	/* Ex. wdt_period=28 bus=333Mhz,reset=~40sec */
>#else
>#define WDT_PERIOD_DEFAULT 3	/* Refer to the PPC40x and PPC4xx manuals */
>#endif				/* for timing information */

And this bit is marked as "FSL_BOOKE" vs. 40x and 44x.

josh

Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module

[Timur Tabi]

Josh Boyer wrote:
> I guess I don't see what you mean.  It talks about e500 but it doesn't
> say "Book-E" anywhere in that comment.  (Though I'm pretty sure that
> comment does apply to 4xx.)

That was my point.  The comment says e500, but the code is also intended for
4xx.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Josh Boyer]

On Fri, Sep 17, 2010 at 6:53 PM, Timur Tabi <timur@freescale.com> wrote:
> Export the global variable 'ppc_tb_freq', so that modules (like the Book-=
E
> watchdog driver) can use it.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> This export is necessary for the Book-E watchdog driver to be compiled as=
 a
> module. =A0Since ppc_proc_freq is already exported, I figured it's okay f=
or
> ppc_tb_freq to be exported as well.
>
> =A0arch/powerpc/kernel/time.c | =A0 =A01 +
> =A01 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 8533b3b..49aa130 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -163,6 +163,7 @@ static long timezone_offset;
> =A0unsigned long ppc_proc_freq;
> =A0EXPORT_SYMBOL(ppc_proc_freq);
> =A0unsigned long ppc_tb_freq;
> +EXPORT_SYMBOL(ppc_tb_freq);

EXPORT_SYMBOL_GPL probably, no?

josh

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Timur Tabi]

On Fri, Sep 17, 2010 at 7:38 PM, Josh Boyer <jwboyer@gmail.com> wrote:

>> =A0unsigned long ppc_proc_freq;
>> =A0EXPORT_SYMBOL(ppc_proc_freq);
>> =A0unsigned long ppc_tb_freq;
>> +EXPORT_SYMBOL(ppc_tb_freq);
>
> EXPORT_SYMBOL_GPL probably, no?

I don't see any reason to limit it to GPL drivers.  Not only that, but
then we'll have this:

EXPORT_SYMBOL(ppc_proc_freq);
EXPORT_SYMBOL_GPL(ppc_tb_freq);

That just looks dumb.


--=20
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Benjamin Herrenschmidt]

On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
> I don't see any reason to limit it to GPL drivers.  Not only that, but
> then we'll have this:

I do

> EXPORT_SYMBOL(ppc_proc_freq);
> EXPORT_SYMBOL_GPL(ppc_tb_freq);
> 
> That just looks dumb. 

Right, so send a patch to fix the first one too :-)

Cheers,
Ben.

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Tabi Timur-B04825]

On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" =
<benh@kernel.crashing.org> wrote:

> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
>> I don't see any reason to limit it to GPL drivers.  Not only that, =
but
>> then we'll have this:
>=20
> I do

Can you elaborate on that, or are you just going to pull rank on me?

>=20
>> EXPORT_SYMBOL(ppc_proc_freq);
>> EXPORT_SYMBOL_GPL(ppc_tb_freq);
>>=20
>> That just looks dumb.=20
>=20
> Right, so send a patch to fix the first one too :-)

Then why doesn't someone post a patch to change all EXPORT_SYMBOL to =
EXPORT_SYMBOL_GPL?  And why do we consider EXPORT_SYMBOL to be "broken"?

I'm not trying to be a troll, but I see a lot of inconsistency with =
respect to  EXPORT_SYMBOL and EXPORT_SYMBOL_GPL. =20
>=20

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Kumar Gala]

On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote:

> On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" =
<benh@kernel.crashing.org> wrote:
>=20
>> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
>>> I don't see any reason to limit it to GPL drivers.  Not only that, =
but
>>> then we'll have this:
>>=20
>> I do
>=20
> Can you elaborate on that, or are you just going to pull rank on me?
>=20
>>=20
>>> EXPORT_SYMBOL(ppc_proc_freq);
>>> EXPORT_SYMBOL_GPL(ppc_tb_freq);
>>>=20
>>> That just looks dumb.=20
>>=20
>> Right, so send a patch to fix the first one too :-)

I don't think either of these should be EXPORT_SYMBOL_GPL.  Why =
shouldn't a binary module be allowed to know these frequencies?  My view =
is why preclude anyone from using this how they want.  If they want to =
live in the gray area so be it.  Who am I to say they shouldn't have =
that choice.

> Then why doesn't someone post a patch to change all EXPORT_SYMBOL to =
EXPORT_SYMBOL_GPL?  And why do we consider EXPORT_SYMBOL to be "broken"?
>=20
> I'm not trying to be a troll, but I see a lot of inconsistency with =
respect to  EXPORT_SYMBOL and EXPORT_SYMBOL_GPL. =20

I'm in agreement with the lack of clarity, it seems to be developer =
whim.

- k

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Vitaly Wool]

On Sat, Sep 18, 2010 at 5:34 PM, Kumar Gala <kumar.gala@freescale.com> wrot=
e:

> I don't think either of these should be EXPORT_SYMBOL_GPL. =A0Why shouldn=
't a binary module be allowed to know these frequencies? =A0My view is why =
preclude anyone from using this how they want. =A0If they want to live in t=
he gray area so be it. =A0Who am I to say they shouldn't have that choice.

W00t, binary modules again... No, please. No binary modules in kernelspace.

Thanks,
   Vitaly

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Josh Boyer]

On Sat, Sep 18, 2010 at 11:34 AM, Kumar Gala <kumar.gala@freescale.com> wro=
te:
>
> On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote:
>
>> On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.cras=
hing.org> wrote:
>>
>>> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
>>>> I don't see any reason to limit it to GPL drivers. =A0Not only that, b=
ut
>>>> then we'll have this:
>>>
>>> I do
>>
>> Can you elaborate on that, or are you just going to pull rank on me?
>>
>>>
>>>> EXPORT_SYMBOL(ppc_proc_freq);
>>>> EXPORT_SYMBOL_GPL(ppc_tb_freq);
>>>>
>>>> That just looks dumb.
>>>
>>> Right, so send a patch to fix the first one too :-)
>
> I don't think either of these should be EXPORT_SYMBOL_GPL. =A0Why shouldn=
't a binary module be allowed to know these frequencies? =A0My view is why =
preclude anyone from using this how they want. =A0If they want to live in t=
he gray area so be it. =A0Who am I to say they shouldn't have that choice.
>

It is not, in my opinion, about what is technically possible and what
isn't.  The kernel is licensed under the GPL.  This is a Linux kernel
only symbol.  One would be hard pressed to claim they have a driver
that wasn't written for Linux that happens to need that symbol.  As a
member of the Linux kernel community, I find it important to encourage
the contribution of code back to the kernel, and this is one way to
help that.  This isn't BSD.

Besides, a developer is free to export it however they wish in their
own kernel tree.  They can deviate from mainline if they so choose.

josh

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Tabi Timur-B04825]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

Josh Boyer wrote:
> It is not, in my opinion, about what is technically possible and what
> isn't.  The kernel is licensed under the GPL.  This is a Linux kernel
> only symbol.  One would be hard pressed to claim they have a driver
> that wasn't written for Linux that happens to need that symbol.  As a
> member of the Linux kernel community, I find it important to encourage
> the contribution of code back to the kernel, and this is one way to
> help that.  This isn't BSD.

Fine, but this goes back to my original question -- if this is how the 
community feels, then why hasn't someone posted a patch that converts all 
EXPORT_SYMBOL into EXPORT_SYMBOL_GPL?

Either we allow non-GPL drivers, or we don't.  If we don't, then we need to 
eliminate EXPORT_SYMBOL once and for all.  Otherwise, the message is 
hypocritical.


-- 
Timur Tabi
Linux kernel developer

[-- Attachment #2: Type: text/html, Size: 1440 bytes --]

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Josh Boyer]

On Sat, Sep 18, 2010 at 1:36 PM, Tabi Timur-B04825 <B04825@freescale.com> w=
rote:
> Josh Boyer wrote:
>> It is not, in my opinion, about what is technically possible and what
>> isn't.=A0 The kernel is licensed under the GPL.=A0 This is a Linux kerne=
l
>> only symbol.=A0 One would be hard pressed to claim they have a driver
>> that wasn't written for Linux that happens to need that symbol.=A0 As a
>> member of the Linux kernel community, I find it important to encourage
>> the contribution of code back to the kernel, and this is one way to
>> help that.=A0 This isn't BSD.
>
> Fine, but this goes back to my original question -- if this is how the
> community feels, then why hasn't someone posted a patch that converts all
> EXPORT_SYMBOL into EXPORT_SYMBOL_GPL?

Because of history in a lot of cases, and like all communities,
opinions vary.  I did say this was my opinion, not a mandate of some
sort.

> Either we allow non-GPL drivers, or we don't.=A0 If we don't, then we nee=
d to
> eliminate EXPORT_SYMBOL once and for all.=A0 Otherwise, the message is
> hypocritical.

I'd be all for it.  I don't think it is as black and white as that
though, as nothing rarely is.  (we can't even get all the code to
adhere to the < 80 column "rule" ).  I also don't think it is
necessarily hypocritical.  This is a new symbol being exported, not
one that has been exported for years.

josh

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Tabi Timur-B04825]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

Josh Boyer wrote:
> This is a new symbol being exported, not
> one that has been exported for years.

Except that Ben says that I should change ppc_proc_freq from EXPORT_SYMBOL to 
EXPORT_SYMBOL_GPL as well.  In a sense, we're in a catch-22.  We have three 
choices:

1. We *arbitrarily* change ppc_proc_freq from EXPORT_SYMBOL to 
EXPORT_SYMBOL_GPL, so that the two symbols are exported the same way

2. We GPL-export only ppc_tb_freq and leave ppc_proc_freq as-is, but then it 
looks dumb.

3. We export ppc_tb_freq the same way we're exporting ppc_proc_freq, 
providing the most options and maintaining consistency.

I just don't see how options #1 or #2 are better than #3, and so far the only 
explanations I've heard are along the lines of "we just like it that way".

Obviously, Linus thinks it's okay to allow some non-GPL modules, otherwise he 
would have long ago changed all EXPORT_SYMBOLs to EXPORT_SYMBOL_GPL.  I'm 
just capitalizing on that mindset.

-- 
Timur Tabi
Linux kernel developer

[-- Attachment #2: Type: text/html, Size: 1579 bytes --]

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Josh Boyer]

On Sat, Sep 18, 2010 at 1:55 PM, Tabi Timur-B04825 <B04825@freescale.com> w=
rote:
> Josh Boyer wrote:
>> This is a new symbol being exported, not
>> one that has been exported for years.
>
> Except that Ben says that I should change ppc_proc_freq from EXPORT_SYMBO=
L
> to
> EXPORT_SYMBOL_GPL as well.=A0 In a sense, we're in a catch-22.=A0 We have=
 three
> choices:
>
> 1. We *arbitrarily* change ppc_proc_freq from EXPORT_SYMBOL to
> EXPORT_SYMBOL_GPL, so that the two symbols are exported the same way
>
> 2. We GPL-export only ppc_tb_freq and leave ppc_proc_freq as-is, but then=
 it
> looks dumb.

I dunno.  I don't think it looks dumb.  It could mean nothing more
than we were paying closer attention this time.

> 3. We export ppc_tb_freq the same way we're exporting ppc_proc_freq,
> providing the most options and maintaining consistency.
>
> I just don't see how options #1 or #2 are better than #3, and so far the
> only
> explanations I've heard are along the lines of "we just like it that way"=
.

Now I think I've been a bit more detailed than that.  I at least
explained why I prefer it that way.  If you disagree, that's fine but
don't make me sound like some kind of petulant child.

> Obviously, Linus thinks it's okay to allow some non-GPL modules, otherwis=
e
> he
> would have long ago changed all EXPORT_SYMBOLs to EXPORT_SYMBOL_GPL.=A0 I=
'm
> just capitalizing on that mindset.

Capitalizing?  The patch you posted that uses this symbol is for a GPL
driver so you gain or lose nothing by having this symbol be
EXPORT_SYMBOL_GPL.  Are you somehow advocating and getting some sort
of gain by allowing non-GPL modules?  If so, I find that unfortunate.
If not, then I guess I don't understand what you mean by capitalizing.

josh

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Scott Wood]

On Sat, 18 Sep 2010 14:22:12 -0400
Josh Boyer <jwboyer@gmail.com> wrote:

> Capitalizing?  The patch you posted that uses this symbol is for a GPL
> driver so you gain or lose nothing by having this symbol be
> EXPORT_SYMBOL_GPL.  Are you somehow advocating and getting some sort
> of gain by allowing non-GPL modules?  If so, I find that unfortunate.
> If not, then I guess I don't understand what you mean by capitalizing.

One can dislike DRM (even a very weak form such as this) without having
a particular desire to go outside the bounds of what it allows.

I thought EXPORT_SYMBOL_GPL was originally meant to indicate the
symbols whose use is likely to be indicitave of code that is, in some
copyright-meaningful way, derived from GPL code?  I have a hard time
seeing that being the case here.  If every symbol is made GPL-only,
then that just gives the binary-only people[1] more incentive to
circumvent the entire mechanism.  It loses its meaning.

Documentation/DocBook/kernel-hacking.tmpl says, "It implies that the
function is considered an internal implementation issue, and not really
an interface."

-Scott

[1] Plus anyone who might want to make a kernel module out of code
which is open source, but not under a license the GPLv2 is compatible
with.

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Detlev Zundel]

Hi Scott,

> On Sat, 18 Sep 2010 14:22:12 -0400
> Josh Boyer <jwboyer@gmail.com> wrote:
>
>> Capitalizing?  The patch you posted that uses this symbol is for a GPL
>> driver so you gain or lose nothing by having this symbol be
>> EXPORT_SYMBOL_GPL.  Are you somehow advocating and getting some sort
>> of gain by allowing non-GPL modules?  If so, I find that unfortunate.
>> If not, then I guess I don't understand what you mean by capitalizing.
>
> One can dislike DRM (even a very weak form such as this) without having
> a particular desire to go outside the bounds of what it allows.
>
> I thought EXPORT_SYMBOL_GPL was originally meant to indicate the
> symbols whose use is likely to be indicitave of code that is, in some
> copyright-meaningful way, derived from GPL code?  

Google finds this, which coincides with what I remmber[1]:

  EXPORT_SYMBOL_GPL 
  
  Some kernel developers are unhappy with providing external interfaces to
  their code, only to see those interfaces being used by binary only
  modules. They view it as their work being appropriated. Whether you
  agree with that view or not is completely irrelevant, the person who
  owns the copyright decides how their work can be used.
  
  EXPORT_SYMBOL_GPL() allows for new interfaces to be marked as only
  available to modules with a GPL compatible license. This is independent
  of the kernel tainting, but obviously takes advantage of
  MODULE_LICENSE() strings.
  
  EXPORT_SYMBOL_GPL() may only be used for new exported symbols, Linus has
  spoken. I believe the phrase involved killer penguins with chainsaws for
  anybody who changed existing exported interfaces.

Cheers
  Detlev

[1] http://lkml.indiana.edu/hypermail/linux/kernel/0110.2/0369.html
  
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Kumar Gala]

On Sep 18, 2010, at 11:56 AM, Josh Boyer wrote:

> On Sat, Sep 18, 2010 at 11:34 AM, Kumar Gala =
<kumar.gala@freescale.com> wrote:
>>=20
>> On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote:
>>=20
>>> On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" =
<benh@kernel.crashing.org> wrote:
>>>=20
>>>> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
>>>>> I don't see any reason to limit it to GPL drivers.  Not only that, =
but
>>>>> then we'll have this:
>>>>=20
>>>> I do
>>>=20
>>> Can you elaborate on that, or are you just going to pull rank on me?
>>>=20
>>>>=20
>>>>> EXPORT_SYMBOL(ppc_proc_freq);
>>>>> EXPORT_SYMBOL_GPL(ppc_tb_freq);
>>>>>=20
>>>>> That just looks dumb.
>>>>=20
>>>> Right, so send a patch to fix the first one too :-)
>>=20
>> I don't think either of these should be EXPORT_SYMBOL_GPL.  Why =
shouldn't a binary module be allowed to know these frequencies?  My view =
is why preclude anyone from using this how they want.  If they want to =
live in the gray area so be it.  Who am I to say they shouldn't have =
that choice.
>>=20
>=20
> It is not, in my opinion, about what is technically possible and what
> isn't.  The kernel is licensed under the GPL.  This is a Linux kernel
> only symbol.  One would be hard pressed to claim they have a driver
> that wasn't written for Linux that happens to need that symbol.  As a
> member of the Linux kernel community, I find it important to encourage
> the contribution of code back to the kernel, and this is one way to
> help that.  This isn't BSD.
>=20
> Besides, a developer is free to export it however they wish in their
> own kernel tree.  They can deviate from mainline if they so choose.

I'll buy this argument as a reason to make both EXPORT_SYMBOL_GPL.

- k=

Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it

[Benjamin Herrenschmidt]

On Sat, 2010-09-18 at 10:34 -0500, Kumar Gala wrote:
> On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote:
> 
> > On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote:
> > 
> >> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote:
> >>> I don't see any reason to limit it to GPL drivers.  Not only that, but
> >>> then we'll have this:
> >> 
> >> I do
> > 
> > Can you elaborate on that, or are you just going to pull rank on me?
> > 
> >> 
> >>> EXPORT_SYMBOL(ppc_proc_freq);
> >>> EXPORT_SYMBOL_GPL(ppc_tb_freq);
> >>> 
> >>> That just looks dumb. 
> >> 
> >> Right, so send a patch to fix the first one too :-)
> 
> I don't think either of these should be EXPORT_SYMBOL_GPL.  Why
> shouldn't a binary module be allowed to know these frequencies?  My
> view is why preclude anyone from using this how they want.  If they
> want to live in the gray area so be it.  Who am I to say they
> shouldn't have that choice.

Well, I'm all for making binary modules life as hard as possible just
for the sake of it :-)

Cheers,
Ben.