* Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module
From: Josh Boyer @ 2010-09-20 19:30 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala, linux-watchdog
In-Reply-To: <AANLkTimp-0zdw-fAaX8qCwHW0NMn--6RK0_USDGOSe47@mail.gmail.com>
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
^ permalink raw reply
* Re: [PATCH v2 03/10] RapidIO: Use stored ingress port number instead of register read
From: Andrew Morton @ 2010-09-20 19:17 UTC (permalink / raw)
To: Bounine, Alexandre; +Cc: linux-kernel, Thomas Moll, linuxppc-dev
In-Reply-To: <0CE8B6BE3C4AD74AB97D9D29BD24E55201303D0E@CORPEXCH1.na.ads.idt.com>
On Mon, 20 Sep 2010 07:31:22 -0700
"Bounine, Alexandre" <Alexandre.Bounine@idt.com> wrote:
> Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > > @@ -219,6 +219,7 @@ struct rio_net {
> > > /**
> > > * struct rio_switch - RIO switch info
> > > * @node: Node in global list of switches
> > > + * @rdev: Associated RIO device structure
> > > * @switchid: Switch ID that is unique across a network
> > > * @hopcount: Hopcount to this switch
> > > * @destid: Associated destid in the path
> > > @@ -234,6 +235,7 @@ struct rio_net {
> > > */
> > > struct rio_switch {
> > > struct list_head node;
> > > + struct rio_dev *rdev;
> > > u16 switchid;
> > > u16 hopcount;
> > > u16 destid;
> >
> > What is the locking for rdev?
>
> This question prompted me consider the following change:
>
> Because the rio_switch structure (in current implementation) is a
> dynamically allocated part of rio_dev, I think it may be simpler to
> combine them into single allocation by using zero length array
> declaration:
>
> struct rio_dev {
> struct list_head global_list;
> struct list_head net_list;
> .....
> ..... rest of rio_dev
> .....
> struct rio_switch switch[0];
> }
>
> This will remove extra memory allocation, remove overlapping structure
> members and clean code sections like one shown below:
>
> u8 hopcount = 0xff;
> u16 destid = rdev->destid;
>
> if (rdev->rswitch) {
> destid = rdev->rswitch->destid;
> hopcount = rdev->rswitch->hopcount;
> }
>
> And this looks better aligned with RapidIO definitions - both: endpoints
> and switches are RIO devices. The current implementation of rio_dev
> somewhat separates rio_switch from its common part (this is why I have
> added that link into rio_switch). We may keep using the pointer to
> associated rio_dev, but reworking the rio_dev structure seems better way
> to me.
>
> Please, let me know what do you think about this conversion. And if
> there are no objections I will make a patch.
>
If you say so ;)
The "variable length array at the end of the struct" thing is pretty
commonly used and works well. As long as we never want to change the
number of switches on the fly (hotplug?).
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it
From: Scott Wood @ 2010-09-20 18:51 UTC (permalink / raw)
To: Josh Boyer
Cc: linuxppc-dev, Gala Kumar-B11780, Tabi Timur-B04825,
linux-watchdog
In-Reply-To: <AANLkTi=RXjc=GZ+RRsKjL3qdYFayXOCX0s1SU5jojeXg@mail.gmail.com>
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.
^ permalink raw reply
* RE: [PATCH 1/2] PPC4xx: Generelizing drivers/dma/ppc4xx/adma.c
From: Tirumala Marri @ 2010-09-20 18:00 UTC (permalink / raw)
To: Wolfgang Denk
Cc: neilb, yur, linux-raid, herbert, linux-crypto, dan.j.williams,
linuxppc-dev
In-Reply-To: <20100918210920.E05AE157D71@gemini.denx.de>
Mr. Wolfgang
>
> Will this driver ever include any 40x processors? If not, you probably
> should use "44x" instead (here and everywhere in the rest of the
> code).
[Marri] Yes there is 40x based DMA engine we planned to include in the
future.
> > +/* Pointer to DMA0, DMA1 CP/CS FIFO */
> > +static void *ppc440spe_dma_fifo_buf;
>
> Seems this should go into "ppc440spe-dma.h"?
>
>
> > +/* This array is used in data-check operations for storing a pattern
> */
> > +static char ppc440spe_qword[16];
> > +
> > +static atomic_t ppc4xx_adma_err_irq_ref;
> > +static dcr_host_t ppc440spe_mq_dcr_host;
> > +static unsigned int ppc440spe_mq_dcr_len;
>
> Ditto?
>
> > +static unsigned long ppc440spe_rxor_state;
> > +
> > +static struct page *ppc440spe_rxor_srcs[32];
>
> And here again - please check globally!
>
>
> > +/**
> > + * ppc440spe_can_rxor - check if the operands may be processed with
> RXOR
> > + */
> > +static int ppc440spe_can_rxor(struct page **srcs, int src_cnt,
> size_t len)
>
> Again, should this then not be in ppc440spe specific files?
>
>
> It seems the split / generalization is highly incomplete yet.
[Marri] Some of the stuff can go into ppc440spe-dma.h. This is just
Beginning to add more new DMA engines support. I expect more changes to
come in.
Regards,
Marri
^ permalink raw reply
* Re: cuImage and multi image?
From: Shawn Jin @ 2010-09-20 17:52 UTC (permalink / raw)
To: tiejun.chen; +Cc: ppcdev, uboot
In-Reply-To: <4C91C77B.8080501@windriver.com>
>> I have a cuImage kernel in order to support legacy u-boot and a
>> ramdisk image. Kernel boots fine if these two images are separate and
>> "bootm $kernel $ramdisk" is used. But I can not make it to work using
>> a single multi image that contains the kernel and ramdisk images. Is
>> it even technically possible to boot a multi-image with cuboot
>> wrapper?
>
> Try the following steps:
> ------
> 1. cp <your ramdisk.gz> arch/powerpc/boot/ramdisk.image.gz
> 2. make cuImage.initrd.<your target>
>
> You can get one Image, cuImage.initrd.<your target>, including kernel and ramdisk.
A follow up question. With this method, the total image size
(uncompressed) is limited to the 4MB (the link address of the boot
wrapper)?
Thanks,
-Shawn.
^ permalink raw reply
* [PATCH 2/2] [v2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module
From: Timur Tabi @ 2010-09-20 16:23 UTC (permalink / raw)
To: benh, linuxppc-dev, kumar.gala, linux-watchdog, jwboyer
In-Reply-To: <1284999822-24753-1-git-send-email-timur@freescale.com>
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_proc_freq and ppc_tb_freq as GPL symbols
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..a6812eb 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 Book-E chips, such as the Freescale
+ MPC85xx SOCs and the IBM PowerPC 440.
+
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
^ permalink raw reply related
* [PATCH 1/2] [v2] powerpc: export ppc_proc_freq and ppc_tb_freq as GPL symbols
From: Timur Tabi @ 2010-09-20 16:23 UTC (permalink / raw)
To: benh, linuxppc-dev, kumar.gala, linux-watchdog, jwboyer
Export the global variable 'ppc_tb_freq', so that modules (like the Book-E
watchdog driver) can use it. To maintain consistency, ppc_proc_freq is changed
to a GPL-only export. This is okay, because any module that needs this symbol
should be an actual Linux driver, which must be GPL-licensed.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
This export is necessary for the Book-E watchdog driver to be compiled as a
module.
arch/powerpc/kernel/time.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 8533b3b..0c0c241 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -161,8 +161,9 @@ extern struct timezone sys_tz;
static long timezone_offset;
unsigned long ppc_proc_freq;
-EXPORT_SYMBOL(ppc_proc_freq);
+EXPORT_SYMBOL_GPL(ppc_proc_freq);
unsigned long ppc_tb_freq;
+EXPORT_SYMBOL_GPL(ppc_tb_freq);
static DEFINE_PER_CPU(u64, last_jiffy);
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc/watchdog: allow the e500 watchdog driver to be compiled as a module
From: Timur Tabi @ 2010-09-20 15:51 UTC (permalink / raw)
To: Josh Boyer; +Cc: kumar.gala, linux-watchdog, linuxppc-dev
In-Reply-To: <AANLkTi=Ks7hkDq1g1SyHLf4Lf8tsew-KnvKOgyQxjcWQ@mail.gmail.com>
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
^ permalink raw reply
* Re: Generating elf kernel ?
From: Scott Wood @ 2010-09-20 15:43 UTC (permalink / raw)
To: tiejun.chen; +Cc: linuxppc-dev, Guillaume Dargaud
In-Reply-To: <4C9569FF.6090807@windriver.com>
On Sun, 19 Sep 2010 09:40:15 +0800
"tiejun.chen" <tiejun.chen@windriver.com> wrote:
> Scott Wood wrote:
> > On Fri, 17 Sep 2010 09:58:41 +0800
> > "tiejun.chen" <tiejun.chen@windriver.com> wrote:
> >
> >> Scott Wood wrote:
> >>> The guest OS *is* the same as native Linux, as far as TLB handling is
> >>> concerned.
> >> Looks you means the TLB exception handler should be same between the native and
> >> the guest OS. Right?
> >
> > Yes.
>
> I don't think so. The HY should assist the guest OS on MMU since I already point
> the guest OS have no authority to create a real TLB directly as I previously said.
Of course the hypervisor assists, when a trap is taken. That doesn't
mean the code is any different in the guest.
> > Yes, of course. But that's not the point. I was just using it as a
> > convenient example because that's what I've recently done ELF loading
> > with... There's no reason U-Boot couldn't do the same if its ELF
> > loader were updated to support device trees. Currently U-Boot loads
> > bootwrapperless uImages to physical address zero.
>
> I never doubt the U-boot can do this for uImage. But I think we're always
> talking about vmlinux, a bare Image.
uImage is pretty much a bare image. It just has a header with a
checksum and some info like OS/architecture, kernel version, build
date, etc.
There would be *no* problem doing this with vmlinux in U-Boot if
someone put in the code to pass a device tree.
-Scott
^ permalink raw reply
* Re: [PATCH 1/3 v4] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: Anton Vorontsov @ 2010-09-20 15:37 UTC (permalink / raw)
To: Roy Zang
Cc: B07421, dedekind1, B25806, linuxppc-dev, linux-mtd, akpm, dwmw2,
B11780
In-Reply-To: <1284706869-12555-1-git-send-email-tie-fei.zang@freescale.com>
On Fri, Sep 17, 2010 at 03:01:07PM +0800, Roy Zang wrote:
[...]
> +struct fsl_lbc_ctrl {
> + /* device info */
> + struct device *dev;
Forward declaration for struct device is needed (and enough).
> + struct fsl_lbc_regs __iomem *regs;
> + int irq;
> + wait_queue_head_t irq_wait;
#include <linux/wait.h> is needed for this.
> + spinlock_t lock;
#include <linux/spinlock.h>
[...]
> diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
> index dceb8d1..4920cd3 100644
> --- a/arch/powerpc/sysdev/fsl_lbc.c
> +++ b/arch/powerpc/sysdev/fsl_lbc.c
[...]
> +#include <linux/slab.h>
> +#include <linux/of_platform.h>
I guess of_platform.h is not needed any longer.
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
>
[...]
> default:
> return -EINVAL;
> @@ -143,14 +130,18 @@ EXPORT_SYMBOL(fsl_upm_find);
> * thus UPM pattern actually executed. Note that mar usage depends on the
> * pre-programmed AMX bits in the UPM RAM.
> */
> +
No need for this empty line.
> int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base, u32 mar)
> {
> int ret = 0;
> unsigned long flags;
[...]
> +static int __devinit fsl_lbc_ctrl_probe(struct platform_device *dev)
> +{
> + int ret;
> +
> + if (!dev->dev.of_node) {
> + dev_err(&dev->dev, "Device OF-Node is NULL");
> + return -EFAULT;
> + }
> + fsl_lbc_ctrl_dev = kzalloc(sizeof(*fsl_lbc_ctrl_dev), GFP_KERNEL);
Btw, the code in the NAND driver checks for !fsl_lbc_ctrl_dev.
So it might make sense to assign the global variable after the
struct is fully initialized.
> + if (!fsl_lbc_ctrl_dev)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&dev->dev, fsl_lbc_ctrl_dev);
> +
> + spin_lock_init(&fsl_lbc_ctrl_dev->lock);
> + init_waitqueue_head(&fsl_lbc_ctrl_dev->irq_wait);
> +
> + fsl_lbc_ctrl_dev->regs = of_iomap(dev->dev.of_node, 0);
> + if (!fsl_lbc_ctrl_dev->regs) {
> + dev_err(&dev->dev, "failed to get memory region\n");
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + fsl_lbc_ctrl_dev->irq = irq_of_parse_and_map(dev->dev.of_node, 0);
> + if (fsl_lbc_ctrl_dev->irq == NO_IRQ) {
> + dev_err(&dev->dev, "failed to get irq resource\n");
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + fsl_lbc_ctrl_dev->dev = &dev->dev;
> +
> + ret = fsl_lbc_ctrl_init(fsl_lbc_ctrl_dev);
> + if (ret < 0)
> + goto err;
> +
> + ret = request_irq(fsl_lbc_ctrl_dev->irq, fsl_lbc_ctrl_irq, 0,
> + "fsl-lbc", fsl_lbc_ctrl_dev);
> + if (ret != 0) {
> + dev_err(&dev->dev, "failed to install irq (%d)\n",
> + fsl_lbc_ctrl_dev->irq);
> + ret = fsl_lbc_ctrl_dev->irq;
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + iounmap(fsl_lbc_ctrl_dev->regs);
> + kfree(fsl_lbc_ctrl_dev);
> + return ret;
> +}
> +
> +static const struct of_device_id fsl_lbc_match[] = {
#include <linux/mod_devicetable.h> is needed for this.
Plus, I think the patch is not runtime bisectable (i.e. you
now do request_irq() here, but not removing it from the nand
driver, so nand will fail to probe).
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 1/2]: Powerpc: Fix EHCA driver on relocatable kernel
From: Alexander Schmidt @ 2010-09-20 14:41 UTC (permalink / raw)
To: Sonny Rao; +Cc: linux-rdma, fenkes, linuxppc-dev, raisch
In-Reply-To: <20100820040809.GS16505@us.ibm.com>
On Thu, 19 Aug 2010 23:08:09 -0500
Sonny Rao <sonnyrao@us.ibm.com> wrote:
> Some modules (like eHCA) want to map all of kernel memory, for this to
> work with a relocated kernel, we need to export kernstart_addr so
> modules can use PHYSICAL_START and memstart_addr so they could use
> MEMORY_START. Note that the 32bit code already exports these symbols.
>
> Signed-off-By: Sonny Rao <sonnyrao@us.ibm.com>
Acked-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
> Index: common/arch/powerpc/mm/init_64.c
> ===================================================================
> --- common.orig/arch/powerpc/mm/init_64.c 2010-08-16 02:38:33.000000000 -0500
> +++ common/arch/powerpc/mm/init_64.c 2010-08-16 02:39:25.000000000 -0500
> @@ -79,7 +79,9 @@
> #endif /* CONFIG_PPC_STD_MMU_64 */
>
> phys_addr_t memstart_addr = ~0;
> +EXPORT_SYMBOL(memstart_addr);
> phys_addr_t kernstart_addr;
> +EXPORT_SYMBOL(kernstart_addr);
>
> void free_initmem(void)
> {
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* RE: [PATCH v2 03/10] RapidIO: Use stored ingress port number instead of register read
From: Bounine, Alexandre @ 2010-09-20 14:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Thomas Moll, linuxppc-dev
In-Reply-To: <20100914151219.5d92c6f9.akpm@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> wrote:
>=20
> > @@ -219,6 +219,7 @@ struct rio_net {
> > /**
> > * struct rio_switch - RIO switch info
> > * @node: Node in global list of switches
> > + * @rdev: Associated RIO device structure
> > * @switchid: Switch ID that is unique across a network
> > * @hopcount: Hopcount to this switch
> > * @destid: Associated destid in the path
> > @@ -234,6 +235,7 @@ struct rio_net {
> > */
> > struct rio_switch {
> > struct list_head node;
> > + struct rio_dev *rdev;
> > u16 switchid;
> > u16 hopcount;
> > u16 destid;
>=20
> What is the locking for rdev?
This question prompted me consider the following change:
Because the rio_switch structure (in current implementation) is a
dynamically allocated part of rio_dev, I think it may be simpler to
combine them into single allocation by using zero length array
declaration:
struct rio_dev {
struct list_head global_list;
struct list_head net_list;
.....
..... rest of rio_dev
.....
struct rio_switch switch[0];
}
This will remove extra memory allocation, remove overlapping structure
members and clean code sections like one shown below:
u8 hopcount =3D 0xff;
u16 destid =3D rdev->destid;
if (rdev->rswitch) {
destid =3D rdev->rswitch->destid;
hopcount =3D rdev->rswitch->hopcount;
} =20
And this looks better aligned with RapidIO definitions - both: endpoints
and switches are RIO devices. The current implementation of rio_dev
somewhat separates rio_switch from its common part (this is why I have
added that link into rio_switch). We may keep using the pointer to
associated rio_dev, but reworking the rio_dev structure seems better way
to me.
Please, let me know what do you think about this conversion. And if
there are no objections I will make a patch.
Alex.
=20
^ permalink raw reply
* Re: [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY)
From: Jan-Bernd Themann @ 2010-09-20 14:26 UTC (permalink / raw)
To: Milton Miller
Cc: linux-kernel-owner, Darren Hart, dvhltc, linux-kernel,
Milton Miller, Will Schmidt, Brian King, niv, Thomas Gleixner,
Doug Maxey, linuxppc-dev
In-Reply-To: <1274433481_6943@mail4.comsite.net>
Hi Milton,
sorry for the delayed answer, was on vacation.
linux-kernel-owner@vger.kernel.org wrote on 21.05.2010 11:18:01:
> linux-kernel-owner@vger.kernel.org
>
> On Thu, 20 May 2010 at 10:21:36 +0200 (CEST) Thomas Gleixner wrote:
> > On Thu, 20 May 2010, Michael Ellerman wrote:
> > > On Wed, 2010-05-19 at 16:38 +0200, Thomas Gleixner wrote:
> > > > On Wed, 19 May 2010, Darren Hart wrote:
> > > >
> > > > > On 05/18/2010 06:25 PM, Michael Ellerman wrote:
> > > > > > On Tue, 2010-05-18 at 15:22 -0700, Darren Hart wrote:
> > >
> > > > > > The result of the discussion about two years ago on this was
that we
> > > > > > needed a custom flow handler for XICS on RT.
> > > > >
> > > > > I'm still not clear on why the ultimate solution wasn't to
> have XICS report
> > > > > edge triggered as edge triggered. Probably some complexity
> of the entire power
> > > > > stack that I am ignorant of.
> > > > >
> > > > > > Apart from the issue of loosing interrupts there is also
> the fact that
> > > > > > masking on the XICS requires an RTAS call which takes a global
lock.
> > > >
> > > > Right, I'd love to avoid that but with real level interrupts we'd
run
> > > > into an interrupt storm. Though another solution would be to issue
the
> > > > EOI after the threaded handler finished, that'd work as well, but
> > > > needs testing.
> > >
> > > Yeah I think that was the idea for the custom flow handler. We'd
reset
> > > the processor priority so we can take other interrupts (which the EOI
> > > usually does for you), then do the actual EOI after the handler
> > > finished.
> >
> > That only works when the card does not issue new interrupts until the
> > EOI happens. If the EOI is only relevant for the interrupt controller,
> > then you are going to lose any edge which comes in before the EOI as
> > well.
>
> Well, the real MSIs have an extra bit to allow the eoi to dally behind
> the mmio on another path and that should cover this race when the irq
> is left enabled.
>
> Jan-Bernd HEA has that change, right?
I don't now. We never hit problems so we did not look very deep into this
area.
We probably have to talk to the HEA HW developers to be sure.
Regards,
Jan-Bernd
^ permalink raw reply
* Re: [PATCH 2/3 v4] P4080/mtd: Only make elbc nand driver detect nand flash partitions
From: Anton Vorontsov @ 2010-09-20 13:19 UTC (permalink / raw)
To: Roy Zang
Cc: B07421, dedekind1, B25806, linuxppc-dev, linux-mtd, akpm, dwmw2,
B11780
In-Reply-To: <1284706869-12555-2-git-send-email-tie-fei.zang@freescale.com>
On Fri, Sep 17, 2010 at 03:01:08PM +0800, Roy Zang wrote:
[...]
> +static struct mutex fsl_elbc_nand_mutex;
> +
> +static int __devinit fsl_elbc_nand_probe(struct platform_device *dev)
> {
> - struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> + struct fsl_lbc_regs __iomem *lbc;
> struct fsl_elbc_mtd *priv;
> struct resource res;
> + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL;
No need for = NULL.
[...]
> - ctrl->chips[bank] = priv;
> + mutex_init(&fsl_elbc_nand_mutex);
This may cause all sorts of misbehaviours, e.g.
A: mutex_init(foo)
A: mutex_lock(foo)
B: mutex_init(foo) <- destroyed "A"-context mutex.
A: mutex_unlock(foo) <- oops
Instead of dynamically initializing the mutex, just define it
with DEFINE_MUTEX() above.
(Btw, #include <linux/mutex.h> is needed.)
> +
> + mutex_lock(&fsl_elbc_nand_mutex);
[...]
> -static int __devinit fsl_elbc_ctrl_init(struct fsl_elbc_ctrl *ctrl)
> +static int fsl_elbc_nand_remove(struct platform_device *dev)
[...]
> + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = fsl_lbc_ctrl_dev->nand;
[...]
> + if (elbc_fcm_ctrl->chips[i])
> + fsl_elbc_chip_remove(elbc_fcm_ctrl->chips[i]);
[...]
> + fsl_lbc_ctrl_dev->nand = NULL;
> + kfree(elbc_fcm_ctrl);
Will cause NULL dereference and/or use-after-free for other
elbc nand instances. To avoid that, reference counting for
elbc_fcm_ctrl is required.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: Initial kernel command string (Was: Generating elf kernel ?)
From: tiejun.chen @ 2010-09-20 8:10 UTC (permalink / raw)
To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <201009200921.09800.dargaud@lpsc.in2p3.fr>
Guillaume Dargaud wrote:
>> I think you should modify the bootargs on your dts.
>> ------
>> chosen {
>> bootargs = "console=ttyS0 root=/dev/ram";
>> linux,stdout-path = "/plb@0/serial@83e00000";
>> } ;
>
> Thanks, that worked great, I now have a fully bootable not only kernel, but
> full OS as well.
Sounds good :)
>
>>> Also my previous kernel would wait for 2 seconds at the beginning to
>>> allow me to change the initial command string via the serial port, but
>>> now it just runs right through. How can I enable this option ?
>> It's possible on PowerPC kernel :)
>>
>> You can take a look at the file, arch/powerpc/boot/main.c.
>> ------
>> static void prep_cmdline(void *chosen)
>> {
>> if (cmdline[0] == '\0')
>> getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1);
>>
>> printf("\n\rLinux/PowerPC load: %s", cmdline);
>> /* If possible, edit the command line */
>> if (console_ops.edit_cmdline)
>> console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
>> printf("\n\r");
>> .......
>>
>>
>> So you have to define one function, console_ops.edit_cmdline -->
>> serial_edit_cmdline. Or you can try bind this to your boot console ops
>> directly. Please refer to the file, arch/powerpc/boot/serial.c.
>
> So I forced
> console_ops.edit_cmdline = serial_edit_cmdline;
> in serial.c, bu that didn't do the trick...
Where are/how do your way implement? Can you paste your code sections here?
If I remember ML405 properly the serial port should be compliant to ns16550. So
I think the sub-functions, putc()/getc()/tstc(), should be from the file,
arch/powerpc/boot/ns16550.c. If so it's fine.
Maybe you can add some extra printf() into the function, serial_edit_cmdline, to
track this in detail.
Cheers
Tiejun
^ permalink raw reply
* Re: Initial kernel command string (Was: Generating elf kernel ?)
From: Guillaume Dargaud @ 2010-09-20 7:21 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <4C9338F3.2010104@windriver.com>
> I think you should modify the bootargs on your dts.
> ------
> chosen {
> bootargs = "console=ttyS0 root=/dev/ram";
> linux,stdout-path = "/plb@0/serial@83e00000";
> } ;
Thanks, that worked great, I now have a fully bootable not only kernel, but
full OS as well.
> > Also my previous kernel would wait for 2 seconds at the beginning to
> > allow me to change the initial command string via the serial port, but
> > now it just runs right through. How can I enable this option ?
>
> It's possible on PowerPC kernel :)
>
> You can take a look at the file, arch/powerpc/boot/main.c.
> ------
> static void prep_cmdline(void *chosen)
> {
> if (cmdline[0] == '\0')
> getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1);
>
> printf("\n\rLinux/PowerPC load: %s", cmdline);
> /* If possible, edit the command line */
> if (console_ops.edit_cmdline)
> console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
> printf("\n\r");
> .......
>
>
> So you have to define one function, console_ops.edit_cmdline -->
> serial_edit_cmdline. Or you can try bind this to your boot console ops
> directly. Please refer to the file, arch/powerpc/boot/serial.c.
So I forced
console_ops.edit_cmdline = serial_edit_cmdline;
in serial.c, bu that didn't do the trick...
--
Guillaume Dargaud
http://www.gdargaud.net/
^ permalink raw reply
* CPU 1 refused to die! (pmac dual G4 MDD)
From: Giuliano Pochini @ 2010-09-19 21:31 UTC (permalink / raw)
To: LinuxPPC-dev
When I do:
echo 0 >/sys/devices/system/cpu/cpu1/online
it waits for a few seconds and dmesg reports:
CPU1 offline
CPU 1 refused to die!
The cpu is not online anymore and top does not list it. I can be re-enabled.
Everything looks ok, except that error message. It worked fine in 2.6.33.
I don't know if it's a ppc specific bug.
$ uname -a
Linux Jay 2.6.35.4 #3 SMP Sat Sep 18 20:03:06 CEST 2010 ppc 7455, altivec supported PowerMac3,6 GNU/Linux
--
Giuliano.
^ permalink raw reply
* Re: [PATCH 1/2] PPC4xx: Generelizing drivers/dma/ppc4xx/adma.c
From: Ilya Yanok @ 2010-09-19 20:00 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1284774145-14543-1-git-send-email-tmarri@apm.com>
Hi Tirumala,
> drivers/dma/ppc4xx/adma.c | 4370 +++-----------------------------------
> drivers/dma/ppc4xx/adma.h | 116 +-
> drivers/dma/ppc4xx/ppc4xx-adma.h | 4020 +++++++++++++++++++++++++++++++++++
You've moved tons of code to the header file. Why?
Regards, Ilya.
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it
From: Benjamin Herrenschmidt @ 2010-09-19 2:42 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Tabi Timur-B04825, linux-watchdog
In-Reply-To: <70810686-1EB6-4AD9-A89B-C2A8BA6AC30D@freescale.com>
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.
^ permalink raw reply
* Re: Generating elf kernel ?
From: tiejun.chen @ 2010-09-19 1:40 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Guillaume Dargaud
In-Reply-To: <20100917124448.255b08cc@schlenkerla.am.freescale.net>
Scott Wood wrote:
> On Fri, 17 Sep 2010 09:58:41 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>
>> Scott Wood wrote:
>>> The guest OS *is* the same as native Linux, as far as TLB handling is
>>> concerned.
>> Looks you means the TLB exception handler should be same between the native and
>> the guest OS. Right?
>
> Yes.
I don't think so. The HY should assist the guest OS on MMU since I already point
the guest OS have no authority to create a real TLB directly as I previously said.
>
>> Here I assume we're talking about e500mc since as far as I know for Freescale
>> only e500mc is designed to support virtual machine based on ISA 2.0.6.
>
> Yes, though there's nothing preventing virtualization on cores without
> category E.HV (KVM supports this) -- it's just slower.
Absolutely.
>
>> I also know all TLB exceptions can direct to the guest OS when we enable
>> EPCR[DTLBGS|ITLBGS|DSIGS|ISIGS]. But some TLB instructions (i.e. tlbwe )are the
>> privileged instructions. So the guest OS always trap into the hypervisor and
>> then the hypervisor should complete the real action with appropriate physical
>> address.
>
> Yes, of course. But that's not the point. I was just using it as a
> convenient example because that's what I've recently done ELF loading
> with... There's no reason U-Boot couldn't do the same if its ELF
> loader were updated to support device trees. Currently U-Boot loads
> bootwrapperless uImages to physical address zero.
I never doubt the U-boot can do this for uImage. But I think we're always
talking about vmlinux, a bare Image.
Here you already assume so many conditions for vmlinux before we were
discussing. Such as bootwrapperlee uImage, its ELF loader can update/support
dtb, the HY... I think this is just why I say we cannot boot vmlinux based on
common boot loader if only change entry point of vmlinux.
>
> And FWIW, we have run setups where our hv loads Linux to true
> physical zero (with the hv living elsewhere), not just guest physical.
That's true. The HY should be allowed to access any address.
Best Regards
Tiejun
>
> -Scott
>
>
^ permalink raw reply
* Re: [PATCH 2/2] PPC4xx: Merge xor.h and dma.h into onefile ppc440spe-dma.h
From: Wolfgang Denk @ 2010-09-18 21:12 UTC (permalink / raw)
To: tmarri
Cc: herbert, neilb, yur, linux-raid, linux-crypto, dan.j.williams,
linuxppc-dev
In-Reply-To: <1284774162-14652-1-git-send-email-tmarri@apm.com>
Dear tmarri@apm.com,
In message <1284774162-14652-1-git-send-email-tmarri@apm.com> you wrote:
> From: Tirumala Marri <tmarri@apm.com>
>
> This patch combines drivers/dma/ppc4xx/xor.h and driver/dma/dma/ppc4xx/dma.h
> into drivers/dma/ppc4xx/ppx440spe-dma.h .
>
> Signed-off-by: Tirumala R Marri <tmarri@apm.com>
> ---
> drivers/dma/ppc4xx/dma.h | 223 -------------------------
> drivers/dma/ppc4xx/ppc440spe-dma.h | 318 ++++++++++++++++++++++++++++++++++++
> drivers/dma/ppc4xx/xor.h | 110 -------------
> 3 files changed, 318 insertions(+), 333 deletions(-)
Please use -M with "git format-patch" so it detects renames (here we
should probably see a rename from dma.h into ppc440spe-dma.h [plus
some changes]) instead of a remove plus add file.
That would make it much easier to review your changes.
Thanks.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
It's a small world, but I wouldn't want to paint it.
^ permalink raw reply
* Re: [PATCH 1/2] PPC4xx: Generelizing drivers/dma/ppc4xx/adma.c
From: Wolfgang Denk @ 2010-09-18 21:09 UTC (permalink / raw)
To: tmarri
Cc: neilb, yur, linux-raid, herbert, linux-crypto, dan.j.williams,
linuxppc-dev
In-Reply-To: <1284774145-14543-1-git-send-email-tmarri@apm.com>
Dear tmarri@apm.com,
In message <1284774145-14543-1-git-send-email-tmarri@apm.com> you wrote:
>
> This patch generalizes the existing drver/dma/ppc4xx/adma.c, so that
> common code can be shared between different similar DMA engine
> drivers in other SoCs.
...
> * This driver supports the asynchrounous DMA copy and RAID engines available
> - * on the AMCC PPC440SPe Processors.
> + * on the AMCC PPC4XX Processors.
Will this driver ever include any 40x processors? If not, you probably
should use "44x" instead (here and everywhere in the rest of the
code).
> diff --git a/drivers/dma/ppc4xx/ppc4xx-adma.h b/drivers/dma/ppc4xx/ppc4xx-adma.h
> new file mode 100644
> index 0000000..7457237
> --- /dev/null
> +++ b/drivers/dma/ppc4xx/ppc4xx-adma.h
...
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <asm/dcr.h>
> +#include <asm/dcr-regs.h>
> +#include "adma.h"
> +#include "ppc440spe-dma.h"
> +
> +/* Default polynomial (for 440SP is only available) */
> +#define PPC4XX_DEFAULT_POLY 0x4d
Should this go into "ppc440spe-dma.h"?
> +/* The list of channels exported by ppc440spe ADMA */
> +struct list_head
> + ppc4xx_adma_chan_list = LIST_HEAD_INIT(ppc4xx_adma_chan_list);
> +
> +/* This flag is set when want to refetch the xor chain in the interrupt
> + * handler
> + */
> +static u32 do_xor_refetch;
> +
> +/* Pointer to DMA0, DMA1 CP/CS FIFO */
> +static void *ppc440spe_dma_fifo_buf;
Seems this should go into "ppc440spe-dma.h"?
> +/* This array is used in data-check operations for storing a pattern */
> +static char ppc440spe_qword[16];
> +
> +static atomic_t ppc4xx_adma_err_irq_ref;
> +static dcr_host_t ppc440spe_mq_dcr_host;
> +static unsigned int ppc440spe_mq_dcr_len;
Ditto?
> +static unsigned long ppc440spe_rxor_state;
> +
> +static struct page *ppc440spe_rxor_srcs[32];
And here again - please check globally!
> +/**
> + * ppc440spe_can_rxor - check if the operands may be processed with RXOR
> + */
> +static int ppc440spe_can_rxor(struct page **srcs, int src_cnt, size_t len)
Again, should this then not be in ppc440spe specific files?
It seems the split / generalization is highly incomplete yet.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
What can it profit a man to gain the whole world and to come to his
property with a gastric ulcer, a blown prostate, and bifocals?
-- John Steinbeck, _Cannery Row_
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it
From: Kumar Gala @ 2010-09-18 18:36 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Tabi Timur-B04825, linux-watchdog
In-Reply-To: <AANLkTi=sa1LJOnw4fK-fX23Egxm7NoJVNKB1vtc9Eg4q@mail.gmail.com>
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=
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it
From: Josh Boyer @ 2010-09-18 18:22 UTC (permalink / raw)
To: Tabi Timur-B04825; +Cc: linuxppc-dev, Gala Kumar-B11780, linux-watchdog
In-Reply-To: <5610599F537DD74A8D1F5CC946A75073034792F9@az33exm25.fsl.freescale.net>
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
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: export ppc_tb_freq so that modules can reference it
From: Tabi Timur-B04825 @ 2010-09-18 17:55 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Gala Kumar-B11780, linux-watchdog
In-Reply-To: <AANLkTimSewEEhbBoBb+fpPgY9EbE5bWsknPS7OkQgkcy@mail.gmail.com>
[-- 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 --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox