* Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
From: K.Prasad @ 2011-02-07 4:52 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, David Kleikamp
In-Reply-To: <1297048239.14982.29.camel@pasglop>
On Mon, Feb 07, 2011 at 02:10:39PM +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2011-02-07 at 08:26 +0530, K.Prasad wrote:
> > On Mon, Feb 07, 2011 at 09:54:13AM +1100, Benjamin Herrenschmidt wrote:
> > > A typo in the #ifdef statement makes us never call it
> > > in flush_thread()
> > >
> >
> > I wish it never compiled for such typos :-)
> >
> >
> > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Acked-by: K.Prasad <prasad@linux.vnet.ibm.com>
>
> Interestingly, that 'fix' now breaks the build:
>
> cc1: warnings being treated as errors
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/process.c:356: error:
> 'set_debug_reg_defaults' defined but not used
>
> This file is is becoming an absolute mess of ifdef's in large part due
> to the new BookE debug stuff and your HW breakpoint stuff... Any chance
> you and Shaggy see if you can improve that situation a bit ?
>
> Cheers,
> Ben.
>
>
Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around the
definition of 'set_debug_reg_defaults'.
There's indeed too much sprinkling of #ifdefs in the code, but most of
it would go away when the BookE code also uses the generic hw-breakpoint
interfaces. Given the advanced debug features that BookE supports, it's
unfortunately not that straight-forward (needs additions to generic
hw-breakpoint infrastructure).
Thanks,
K.Prasad
^ permalink raw reply
* Re: [PATCH 1/6] nvram: Generalize code for OS partitions in NVRAM
From: Benjamin Herrenschmidt @ 2011-02-07 4:57 UTC (permalink / raw)
To: Jim Keniston; +Cc: linuxppc-dev
In-Reply-To: <20101114041516.9457.2462.stgit@localhost.localdomain>
On Sat, 2010-11-13 at 20:15 -0800, Jim Keniston wrote:
> Adapt the functions used to create and write to the RTAS-log partition
> to work with any OS-type partition.
>
> Signed-off-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
Overall pretty good (sorry for taking that long to review, I've been
swamped down). Just a handful of minor nits:
> +/*
> + * Per the criteria passed via nvram_remove_partition(), should this
> + * partition be removed? 1=remove, 0=keep
> + */
> +static int nvram_condemn_partition(struct nvram_partition *part,
> + const char *name, int sig, const char *exceptions[])
As "fun" as this name is, it didn't help me understand what the function
was about until I read the code for the next one :-)
What about instead something like nvram_can_remove_partition() or
something a bit more explicit like that ?
"comdemn" made me thought it was a function used to "mark" partitions
to be killed later, which is not what the function does.
.../...
> +static const char *valid_os_partitions[] = {
> + "ibm,rtas-log",
> + NULL
> +};
Can you pick a name that will be less confusing in the global
symbol table ? Something like "pseries_nvram_os_partitions"
or whatever shorter you can come up with which doesn't suck ?
Also, should we move that "os partition" core out of pseries ?
Looks like it will be useful for a few other platforms, I'd like
to move that to a more generically useful location in arch/powerpc
but that's not a blocker for this series but something to do next
maybe ?
In that case "struct os_partition" should also find itself a better
name, maybe struct nvram_os_partition ?
> static ssize_t pSeries_nvram_read(char *buf, size_t count, loff_t *index)
> {
> @@ -134,7 +147,7 @@ static ssize_t pSeries_nvram_get_size(void)
> }
>
>
> -/* nvram_write_error_log
> +/* nvram_write_os_partition, nvram_write_error_log
> *
> * We need to buffer the error logs into nvram to ensure that we have
> * the failure information to decode. If we have a severe error there
> @@ -156,48 +169,55 @@ static ssize_t pSeries_nvram_get_size(void)
> * The 'data' section would look like (in bytes):
> * +--------------+------------+-----------------------------------+
> * | event_logged | sequence # | error log |
> - * |0 3|4 7|8 nvram_error_log_size-1|
> + * |0 3|4 7|8 error_log_size-1|
> * +--------------+------------+-----------------------------------+
> *
> * event_logged: 0 if event has not been logged to syslog, 1 if it has
> * sequence #: The unique sequence # for each event. (until it wraps)
> * error log: The error log from event_scan
> */
> -int nvram_write_error_log(char * buff, int length,
> +int nvram_write_os_partition(struct os_partition *part, char * buff, int length,
> unsigned int err_type, unsigned int error_log_cnt)
> {
> int rc;
> loff_t tmp_index;
> struct err_log_info info;
>
> - if (nvram_error_log_index == -1) {
> + if (part->index == -1) {
> return -ESPIPE;
> }
>
> - if (length > nvram_error_log_size) {
> - length = nvram_error_log_size;
> + if (length > part->size) {
> + length = part->size;
> }
>
> info.error_type = err_type;
> info.seq_num = error_log_cnt;
>
> - tmp_index = nvram_error_log_index;
> + tmp_index = part->index;
>
> rc = ppc_md.nvram_write((char *)&info, sizeof(struct err_log_info), &tmp_index);
> if (rc <= 0) {
> - printk(KERN_ERR "nvram_write_error_log: Failed nvram_write (%d)\n", rc);
> + printk(KERN_ERR "nvram_write_os_partition: Failed nvram_write (%d)\n", rc);
> return rc;
> }
>
> rc = ppc_md.nvram_write(buff, length, &tmp_index);
> if (rc <= 0) {
> - printk(KERN_ERR "nvram_write_error_log: Failed nvram_write (%d)\n", rc);
> + printk(KERN_ERR "nvram_write_os_partition: Failed nvram_write (%d)\n", rc);
> return rc;
> }
>
> return 0;
> }
While at it, turn these into pr_err and use __FUNCTION__ or __func__
> +int nvram_write_error_log(char * buff, int length,
> + unsigned int err_type, unsigned int error_log_cnt)
> +{
> + return nvram_write_os_partition(&rtas_log_partition, buff, length,
> + err_type, error_log_cnt);
> +}
That's a candidate for a static inline in a .h
> /* nvram_read_error_log
> *
> * Reads nvram for error log for at most 'length'
> @@ -209,13 +229,13 @@ int nvram_read_error_log(char * buff, int length,
> loff_t tmp_index;
> struct err_log_info info;
>
> - if (nvram_error_log_index == -1)
> + if (rtas_log_partition.index == -1)
> return -1;
>
> - if (length > nvram_error_log_size)
> - length = nvram_error_log_size;
> + if (length > rtas_log_partition.size)
> + length = rtas_log_partition.size;
>
> - tmp_index = nvram_error_log_index;
> + tmp_index = rtas_log_partition.index;
>
> rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
> if (rc <= 0) {
> @@ -244,10 +264,10 @@ int nvram_clear_error_log(void)
> int clear_word = ERR_FLAG_ALREADY_LOGGED;
> int rc;
>
> - if (nvram_error_log_index == -1)
> + if (rtas_log_partition.index == -1)
> return -1;
>
> - tmp_index = nvram_error_log_index;
> + tmp_index = rtas_log_partition.index;
>
> rc = ppc_md.nvram_write((char *)&clear_word, sizeof(int), &tmp_index);
> if (rc <= 0) {
> @@ -258,67 +278,71 @@ int nvram_clear_error_log(void)
> return 0;
> }
>
> -/* pseries_nvram_init_log_partition
> +/* pseries_nvram_init_os_partition
> *
> - * This will setup the partition we need for buffering the
> - * error logs and cleanup partitions if needed.
> + * This set up a partition with an "OS" signature.
> *
> * The general strategy is the following:
> - * 1.) If there is log partition large enough then use it.
> - * 2.) If there is none large enough, search
> - * for a free partition that is large enough.
> - * 3.) If there is not a free partition large enough remove
> - * _all_ OS partitions and consolidate the space.
> - * 4.) Will first try getting a chunk that will satisfy the maximum
> - * error log size (NVRAM_MAX_REQ).
> - * 5.) If the max chunk cannot be allocated then try finding a chunk
> - * that will satisfy the minum needed (NVRAM_MIN_REQ).
> + * 1.) If a partition with the indicated name already exists...
> + * - If it's large enough, use it.
> + * - Otherwise, recycle it and keep going.
> + * 2.) Search for a free partition that is large enough.
> + * 3.) If there's not a free partition large enough, recycle any obsolete
> + * OS partitions and try again.
> + * 4.) Will first try getting a chunk that will satisfy the requested size.
> + * 5.) If a chunk of the requested size cannot be allocated, then try finding
> + * a chunk that will satisfy the minum needed.
> + *
> + * Returns 0 on success, else -1.
> */
> -static int __init pseries_nvram_init_log_partition(void)
> +static int __init pseries_nvram_init_os_partition(struct os_partition *part)
> {
> loff_t p;
> int size;
>
> - p = nvram_find_partition(NVRAM_LOG_PART_NAME, NVRAM_SIG_OS, &size);
> + p = nvram_find_partition(part->name, NVRAM_SIG_OS, &size);
>
> /* Found one but too small, remove it */
> - if (p && size < NVRAM_MIN_REQ) {
> - pr_info("nvram: Found too small "NVRAM_LOG_PART_NAME" partition"
> - ",removing it...");
> - nvram_remove_partition(NVRAM_LOG_PART_NAME, NVRAM_SIG_OS);
> + if (p && size < part->min_size) {
> + pr_info("nvram: Found too small %s partition,"
> + " removing it...\n", part->name);
> + nvram_remove_partition(part->name, NVRAM_SIG_OS, NULL);
> p = 0;
> }
>
> /* Create one if we didn't find */
> if (!p) {
> - p = nvram_create_partition(NVRAM_LOG_PART_NAME, NVRAM_SIG_OS,
> - NVRAM_MAX_REQ, NVRAM_MIN_REQ);
> - /* No room for it, try to get rid of any OS partition
> - * and try again
> - */
> + p = nvram_create_partition(part->name, NVRAM_SIG_OS,
> + part->req_size, part->min_size);
> if (p == -ENOSPC) {
> - pr_info("nvram: No room to create "NVRAM_LOG_PART_NAME
> - " partition, deleting all OS partitions...");
> - nvram_remove_partition(NULL, NVRAM_SIG_OS);
> - p = nvram_create_partition(NVRAM_LOG_PART_NAME,
> - NVRAM_SIG_OS, NVRAM_MAX_REQ,
> - NVRAM_MIN_REQ);
> + pr_info("nvram: No room to create %s partition, "
> + "deleting any obsolete OS partitions...\n",
> + part->name);
> + nvram_remove_partition(NULL, NVRAM_SIG_OS,
> + valid_os_partitions);
> + p = nvram_create_partition(part->name, NVRAM_SIG_OS,
> + part->req_size, part->min_size);
> }
> }
>
> if (p <= 0) {
> - pr_err("nvram: Failed to find or create "NVRAM_LOG_PART_NAME
> - " partition, err %d\n", (int)p);
> - return 0;
> + pr_err("nvram: Failed to find or create %s"
> + " partition, err %d\n", part->name, (int)p);
> + return -1;
> }
>
> - nvram_error_log_index = p;
> - nvram_error_log_size = nvram_get_partition_size(p) -
> - sizeof(struct err_log_info);
> + part->index = p;
> + part->size = nvram_get_partition_size(p) - sizeof(struct err_log_info);
>
> return 0;
> }
> -machine_late_initcall(pseries, pseries_nvram_init_log_partition);
> +
> +static int __init pseries_nvram_init_log_partitions(void)
> +{
> + (void) pseries_nvram_init_os_partition(&rtas_log_partition);
> + return 0;
> +}
> +machine_late_initcall(pseries, pseries_nvram_init_log_partitions);
>
> int __init pSeries_nvram_init(void)
> {
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 2/6] nvram: Capture oops/panic reports in ibm, oops-log partition
From: Benjamin Herrenschmidt @ 2011-02-07 5:01 UTC (permalink / raw)
To: Jim Keniston; +Cc: linuxppc-dev
In-Reply-To: <20101114041521.9457.69485.stgit@localhost.localdomain>
On Sat, 2010-11-13 at 20:15 -0800, Jim Keniston wrote:
> Create the ibm,oops-log NVRAM partition, and capture the end of the printk
> buffer in it when there's an oops or panic. If we can't create the
> ibm,oops-log partition, capture the oops/panic report in ibm,rtas-log.
Won't the parsing tools choke on the oops log in the RTAS log
partition ?
Also, maybe remove the "ibm," prefix on the nvram partition, make it
"lnx," instead.
Then, if you move the rest of the code out of pseries, you can move that
out too, just make pseries platform code call the init code for it, that
way other platforms can re-use that as-is.
But that later part can wait a separate patch series, just make sure you
change the partition name now.
Cheers,
Ben.
> Signed-off-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
>
> arch/powerpc/platforms/pseries/nvram.c | 89 ++++++++++++++++++++++++++++++++
> 1 files changed, 88 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 43d5c52..6c88cda 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -16,6 +16,8 @@
> #include <linux/errno.h>
> #include <linux/init.h>
> #include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/kmsg_dump.h>
> #include <asm/uaccess.h>
> #include <asm/nvram.h>
> #include <asm/rtas.h>
> @@ -50,11 +52,32 @@ static struct os_partition rtas_log_partition = {
> .index = -1
> };
>
> +static struct os_partition oops_log_partition = {
> + .name = "ibm,oops-log",
> + .req_size = 4000,
> + .min_size = 2000,
> + .index = -1
> +};
> +
> static const char *valid_os_partitions[] = {
> "ibm,rtas-log",
> + "ibm,oops-log",
> NULL
> };
>
> +static void oops_to_nvram(struct kmsg_dumper *dumper,
> + enum kmsg_dump_reason reason,
> + const char *old_msgs, unsigned long old_len,
> + const char *new_msgs, unsigned long new_len);
> +
> +static struct kmsg_dumper nvram_kmsg_dumper = {
> + .dump = oops_to_nvram
> +};
> +
> +/* We preallocate oops_buf during init to avoid kmalloc during oops/panic. */
> +static size_t oops_buf_sz;
> +static char *oops_buf;
> +
> static ssize_t pSeries_nvram_read(char *buf, size_t count, loff_t *index)
> {
> unsigned int i;
> @@ -337,9 +360,36 @@ static int __init pseries_nvram_init_os_partition(struct os_partition *part)
> return 0;
> }
>
> +static void __init nvram_init_oops_partition(int rtas_partition_exists)
> +{
> + int rc;
> +
> + rc = pseries_nvram_init_os_partition(&oops_log_partition);
> + if (rc != 0) {
> + if (!rtas_partition_exists)
> + return;
> + pr_notice("nvram: Using %s partition to log both"
> + " RTAS errors and oops/panic reports\n",
> + rtas_log_partition.name);
> + memcpy(&oops_log_partition, &rtas_log_partition,
> + sizeof(rtas_log_partition));
> + }
> + oops_buf_sz = oops_log_partition.size - sizeof(struct err_log_info);
> + oops_buf = kmalloc(oops_buf_sz, GFP_KERNEL);
> + rc = kmsg_dump_register(&nvram_kmsg_dumper);
> + if (rc != 0) {
> + pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
> + kfree(oops_buf);
> + return;
> + }
> +}
> +
> static int __init pseries_nvram_init_log_partitions(void)
> {
> - (void) pseries_nvram_init_os_partition(&rtas_log_partition);
> + int rc;
> +
> + rc = pseries_nvram_init_os_partition(&rtas_log_partition);
> + nvram_init_oops_partition(rc == 0);
> return 0;
> }
> machine_late_initcall(pseries, pseries_nvram_init_log_partitions);
> @@ -373,3 +423,40 @@ int __init pSeries_nvram_init(void)
>
> return 0;
> }
> +
> +/*
> + * Try to capture the last capture_len bytes of the printk buffer. Return
> + * the amount actually captured.
> + */
> +static size_t capture_last_msgs(const char *old_msgs, size_t old_len,
> + const char *new_msgs, size_t new_len,
> + char *captured, size_t capture_len)
> +{
> + if (new_len >= capture_len) {
> + memcpy(captured, new_msgs + (new_len - capture_len),
> + capture_len);
> + return capture_len;
> + } else {
> + /* Grab the end of old_msgs. */
> + size_t old_tail_len = min(old_len, capture_len - new_len);
> + memcpy(captured, old_msgs + (old_len - old_tail_len),
> + old_tail_len);
> + memcpy(captured + old_tail_len, new_msgs, new_len);
> + return old_tail_len + new_len;
> + }
> +}
> +
> +/* our kmsg_dump callback */
> +static void oops_to_nvram(struct kmsg_dumper *dumper,
> + enum kmsg_dump_reason reason,
> + const char *old_msgs, unsigned long old_len,
> + const char *new_msgs, unsigned long new_len)
> +{
> + static unsigned int oops_count = 0;
> + size_t text_len;
> +
> + text_len = capture_last_msgs(old_msgs, old_len, new_msgs, new_len,
> + oops_buf, oops_buf_sz);
> + (void) nvram_write_os_partition(&oops_log_partition, oops_buf,
> + (int) text_len, ERR_TYPE_KERNEL_PANIC, ++oops_count);
> +}
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc: Add IO event interrupt support
From: Benjamin Herrenschmidt @ 2011-02-07 5:10 UTC (permalink / raw)
To: Tseng-Hui (Frank) Lin; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1294267483.24377.30.camel@flin.austin.ibm.com>
On Wed, 2011-01-05 at 16:44 -0600, Tseng-Hui (Frank) Lin wrote:
> This patch adds support for handling IO Event interrupts which come
> through at the /event-sources/ibm,io-events device tree node.
.../...
The previous patches regarding the error log sometimes call it error log
and sometimes event log. Can you consolidate that ?
> +int pseries_ioei_register_handler(pseries_ioei_handler_t handler)
> +{
Shouldn't we have a void * to attach with each client so it can put
its private data there and get them back in the handler ?
We are -really- re-inventing interrupts and/or notifiers here, which
I find a tad annoying..
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
From: Benjamin Herrenschmidt @ 2011-02-07 5:13 UTC (permalink / raw)
To: prasad; +Cc: linuxppc-dev, David Kleikamp
In-Reply-To: <20110207045230.GB4287@in.ibm.com>
On Mon, 2011-02-07 at 10:22 +0530, K.Prasad wrote:
> Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around
> the
> definition of 'set_debug_reg_defaults'.
Can you send a patch ?
> There's indeed too much sprinkling of #ifdefs in the code, but most of
> it would go away when the BookE code also uses the generic
> hw-breakpoint interfaces.
What's your status for those patches ?
> Given the advanced debug features that BookE supports, it's
> unfortunately not that straight-forward (needs additions to generic
> hw-breakpoint infrastructure).
Might want to move a lot of that code to a separate set of files maybe
and just call a single hook from the various process.c locations ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH V2 1/6] powerpc: Move udbg_early_init() after early_init_devtree()
From: David Gibson @ 2011-02-07 8:29 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296691225.12290.112.camel@shaggy-w500>
On Wed, Feb 02, 2011 at 06:00:25PM -0600, Dave Kleikamp wrote:
> On Thu, 2011-02-03 at 10:06 +1100, David Gibson wrote:
> > On Tue, Feb 01, 2011 at 12:48:41PM -0600, Dave Kleikamp wrote:
> > > so that it can use information from the device tree.
> >
> > Hrm. On the other hand this means that the early_init_devtree() code
> > can't benefit from hardcoded early debugging. Since you don't
> > actually appear to use devtree information in udbg_early_init() in the
> > latest series, I'd suggest dropping this patch.
>
> Patch 2 depends on early_init_devtree() being run. Until then, I don't
> know of a way to get at the bootargs.
Ah, yes. Drat.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: David Gibson @ 2011-02-07 8:30 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296774957.14077.23.camel@shaggy-w500>
On Thu, Feb 03, 2011 at 05:15:57PM -0600, Dave Kleikamp wrote:
> On Thu, 2011-02-03 at 16:03 +1100, David Gibson wrote:
> > On Wed, Feb 02, 2011 at 05:53:59PM -0600, Dave Kleikamp wrote:
> > > On Thu, 2011-02-03 at 10:08 +1100, David Gibson wrote:
> > > > On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> > > > > Since other OS's may be running on the other cores don't use tlbivax
> > > >
> > > > [snip]
> > > > > +#ifdef CONFIG_44x
> > > > > +void __init early_init_mmu_44x(void)
> > > > > +{
> > > > > + unsigned long root = of_get_flat_dt_root();
> > > > > + if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> > > > > + amp = 1;
> > > > > +}
> > > > > +#endif /* CONFIG_44x */
> > > >
> > > > A test against a hardcoded compatible string seems a nasty way to do
> > > > this. Maybe we should define a new boolean property for the root
> > > > node.
> > >
> > > I'm not crazy about this string, but I needed something in the device
> > > tree to key off of. Freescale has something similar (i.e.
> > > MPC8572DS-CAMP), so I chose to follow their example. I'd be happy to
> > > replace it with a boolean property. Any objection to just using
> > > "amp"?
> >
> > Bit too short, I think. I'd suggest either spelling out
> > 'asymmetric-multiprocessor' or 'cooperative-partition' (a more
> > accurate term, IMO).
>
> I could be wrong, but I thought the A stands for Asynchronous, not
> Asymmetric. I thought Asymmetric means that different types of tasks
> run on the secondary processors, as on the Cell.
Yeah, I thought so too, but Freescale at least seem to use it this
way. 'Asynchronous' would make even less sense.
> Anyway, going with
> 'cooperative-partition' would avoid that confusion.
>
> Shaggy
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH] sched: provide scheduler_ipi() callback in response to smp_send_reschedule()
From: Peter Zijlstra @ 2011-02-07 13:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m32r-ja, linux-mips, linux-ia64, linux-sh, H. Peter Anvin,
Heiko Carstens, Paul Mackerras, Helge Deller, sparclinux,
Linux-Arch, linux-s390, Jesper Nilsson, Jeremy Fitzhardinge,
Russell King, Hirokazu Takata, x86, James E.J. Bottomley,
virtualization, Ingo Molnar, Matt Turner, Fenghua Yu,
Mike Frysinger, user-mode-linux-devel, Konrad Rzeszutek Wilk,
Jeff Dike, Chris Metcalf, xen-devel, Mikael Starvik, linux-m32r,
Ivan Kokshaysky, user-mode-linux-user, uclinux-dist-devel,
Thomas Gleixner, linux-arm-kernel, Richard Henderson, Tony Luck,
linux-parisc, linux-cris-kernel, linux-am33-list, linux-kernel,
Ralf Baechle, Kyle McMartin, Paul Mundt, linux-alpha,
Martin Schwidefsky, linux390, Koichi Yasutake, linuxppc-dev,
David S. Miller
In-Reply-To: <1297034792.14982.10.camel@pasglop>
On Mon, 2011-02-07 at 10:26 +1100, Benjamin Herrenschmidt wrote:
> You missed:
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 9813605..467d122 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -98,6 +98,7 @@ void smp_message_recv(int msg)
> break;
> case PPC_MSG_RESCHEDULE:
> /* we notice need_resched on exit */
> + scheduler_ipi();
> break;
> case PPC_MSG_CALL_FUNC_SINGLE:
> generic_smp_call_function_single_interrupt();
>
> Fold that in and add:
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Thanks Ben!
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 14:13 UTC (permalink / raw)
To: Mark Brown
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <1297081335-13631-1-git-send-email-broonie@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]
Hi Mark,
On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 2657299..99e3c52 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -1,23 +1,6 @@
> config PM
> - bool "Power Management support"
> - depends on !IA64_HP_SIM
> - ---help---
> - "Power Management" means that parts of your computer are shut
> - off or put into a power conserving "sleep" mode if they are not
> - being used. There are two competing standards for doing this: APM
> - and ACPI. If you want to use either one, say Y here and then also
> - to the requisite support below.
> -
> - Power Management is most important for battery powered laptop
> - computers; if you have a laptop, check out the Linux Laptop home
> - page on the WWW at <http://www.linux-on-laptops.com/> or
> - Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
> - and the Battery Powered Linux mini-HOWTO, available from
> - <http://www.tldp.org/docs.html#howto>.
> -
> - Note that, even if you say N here, Linux on the x86 architecture
> - will issue the hlt instruction if nothing is to be done, thereby
> - sending the processor to sleep and saving power.
> + bool
> + default y if !IA64_HP_SIM
Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
server configs), so this will unexpectedly turn it on for them.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 14:18 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110208011324.d5371c4c.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > + bool
> > + default y if !IA64_HP_SIM
> Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> server configs), so this will unexpectedly turn it on for them.
Do you mean that these systems require CONFIG_PM be turned off, or just
that people tend not to turn it on? If the latter would you expect any
ill effects from doing so?
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 14:44 UTC (permalink / raw)
To: Mark Brown
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110207141829.GK10564@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
Hi Mark,
On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> > On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> > > + bool
> > > + default y if !IA64_HP_SIM
>
> > Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> > server configs), so this will unexpectedly turn it on for them.
>
> Do you mean that these systems require CONFIG_PM be turned off, or just
> that people tend not to turn it on? If the latter would you expect any
> ill effects from doing so?
I don't know the answer to either question without testing. All I am
saying is that currently the default for CONFIG_PM is "off" and you are
changing it to be "on" and there may not have been any testing done of
that in some situations. We don't know where it was explicitly
turned off any more since we shrank our defconfig files (which was done
automatically) ... since it is off by default, it doesn't need to be
mentioned in a defconfig unless it needs to be turned on.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 14:50 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110208014432.2c9288e4.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > Do you mean that these systems require CONFIG_PM be turned off, or just
> > that people tend not to turn it on? If the latter would you expect any
> > ill effects from doing so?
> I don't know the answer to either question without testing. All I am
> saying is that currently the default for CONFIG_PM is "off" and you are
> changing it to be "on" and there may not have been any testing done of
> that in some situations. We don't know where it was explicitly
> turned off any more since we shrank our defconfig files (which was done
> automatically) ... since it is off by default, it doesn't need to be
> mentioned in a defconfig unless it needs to be turned on.
My suspicion would be that it'll have been turned off by someone hitting
return through a config upgrade rather than through deliberate effort.
On the other hand if it is essential for some machines to have it
disabled they probably want to have somethnig in Kconfig.
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Geert Uytterhoeven @ 2011-02-07 15:00 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110207145031.GL10564@opensource.wolfsonmicro.com>
On Mon, Feb 7, 2011 at 15:50, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
>> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfson=
micro.com> wrote:
>
>> > Do you mean that these systems require CONFIG_PM be turned off, or jus=
t
>> > that people tend not to turn it on? =C2=A0If the latter would you expe=
ct any
>> > ill effects from doing so?
>
>> I don't know the answer to either question without testing. =C2=A0All I =
am
>> saying is that currently the default for CONFIG_PM is "off" and you are
>> changing it to be "on" and there may not have been any testing done of
>> that in some situations. =C2=A0 We don't know where it was explicitly
>> turned off any more since we shrank our defconfig files (which was done
>> automatically) ... since it is off by default, it doesn't need to be
>> mentioned in a defconfig unless it needs to be turned on.
>
> My suspicion would be that it'll have been turned off by someone hitting
> return through a config upgrade rather than through deliberate effort.
> On the other hand if it is essential for some machines to have it
> disabled they probably want to have somethnig in Kconfig.
$ git grep "CONFIG_PM is not set"
7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
256
$
7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
savedefconfig, so that's a safe revision with untrimmed defconfigs.
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 15:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Len Brown, Dmitry Torokhov, linux-embedded, Mark Brown,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <AANLkTikw7j5Q7CGZK=Xbcgubt9LYBGiXf01wQ4nbg8NR@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
Hi Geert,
On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> $ git grep "CONFIG_PM is not set"
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> 256
> $
>
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> savedefconfig, so that's a safe revision with untrimmed defconfigs.
Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
defconfigs, or just off because noone explicitly turned it on.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 15:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Len Brown, Dmitry Torokhov, linux-embedded, Mark Brown,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110208021045.699654ca.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
On Tue, 8 Feb 2011 02:10:45 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > $ git grep "CONFIG_PM is not set"
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> > 256
> > $
> >
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> > savedefconfig, so that's a safe revision with untrimmed defconfigs.
>
> Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
> defconfigs, or just off because noone explicitly turned it on.
At least some of the powerpc defconfigs were added with CONFIG_PM
disabled. I assume that was on purpose (though it may not have been).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 15:21 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Geert Uytterhoeven, Alan Stern, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <20110208021916.68b33b37.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
> At least some of the powerpc defconfigs were added with CONFIG_PM
> disabled. I assume that was on purpose (though it may not have been).
I'd not be so sure - since it's a bool without an explicit default set
Kconfig will default to disabling it and if anything enabling it is the
option that requires special effort.
^ permalink raw reply
* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Timur Tabi @ 2011-02-07 15:13 UTC (permalink / raw)
To: Dan Malek; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9E25355F-62B4-46A3-9991-FDE89FB6B862@digitaldans.com>
Dan Malek wrote:
> So, I did a little research. It appears in the case of PowerPC,
> the GFP_DMA can change the way the allocation operates.
> Since the coherent allocator works with pages of memory,
> in the case of a system with highmem, not using GFP_DMA
> could hand you a physical page out of the highmem pool.
I think that's true only if SWIOTLB is enabled. dma_direct_alloc_coherent()
does this:
/* ignore region specifiers */
flag &= ~(__GFP_HIGHMEM);
> This behavior is modified if you specify a restricted DMA
> mask for the device. In this case, dma_alloc_coherent
> will force GFP_DMA on your behalf (on PowerPC).
Isn't it required for all callers of dma_alloc_coherent to specify a mask (via
dma_set_mask) first?
--
Timur Tabi
Linux kernel developer at Freescale
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Alan Stern @ 2011-02-07 15:36 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Geert Uytterhoeven, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <20110207152132.GM10564@opensource.wolfsonmicro.com>
On Mon, 7 Feb 2011, Mark Brown wrote:
> On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
>
> > At least some of the powerpc defconfigs were added with CONFIG_PM
> > disabled. I assume that was on purpose (though it may not have been).
>
> I'd not be so sure - since it's a bool without an explicit default set
> Kconfig will default to disabling it and if anything enabling it is the
> option that requires special effort.
This may be a naive suggestion, but have you considered simply _asking_
the people who added those defconfigs?
Alan Stern
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 15:49 UTC (permalink / raw)
To: Alan Stern
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Geert Uytterhoeven, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <Pine.LNX.4.44L0.1102071035320.1924-100000@iolanthe.rowland.org>
On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> On Mon, 7 Feb 2011, Mark Brown wrote:
> > I'd not be so sure - since it's a bool without an explicit default set
> > Kconfig will default to disabling it and if anything enabling it is the
> > option that requires special effort.
> This may be a naive suggestion, but have you considered simply _asking_
> the people who added those defconfigs?
I'm rather hoping that they'll notice the mailing list thread or that
someone else who knows what's going on with them does - as Geert pointed
out there's a considerable number of defconfigs that have this turned
off. It seems more sensible to get some idea if this seems sane to
people in the general case before going trying to identify and contact
so many individuals.
If there are systems that really require disabling CONFIG_PM we probably
need to add stuff to Kconfig to make sure it can't be enabled anyway;
this shouldn't enable any new configurations.
^ permalink raw reply
* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-07 18:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1297035336.14982.14.camel@pasglop>
On 02/06/2011 05:35 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2011-02-04 at 17:25 -0600, Meador Inge wrote:
>> In a recent thread [1,2,3] concerning device trees for AMP systems, the
>> question of whether we really need 'protected-sources' arose. The general
>> consensus was that a new boolean property 'pic-no-reset' (described in more
>> detail in a following patch) could be expanded to cover the use cases that
>> 'protected-sources' was covering.
>>
>> One concern that was raised was for legacy systems which already use the
>> 'protected-sources' property [4]. For legacy use cases, 'protected-sources'
>> will be treated as an alias of 'pic-no-reset'. The sources
>> encoded in the 'protected-sources' cells, however, will not be processed. This
>> legacy check is added in a later patch in the series.
>
> I'm a bit annoyed here. Why do we need to do that ? Those Cell machines
> are going to be real bastards to find and test with, and I don't really
> see the point...
In my previous reply I said that "it is not so much as a need as it is a
potential simplification." After further reflection, I don't think that
is completely true. As we get into AMP systems with higher core counts,
then implementing this functionality using the existing
"protected-sources" implementation versus the new "pic-no-reset" work is
going to be harder to maintain.
The reason being that *every* OS instance has to know about *every*
other OSes interrupt sources, which is a little gross. You can see this
happening already in "arch/powerpc/boot/dts/p2020rdb_camp_core0.dts" and
"arch/powerpc/boot/dts/p2020rdb_camp_core1.dts":
// p2020rdb_camp_core0.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 1
protected-sources = <
42 76 77 78 79 /* serial1 , dma2 */
29 30 34 26 /* enet0, pci1 */
0xe0 0xe1 0xe2 0xe3 /* msi */
0xe4 0xe5 0xe6 0xe7
>;
};
// p2020rdb_camp_core1.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 0
protected-sources = <
17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
16 20 21 22 23 28 /* L2, dma1, USB */
03 35 36 40 31 32 33 /* mdio, enet1, enet2 */
72 45 58 25 /* sdhci, crypto , pci */
>;
};
It is going to be a real pain to keep all of the lists up to date.
Especially considering we already have sufficient information in the
device tree to do this work. I do understand the concern of
finding/testing the older systems. However, is the testing of those
systems enough to keep out the proposed change and potentially lower
maintenance in the future? Is the legacy system argument the only
reason to keep this change out or are there other technical deficiencies?
Also, in the proposed MPIC modifications there is a check for protected
sources (it is treated as an alias for "pic-no-reset"; see PATCH 3 in
the set) that should provide functionality equivalent to what systems
using "protected-sources" already have. That check only looks for the
presence of "protected-sources" and does not process the cells. Another
option would be to leave in the protected sources implementation (but
undocumented in the binding) and have the full "pic-no-reset" behavior
there as well (and documented in the binding).
If this has no chance of acceptance (?), then I will just re-submit the
binding and implementation with "protected-sources" and the limited form
of "pic-no-reset".
--
Meador Inge | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
^ permalink raw reply
* RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG CoreInterface Layer
From: Tirumala Marri @ 2011-02-07 18:45 UTC (permalink / raw)
To: David Laight; +Cc: linux-usb, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AC2C@saturn3.aculab.com>
-----Original Message-----
From: linuxppc-dev-bounces+tmarri=amcc.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=amcc.com@lists.ozlabs.org] On Behalf
Of David Laight
Sent: Wednesday, January 26, 2011 8:35 AM
Cc: linux-usb@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
Subject: RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG
CoreInterface Layer
> Also in_le32/out_le32/in_be32/out_be32 are
> architecture-specific AFAIK.
Isn't the whole patch architecture-specific ?
> I'd suggest using readl/writel for LE ops and
> __be32_to_cpu(__raw_readl(addr))/__raw_writel(__cpu_to_be32(b),addr)
> for BE ops.
Since the ppc doesn't have a byteswap instruction (and the LE
memory transfers might even be slow!) you really don't want to
be doing software byteswap.
Not to mention the horrific synchronistion instructions
that in_le32() and out_le32() actually contain.
I didn't find __raw_readl() when I was looking for asm
patterns that byteswapped memory transfers.
I did find st_le32() and ld_le32() in arch/powerpc/include/asm/swab.h
but that file is actually quite difficult to #include because
there is another swab.h that gets included instead.
[marri] Initial version of DWC-IP did not have feature to swap endianness.
Whereas next versions have it.
^ permalink raw reply
* RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG Core Interface Layer
From: Tirumala Marri @ 2011-02-07 18:53 UTC (permalink / raw)
To: Alexander Gordeev
Cc: greg, linux-usb, linuxppc-dev, Fushen Chen, Mark Miesfeld
In-Reply-To: <20110126191734.5dff9641@desktopvm.lvknet>
dwc_read_reg32 is used nowhere throughout the code. One of dwc_read32 and
dwc_read_reg32 should be removed IMO. There was once only dwc_read_reg32. In
version 5 of your patchset I believe. Why did you add another function?
AFAIK it is not correct to store pointers in u32 because they need 8 bytes
on 64-bit archs. So it was ok with the old dwc_read_reg32.
[Marri] If u32 is 8bytes isn't pointer type would be 8bytes as well. I had
change the API to avoid type castings to register addresses.
^ permalink raw reply
* RE: [PATCH V8 04/10] USB/ppc4xx: Add Synopsys DWC OTG HCD function
From: Tirumala Marri @ 2011-02-07 18:53 UTC (permalink / raw)
To: Alexander Gordeev
Cc: greg, linux-usb, linuxppc-dev, Fushen Chen, Mark Miesfeld
In-Reply-To: <20110126192743.646af6d2@desktopvm.lvknet>
}
dev_set_drvdata(_dev, dwc_otg_device);
hcd->regs = otg_dev->base;
+ hcd->rsrc_start = otg_dev->phys_addr;
+ hcd->rsrc_len = otg_dev->base_len;
hcd->self.otg_port = 1;
[Marri] Sure
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Rafael J. Wysocki @ 2011-02-07 19:16 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Alan Stern, Geert Uytterhoeven, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110207154953.GN10564@opensource.wolfsonmicro.com>
On Monday, February 07, 2011, Mark Brown wrote:
> On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> > On Mon, 7 Feb 2011, Mark Brown wrote:
>
> > > I'd not be so sure - since it's a bool without an explicit default set
> > > Kconfig will default to disabling it and if anything enabling it is the
> > > option that requires special effort.
>
> > This may be a naive suggestion, but have you considered simply _asking_
> > the people who added those defconfigs?
>
> I'm rather hoping that they'll notice the mailing list thread or that
> someone else who knows what's going on with them does - as Geert pointed
> out there's a considerable number of defconfigs that have this turned
> off. It seems more sensible to get some idea if this seems sane to
> people in the general case before going trying to identify and contact
> so many individuals.
>
> If there are systems that really require disabling CONFIG_PM we probably
> need to add stuff to Kconfig to make sure it can't be enabled anyway;
> this shouldn't enable any new configurations.
Well, as I've just said, I don't like this change. I'd very much prefer it if
CONFIG_PM_OPS were renamed to CONFIG_PM.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 21/23] hvc_console: Fix race between hvc_close and hvc_remove
From: Anton Blanchard @ 2011-02-07 21:16 UTC (permalink / raw)
To: Willy Tarreau
Cc: Rusty Russell, Greg Kroah-Hartman, linux-kernel, stable,
linuxppc-dev, Amit Shah, stable-review, Alan Cox
In-Reply-To: <20110206232253.421321729@pcw.home.local>
Hi,
> From: Amit Shah <amit.shah@redhat.com>
>
> commit e74d098c66543d0731de62eb747ccd5b636a6f4c upstream.
>
> Alan pointed out a race in the code where hvc_remove is invoked. The
> recent virtio_console work is the first user of hvc_remove().
I faintly remember this bug caused boot issues and the following patch
must be applied to fix it.
Anton
--
commit 320718ee074acce5ffced6506cb51af1388942aa
Author: Anton Blanchard <anton@samba.org>
Date: Tue Apr 6 21:42:38 2010 +1000
hvc_console: Fix race between hvc_close and hvc_remove
I don't claim to understand the tty layer, but it seems like hvc_open and
hvc_close should be balanced in their kref reference counting.
Right now we get a kref every call to hvc_open:
if (hp->count++ > 0) {
tty_kref_get(tty); <----- here
spin_unlock_irqrestore(&hp->lock, flags);
hvc_kick();
return 0;
} /* else count == 0 */
tty->driver_data = hp;
hp->tty = tty_kref_get(tty); <------ or here if hp->count was 0
But hvc_close has:
tty_kref_get(tty);
if (--hp->count == 0) {
...
/* Put the ref obtained in hvc_open() */
tty_kref_put(tty);
...
}
tty_kref_put(tty);
Since the outside kref get/put balance we only do a single kref_put when
count reaches 0.
The patch below changes things to call tty_kref_put once for every
hvc_close call, and with that my machine boots fine.
Signed-off-by: Anton Blanchard <anton@samba.org>
Acked-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index d3890e8..35cca4c 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -368,16 +368,12 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
hp = tty->driver_data;
spin_lock_irqsave(&hp->lock, flags);
- tty_kref_get(tty);
if (--hp->count == 0) {
/* We are done with the tty pointer now. */
hp->tty = NULL;
spin_unlock_irqrestore(&hp->lock, flags);
- /* Put the ref obtained in hvc_open() */
- tty_kref_put(tty);
-
if (hp->ops->notifier_del)
hp->ops->notifier_del(hp, hp->data);
^ permalink raw reply related
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