Re: [PATCH][RFC] preempt_count corruption across H_CEDE call with CONFIG_PREEMPT on pseries
On 08/19/2010 08:58 AM, Ankita Garg wrote: > Hi Darren, > > On Thu, Jul 22, 2010 at 11:24:13AM -0700, Darren Hart wrote: >> >> With some instrumentation we were able to determine that the >> preempt_count() appears to change across the extended_cede_processor() >> call. Specifically across the plpar_hcall_norets(H_CEDE) call. On >> PREEMPT_RT we call this with preempt_count=1 and return with >> preempt_count=0x. On mainline with CONFIG_PREEMPT=y, the value >> is different (0x65) but is still incorrect. > > I was trying to reproduce this issue on a 2.6.33.7-rt29 kernel. I could > easily reproduce this on the RT kernel and not the non-RT kernel. This matches my experience. > However, I hit it every single time I did a cpu online operation. I also > noticed that the issue persists even when I disable H_CEDE by passing > the "cede_offline=0" kernel commandline parameter. Could you pl confirm > if you observe the same in your setup ? Yes, this is my experience as well. > > However, the issue still remains. Will spend few cycles looking into > this issue. > I've spent today trying to collect some useful traces. I consistently see the preempt_count() change to 0x across the H_CEDE call, but the irq and sched events (to ftrace) do not indicate anything else running on the CPU that could change that. -0 1d.h1. 11408us : irq_handler_entry: irq=16 name=IPI -0 1d.h1. 11411us : irq_handler_exit: irq=16 ret=handled ... -0 1d 22101us : .pseries_mach_cpu_die: start -0 1d 22108us : .pseries_mach_cpu_die: cpu 1 (hwid 1) ceding for offline with hint 2 ... -0 1d.Hff. 33804us : .pseries_mach_cpu_die: returned from cede with pcnt: -0 1d.Hff. 33805us : .pseries_mach_cpu_die: forcing pcnt to 0 -0 1d 33807us : .pseries_mach_cpu_die: cpu 1 (hwid 1) returned from cede. -0 1d 33808us : .pseries_mach_cpu_die: Decrementer value = 7fa49474 Timebase value = baefee6c88113 -0 1d 33809us : .pseries_mach_cpu_die: cpu 1 (hwid 1) got prodded to go online -0 1d 33816us : .pseries_mach_cpu_die: calling start_seconday, pcnt: 0 -0 1d 33816us : .pseries_mach_cpu_die: forcing pcnt to 0 - Modules linked in: autofs4 binfmt_misc dm_mirror dm_region_hash dm_log [last unloaded: scsi_wait_scan] NIP: c006aa50 LR: c006ac40 CTR: c006ac14 REGS: c0008e79ffc0 TRAP: 0300 Not tainted (2.6.33-rt-dvhrt16-02358-g61223dd-dirty) MSR: 80001032 CR: 2822 XER: DAR: c18000ba44c0, DSISR: 4000 TASK = c0010e6de040[0] 'swapper' THREAD: c0008e7a CPU: 1 GPR00: 01800040 c0008e7a0240 c0b55008 c0010e6de040 GPR04: c00085d800c0 000f GPR08: 0180 c0008e7a c0ba4480 c0a32c80 GPR12: 80009032 c0ba4680 c0008e7a08f0 0001 GPR16: f2fa c0010e6de040 7fff GPR20: 0001 0001 c0f42c80 GPR24: c000850b7638 0001 GPR28: c0f42c80 c0010e6de040 c0ad7698 c0008e7a0240 NIP [c006aa50] .resched_task+0x48/0xd8 LR [c006ac40] .check_preempt_curr_idle+0x2c/0x44 Call Trace: Instruction dump: f821ff71 7c3f0b78 ebc2ab28 7c7d1b78 6000 6000 e95e8008 e97e8000 e93d0008 81090010 79084da4 38080040 <7d4a002a> 7c0b502e 7c74 7800d182 ---[ end trace 6d3f1cddaa17382c ]--- Kernel panic - not syncing: Attempted to kill the idle task! Call Trace: Rebooting in 180 seconds.. When running with the function plugin I had to stop the trace immediately before entering start_secondary after an online or my traces would not include the pseries_mach_cpu_die function, nor the tracing I added there (possibly buffer size, I am using 2048). The following trace was collected using "trace-cmd record -p function -e irq -e sched" and has been filtered to only show CPU [001] (the CPU undergoing the offline/online test, and the one seeing preempt_count (pcnt) go to after cede. The function tracer does not indicate anything running on the CPU other than the HCALL - unless the __trace_hcall* commands might be to blame. Can a POWER guru comment? -0 [001] 417.625286: function: .cpu_die -0 [001] 417.625287: function: .pseries_mach_cpu_die -0 [001] 417.625290: bprint: .pseries_mach_cpu_die : start -0 [001] 417.625291: function: .idle_task_exit -0 [001] 417.625292: function: .switch_slb -0 [001] 417.625294: function: .xics_teardown_cpu -0 [001] 417.625294: function: .xics_set_cpu_prio
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote: > With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, > so the following pops up on PowerPC: > > cc1: warnings being treated as errors > In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: > include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared > inside parameter list > include/linux/of_gpio.h:74: warning: its scope is only this definition > or declaration, which is probably not what > you want > include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared > inside parameter list > make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 > > This patch fixes the issue by providing the proper forward declaration. > > Signed-off-by: Anton Vorontsov This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig. g. > --- > > On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote: > > I get that with my current stuff: > > > > cc1: warnings being treated as errors > > In file included from [..]/mpc52xx_common.c:19: > > of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list > [...] > > make[3]: *** Waiting for unfinished jobs > > That's because with GPIOCHIP=n no one declares struct gpio_chip. > > It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as > this feels more generic, and we already have some !GPIOLIB handling > in there. > > include/linux/gpio.h |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/include/linux/gpio.h b/include/linux/gpio.h > index 03f616b..85207d2 100644 > --- a/include/linux/gpio.h > +++ b/include/linux/gpio.h > @@ -15,6 +15,12 @@ > struct device; > > /* > + * Some code might rely on the declaration. Still, it is illegal > + * to dereference it for !GPIOLIB case. > + */ > +struct gpio_chip; > + > +/* > * Some platforms don't support the GPIO programming interface. > * > * In case some driver uses it anyway (it should normally have > -- > 1.7.0.5 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 31, 2010 at 02:03:44AM -0600, Grant Likely wrote: > On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote: > > With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, > > so the following pops up on PowerPC: > > > > cc1: warnings being treated as errors > > In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: > > include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared > > inside parameter list > > include/linux/of_gpio.h:74: warning: its scope is only this definition > > or declaration, which is probably not what > > you want > > include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared > > inside parameter list > > make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 > > > > This patch fixes the issue by providing the proper forward declaration. > > > > Signed-off-by: Anton Vorontsov > > This doesn't actually solve the problem, and gpiochip should > remain undefined when CONFIG_GPIOLIB=n to catch exactly these > build failures. The real problem is that I merged a change > into the mpc5200 code that required CONFIG_GPIOLIB be enabled > without reflecting that requirement in Kconfig. No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y. > > --- > > > > On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote: > > > I get that with my current stuff: > > > > > > cc1: warnings being treated as errors > > > In file included from [..]/mpc52xx_common.c:19: > > > of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list > > [...] > > > make[3]: *** Waiting for unfinished jobs > > > > That's because with GPIOCHIP=n no one declares struct gpio_chip. > > > > It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as > > this feels more generic, and we already have some !GPIOLIB handling > > in there. > > > > include/linux/gpio.h |6 ++ > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > diff --git a/include/linux/gpio.h b/include/linux/gpio.h > > index 03f616b..85207d2 100644 > > --- a/include/linux/gpio.h > > +++ b/include/linux/gpio.h > > @@ -15,6 +15,12 @@ > > struct device; > > > > /* > > + * Some code might rely on the declaration. Still, it is illegal > > + * to dereference it for !GPIOLIB case. > > + */ > > +struct gpio_chip; > > + > > +/* > > * Some platforms don't support the GPIO programming interface. > > * > > * In case some driver uses it anyway (it should normally have > > -- > > 1.7.0.5 > > -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/512x: fix clk_get() return value
clk_get() should return an ERR_PTR value on error, not NULL.
Signed-off-by: Akinobu Mita
Cc: Grant Likely
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/platforms/512x/clock.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/512x/clock.c
b/arch/powerpc/platforms/512x/clock.c
index 5b243bd..3dc2a8d 100644
--- a/arch/powerpc/platforms/512x/clock.c
+++ b/arch/powerpc/platforms/512x/clock.c
@@ -57,7 +57,7 @@ static struct clk *mpc5121_clk_get(struct device *dev, const
char *id)
int id_match = 0;
if (dev == NULL || id == NULL)
- return NULL;
+ return clk;
mutex_lock(&clocks_mutex);
list_for_each_entry(p, &clocks, node) {
--
1.7.1.231.gd0b16
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2 v2] powerpc, pseries: Re-enable dispatch trace log userspace interface
Since the cpu accounting code uses the hypervisor dispatch trace log
now when CONFIG_VIRT_CPU_ACCOUNTING = y, the previous commit disabled
access to it via files in the /sys/kernel/debug/powerpc/dtl/ directory
in that case. This restores those files.
To do this, we now have a hook that the cpu accounting code will call
as it processes each entry from the hypervisor dispatch trace log.
The code in dtl.c now uses that to fill up its ring buffer, rather
than having the hypervisor fill the ring buffer directly.
This also fixes dtl_file_read() to handle overflow conditions a bit
better and adds a spinlock to ensure that race conditions (multiple
processes opening or reading the file concurrently) are handled
correctly.
Signed-off-by: Paul Mackerras
---
Removed extraneous increment of dtl->last_idx in this version.
Patch 1/1 of this series is unchanged.
arch/powerpc/include/asm/lppaca.h|8 ++
arch/powerpc/kernel/time.c |6 +-
arch/powerpc/platforms/pseries/dtl.c | 207 +++---
3 files changed, 180 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/lppaca.h
b/arch/powerpc/include/asm/lppaca.h
index cfb85ec..7f5e0fe 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -191,6 +191,14 @@ struct dtl_entry {
#define DISPATCH_LOG_BYTES 4096/* bytes per cpu */
#define N_DISPATCH_LOG (DISPATCH_LOG_BYTES / sizeof(struct dtl_entry))
+/*
+ * When CONFIG_VIRT_CPU_ACCOUNTING = y, the cpu accounting code controls
+ * reading from the dispatch trace log. If other code wants to consume
+ * DTL entries, it can set this pointer to a function that will get
+ * called once for each DTL entry that gets processed.
+ */
+extern void (*dtl_consumer)(struct dtl_entry *entry, u64 index);
+
#endif /* CONFIG_PPC_BOOK3S */
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_LPPACA_H */
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index fca2064..bcb738b 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -183,6 +183,8 @@ DEFINE_PER_CPU(unsigned long, cputime_scaled_last_delta);
cputime_t cputime_one_jiffy;
+void (*dtl_consumer)(struct dtl_entry *, u64);
+
static void calc_cputime_factors(void)
{
struct div_result res;
@@ -218,7 +220,7 @@ static u64 read_spurr(u64 tb)
*/
static u64 scan_dispatch_log(u64 stop_tb)
{
- unsigned long i = local_paca->dtl_ridx;
+ u64 i = local_paca->dtl_ridx;
struct dtl_entry *dtl = local_paca->dtl_curr;
struct dtl_entry *dtl_end = local_paca->dispatch_log_end;
struct lppaca *vpa = local_paca->lppaca_ptr;
@@ -229,6 +231,8 @@ static u64 scan_dispatch_log(u64 stop_tb)
if (i == vpa->dtl_idx)
return 0;
while (i < vpa->dtl_idx) {
+ if (dtl_consumer)
+ dtl_consumer(dtl, i);
dtb = dtl->timebase;
tb_delta = dtl->enqueue_to_dispatch_time +
dtl->ready_to_enqueue_time;
diff --git a/arch/powerpc/platforms/pseries/dtl.c
b/arch/powerpc/platforms/pseries/dtl.c
index 0357655..68cb2f2 100644
--- a/arch/powerpc/platforms/pseries/dtl.c
+++ b/arch/powerpc/platforms/pseries/dtl.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -37,6 +38,7 @@ struct dtl {
int cpu;
int buf_entries;
u64 last_idx;
+ spinlock_t lock;
};
static DEFINE_PER_CPU(struct dtl, cpu_dtl);
@@ -55,25 +57,97 @@ static u8 dtl_event_mask = 0x7;
static int dtl_buf_entries = (16 * 85);
-static int dtl_enable(struct dtl *dtl)
+#ifdef CONFIG_VIRT_CPU_ACCOUNTING
+struct dtl_ring {
+ u64 write_index;
+ struct dtl_entry *write_ptr;
+ struct dtl_entry *buf;
+ struct dtl_entry *buf_end;
+ u8 saved_dtl_mask;
+};
+
+static DEFINE_PER_CPU(struct dtl_ring, dtl_rings);
+
+static atomic_t dtl_count;
+
+/*
+ * The cpu accounting code controls the DTL ring buffer, and we get
+ * given entries as they are processed.
+ */
+static void consume_dtle(struct dtl_entry *dtle, u64 index)
{
- unsigned long addr;
- int ret, hwcpu;
+ struct dtl_ring *dtlr = &__get_cpu_var(dtl_rings);
+ struct dtl_entry *wp = dtlr->write_ptr;
+ struct lppaca *vpa = local_paca->lppaca_ptr;
- /* only allow one reader */
- if (dtl->buf)
- return -EBUSY;
+ if (!wp)
+ return;
- /* we need to store the original allocation size for use during read */
- dtl->buf_entries = dtl_buf_entries;
+ *wp = *dtle;
+ barrier();
- dtl->buf = kmalloc_node(dtl->buf_entries * sizeof(struct dtl_entry),
- GFP_KERNEL, cpu_to_node(dtl->cpu));
- if (!dtl->buf) {
- printk(KERN_WARNING "%s: buffer alloc failed for cpu %d\n",
-
SPI driver for MPC837xERDB
Hi all,
I am new to linux device driver development. I have to develop a SPI driver
for MPC8377 processor based board. Right now I don't have a that board
ready(but I have the MPC837xERDB with me), so I was thinking of developing
and testing it on the RDB. But in MPC837xERDB, I don't have an SPI slave
device, the SPI pins on the board is connected to the SD card slot. Is it
possible to use SD card slot as SPI device? Also I have written a sample SPI
driver, but its probe function is not getting called?
#include
#include
static int __devinit spi_probe(struct spi_device *spi)
{
printk(KERN_DEBUG "spi_probe...");
return 0;
}
static int __devexit spi_remove(struct spi_device *spi)
{
printk(KERN_DEBUG "spi_remove\n");
return 0;
}
static struct spi_driver sample_spi_driver = {
.driver = {
.name = "sample-spi",
.bus = &spi_bus_type,
.owner = THIS_MODULE,
},
.probe = spi_probe,
.remove = spi_remove,
};
static __init int spi_module_init(void)
{
printk(KERN_INFO "SPI Driver Version 0.1\n");
return spi_register_driver(&sample_spi_driver);
}
static __exit void spi_module_exit(void)
{
/* unregister SPI device */
spi_unregister_driver(&sample_spi_driver);
}
MODULE_LICENSE("GPL");
module_init(spi_module_init);
module_exit(spi_module_exit);
Any help would be greatly appreciated.
Thanks in advance,
Ravi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ucc_geth: fix ethtool set ring param bug
On Mon, 2010-08-30 at 22:47 +0800, Liang Li wrote:
> It's common sense that when we should do change to driver ring
> desc/buffer etc only after 'stop/shutdown' the device. When we
> do change while devices/driver is running, kernel oops occur:
[...]
> diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
> index 6f92e48..1b37aaa 100644
> --- a/drivers/net/ucc_geth_ethtool.c
> +++ b/drivers/net/ucc_geth_ethtool.c
> @@ -255,13 +255,18 @@ uec_set_ringparam(struct net_device *netdev,
> return -EINVAL;
> }
>
> - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> -
> if (netif_running(netdev)) {
> - /* FIXME: restart automatically */
> - printk(KERN_INFO
> - "Please re-open the interface.\n");
> + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> + ucc_geth_close(netdev);
> +
> + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> +
> + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> + ucc_geth_open(netdev);
What if ucc_geth_open() fails?
Ben.
> + } else {
> + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> }
>
> return ret;
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ucc_geth: fix ethtool set ring param bug
On Tue, Aug 31, 2010 at 03:41:22PM +0100, Ben Hutchings wrote:
> On Mon, 2010-08-30 at 22:47 +0800, Liang Li wrote:
> > It's common sense that when we should do change to driver ring
> > desc/buffer etc only after 'stop/shutdown' the device. When we
> > do change while devices/driver is running, kernel oops occur:
> [...]
> > diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
> > index 6f92e48..1b37aaa 100644
> > --- a/drivers/net/ucc_geth_ethtool.c
> > +++ b/drivers/net/ucc_geth_ethtool.c
> > @@ -255,13 +255,18 @@ uec_set_ringparam(struct net_device *netdev,
> > return -EINVAL;
> > }
> >
> > - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > -
> > if (netif_running(netdev)) {
> > - /* FIXME: restart automatically */
> > - printk(KERN_INFO
> > - "Please re-open the interface.\n");
> > + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> > + ucc_geth_close(netdev);
> > +
> > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > +
> > + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> > + ucc_geth_open(netdev);
>
> What if ucc_geth_open() fails?
I did some runtime tests but did not witness the ucc_geth_open fail.
Assume it may fail for some reason, then I tend to think give out
warnings for request user to open/enable it mannually? Or we may need
to keep the 'FIXME' line?
Thanks,
-Liang Li
>
> Ben.
>
> > + } else {
> > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > }
> >
> > return ret;
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare Communications
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ucc_geth: fix ethtool set ring param bug
On Tue, 2010-08-31 at 23:16 +0800, Liang Li wrote:
> On Tue, Aug 31, 2010 at 03:41:22PM +0100, Ben Hutchings wrote:
> > On Mon, 2010-08-30 at 22:47 +0800, Liang Li wrote:
> > > It's common sense that when we should do change to driver ring
> > > desc/buffer etc only after 'stop/shutdown' the device. When we
> > > do change while devices/driver is running, kernel oops occur:
> > [...]
> > > diff --git a/drivers/net/ucc_geth_ethtool.c
> > > b/drivers/net/ucc_geth_ethtool.c
> > > index 6f92e48..1b37aaa 100644
> > > --- a/drivers/net/ucc_geth_ethtool.c
> > > +++ b/drivers/net/ucc_geth_ethtool.c
> > > @@ -255,13 +255,18 @@ uec_set_ringparam(struct net_device *netdev,
> > > return -EINVAL;
> > > }
> > >
> > > - ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > - ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > -
> > > if (netif_running(netdev)) {
> > > - /* FIXME: restart automatically */
> > > - printk(KERN_INFO
> > > - "Please re-open the interface.\n");
> > > + printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
> > > + ucc_geth_close(netdev);
> > > +
> > > + ug_info->bdRingLenRx[queue] = ring->rx_pending;
> > > + ug_info->bdRingLenTx[queue] = ring->tx_pending;
> > > +
> > > + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> > > + ucc_geth_open(netdev);
> >
> > What if ucc_geth_open() fails?
>
> I did some runtime tests but did not witness the ucc_geth_open fail.
> Assume it may fail for some reason, then I tend to think give out
> warnings for request user to open/enable it mannually? Or we may need
> to keep the 'FIXME' line?
The easy way out is to allow changing the ring size only while the
interface is down. The hard way is to make the change in such a way
that you can roll back in case of failure.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/4] drivers/serial/ucc_uart.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_compatible_node or of_find_node_by_type.
This patch also substantially reorganizes the error handling code in the
function, to that it is possible first to jump to code that frees qe_port
and then to jump to code that also puts np.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
//
Signed-off-by: Julia Lawall
---
drivers/serial/ucc_uart.c | 67 --
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/serial/ucc_uart.c b/drivers/serial/ucc_uart.c
index 3f4848e..38a5ef0 100644
--- a/drivers/serial/ucc_uart.c
+++ b/drivers/serial/ucc_uart.c
@@ -1270,13 +1270,12 @@ static int ucc_uart_probe(struct platform_device *ofdev,
ret = of_address_to_resource(np, 0, &res);
if (ret) {
dev_err(&ofdev->dev, "missing 'reg' property in device tree\n");
- kfree(qe_port);
- return ret;
+ goto out_free;
}
if (!res.start) {
dev_err(&ofdev->dev, "invalid 'reg' property in device tree\n");
- kfree(qe_port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
qe_port->port.mapbase = res.start;
@@ -1286,17 +1285,17 @@ static int ucc_uart_probe(struct platform_device *ofdev,
if (!iprop) {
iprop = of_get_property(np, "device-id", NULL);
if (!iprop) {
- kfree(qe_port);
dev_err(&ofdev->dev, "UCC is unspecified in "
"device tree\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
}
if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->ucc_num = *iprop - 1;
@@ -1310,16 +1309,16 @@ static int ucc_uart_probe(struct platform_device *ofdev,
sprop = of_get_property(np, "rx-clock-name", NULL);
if (!sprop) {
dev_err(&ofdev->dev, "missing rx-clock-name in device tree\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->us_info.rx_clock = qe_clock_source(sprop);
if ((qe_port->us_info.rx_clock < QE_BRG1) ||
(qe_port->us_info.rx_clock > QE_BRG16)) {
dev_err(&ofdev->dev, "rx-clock-name must be a BRG for UART\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
#ifdef LOOPBACK
@@ -1329,39 +1328,39 @@ static int ucc_uart_probe(struct platform_device *ofdev,
sprop = of_get_property(np, "tx-clock-name", NULL);
if (!sprop) {
dev_err(&ofdev->dev, "missing tx-clock-name in device tree\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
qe_port->us_info.tx_clock = qe_clock_source(sprop);
#endif
if ((qe_port->us_info.tx_clock < QE_BRG1) ||
(qe_port->us_info.tx_clock > QE_BRG16)) {
dev_err(&ofdev->dev, "tx-clock-name must be a BRG for UART\n");
- kfree(qe_port);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_free;
}
/* Get the port number, numbered 0-3 */
iprop = of_get_property(np, "port-number", NULL);
if (!iprop) {
dev_err(&ofdev->dev, "missing port-number in device tree\n");
- kfree(qe_port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
qe_port->port.line = *iprop;
if (qe_port->port.line >= UCC_MAX_UART) {
dev_err(&ofdev->dev, "port-number must be 0-%u\n",
UCC_MAX_UART - 1);
- kfree(qe_port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
qe_port->port.irq = irq_of_parse_and_map
[PATCH 4/4] arch/powerpc/platforms/chrp/nvram.c: Add of_node_put to avoid memory leak
Add a call to of_node_put in the error handling code following a call to
of_find_node_by_type.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@r exists@
local idexpression x;
expression E,E1,E2;
statement S;
@@
*x =
(of_find_node_by_path
|of_find_node_by_name
|of_find_node_by_phandle
|of_get_parent
|of_get_next_parent
|of_get_next_child
|of_find_compatible_node
|of_match_node
|of_find_node_by_type
|of_find_node_with_property
|of_find_matching_node
|of_parse_phandle
)(...);
...
if (x == NULL) S
<... when != x = E
*if (...) {
... when != of_node_put(x)
when != if (...) { ... of_node_put(x); ... }
(
return <+...x...+>;
|
* return ...;
)
}
...>
(
E2 = x;
|
of_node_put(x);
)
//
Signed-off-by: Julia Lawall
---
arch/powerpc/platforms/chrp/nvram.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/chrp/nvram.c
b/arch/powerpc/platforms/chrp/nvram.c
index ba3588f..d3ceff0 100644
--- a/arch/powerpc/platforms/chrp/nvram.c
+++ b/arch/powerpc/platforms/chrp/nvram.c
@@ -74,8 +74,10 @@ void __init chrp_nvram_init(void)
return;
nbytes_p = of_get_property(nvram, "#bytes", &proplen);
- if (nbytes_p == NULL || proplen != sizeof(unsigned int))
+ if (nbytes_p == NULL || proplen != sizeof(unsigned int)) {
+ of_node_put(nvram);
return;
+ }
nvram_size = *nbytes_p;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
On Tue, 31 Aug 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > Add a call to of_node_put in the error handling code following a call to
> > of_find_node_by_path.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > //
> > @r exists@
> > local idexpression x;
> > expression E,E1;
> > statement S;
> > @@
> >
> > *x =
> > (of_find_node_by_path
> > |of_find_node_by_name
> > |of_find_node_by_phandle
> > |of_get_parent
> > |of_get_next_parent
> > |of_get_next_child
> > |of_find_compatible_node
> > |of_match_node
> > )(...);
> > ...
> > if (x == NULL) S
> > <... when != x = E
> > *if (...) {
> > ... when != of_node_put(x)
> > when != if (...) { ... of_node_put(x); ... }
> > (
> > return <+...x...+>;
> > |
> > * return ...;
> > )
> > }
> > ...>
> > of_node_put(x);
> > //
> >
> > Signed-off-by: Julia Lawall
> >
> > ---
> > drivers/macintosh/via-pmu-led.c |4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/macintosh/via-pmu-led.c
> > b/drivers/macintosh/via-pmu-led.c
> > index d242976..19c3718 100644
> > --- a/drivers/macintosh/via-pmu-led.c
> > +++ b/drivers/macintosh/via-pmu-led.c
> > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > if (dt == NULL)
> > return -ENODEV;
> > model = of_get_property(dt, "model", NULL);
> > - if (model == NULL)
> > + if (model == NULL) {
> > + of_node_put(dt);
> > return -ENODEV;
> > + }
> > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > strcmp(model, "PowerMac7,2") != 0 &&
> >
>
> is there any rule that says when to use strncmp ? it seems perfecly valid to
> use strcpy here
> (what is done in the last cmp).
Perhaps there are some characters after eg PowerBook that one doesn't want
to compare with?
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
On Tue, Aug 31, 2010 at 06:08:19PM +0200, Julia Lawall wrote:
> On Tue, 31 Aug 2010, walter harms wrote:
> > > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > > if (dt == NULL)
> > > return -ENODEV;
> > > model = of_get_property(dt, "model", NULL);
> > > - if (model == NULL)
> > > + if (model == NULL) {
> > > + of_node_put(dt);
> > > return -ENODEV;
> > > + }
> > > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > > strcmp(model, "PowerMac7,2") != 0 &&
> > >
> >
> > is there any rule that says when to use strncmp ? it seems perfecly valid
> > to use strcpy here
> > (what is done in the last cmp).
>
> Perhaps there are some characters after eg PowerBook that one doesn't want
> to compare with?
Yes. The model property on powermacs always has the version number. strncmp
makes sure that *all* PowerBooks and iBooks are matched.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
On Tue, Aug 31, 2010 at 18:08 +0200, Julia Lawall wrote:
> On Tue, 31 Aug 2010, walter harms wrote:
>
> >
> >
> > Julia Lawall schrieb:
> > > Add a call to of_node_put in the error handling code following a call to
> > > of_find_node_by_path.
[...]
> > > --- a/drivers/macintosh/via-pmu-led.c
> > > +++ b/drivers/macintosh/via-pmu-led.c
> > > @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> > > if (dt == NULL)
> > > return -ENODEV;
> > > model = of_get_property(dt, "model", NULL);
> > > - if (model == NULL)
> > > + if (model == NULL) {
> > > + of_node_put(dt);
> > > return -ENODEV;
> > > + }
> > > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> > > strncmp(model, "iBook", strlen("iBook")) != 0 &&
> > > strcmp(model, "PowerMac7,2") != 0 &&
> > >
> >
> > is there any rule that says when to use strncmp ? it seems perfecly valid
> > to use strcpy here
> > (what is done in the last cmp).
>
> Perhaps there are some characters after eg PowerBook that one doesn't want
> to compare with?
It seems to me that model has no '\0' in the end. If model is got from
the hardware then we should double check it - maybe harware is buggy.
Otherwise we'll overflow model.
But why strcmp(model, "PowerMac7,2")? IMO it should be replaced
with strncmp().
--
Vasiliy
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: mtmsrd not defined
On Aug 22, 2010, at 5:48 PM, Sean MacLennan wrote: > On Mon, 23 Aug 2010 08:34:54 +1000 > Benjamin Herrenschmidt wrote: > >> I'd rather have a macro somewhere in ppc_asm.h (MTMSR ?) that does the >> right thing. We might even already have one... > > We do here is a new, improved patch. > > Replace the BOOK3S_64 specific mtmsrd with the generic MTMSRD macro. > > Signed-off-by: Sean MacLennan > --- Can we add proper CONFIG_PPC_FPU into this file. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
On Tue, Aug 31, 2010 at 10:16 AM, Vasiliy Kulikov wrote:
> On Tue, Aug 31, 2010 at 18:08 +0200, Julia Lawall wrote:
>> On Tue, 31 Aug 2010, walter harms wrote:
>> > > if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
>> > > strncmp(model, "iBook", strlen("iBook")) != 0 &&
>> > > strcmp(model, "PowerMac7,2") != 0 &&
>> > >
>> >
>> > is there any rule that says when to use strncmp ? it seems perfecly valid
>> > to use strcpy here
>> > (what is done in the last cmp).
>>
>> Perhaps there are some characters after eg PowerBook that one doesn't want
>> to compare with?
>
> It seems to me that model has no '\0' in the end. If model is got from
> the hardware then we should double check it - maybe harware is buggy.
> Otherwise we'll overflow model.
Model does have \0 at the end. This code is using strncmp to
purposefully ignore the model suffix.
> But why strcmp(model, "PowerMac7,2")? IMO it should be replaced
> with strncmp().
We use strcmp when parsing the device tree because the the length of
the model property string is unknown and in most cases we *must* match
the exact entire string, such as with this PowerMac7,2 example. Using
strncmp would also happen to match with something like
"PowerMac7,2345" which is not the desired behaviour.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr
On Aug 28, 2010, at 5:34 PM, Timur Tabi wrote:
>> wrote:
>
>> +
>> + for_each_node_by_name(np, "global-utilities") {
>> + if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
>> + rstcr = of_iomap(np, 0) + 0xb0;
>> + if (!rstcr)
>> + printk (KERN_EMERG "Error: reset control "
>
> I'm not sure KERN_EMERG is warranted for this kind of error.
I'm not sure either - I left it as it was before.
>
>> + "register not mapped!\n");
>> + }
>
> So if a node has an fsl,rstcr property, but the of_iomap() fails, we
> jump to the next global-utilities node? Perhaps you need a 'break'
> after the printk()?
Or potentially a continue to be more robust? Or would two (or more) "has-rstcr"
nodes be wrong?
>
>> + }
>> +
>> + if (!rstcr && ppc_md.restart == fsl_rstcr_restart)
>
> Wouldn't it make more sense to assign fsl_rstcr_restart to
> ppc_md.restart only if we find a valid fsl,has-rstcr property?
Again I'm not entirely sure, I left this as it was before. Is there another way
to reset the board if the rstcr node was not found correctly?
-M
>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/85xx: Fix compile issue with p1022_ds due to lmb rename to memblock
arch/powerpc/platforms/85xx/p1022_ds.c:22:23: error: linux/lmb.h: No such file
or directory
arch/powerpc/platforms/85xx/p1022_ds.c: In function 'p1022_ds_setup_arch':
arch/powerpc/platforms/85xx/p1022_ds.c:100: error: implicit declaration of
function 'memblock_end_of_DRAM'
arch/powerpc/platforms/85xx/p1022_ds.c: At top level:
arch/powerpc/platforms/85xx/p1022_ds.c:147: error: 'udbg_progress' undeclared
here (not in a function)
make[2]: *** [arch/powerpc/platforms/85xx/p1022_ds.o] Error 1
Signed-off-by: Kumar Gala
---
arch/powerpc/platforms/85xx/p1022_ds.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c
b/arch/powerpc/platforms/85xx/p1022_ds.c
index e1467c9..34e0090 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -19,7 +19,7 @@
#include
#include
-#include
+#include
#include
#include
@@ -97,7 +97,7 @@ static void __init p1022_ds_setup_arch(void)
#endif
#ifdef CONFIG_SWIOTLB
- if (lmb_end_of_DRAM() > max) {
+ if (memblock_end_of_DRAM() > max) {
ppc_swiotlb_enable = 1;
set_pci_dma_ops(&swiotlb_dma_ops);
ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
--
1.6.0.6
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 31, 2010 at 2:37 AM, Anton Vorontsov wrote: > On Tue, Aug 31, 2010 at 02:03:44AM -0600, Grant Likely wrote: >> On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote: >> > With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, >> > so the following pops up on PowerPC: >> > >> > cc1: warnings being treated as errors >> > In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19: >> > include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared >> > inside parameter list >> > include/linux/of_gpio.h:74: warning: its scope is only this definition >> > or declaration, which is probably not what >> > you want >> > include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared >> > inside parameter list >> > make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1 >> > >> > This patch fixes the issue by providing the proper forward declaration. >> > >> > Signed-off-by: Anton Vorontsov >> >> This doesn't actually solve the problem, and gpiochip should >> remain undefined when CONFIG_GPIOLIB=n to catch exactly these >> build failures. The real problem is that I merged a change >> into the mpc5200 code that required CONFIG_GPIOLIB be enabled >> without reflecting that requirement in Kconfig. > > No, look closer. The error is in of_gpio.h, and it's perfectly > fine to include it w/o GPIOLIB=y. Looking even closer, we're both wrong. You're right I didn't look carefully enough, and the error is in of_gpio.h, not the .c file. However, it is not okay to get the definitions from of_gpio.h when CONFIG_GPIOLIB=n. If GPIOLIB, or more specifically OF_GPIO isn't set, then the of_gpio.h definitions should either not be defined, or should be defined as empty stubs (where appropriate). So, instead of adding a forward declarations of the struct, the correct thing I think to do is to #ifdef out the contents of of_gpio.h when GPIOLIB isn't selected so that extra #includes are completely benign. I've been doing the same thing on the other linux/of*.h headers. I've got a patch in my tree that I'm testing right now and I'll post later today. g. > >> > --- >> > >> > On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote: >> > > I get that with my current stuff: >> > > >> > > cc1: warnings being treated as errors >> > > In file included from [..]/mpc52xx_common.c:19: >> > > of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list >> > [...] >> > > make[3]: *** Waiting for unfinished jobs >> > >> > That's because with GPIOCHIP=n no one declares struct gpio_chip. >> > >> > It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as >> > this feels more generic, and we already have some !GPIOLIB handling >> > in there. >> > >> > include/linux/gpio.h | 6 ++ >> > 1 files changed, 6 insertions(+), 0 deletions(-) >> > >> > diff --git a/include/linux/gpio.h b/include/linux/gpio.h >> > index 03f616b..85207d2 100644 >> > --- a/include/linux/gpio.h >> > +++ b/include/linux/gpio.h >> > @@ -15,6 +15,12 @@ >> > struct device; >> > >> > /* >> > + * Some code might rely on the declaration. Still, it is illegal >> > + * to dereference it for !GPIOLIB case. >> > + */ >> > +struct gpio_chip; >> > + >> > +/* >> > * Some platforms don't support the GPIO programming interface. >> > * >> > * In case some driver uses it anyway (it should normally have >> > -- >> > 1.7.0.5 >> > > > -- > Anton Vorontsov > email: cbouatmai...@gmail.com > irc://irc.freenode.net/bd2 > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr
On Tue, Aug 31, 2010 at 11:26 AM, Matthew McClintock wrote: >> I'm not sure KERN_EMERG is warranted for this kind of error. > > I'm not sure either - I left it as it was before. My vote is to change it to KERN_ERR, but it's your call. >> So if a node has an fsl,rstcr property, but the of_iomap() fails, we >> jump to the next global-utilities node? Perhaps you need a 'break' >> after the printk()? > > Or potentially a continue to be more robust? Or would two (or more) > "has-rstcr" nodes be wrong? My point is that if the of_iomap() call fails, looking for another global-utilities node is not the right course of action. of_iomap() failing is a serious error that should result in an immediate exit. >> Wouldn't it make more sense to assign fsl_rstcr_restart to >> ppc_md.restart only if we find a valid fsl,has-rstcr property? > > Again I'm not entirely sure, I left this as it was before. Is there another > way to reset the board if the rstcr node was not found correctly? Not on our 85xx boards. On 83xx, there's mpc83xx_restart(). I just don't like "ppc_md.restart == fsl_rstcr_restart", because it assumes that the define_machine() entry is incorrect. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case
On Tue, Aug 31, 2010 at 10:44:14AM -0600, Grant Likely wrote:
> On Tue, Aug 31, 2010 at 2:37 AM, Anton Vorontsov
> wrote:
> > On Tue, Aug 31, 2010 at 02:03:44AM -0600, Grant Likely wrote:
> >> On Tue, Aug 24, 2010 at 01:26:23PM +0400, Anton Vorontsov wrote:
> >> > With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared,
> >> > so the following pops up on PowerPC:
> >> >
> >> > cc1: warnings being treated as errors
> >> > In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
> >> > include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared
> >> > inside parameter list
> >> > include/linux/of_gpio.h:74: warning: its scope is only this definition
> >> > or declaration, which is probably not what
> >> > you want
> >> > include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared
> >> > inside parameter list
> >> > make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1
> >> >
> >> > This patch fixes the issue by providing the proper forward declaration.
> >> >
> >> > Signed-off-by: Anton Vorontsov
> >>
> >> This doesn't actually solve the problem, and gpiochip should
> >> remain undefined when CONFIG_GPIOLIB=n to catch exactly these
> >> build failures. The real problem is that I merged a change
> >> into the mpc5200 code that required CONFIG_GPIOLIB be enabled
> >> without reflecting that requirement in Kconfig.
> >
> > No, look closer. The error is in of_gpio.h, and it's perfectly
> > fine to include it w/o GPIOLIB=y.
>
> Looking even closer, we're both wrong. You're right I didn't look
> carefully enough, and the error is in of_gpio.h, not the .c file.
>
> However, it is not okay to get the definitions from of_gpio.h when
> CONFIG_GPIOLIB=n. If GPIOLIB, or more specifically OF_GPIO isn't set,
> then the of_gpio.h definitions should either not be defined, or should
> be defined as empty stubs (where appropriate).
Grrr. Grant, look again, even closer than you did.
They are stubs!
#else /* CONFIG_OF_GPIO */ << !OF_GPIO (or !GPIOLIB) case.
/* Drivers may not strictly depend on the GPIO support, so let them link. */
static inline int of_get_gpio_flags(struct device_node *np, int index,
enum of_gpio_flags *flags)
{
return -ENOSYS;
}
static inline unsigned int of_gpio_count(struct device_node *np)
{
return 0;
}
static inline void of_gpiochip_add(struct gpio_chip *gc) { }
static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
#endif /* CONFIG_OF_GPIO */
The errors are triggered by the of_gpiochip_*() stubs, which
are needed by the drivers/gpio/gpiolib.c.
Do you want to add another #ifdef CONFIG_GPIOLIB around
of_gpiochip_*()? That would be ugly.
There's nothing wrong in providing the forward decls, you
can't dereference it anyway (so you would still catch the
bogus users). And at the same time it's will work great
for these stubs.
--
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: mtmsrd not defined
On Tue, 31 Aug 2010 11:17:17 -0500 Kumar Gala wrote: > Can we add proper CONFIG_PPC_FPU into this file. If nobody beats me to it I can try this evening. Cheers, Sean ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] drivers/macintosh/via-pmu-led.c: Add of_node_put to avoid memory leak
Julia Lawall schrieb:
> Add a call to of_node_put in the error handling code following a call to
> of_find_node_by_path.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @r exists@
> local idexpression x;
> expression E,E1;
> statement S;
> @@
>
> *x =
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> drivers/macintosh/via-pmu-led.c |4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/macintosh/via-pmu-led.c b/drivers/macintosh/via-pmu-led.c
> index d242976..19c3718 100644
> --- a/drivers/macintosh/via-pmu-led.c
> +++ b/drivers/macintosh/via-pmu-led.c
> @@ -92,8 +92,10 @@ static int __init via_pmu_led_init(void)
> if (dt == NULL)
> return -ENODEV;
> model = of_get_property(dt, "model", NULL);
> - if (model == NULL)
> + if (model == NULL) {
> + of_node_put(dt);
> return -ENODEV;
> + }
> if (strncmp(model, "PowerBook", strlen("PowerBook")) != 0 &&
> strncmp(model, "iBook", strlen("iBook")) != 0 &&
> strcmp(model, "PowerMac7,2") != 0 &&
>
is there any rule that says when to use strncmp ? it seems perfecly valid to
use strcpy here
(what is done in the last cmp).
re,
wh
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] PPC: Fix compilation of fsl_rio.c
On Tue, 31 Aug 2010 04:15:21 +0200 Alexander Graf wrote: > Commit a52c8f52 introduced machine check magic for the RapidIO chip. > Unfortunately it was so magical that it used constants that aren't even > defined! > > This patch bluntly comments out the broken constant's usage. This > probably means that said functionality thus doesn't work, but at > least it makes it compile for me. The MCSR_MASK is actually completely unnecessary -- it doesn't change the result of testing bits that are within the mask. Multiple patches have been posted for this already, including: http://patchwork.ozlabs.org/patch/56135/ Someone just needs to apply it. :-) -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] v5 De-couple sysfs memory directories from memory sections
On Mon, 2010-08-16 at 09:34 -0500, Nathan Fontenot wrote: > > It's not an unresolvable issue, as this is a must-fix problem. But you > > should tell us what your proposal is to prevent breakage of existing > > installations. A Kconfig option would be good, but a boot-time kernel > > command line option which selects the new format would be much better. > > This shouldn't break existing installations, unless an architecture chooses > to do so. With my patch only the powerpc/pseries arch is updated such that > what is seen in userspace is different. Even if an arch defines the override for the sysfs dir size, I still don't think this breaks anything (it shouldn't). We move _all_ of the directories over, all at once, to a single, uniform size. The only apparent change to a user moving kernels would be a larger block_size_bytes (which is certainly not changing the ABI) and a new sysfs file for the end of the section. The new sysfs file is _completely_ redundant at this point. The architecture is only supposed to bump up the directory size when it *KNOWS* that all operations will be done at the larger section size, such as if the specific hardware has physical DIMMs which are much larger than SECTION_SIZE. Let's say we have a system with 20MB of memory, SECTION_SIZE of 1MB and a sysfs dir size of 4MB. Before the patch, we have 20 directories: one for each section. After this patch, we have 5 directories. The thing that I think is the next step, but that we _will_ probably need eventually is this, take the 5 sysfs dirs in the above case: 0->3, 4->7, 8->11, 12->15, 16->19 and turn that into a single one: 0->19 *That* will require changing the ABI, but we could certainly have some bloated and slow, but backward-compatible mode. -- Dave ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] PPC: Fix compilation of fsl_rio.c
On Aug 31, 2010, at 1:10 PM, Scott Wood wrote: > On Tue, 31 Aug 2010 04:15:21 +0200 > Alexander Graf wrote: > >> Commit a52c8f52 introduced machine check magic for the RapidIO chip. >> Unfortunately it was so magical that it used constants that aren't even >> defined! >> >> This patch bluntly comments out the broken constant's usage. This >> probably means that said functionality thus doesn't work, but at >> least it makes it compile for me. > > The MCSR_MASK is actually completely unnecessary -- it doesn't change > the result of testing bits that are within the mask. > > Multiple patches have been posted for this already, including: > http://patchwork.ozlabs.org/patch/56135/ > > Someone just needs to apply it. :-) I've got a different version as I don't want to override the mcheck handler. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] PPC: Fix compilation of mpc85xx_mds.c
On Aug 30, 2010, at 9:15 PM, Alexander Graf wrote: > Commit 99d8238f berobbed the for_each loop of its iterator! Let's be > nice and give it back, so it compiles for us. > > CC: Anton Vorontsov > Signed-off-by: Alexander Graf > --- > arch/powerpc/platforms/85xx/mpc85xx_mds.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) applied to merge - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/4] fsl_rio: fix compile errors
On Jun 18, 2010, at 1:24 AM, Li Yang wrote: > Fixes the following compile problem on E500 platforms: > arch/powerpc/sysdev/fsl_rio.c: In function 'fsl_rio_mcheck_exception': > arch/powerpc/sysdev/fsl_rio.c:248: error: 'MCSR_MASK' undeclared (first use > in this function) > > Also fixes the compile problem on non-E500 platforms. > > Signed-off-by: Li Yang > --- > arch/powerpc/sysdev/fsl_rio.c |6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) applied to merge - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/85xx: Fix compile issue with p1022_ds due to lmb rename to memblock
On Aug 31, 2010, at 11:42 AM, Kumar Gala wrote: > arch/powerpc/platforms/85xx/p1022_ds.c:22:23: error: linux/lmb.h: No such > file or directory > arch/powerpc/platforms/85xx/p1022_ds.c: In function 'p1022_ds_setup_arch': > arch/powerpc/platforms/85xx/p1022_ds.c:100: error: implicit declaration of > function 'memblock_end_of_DRAM' > arch/powerpc/platforms/85xx/p1022_ds.c: At top level: > arch/powerpc/platforms/85xx/p1022_ds.c:147: error: 'udbg_progress' undeclared > here (not in a function) > make[2]: *** [arch/powerpc/platforms/85xx/p1022_ds.o] Error 1 > > Signed-off-by: Kumar Gala > --- > arch/powerpc/platforms/85xx/p1022_ds.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) applied to merge - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/platforms/83xx/mpc837x_mds.c: Add missing iounmap
On Aug 29, 2010, at 2:47 PM, Julia Lawall wrote:
> The function of_iomap returns the result of calling ioremap, so iounmap
> should be called on the result in the error handling code, as done in the
> normal exit of the function.
>
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @r exists@
> local idexpression x;
> expression E,E1;
> identifier l;
> statement S;
> @@
>
> *x = of_iomap(...);
> ... when != iounmap(x)
> when != if (...) { ... iounmap(x); ... }
> when != E = x
> when any
> (
> if (x == NULL) S
> |
> if (...) {
> ... when != iounmap(x)
> when != if (...) { ... iounmap(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> )
> ... when != x = E1
>when any
> iounmap(x);
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> arch/powerpc/platforms/83xx/mpc837x_mds.c |9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
applied to merge
- k
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 5/7] arch/powerpc/sysdev/qe_lib/qe.c: Add of_node_put to avoid memory leak
On Aug 29, 2010, at 4:52 AM, Julia Lawall wrote:
> Add a call to of_node_put in the error handling code following a call to
> of_find_compatible_node.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @r exists@
> local idexpression x;
> expression E,E1;
> statement S;
> @@
>
> *x =
> (of_find_node_by_path
> |of_find_node_by_name
> |of_find_node_by_phandle
> |of_get_parent
> |of_get_next_parent
> |of_get_next_child
> |of_find_compatible_node
> |of_match_node
> )(...);
> ...
> if (x == NULL) S
> <... when != x = E
> *if (...) {
> ... when != of_node_put(x)
> when != if (...) { ... of_node_put(x); ... }
> (
> return <+...x...+>;
> |
> * return ...;
> )
> }
> ...>
> of_node_put(x);
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> arch/powerpc/sysdev/qe_lib/qe.c |1 +
> 1 file changed, 1 insertion(+)
applied to merge
- k
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/85xx: Add P1021 PCI IDs and quirks
On Aug 8, 2010, at 9:03 AM, Anton Vorontsov wrote: > This is needed for proper PCI-E support on P1021 SoCs. > > Signed-off-by: Anton Vorontsov > --- > arch/powerpc/sysdev/fsl_pci.c |2 ++ > include/linux/pci_ids.h |2 ++ > 2 files changed, 4 insertions(+), 0 deletions(-) applied - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[git pull] Please pull powerpc.git merge branch
The following changes since commit 54a834043314c257210db2a9d59f8cc605571639: Michael Neuling (1): powerpc: Don't use kernel stack with translation off are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git ..BRANCH.NOT.VERIFIED.. Alexander Graf (1): powerpc/85xx: Fix compilation of mpc85xx_mds.c Anton Vorontsov (1): powerpc/85xx: Add P1021 PCI IDs and quirks Julia Lawall (2): arch/powerpc/platforms/83xx/mpc837x_mds.c: Add missing iounmap arch/powerpc/sysdev/qe_lib/qe.c: Add of_node_put to avoid memory leak Kumar Gala (1): powerpc/85xx: Fix compile issue with p1022_ds due to lmb rename to memblock Li Yang (1): fsl_rio: fix compile errors arch/powerpc/platforms/83xx/mpc837x_mds.c |9 ++--- arch/powerpc/platforms/85xx/mpc85xx_mds.c |1 + arch/powerpc/platforms/85xx/p1022_ds.c|4 ++-- arch/powerpc/sysdev/fsl_pci.c |2 ++ arch/powerpc/sysdev/fsl_rio.c |6 +- arch/powerpc/sysdev/qe_lib/qe.c |1 + include/linux/pci_ids.h |2 ++ 7 files changed, 19 insertions(+), 6 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] v5 De-couple sysfs memory directories from memory sections
Hi Nathan, > This set of patches de-couples the idea that there is a single > directory in sysfs for each memory section. The intent of the > patches is to reduce the number of sysfs directories created to > resolve a boot-time performance issue. On very large systems > boot time are getting very long (as seen on powerpc hardware) > due to the enormous number of sysfs directories being created. > On a system with 1 TB of memory we create ~63,000 directories. > For even larger systems boot times are being measured in hours. > > This set of patches allows for each directory created in sysfs > to cover more than one memory section. The default behavior for > sysfs directory creation is the same, in that each directory > represents a single memory section. A new file 'end_phys_index' > in each directory contains the physical_id of the last memory > section covered by the directory so that users can easily > determine the memory section range of a directory. I tested this on a POWER7 with 2TB memory and the boot time improved from greater than 6 hours (I gave up), to under 5 minutes. Nice! Tested-by: Anton Blanchard Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powerpc/fsl_soc: Search all global-utilities nodes for rstccr
The first global-utilities node might not contain the rstcr
property, so we should search all the nodes
Signed-off-by: Matthew McClintock
---
-Changed KERN_EMERG to KERN_ERR
-Break if we do not find rstcr mapped
-Restore of_put_node that was dropped
arch/powerpc/sysdev/fsl_soc.c | 20 +---
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index b91f7ac..6c67d9e 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -378,17 +378,23 @@ static __be32 __iomem *rstcr;
static int __init setup_rstcr(void)
{
struct device_node *np;
- np = of_find_node_by_name(NULL, "global-utilities");
- if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
- rstcr = of_iomap(np, 0) + 0xb0;
- if (!rstcr)
- printk (KERN_EMERG "Error: reset control register "
- "not mapped!\n");
- } else if (ppc_md.restart == fsl_rstcr_restart)
+
+ for_each_node_by_name(np, "global-utilities") {
+ if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
+ rstcr = of_iomap(np, 0) + 0xb0;
+ if (!rstcr)
+ printk (KERN_ERR "Error: reset control "
+ "register not mapped!\n");
+ break;
+ }
+ }
+
+ if (!rstcr && ppc_md.restart == fsl_rstcr_restart)
printk(KERN_ERR "No RSTCR register, warm reboot won't work\n");
if (np)
of_node_put(np);
+
return 0;
}
--
1.6.6.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] powerpc/fsl_booke: Add support to boot from core other than 0
First we check to see if we are the first core booting up. This is accomplished by comparing the boot_cpuid with -1, if it is we assume this is the first core coming up. Secondly, we need to update the initial thread info structure to reflect the actual cpu we are running on otherwise smp_processor_id() and related functions will return the default initialization value of the struct or 0. Signed-off-by: Matthew McClintock --- arch/powerpc/kernel/head_fsl_booke.S | 10 -- arch/powerpc/kernel/setup_32.c |2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S index 258315a..5bbf593 100644 --- a/arch/powerpc/kernel/head_fsl_booke.S +++ b/arch/powerpc/kernel/head_fsl_booke.S @@ -152,8 +152,11 @@ _ENTRY(__early_start) /* Check to see if we're the second processor, and jump * to the secondary_start code if so */ - mfspr r24,SPRN_PIR - cmpwi r24,0 + lis r24, boot_cp...@h + ori r24, r24, boot_cp...@l + lwz r24, 0(r24) + cmpwi r24, -1 + mfspr r24,SPRN_PIR bne __secondary_start #endif @@ -175,6 +178,9 @@ _ENTRY(__early_start) li r0,0 stwur0,THREAD_SIZE-STACK_FRAME_OVERHEAD(r1) + rlwinm r22,r1,0,0,31-THREAD_SHIFT /* current thread_info */ + stw r24, TI_CPU(r22) + bl early_init #ifdef CONFIG_RELOCATABLE diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c index 8f58986..4be3ef4 100644 --- a/arch/powerpc/kernel/setup_32.c +++ b/arch/powerpc/kernel/setup_32.c @@ -46,7 +46,7 @@ extern void bootx_init(unsigned long r4, unsigned long phys); -int boot_cpuid; +int boot_cpuid = -1; EXPORT_SYMBOL_GPL(boot_cpuid); int boot_cpuid_phys; -- 1.6.6.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] powerpc/mm: Assume first cpu is boot_cpuid not 0
arch/powerpc/mm/mmu_context_nohash.c assumes the boot cpu
will always have smp_processor_id() == 0. This patch fixes
that assumption
Signed-off-by: Matthew McClintock
---
arch/powerpc/mm/mmu_context_nohash.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_nohash.c
b/arch/powerpc/mm/mmu_context_nohash.c
index 1f2d9ff..cf98c1e 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -334,7 +334,7 @@ static int __cpuinit mmu_context_cpu_notify(struct
notifier_block *self,
/* We don't touch CPU 0 map, it's allocated at aboot and kept
* around forever
*/
- if (cpu == 0)
+ if (cpu == boot_cpuid)
return NOTIFY_OK;
switch (action) {
@@ -412,9 +412,11 @@ void __init mmu_context_init(void)
*/
context_map = alloc_bootmem(CTX_MAP_SIZE);
context_mm = alloc_bootmem(sizeof(void *) * (last_context + 1));
+#ifndef CONFIG_SMP
stale_map[0] = alloc_bootmem(CTX_MAP_SIZE);
+#else
+ stale_map[boot_cpuid] = alloc_bootmem(CTX_MAP_SIZE);
-#ifdef CONFIG_SMP
register_cpu_notifier(&mmu_context_cpu_nb);
#endif
--
1.6.6.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[v1 PATCH] ucc_geth: fix ethtool set ring param bug
It's common sense that when we should do change to driver ring
desc/buffer etc only after 'stop/shutdown' the device. When we
do change while devices/driver is running, kernel oops occur:
[
r...@fsl_8569mds:/root> ethtool -G eth0 tx 256
r...@fsl_8569mds:/root> Oops: Kernel access of bad area, sig: 11 [#1]
MPC8569 MDS
last sysfs file: /sys/kernel/uevent_seqnum
Modules linked in:
NIP: LR: c0072fbc CTR:
REGS: effefef0 TRAP: 0400 Not tainted (2.6.36-rc3-2-g6c3b118-dirty)
MSR: 00021000 CR: 24442048 XER:
TASK = c0518350[0] 'swapper' THREAD: c0544000
GPR00: effeffa0 c0518350 0020 ef0be000 ef005000 8000 0200
GPR08: c03b5d00 f1010080 ef08d458 000dda96 3ffb2900
GPR16: 3ffa8948 3fff1314 3ffac3f8
GPR24: c053 0020 ef3709c0
NIP [] (null)
LR [c0072fbc] handle_IRQ_event+0x4c/0x12c
Call Trace:
[effeffa0] [c0019414] qe_ic_mask_irq+0x1c/0x90 (unreliable)
[effeffc0] [c0075b74] handle_level_irq+0x88/0x128
[effeffd0] [c001ca44] qe_ic_cascade_muxed_mpic+0x50/0x88
[effefff0] [c000d5fc] call_handle_irq+0x18/0x28
[c0545ea0] [c0004f3c] do_IRQ+0xa8/0x140
[c0545ed0] [c000e2bc] ret_from_except+0x0/0x18
-- Exception: 501 at cpu_idle+0x9c/0xdc
LR = cpu_idle+0x9c/0xdc
[c0545f90] [c0008318] cpu_idle+0x54/0xdc (unreliable)
[c0545fb0] [c000231c] rest_init+0x68/0x7c
[c0545fc0] [c04e686c] start_kernel+0x230/0x2b0
[c0545ff0] [c39c] skpinv+0x2b4/0x2f0
Instruction dump:
Kernel panic - not syncing: Fatal exception in interrupt
Rebooting in 180 seconds..
]
Then the natural solution would be 'stop driver/device then adjust
ring buffer parameter then reactivate driver/device'.
v1: add roll back branch for 'auto reopen fail' statement
Signed-off-by: Liang Li
---
drivers/net/ucc_geth.c |4 ++--
drivers/net/ucc_geth.h |2 ++
drivers/net/ucc_geth_ethtool.c | 29 +++--
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 7d2e33a..5eadfa3 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3485,7 +3485,7 @@ err:
/* Called when something needs to use the ethernet device */
/* Returns 0 for success. */
-static int ucc_geth_open(struct net_device *dev)
+int ucc_geth_open(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
int err;
@@ -3542,7 +3542,7 @@ err:
}
/* Stops the kernel queue, and halts the controller */
-static int ucc_geth_close(struct net_device *dev)
+int ucc_geth_close(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
diff --git a/drivers/net/ucc_geth.h b/drivers/net/ucc_geth.h
index 05a9558..1cfb400 100644
--- a/drivers/net/ucc_geth.h
+++ b/drivers/net/ucc_geth.h
@@ -1235,5 +1235,7 @@ int init_flow_control_params(u32
automatic_flow_control_mode,
u32 __iomem *upsmr_register, u32 __iomem *uempr_register,
u32 __iomem *maccfg1_register);
+extern int ucc_geth_open(struct net_device *);
+extern int ucc_geth_close(struct net_device *);
#endif /* __UCC_GETH_H__ */
diff --git a/drivers/net/ucc_geth_ethtool.c b/drivers/net/ucc_geth_ethtool.c
index 6f92e48..644a6db 100644
--- a/drivers/net/ucc_geth_ethtool.c
+++ b/drivers/net/ucc_geth_ethtool.c
@@ -255,13 +255,30 @@ uec_set_ringparam(struct net_device *netdev,
return -EINVAL;
}
- ug_info->bdRingLenRx[queue] = ring->rx_pending;
- ug_info->bdRingLenTx[queue] = ring->tx_pending;
-
if (netif_running(netdev)) {
- /* FIXME: restart automatically */
- printk(KERN_INFO
- "Please re-open the interface.\n");
+ u16 rx_t;
+ u16 tx_t;
+ printk(KERN_INFO "Stopping interface %s.\n", netdev->name);
+ ucc_geth_close(netdev);
+
+ rx_t = ug_info->bdRingLenRx[queue];
+ tx_t = ug_info->bdRingLenTx[queue];
+
+ ug_info->bdRingLenRx[queue] = ring->rx_pending;
+ ug_info->bdRingLenTx[queue] = ring->tx_pending;
+
+ printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
+ ret = ucc_geth_open(netdev);
+ if (ret) {
+ printk(KERN_WARNING "uec_set_ringparam: set ring param
for running"
+ " interface %s failed. Please try to
make the interface "
+ " down, then try again.\n",
netdev->name);
+ ug_info->bdRingLenRx[queue] = rx_t;
+ ug_info->bdRingLenTx[queue] = tx_t;
+ }
+ } else {
+ ug_info-
Re: [PATCH] powerpc: mtmsrd not defined
On Tue, 31 Aug 2010 13:46:05 -0400 Sean MacLennan wrote: > On Tue, 31 Aug 2010 11:17:17 -0500 > Kumar Gala wrote: > > > Can we add proper CONFIG_PPC_FPU into this file. > > If nobody beats me to it I can try this evening. I had to give up. Without the CONFIG_PPC_FPU it compiles fine for an FPU less 44x. But with the CONFIG_PPC_FPU, I get the following errors: arch/powerpc/lib/built-in.o: In function `__copy_tofrom_user': arch/powerpc/lib/copy_32.S:(.kprobes.text+0xf8e): undefined reference to `do_lfs' arch/powerpc/lib/copy_32.S:(.kprobes.text+0xf96): undefined reference to `do_lfs' arch/powerpc/lib/copy_32.S:(.kprobes.text+0xfc2): undefined reference to `do_lfd' arch/powerpc/lib/copy_32.S:(.kprobes.text+0xfca): undefined reference to `do_lfd' arch/powerpc/lib/copy_32.S:(.kprobes.text+0xff6): undefined reference to `do_stfs' arch/powerpc/lib/copy_32.S:(.kprobes.text+0xffe): undefined reference to `do_stfs' arch/powerpc/lib/copy_32.S:(.kprobes.text+0x102a): undefined reference to `do_stfd' arch/powerpc/lib/copy_32.S:(.kprobes.text+0x1032): undefined reference to `do_stfd' make: *** [.tmp_vmlinux1] Error 1 Oops, sorry, it will say arch/powerpc/lib/copy32.S... I corrected that. But I cannot find how copy_32.S is including those functions. Cheers, Sean ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/4] fsl_rio: fix compile errors
On Wed, Sep 1, 2010 at 5:39 AM, Kumar Gala wrote: > > On Jun 18, 2010, at 1:24 AM, Li Yang wrote: > >> Fixes the following compile problem on E500 platforms: >> arch/powerpc/sysdev/fsl_rio.c: In function 'fsl_rio_mcheck_exception': >> arch/powerpc/sysdev/fsl_rio.c:248: error: 'MCSR_MASK' undeclared (first use >> in this function) >> >> Also fixes the compile problem on non-E500 platforms. >> >> Signed-off-by: Li Yang >> --- >> arch/powerpc/sysdev/fsl_rio.c | 6 +- >> 1 files changed, 5 insertions(+), 1 deletions(-) > > applied to merge Thanks, Kumar. How about the other ones in the series? - Leo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH][RFC] preempt_count corruption across H_CEDE call with CONFIG_PREEMPT on pseries
On Tue, 2010-08-31 at 00:12 -0700, Darren Hart wrote: .. > > When running with the function plugin I had to stop the trace > immediately before entering start_secondary after an online or my traces > would not include the pseries_mach_cpu_die function, nor the tracing I > added there (possibly buffer size, I am using 2048). The following trace > was collected using "trace-cmd record -p function -e irq -e sched" and > has been filtered to only show CPU [001] (the CPU undergoing the > offline/online test, and the one seeing preempt_count (pcnt) go to > after cede. The function tracer does not indicate anything > running on the CPU other than the HCALL - unless the __trace_hcall* > commands might be to blame. It's not impossible. Though normally they're disabled right, so the only reason they're running is because you're tracing. So if they are causing the bug then that doesn't explain why you see it normally. Still, might be worth disabling just the hcall tracepoints just to be 100% sure. cheers signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/4] fsl_rio: fix compile errors
On Aug 31, 2010, at 10:40 PM, Li Yang wrote: > On Wed, Sep 1, 2010 at 5:39 AM, Kumar Gala wrote: >> >> On Jun 18, 2010, at 1:24 AM, Li Yang wrote: >> >>> Fixes the following compile problem on E500 platforms: >>> arch/powerpc/sysdev/fsl_rio.c: In function 'fsl_rio_mcheck_exception': >>> arch/powerpc/sysdev/fsl_rio.c:248: error: 'MCSR_MASK' undeclared (first use >>> in this function) >>> >>> Also fixes the compile problem on non-E500 platforms. >>> >>> Signed-off-by: Li Yang >>> --- >>> arch/powerpc/sysdev/fsl_rio.c |6 +- >>> 1 files changed, 5 insertions(+), 1 deletions(-) >> >> applied to merge > > Thanks, Kumar. > > How about the other ones in the series? I want to rework how the whole RIO mcheck works and will do so for 2.6.37. Part of the problem I have is that we are ignoring the fact that this code isn't right for 8641 support of SRIO. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: mtmsrd not defined
On Aug 31, 2010, at 9:55 PM, Sean MacLennan wrote: > On Tue, 31 Aug 2010 13:46:05 -0400 > Sean MacLennan wrote: > >> On Tue, 31 Aug 2010 11:17:17 -0500 >> Kumar Gala wrote: >> >>> Can we add proper CONFIG_PPC_FPU into this file. >> >> If nobody beats me to it I can try this evening. > > I had to give up. Without the CONFIG_PPC_FPU it compiles fine for an > FPU less 44x. But with the CONFIG_PPC_FPU, I get the following errors: > > arch/powerpc/lib/built-in.o: In function `__copy_tofrom_user': > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xf8e): undefined reference to > `do_lfs' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xf96): undefined reference to > `do_lfs' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xfc2): undefined reference to > `do_lfd' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xfca): undefined reference to > `do_lfd' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xff6): undefined reference to > `do_stfs' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0xffe): undefined reference to > `do_stfs' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0x102a): undefined reference to > `do_stfd' > arch/powerpc/lib/copy_32.S:(.kprobes.text+0x1032): undefined reference to > `do_stfd' > make: *** [.tmp_vmlinux1] Error 1 > > > Oops, sorry, it will say arch/powerpc/lib/copy32.S... I corrected that. > But I cannot find how copy_32.S is including those functions. > > Cheers, > Sean For what defconfig setup do you get the errors above? - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: mtmsrd not defined
On Wed, 1 Sep 2010 01:45:46 -0500 Kumar Gala wrote: > For what defconfig setup do you get the errors above? 44x/ebony_defconfig Then enable KPROBES (or XMON). Then put an #ifdef CONFIG_PPC_FPU in ldstfp.S. Cheers, Sean ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
