* [RFC PATCH] of_gpio: implement of_get_gpio_flags()
From: Anton Vorontsov @ 2008-07-25 16:48 UTC (permalink / raw)
To: Trent Piepho; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <Pine.LNX.4.64.0807231551230.26456@t2.domain.actdsltmp>
This function is alike to the of_get_gpio(), but accepts new argument:
flags. Now the of_get_gpio() call is a wrapper around of_get_gpio_flags().
This new function will be used by the drivers that need to retrive GPIO
information, such as active-low flag.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
On Wed, Jul 23, 2008 at 04:42:24PM -0700, Trent Piepho wrote:
> On Wed, 23 Jul 2008, Anton Vorontsov wrote:
> > On Mon, Jul 21, 2008 at 02:12:20PM -0700, Trent Piepho wrote:
> >> On Mon, 21 Jul 2008, Anton Vorontsov wrote:
> >>> On Sat, Jul 19, 2008 at 02:08:01PM -0700, Trent Piepho wrote:
> >>>> It doesn't look like you have any way to unset the active low flag. What if
> >>>> I unload the leds-gpio driver (or another gpio user) and then try to use the
> >>>> gpio with something else? The active low flag is stuck on!
> >>>
> >>> Why would you want to unset the flags? It is specified in the device
> >>> tree, and can't be changed. Specifying different flags for the same GPIO
> >>> would be an error (plus, Linux forbids shared gpios, so you simply can't
> >>> specify one gpio for several devices).
> >>
> >> You can't use the same gpio for two different things at the same time, but you
> >> can load a driver, unload it, and then load another.
> >
> > Hm.. yeah, this might happen. Now I tend to think that transparent
> > active-low handling wasn't that good idea after all. So, something like
> > of_gpio_is_active_low(device_node, gpioidx) should be implemented
> > instead. This function will parse the gpio's = <> flags. Please speak up
> > if you have any better ideas though.
>
> The flags could be provided via of_get_gpio. E.g., something like
> of_get_gpio(...., u32 *flags), and if flags is non-NULL the gpio flags are
> left there. of_get_gpio already has the flags and some other of_get_*
> functions return multiple things like this.
Good idea, thanks. This patch implements of_get_gpio_flags() in addition
to of_get_gpio() (since we don't want to break existing users).
The name seems a bit unfortunate though, because one could read the
function as it gets gpio flags only (though, we might implement
this instead, but this way average driver will end up with parsing
gpios = <> twice).
of_get_gpio_wflags() would be better. Or maybe leave it as is, since
it is documented anyway. ;-)
> Or just have an active low property for the led:
> led.active_low = !!of_get_property(np, "active-low", NULL);
>
> Pretty simple, just one line of code. At least if one looks just at
> leds-gpio, that's obviously the simplest way. Is active low a property of
> the led or a property of the gpio? I guess one could argue either way.
As I said previously, I'm not against this as a temporary solution.
Because we can always deprecate the active-low property later, in a
backward compatible way.
See http://ozlabs.org/pipermail/linuxppc-dev/2008-April/055202.html
So, assuming that the of_get_gpio_flags() will appear only in 2.6.28,
I'm fine with explicit active-low property for now.
> It seems like putting one line of code leds-gpio driver is better than
> putting much more complex code into the gpio controller driver. And each
> gpio controller has to have that code too.
>
> Now you could say that each gpio user needing to support inverting gpios is
> a lot of code too,
[...]
Quite the reverse, actually. ;-) I tend to agree. With this patch
every gpio controller will able to parse flags, w/o single line
added. So this approach is obviously better.
Plus, with transparent active-low handling, most drivers will end up
with checking active-low twice, because most drivers are doing if
(active_low) already.
Thanks.
drivers/of/gpio.c | 35 ++++++++++++++++++++++++++++-------
include/linux/of_gpio.h | 38 ++++++++++++++++++++++++++++++++++----
2 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 1c9cab8..5be67dd 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -19,14 +19,16 @@
#include <asm/prom.h>
/**
- * of_get_gpio - Get a GPIO number from the device tree to use with GPIO API
+ * of_get_gpio - Get a GPIO number and flags to use with GPIO API
* @np: device node to get GPIO from
* @index: index of the GPIO
+ * @flags: a flags pointer to fill in
*
* Returns GPIO number to use with Linux generic GPIO API, or one of the errno
- * value on the error condition.
+ * value on the error condition. This function also will fill in the flags.
*/
-int of_get_gpio(struct device_node *np, int index)
+int of_get_gpio_flags(struct device_node *np, int index,
+ enum of_gpio_flags *flags)
{
int ret = -EINVAL;
struct device_node *gc;
@@ -102,7 +104,11 @@ int of_get_gpio(struct device_node *np, int index)
goto err0;
}
- ret = of_gc->xlate(of_gc, np, gpio_spec);
+ /* .xlate might decide to not fill in the flags, so clear it here */
+ if (flags)
+ *flags = 0;
+
+ ret = of_gc->xlate(of_gc, np, gpio_spec, flags);
if (ret < 0)
goto err1;
@@ -113,26 +119,41 @@ err0:
pr_debug("%s exited with status %d\n", __func__, ret);
return ret;
}
-EXPORT_SYMBOL(of_get_gpio);
+EXPORT_SYMBOL(of_get_gpio_flags);
/**
- * of_gpio_simple_xlate - translate gpio_spec to the GPIO number
+ * of_gpio_simple_xlate - translate gpio_spec to the GPIO number and flags
* @of_gc: pointer to the of_gpio_chip structure
* @np: device node of the GPIO chip
* @gpio_spec: gpio specifier as found in the device tree
+ * @flags: a flags pointer to fill in
*
* This is simple translation function, suitable for the most 1:1 mapped
* gpio chips. This function performs only one sanity check: whether gpio
* is less than ngpios (that is specified in the gpio_chip).
*/
int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
- const void *gpio_spec)
+ const void *gpio_spec, enum of_gpio_flags *flags)
{
const u32 *gpio = gpio_spec;
+ /*
+ * We're discouraging gpio_cells < 2, since this way you'll have to
+ * write your own xlate function, which will retrive the GPIO number
+ * and its flags from a single gpio cell. This is possible, but not
+ * recommended.
+ */
+ if (of_gc->gpio_cells < 2) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
if (*gpio > of_gc->gc.ngpio)
return -EINVAL;
+ if (flags)
+ *flags = gpio[1];
+
return *gpio;
}
EXPORT_SYMBOL(of_gpio_simple_xlate);
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 67db101..8dc9bcb 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -14,19 +14,32 @@
#ifndef __LINUX_OF_GPIO_H
#define __LINUX_OF_GPIO_H
+#include <linux/compiler.h>
+#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/gpio.h>
+struct device_node;
+
#ifdef CONFIG_OF_GPIO
/*
+ * This is Linux-specific flags. By default controllers' and Linux' mapping
+ * match, but GPIO controllers are free to translate their own flags to
+ * Linux-specific in their .xlate callback. Though, 1:1 mapping is recommended.
+ */
+enum of_gpio_flags {
+ OF_GPIO_ACTIVE_LOW = 0x1,
+};
+
+/*
* Generic OF GPIO chip
*/
struct of_gpio_chip {
struct gpio_chip gc;
int gpio_cells;
int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
- const void *gpio_spec);
+ const void *gpio_spec, enum of_gpio_flags *flags);
};
static inline struct of_gpio_chip *to_of_gpio_chip(struct gpio_chip *gc)
@@ -50,20 +63,37 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc)
return container_of(of_gc, struct of_mm_gpio_chip, of_gc);
}
-extern int of_get_gpio(struct device_node *np, int index);
+extern int of_get_gpio_flags(struct device_node *np, int index,
+ enum of_gpio_flags *flags);
+
extern int of_mm_gpiochip_add(struct device_node *np,
struct of_mm_gpio_chip *mm_gc);
extern int of_gpio_simple_xlate(struct of_gpio_chip *of_gc,
struct device_node *np,
- const void *gpio_spec);
+ const void *gpio_spec,
+ enum of_gpio_flags *flags);
#else
/* Drivers may not strictly depend on the GPIO support, so let them link. */
-static inline int of_get_gpio(struct device_node *np, int index)
+static inline int of_get_gpio_flags(struct device_node *np, int index,
+ enum of_gpio_flags *flags)
{
return -ENOSYS;
}
#endif /* CONFIG_OF_GPIO */
+/**
+ * of_get_gpio - Get a GPIO number to use with GPIO API
+ * @np: device node to get GPIO from
+ * @index: index of the GPIO
+ *
+ * Returns GPIO number to use with Linux generic GPIO API, or one of the errno
+ * value on the error condition.
+ */
+static inline int of_get_gpio(struct device_node *np, int index)
+{
+ return of_get_gpio_flags(np, index, NULL);
+}
+
#endif /* __LINUX_OF_GPIO_H */
--
1.5.5.4
^ permalink raw reply related
* Re: [PATCH 1/4][V2] powerpc : add support for linux, usable-memory properties for drconf memory
From: Chandru @ 2008-07-25 16:51 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Michael Neuling, Stephen Rothwell, linuxppc-dev
In-Reply-To: <18565.42340.817405.289271@cargo.ozlabs.ibm.com>
On Tuesday 22 July 2008 14:46:20 Paul Mackerras wrote:
> Chandru writes:
>
> > Scan for linux,usable-memory properties in case of dynamic reconfiguration
> > memory . Support for kexec/kdump.
> >
> > Signed-off-by: Chandru Siddalingappa <chandru@in.ibm.com>
>
> Could we *please* have a more comprehensive patch description that
> that? Something which will help people coming along in two (or five
> or ten) years time to understand what problem exists in the code, how
> this patch solves it, and why this approach was chosen over any
> alternative approaches?
>
> Thanks,
> Paul.
>
Another alternate approach could be to create one 'linux,usable-drconf-memory' property and add all the usable memory regions into it, in a similar fashion to ibm,dynamic-memory property. For a given lmb in ibm,dynamic-memory , a corresponding usable-memory entry could be created which will contain 1 or more of (base,size) duple. For each entry in this new 'linux,usable-drconf-memory' property, a counter within it will tell us how many (base,size) duple are available in it. Some part of the current code may get duplicated. Let me go back and check if I can work on a patch for this approach .
thx,
Chandru
^ permalink raw reply
* Re: [PATCH v3 1/4] of: adapt of_find_i2c_driver() to be usable by SPI also
From: Jon Smirl @ 2008-07-25 17:02 UTC (permalink / raw)
To: Grant Likely
Cc: spi-devel-general, akpm, dbrownell, linux-kernel, linuxppc-dev
In-Reply-To: <fa686aa40807250921v73bec21cya6b4cd494f12f2e8@mail.gmail.com>
On 7/25/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jul 25, 2008 at 11:40 AM, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 7/25/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> >> From: Grant Likely <grant.likely@secretlab.ca>
> >>
>
> >> + * At the moment, a single table is used for all bus types because it is
> >> + * assumed that the data size is small and that the compatible values
> >> + * should already be distinct enough to differentiate between SPI, I2C
> >> + * and other devices.
> >
> > Maybe add a section recommending to update the alias list in the linux
> > device driver before adding entries here? This table should be a last
> > resort. I'm not even sure this table should exist, what would be a
> > case where we would need to make an entry here instead of fixing the
> > device driver by adding an alias name?
>
>
> In principle I agree. However, this patch is simply porting the i2c
> specific code to something that can be used by both SPI and I2C. I
> don't want to rework the actual mechanism in this particular patch. I
> can submit an additional patch to change this along with reworking
> some of the behavior that needs to be improved.
>
>
> >> + * First method is to lookup the compatible value in of_modalias_table.
> >> + * Second is to look for a "linux,<modalias>" entry in the compatible list
> >> + * and used that for modalias. Third is to strip off the manufacturer
> >> + * prefix from the first compatible entry and use the remainder as modalias
> >
> > I also think this is a problem. Embedding the name of Linux device
> > drivers into device firmware makes it almost impossible to rename the
> > device driver. Again, what is a case where generic part numbers can't
> > be listed in the alias section of the linux device driver?
> >
> > Even eeprom was just fixed to take generic part numbers (at24).
>
>
> Again, I agree, but this change is very much a stop gap measure to get
> things working in a sane way without having to create bad device tree
> bindings (device tree bindings are hard to change, code is not). I've
> been considering posting a patch to remove this clause from the
> functions, but that needs to be reviewed separately from this change.
Isn't putting "compatible="linux,modalias"" into your device tree a
really bad idea?
>
> Thanks,
>
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFC] cpm_uart: Add generic clock API support to set baudrates
From: Kumar Gala @ 2008-07-25 17:39 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linuxppc-dev list
In-Reply-To: <200807241405.21770.laurentp@cse-semaphore.com>
On Jul 24, 2008, at 7:05 AM, Laurent Pinchart wrote:
> This patch introduces baudrate setting support via the generic clock
> API.
> When present the optional device tree clock property is used instead
> of
> fsl-cpm-brg. Platforms can then define complex clock schemes, to
> output
> the serial clock on an external pin for instance.
>
> This is required to support RS485 ports on a platform I'm working on
> (TBox
> CPU32 - see http://www.cse-semaphore.com/products/t-box/t-box-ms.php).
>
> ---
> drivers/serial/cpm_uart/cpm_uart.h | 1 +
> drivers/serial/cpm_uart/cpm_uart_core.c | 26 ++++++++++++++++++
> +-------
> 2 files changed, 20 insertions(+), 7 deletions(-)
I'm having problems applying due to mailer formatting.
- k
^ permalink raw reply
* Re: [PATCHv4] cpm2: Implement GPIO LIB API on CPM2 Freescale SoC.
From: Kumar Gala @ 2008-07-25 17:42 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Scott Wood, linuxppc-dev list
In-Reply-To: <200807241800.27146.laurentp@cse-semaphore.com>
On Jul 24, 2008, at 11:00 AM, Laurent Pinchart wrote:
> This patch implement GPIO LIB support for the CPM2 GPIOs. The code
> can also be
> used for CPM1 GPIO port E, as both cores are compatible at the
> register level.
>
> Based on earlier work by Jochen Friedrich.
>
> Signed-off-by: Laurent Pinchart <laurentp@cse-semaphore.com>
> Cc: Jochen Friedrich <jochen@scram.de>
> ---
> arch/powerpc/platforms/Kconfig | 2 +
> arch/powerpc/sysdev/cpm2.c | 11 ++++
> arch/powerpc/sysdev/cpm_common.c | 123 +++++++++++++++++++++++++++++
> +++++++++
> include/asm-powerpc/cpm.h | 3 +
> 4 files changed, 139 insertions(+), 0 deletions(-)
I'm having similar mailer issues with this patch.
- k
^ permalink raw reply
* Re: GPIO drivers, other patches?
From: Anton Vorontsov @ 2008-07-25 17:54 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev list
In-Reply-To: <20080715174201.GA2231@polina.dev.rtsoft.ru>
On Tue, Jul 15, 2008 at 09:42:01PM +0400, Anton Vorontsov wrote:
> On Tue, Jul 15, 2008 at 08:00:48PM +0400, Anton Vorontsov wrote:
> > On Tue, Jul 15, 2008 at 10:10:21AM -0500, Kumar Gala wrote:
> > > Anton,
> > >
> > > I think I've gotten most of the patches from you,
> >
> > Yes, much thanks!
> >
> > > but I know there are
> > > few I've lost. Can you just repost any patches that aren't in my
> > > powerpc-next tree.
>
> While testing the tree in run-time found that this patch is missing:
>
> [PATCH 3/4] powerpc: rtc_cmos_setup: assign interrupts only if there is i8259 PIC
> http://patchwork.ozlabs.org/linuxppc/patch?id=18915
>
> Without this patch rtc will fail to probe on MPC8610HPCD:
>
> rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
> rtc_cmos: probe of rtc_cmos failed with error -38
Kumar, sorry for bothering you again... are there any open issues
with the patch, can we merge it?
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [git pull] Please pull from powerpc.git merge branch
From: Linus Torvalds @ 2008-07-25 18:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, akpm, Linux Kernel list
In-Reply-To: <1216973590.11188.99.camel@pasglop>
On Fri, 25 Jul 2008, Benjamin Herrenschmidt wrote:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
Ok, I got a conflict in arch/powerpc/platforms/52xx/Kconfig, you should
check that I fixed it up right and send me follow-up patches if required.
Linus
^ permalink raw reply
* Re: GPIO drivers, other patches?
From: Kumar Gala @ 2008-07-25 18:14 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev list
In-Reply-To: <20080725175410.GA12278@polina.dev.rtsoft.ru>
On Jul 25, 2008, at 12:54 PM, Anton Vorontsov wrote:
> On Tue, Jul 15, 2008 at 09:42:01PM +0400, Anton Vorontsov wrote:
>> On Tue, Jul 15, 2008 at 08:00:48PM +0400, Anton Vorontsov wrote:
>>> On Tue, Jul 15, 2008 at 10:10:21AM -0500, Kumar Gala wrote:
>>>> Anton,
>>>>
>>>> I think I've gotten most of the patches from you,
>>>
>>> Yes, much thanks!
>>>
>>>> but I know there are
>>>> few I've lost. Can you just repost any patches that aren't in my
>>>> powerpc-next tree.
>>
>> While testing the tree in run-time found that this patch is missing:
>>
>> [PATCH 3/4] powerpc: rtc_cmos_setup: assign interrupts only if
>> there is i8259 PIC
>> http://patchwork.ozlabs.org/linuxppc/patch?id=18915
>>
>> Without this patch rtc will fail to probe on MPC8610HPCD:
>>
>> rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
>> rtc_cmos: probe of rtc_cmos failed with error -38
>
> Kumar, sorry for bothering you again... are there any open issues
> with the patch, can we merge it?
I'd like Ben to ack it.
- k
^ permalink raw reply
* Re: [PATCH v3 4/4] powerpc/mpc5200: Add mpc5200-spi (non-PSC) device driver
From: Daniel Walker @ 2008-07-25 18:19 UTC (permalink / raw)
To: Grant Likely
Cc: dbrownell, linux-kernel, linuxppc-dev, spi-devel-general, akpm
In-Reply-To: <20080725073326.8485.99210.stgit@trillian.secretlab.ca>
On Fri, 2008-07-25 at 03:33 -0400, Grant Likely wrote:
> + if (status && (irq != NO_IRQ))
> + dev_err(&ms->master->dev, "spurious irq, status=0x%.2x\n",
> + status);
> +
> + /* Check if there is another transfer waiting */
> + if (list_empty(&ms->queue))
> + return FSM_STOP;
I don't think doing list_empty outside the critical section is totally
safe.. You might want to move it down inside the spin_lock() section.
> + /* Get the next message */
> + spin_lock(&ms->lock);
The part that's a little confusing here is that the interrupt can
actually activate the workqueue .. So I'm wondering if maybe you could
have this interrupt driven any workqueue driven at the same time? If you
could then you would need the above to be
spin_lock_irq/spin_lock_irqsave ..
Daniel
^ permalink raw reply
* [PATCH] Allow non-hcall return values for lparcfg writes
From: Nathan Fontenot @ 2008-07-25 18:27 UTC (permalink / raw)
To: linuxppc-dev
The code to handle writes to /proc/ppc64/lparcfg incorrectly
assumes that the return code from the helper routines to update
processor or memory entitlement return a hcall return value. It
then assumes any non-hcall return value is bad and sets the return
code for the write to be -EIO.
The update_[mp]pp routines can return values other than a hcall
return value. This patch removes the automatic setting of any
return code that is not an hcall return value from these routines
to -EIO.
NOTE: This patch applies on top of the CMO patches and my previous
patch for lparcfg.c.
Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
---
Index: linux-2.6.git/arch/powerpc/kernel/lparcfg.c
===================================================================
--- linux-2.6.git.orig/arch/powerpc/kernel/lparcfg.c 2008-07-23 13:40:50.000000000 -0500
+++ linux-2.6.git/arch/powerpc/kernel/lparcfg.c 2008-07-25 13:06:06.000000000 -0500
@@ -636,10 +636,6 @@
retval = -EIO;
} else if (retval == H_PARAMETER) {
retval = -EINVAL;
- } else {
- printk(KERN_WARNING "%s: received unknown hv return code %ld",
- __func__, retval);
- retval = -EIO;
}
return retval;
^ permalink raw reply
* Re: GPIO drivers, other patches?
From: Anton Vorontsov @ 2008-07-25 18:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
In-Reply-To: <FC85A2C9-A382-4E26-8A0A-33CD324DB632@kernel.crashing.org>
On Fri, Jul 25, 2008 at 01:14:58PM -0500, Kumar Gala wrote:
>
> On Jul 25, 2008, at 12:54 PM, Anton Vorontsov wrote:
>
>> On Tue, Jul 15, 2008 at 09:42:01PM +0400, Anton Vorontsov wrote:
>>> On Tue, Jul 15, 2008 at 08:00:48PM +0400, Anton Vorontsov wrote:
>>>> On Tue, Jul 15, 2008 at 10:10:21AM -0500, Kumar Gala wrote:
>>>>> Anton,
>>>>>
>>>>> I think I've gotten most of the patches from you,
>>>>
>>>> Yes, much thanks!
>>>>
>>>>> but I know there are
>>>>> few I've lost. Can you just repost any patches that aren't in my
>>>>> powerpc-next tree.
>>>
>>> While testing the tree in run-time found that this patch is missing:
>>>
>>> [PATCH 3/4] powerpc: rtc_cmos_setup: assign interrupts only if there
>>> is i8259 PIC
>>> http://patchwork.ozlabs.org/linuxppc/patch?id=18915
>>>
>>> Without this patch rtc will fail to probe on MPC8610HPCD:
>>>
>>> rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
>>> rtc_cmos: probe of rtc_cmos failed with error -38
>>
>> Kumar, sorry for bothering you again... are there any open issues
>> with the patch, can we merge it?
>
> I'd like Ben to ack it.
Benjamin, could you please take a look at this patch?
[PATCH 3/4] powerpc: rtc_cmos_setup: assign interrupts only if there is i8259 PIC
http://patchwork.ozlabs.org/linuxppc/patch?id=18915
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: CONFIG_FRAME_POINTER [was [PATCH] x86: BUILD_IRQ say .text]
From: Hugh Dickins @ 2008-07-25 18:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: the arch/x86 maintainers, Linuxppc-dev, linux-kernel, Mike Travis
In-Reply-To: <20080724104459.GI28817@elte.hu>
On Thu, 24 Jul 2008, Ingo Molnar wrote:
> * Hugh Dickins <hugh@veritas.com> wrote:
>
> > I've been using -fno-unit-at-a-time (to lessen inlining, for easier
> > debugging) for a long time
>
> Should we perhaps enable this automatically on CONFIG_FRAME_POINTER=y
> builds? Although a separate, default-off config option might be better,
I'm content to go on patching the Makefile to suit my own private debug
proclivities, somewhat reluctant to foist them upon others. But I must
have found no-unit-at-a-time made things clearer at some point,
and assume it is still being useful.
Whether it deserves its own config option, hmm, not sure of that.
But agree it shouldn't really go into CONFIG_FRAME_POINTER=y
(though notice that already bundles no-optimize-sibling-calls).
I rather think CONFIG_FRAME_POINTER shouldn't exist at all (or be
a private, config-user-invisible, specific-to-a-few-arches thing):
what one wants to configure is how far to sacrifice cpu performance
and kernel smallness to getting a good stacktrace. Frame pointer
is just an implementation detail on that, appropriate to some arches.
Perhaps three settings: no stacktrace, fair stacktrace, best stacktrace.
I've Cc'ed Ben and linuxppc-dev because I wonder if they're aware
that several options (I got it from LATENCYTOP, but I think LOCKDEP
and FTRACE and some others) are doing a "select FRAME_POINTER",
which forces CONFIG_FRAME_POINTER=y on PowerPC, even though
FRAME_POINTER is not an option offered on PowerPC. The
resulting kernels appear to run okay, but I was surprised.
> i'd not be surprised if there were more regressions in this area, it's a
> seldom used build vector.
There'd probably be a lot more if we weren't (recently?) supporting
compilers which only manage no-unit-at-a-time. The only issue I get
in my kernel builds is with sched_clock() - but I've not been trying
allyesconfig or randconfig. Here's a patch: but whereas the BUILD_IRQ
one seemed worth sending because the problem might crop up in other
ways and waste someone's time, this one just depends on your taste.
[PATCH] sched: move sched_clock before first use
Move sched_clock() up to stop warning: weak declaration of `sched_clock'
after first use results in unspecified behavior (if -fno-unit-at-a-time).
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
kernel/sched_clock.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
--- 2.6.26-git/kernel/sched_clock.c 2008-07-18 11:33:34.000000000 +0100
+++ linux/kernel/sched_clock.c 2008-07-18 11:57:46.000000000 +0100
@@ -32,6 +32,15 @@
#include <linux/ktime.h>
#include <linux/module.h>
+/*
+ * Scheduler clock - returns current time in nanosec units.
+ * This is default implementation.
+ * Architectures and sub-architectures can override this.
+ */
+unsigned long long __attribute__((weak)) sched_clock(void)
+{
+ return (unsigned long long)jiffies * (NSEC_PER_SEC / HZ);
+}
#ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK
@@ -321,16 +330,6 @@ EXPORT_SYMBOL_GPL(sched_clock_idle_wakeu
#endif
-/*
- * Scheduler clock - returns current time in nanosec units.
- * This is default implementation.
- * Architectures and sub-architectures can override this.
- */
-unsigned long long __attribute__((weak)) sched_clock(void)
-{
- return (unsigned long long)jiffies * (NSEC_PER_SEC / HZ);
-}
-
unsigned long long cpu_clock(int cpu)
{
unsigned long long clock;
^ permalink raw reply
* Re: [PATCH v3 1/4] of: adapt of_find_i2c_driver() to be usable by SPI also
From: Grant Likely @ 2008-07-25 18:52 UTC (permalink / raw)
To: Jon Smirl; +Cc: spi-devel-general, akpm, dbrownell, linux-kernel, linuxppc-dev
In-Reply-To: <9e4733910807251002w7da115e5r53600f1cf4e3891@mail.gmail.com>
On Fri, Jul 25, 2008 at 1:02 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 7/25/08, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Fri, Jul 25, 2008 at 11:40 AM, Jon Smirl <jonsmirl@gmail.com> wrote:
>> > On 7/25/08, Grant Likely <grant.likely@secretlab.ca> wrote:
>> >> + * First method is to lookup the compatible value in of_modalias_table.
>> >> + * Second is to look for a "linux,<modalias>" entry in the compatible list
>> >> + * and used that for modalias. Third is to strip off the manufacturer
>> >> + * prefix from the first compatible entry and use the remainder as modalias
>> >
>> > I also think this is a problem. Embedding the name of Linux device
>> > drivers into device firmware makes it almost impossible to rename the
>> > device driver. Again, what is a case where generic part numbers can't
>> > be listed in the alias section of the linux device driver?
>> >
>> > Even eeprom was just fixed to take generic part numbers (at24).
>>
>> Again, I agree, but this change is very much a stop gap measure to get
>> things working in a sane way without having to create bad device tree
>> bindings (device tree bindings are hard to change, code is not). I've
>> been considering posting a patch to remove this clause from the
>> functions, but that needs to be reviewed separately from this change.
>
> Isn't putting "compatible="linux,modalias"" into your device tree a
> really bad idea?
Yes, it is, but I still need to preserve existing behavior in this
patch. That change needs to be reviewed separately. I'll submit
another patch to deal with this.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [git pull] Please pull from powerpc.git merge branch
From: Grant Likely @ 2008-07-25 18:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: akpm, Linux Kernel list, linuxppc-dev list
In-Reply-To: <alpine.LFD.1.10.0807251108320.5249@nehalem.linux-foundation.org>
T24gRnJpLCBKdWwgMjUsIDIwMDggYXQgMjoxMCBQTSwgTGludXMgVG9ydmFsZHMKPHRvcnZhbGRz
QGxpbnV4LWZvdW5kYXRpb24ub3JnPiB3cm90ZToKPgo+Cj4gT24gRnJpLCAyNSBKdWwgMjAwOCwg
QmVuamFtaW4gSGVycmVuc2NobWlkdCB3cm90ZToKPj4KPj4g77u/Z2l0Oi8vZ2l0Lmtlcm5lbC5v
cmcvcHViL3NjbS9saW51eC9rZXJuZWwvZ2l0L2JlbmgvcG93ZXJwYy5naXQgbWVyZ2UKPgo+IE9r
LCBJIGdvdCBhIGNvbmZsaWN0IGluIGFyY2gvcG93ZXJwYy9wbGF0Zm9ybXMvNTJ4eC9LY29uZmln
LCB5b3Ugc2hvdWxkCj4gY2hlY2sgdGhhdCBJIGZpeGVkIGl0IHVwIHJpZ2h0IGFuZCBzZW5kIG1l
IGZvbGxvdy11cCBwYXRjaGVzIGlmIHJlcXVpcmVkLgoKT2theSwgSSdsbCBkbyB0aGF0CgpnLgoK
LS0gCkdyYW50IExpa2VseSwgQi5TYy4sIFAuRW5nLgpTZWNyZXQgTGFiIFRlY2hub2xvZ2llcyBM
dGQuCg==
^ permalink raw reply
* Re: [PATCH v3 2/4] spi: split up spi_new_device() to allow two stage registration.
From: David Brownell @ 2008-07-25 19:00 UTC (permalink / raw)
To: Grant Likely; +Cc: spi-devel-general, akpm, linux-kernel, linuxppc-dev
In-Reply-To: <20080725073316.8485.84001.stgit@trillian.secretlab.ca>
On Friday 25 July 2008, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> spi_new_device() allocates and registers an spi device all in one swoop.
> If the driver needs to add extra data to the spi_device before it is
> registered, then this causes problems.
Mention an example please ... you have dev->archdata in mind, yes?
> This patch splits the allocation and registration portions of code out
> of spi_new_device() and creates three new functions; spi_alloc_device(),
> spi_register_device(), and spi_device_release().
Comment needs fixing: *TWO* new functions, spi_device_release() was
not needed ...
> spi_new_device() is
> modified to use the new functions for allocation and registration.
> None of the existing users of spi_new_device() should be affected by
> this change.
>
> Drivers using the new API can forego the use of an spi_board_info
Strike "an". :)
> structure to describe the device layout and populate data into the
> spi_device structure directly.
>
> This change is in preparation for adding an OF device tree parser to
> generate spi_devices based on data in the device tree.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Given the comment fixes noted above and below:
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
Thanks.
> ---
>
> drivers/spi/spi.c | 139 ++++++++++++++++++++++++++++++++---------------
> include/linux/spi/spi.h | 10 +++
> 2 files changed, 105 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ecca4a6..2077ef5 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -178,6 +178,96 @@ struct boardinfo {
> static LIST_HEAD(board_list);
> static DEFINE_MUTEX(board_lock);
>
> +/**
> + * spi_alloc_device - Allocate a new SPI device
> + * @master: Controller to which device is connected
> + * Context: can sleep
> + *
> + * Allows a driver to allocate and initialize and spi_device without
"a" spi_device
> + * registering it immediately. This allows a driver to directly
> + * fill the spi_device with device parameters before calling
> + * spi_add_device() on it.
> + *
> + * Caller is responsible to call spi_add_device() on the returned
> + * spi_device structure to add it to the SPI master. If the caller
> + * needs to discard the spi_device without adding it, then it should
> + * call spi_dev_put() on it.
> + *
> + * Returns a pointer to the new device, or NULL.
> + */
> +struct spi_device *spi_alloc_device(struct spi_master *master)
> +{
> + struct spi_device *spi;
> + struct device *dev = master->dev.parent;
> +
> + if (!spi_master_get(master))
> + return NULL;
> +
> + spi = kzalloc(sizeof *spi, GFP_KERNEL);
> + if (!spi) {
> + dev_err(dev, "cannot alloc spi_device\n");
> + spi_master_put(master);
> + return NULL;
> + }
> +
> + spi->master = master;
> + spi->dev.parent = dev;
> + spi->dev.bus = &spi_bus_type;
> + spi->dev.release = spidev_release;
> + device_initialize(&spi->dev);
> + return spi;
> +}
> +EXPORT_SYMBOL_GPL(spi_alloc_device);
> +
> +/**
> + * spi_add_device - Add an spi_device allocated with spi_alloc_device
Strike "an".
> + * @spi: spi_device to register
> + *
> + * Companion function to spi_alloc_device. Devices allocated with
> + * spi_alloc_device can be added onto the spi bus with this function.
> + *
> + * Returns 0 on success; non-zero on failure
> + */
> +int spi_add_device(struct spi_device *spi)
> +{
> + struct device *dev = spi->master->dev.parent;
> + int status;
> +
> + /* Chipselects are numbered 0..max; validate. */
> + if (spi->chip_select >= spi->master->num_chipselect) {
> + dev_err(dev, "cs%d > max %d\n",
Nit; "cs%d >= max %d" ... no need to carry this minor bug forward.
> + spi->chip_select,
> + spi->master->num_chipselect);
> + return -EINVAL;
> + }
> +
> + /* Set the bus ID string */
> + snprintf(spi->dev.bus_id, sizeof spi->dev.bus_id,
> + "%s.%u", spi->master->dev.bus_id,
> + spi->chip_select);
> +
> + /* drivers may modify this initial i/o setup */
> + status = spi->master->setup(spi);
Hmm, I just noticed this *pre-existing* bug: if there's already
a device with this chipselect (so the device_add will fail, later),
its configuration will be trashed here. That's not a reason to NAK
this patch. (I think the fix would be as simple as calling setup
in spi_drv_probe, and making sure that's always called even if for
some odd reason the driver doesn't have its own probe routine. If
you want to fix that, great ... otherwise I'll try to get it done
before 2.6.27 finishes.)
> + if (status < 0) {
> + dev_err(dev, "can't %s %s, status %d\n",
> + "setup", spi->dev.bus_id, status);
> + return status;
> + }
> +
> + /* driver core catches callers that misbehave by defining
> + * devices that already exist.
> + */
> + status = device_add(&spi->dev);
> + if (status < 0) {
> + dev_err(dev, "can't %s %s, status %d\n",
> + "add", spi->dev.bus_id, status);
> + return status;
> + }
> +
> + dev_dbg(dev, "registered child %s\n", spi->dev.bus_id);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_add_device);
>
> /**
> * spi_new_device - instantiate one new SPI device
> @@ -197,7 +287,6 @@ struct spi_device *spi_new_device(struct spi_master *master,
> struct spi_board_info *chip)
> {
> struct spi_device *proxy;
> - struct device *dev = master->dev.parent;
> int status;
>
> /* NOTE: caller did any chip->bus_num checks necessary.
> @@ -207,66 +296,28 @@ struct spi_device *spi_new_device(struct spi_master *master,
> * suggests syslogged diagnostics are best here (ugh).
> */
>
> - /* Chipselects are numbered 0..max; validate. */
> - if (chip->chip_select >= master->num_chipselect) {
> - dev_err(dev, "cs%d > max %d\n",
> - chip->chip_select,
> - master->num_chipselect);
> - return NULL;
> - }
> -
> - if (!spi_master_get(master))
> + proxy = spi_alloc_device(master);
> + if (!proxy)
> return NULL;
>
> WARN_ON(strlen(chip->modalias) >= sizeof(proxy->modalias));
>
> - proxy = kzalloc(sizeof *proxy, GFP_KERNEL);
> - if (!proxy) {
> - dev_err(dev, "can't alloc dev for cs%d\n",
> - chip->chip_select);
> - goto fail;
> - }
> - proxy->master = master;
> proxy->chip_select = chip->chip_select;
> proxy->max_speed_hz = chip->max_speed_hz;
> proxy->mode = chip->mode;
> proxy->irq = chip->irq;
> strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
> -
> - snprintf(proxy->dev.bus_id, sizeof proxy->dev.bus_id,
> - "%s.%u", master->dev.bus_id,
> - chip->chip_select);
> - proxy->dev.parent = dev;
> - proxy->dev.bus = &spi_bus_type;
> proxy->dev.platform_data = (void *) chip->platform_data;
> proxy->controller_data = chip->controller_data;
> proxy->controller_state = NULL;
> - proxy->dev.release = spidev_release;
>
> - /* drivers may modify this initial i/o setup */
> - status = master->setup(proxy);
> + status = spi_add_device(proxy);
> if (status < 0) {
> - dev_err(dev, "can't %s %s, status %d\n",
> - "setup", proxy->dev.bus_id, status);
> - goto fail;
> + spi_dev_put(proxy);
> + return NULL;
> }
>
> - /* driver core catches callers that misbehave by defining
> - * devices that already exist.
> - */
> - status = device_register(&proxy->dev);
> - if (status < 0) {
> - dev_err(dev, "can't %s %s, status %d\n",
> - "add", proxy->dev.bus_id, status);
> - goto fail;
> - }
> - dev_dbg(dev, "registered child %s\n", proxy->dev.bus_id);
> return proxy;
> -
> -fail:
> - spi_master_put(master);
> - kfree(proxy);
> - return NULL;
> }
> EXPORT_SYMBOL_GPL(spi_new_device);
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index a9cc29d..d203d08 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -778,8 +778,18 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
> * use spi_new_device() to describe each device. You can also call
> * spi_unregister_device() to start making that device vanish, but
> * normally that would be handled by spi_unregister_master().
> + *
> + * You can also use spi_alloc_device() and spi_add_device() to
> + * for
to, for, sex, ate ... voudou will appreciate!! ;)
Make that "to use" a two stage registration "sequence".
> a two stage registration of an SPI device. This gives the caller
> + * some more control over the spi_device structure before it is registered
"... before it is registered, but requires that caller to initialize fields that
would otherwise be defined using the board info."
> */
> extern struct spi_device *
> +spi_alloc_device(struct spi_master *master);
> +
> +extern int
> +spi_add_device(struct spi_device *spi);
> +
> +extern struct spi_device *
> spi_new_device(struct spi_master *, struct spi_board_info *);
>
> static inline void
>
^ permalink raw reply
* Re: [PATCH / RFC] net: don't grab a mutex within a timer context in gianfar
From: Andy Fleming @ 2008-07-25 19:02 UTC (permalink / raw)
To: Sebastian Siewior
Cc: Nate Case, netdev, linuxppc-dev, Vitaly Bordug, Li Yang,
Jeff Garzik
In-Reply-To: <20080723200337.GA5122@Chamillionaire.breakpoint.cc>
On Jul 23, 2008, at 16:03, Sebastian Siewior wrote:
> From: Sebastian Siewior <bigeasy@linutronix.de>
>
> I got the following backtrace while network was unavailble:
>
> |NETDEV WATCHDOG: eth0: transmit timed out
> |BUG: sleeping function called from invalid context at /home/bigeasy/
> git/linux-2.6-powerpc/kernel/mutex.c:87
> |in_atomic():1, irqs_disabled():0
> |Call Trace:
> |[c0383d90] [c0006dd8] show_stack+0x48/0x184 (unreliable)
> |[c0383db0] [c001e938] __might_sleep+0xe0/0xf4
> |[c0383dc0] [c025a43c] mutex_lock+0x24/0x3c
> |[c0383de0] [c019005c] phy_stop+0x20/0x70
> |[c0383df0] [c018d4ec] stop_gfar+0x28/0xf4
> |[c0383e10] [c018e8c4] gfar_timeout+0x30/0x60
> |[c0383e20] [c01fe7c0] dev_watchdog+0xa8/0x144
> |[c0383e30] [c002f93c] run_timer_softirq+0x148/0x1c8
> |[c0383e60] [c002b084] __do_softirq+0x5c/0xc4
> |[c0383e80] [c00046fc] do_softirq+0x3c/0x54
> |[c0383e90] [c002ac60] irq_exit+0x3c/0x5c
> |[c0383ea0] [c000b378] timer_interrupt+0xe0/0xf8
> |[c0383ec0] [c000e5ac] ret_from_except+0x0/0x18
> |[c0383f80] [c000804c] cpu_idle+0xcc/0xdc
> |[c0383fa0] [c025c07c] etext+0x7c/0x90
> |[c0383fc0] [c0338960] start_kernel+0x294/0x2a8
> |[c0383ff0] [c00003dc] skpinv+0x304/0x340
> |------------[ cut here ]------------
>
> The phylock was once a spinlock but got changed into a mutex via
> commit 35b5f6b1a aka [PHYLIB: Locking fixes for PHY I/O potentially
> sleeping]
>
> Signed-off-by: Sebastian Siewior <bigeasy@linutronix.de>
> ---
Looks good to me. Thanks for taking care of this.
Acked-by: Andy Fleming <afleming@freescale.com>
^ permalink raw reply
* [PATCH] powerpc: clean up the Book-E HW watchpoint support
From: Kumar Gala @ 2008-07-25 19:27 UTC (permalink / raw)
To: linuxppc-dev
* CONFIG_BOOKE is selected by CONFIG_44x so we dont need both
* Fixed a few comments
* Go back to only using DBCR0_IDM to determine if we are using
debug resources.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Luis, can you test this on 44x. I don't expect any breakage.
- k
arch/powerpc/kernel/entry_32.S | 6 +++---
arch/powerpc/kernel/process.c | 8 ++++----
arch/powerpc/kernel/ptrace.c | 7 ++++---
arch/powerpc/kernel/signal.c | 2 +-
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 81c8324..da52269 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -148,7 +148,7 @@ transfer_to_handler:
/* Check to see if the dbcr0 register is set up to debug. Use the
internal debug mode bit to do this. */
lwz r12,THREAD_DBCR0(r12)
- andis. r12,r12,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
+ andis. r12,r12,DBCR0_IDM@h
beq+ 3f
/* From user and task is ptraced - load up global dbcr0 */
li r12,-1 /* clear all pending debug events */
@@ -292,7 +292,7 @@ syscall_exit_cont:
/* If the process has its own DBCR0 value, load it up. The internal
debug mode bit tells us that dbcr0 should be loaded. */
lwz r0,THREAD+THREAD_DBCR0(r2)
- andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
+ andis. r10,r0,DBCR0_IDM@h
bnel- load_dbcr0
#endif
#ifdef CONFIG_44x
@@ -720,7 +720,7 @@ restore_user:
/* Check whether this process has its own DBCR0 value. The internal
debug mode bit tells us that dbcr0 should be loaded. */
lwz r0,THREAD+THREAD_DBCR0(r2)
- andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
+ andis. r10,r0,DBCR0_IDM@h
bnel- load_dbcr0
#endif
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index db2497c..e030f3b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -254,7 +254,7 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
return;
/* Clear the DAC and struct entries. One shot trigger */
-#if (defined(CONFIG_44x) || defined(CONFIG_BOOKE))
+#if defined(CONFIG_BOOKE)
mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
| DBCR0_IDM));
#endif
@@ -286,7 +286,7 @@ int set_dabr(unsigned long dabr)
mtspr(SPRN_DABR, dabr);
#endif
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
mtspr(SPRN_DAC1, dabr);
#endif
@@ -373,7 +373,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
set_dabr(new->thread.dabr);
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
/* If new thread DAC (HW breakpoint) is the same then leave it */
if (new->thread.dabr)
set_dabr(new->thread.dabr);
@@ -568,7 +568,7 @@ void flush_thread(void)
current->thread.dabr = 0;
set_dabr(0);
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W);
#endif
}
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index a5d0e78..66204cb 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -717,7 +717,7 @@ void user_disable_single_step(struct task_struct *task)
struct pt_regs *regs = task->thread.regs;
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
/* If DAC then do not single step, skip */
if (task->thread.dabr)
return;
@@ -744,10 +744,11 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
if (addr > 0)
return -EINVAL;
+ /* The bottom 3 bits in dabr are flags */
if ((data & ~0x7UL) >= TASK_SIZE)
return -EIO;
-#ifdef CONFIG_PPC64
+#ifndef CONFIG_BOOKE
/* For processors using DABR (i.e. 970), the bottom 3 bits are flags.
* It was assumed, on previous implementations, that 3 bits were
@@ -769,7 +770,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
task->thread.dabr = data;
#endif
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
/* As described above, it was assumed 3 bits were passed with the data
* address, but we will assume only the mode bits will be passed
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 7aada78..2b5eaa6 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -147,7 +147,7 @@ int do_signal(sigset_t *oldset, struct pt_regs *regs)
*/
if (current->thread.dabr) {
set_dabr(current->thread.dabr);
-#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE)
mtspr(SPRN_DBCR0, current->thread.dbcr0);
#endif
}
--
1.5.5.1
^ permalink raw reply related
* Re: [PATCH v3 2/4] spi: split up spi_new_device() to allow two stage registration.
From: Grant Likely @ 2008-07-25 19:34 UTC (permalink / raw)
To: David Brownell; +Cc: spi-devel-general, akpm, linux-kernel, linuxppc-dev
In-Reply-To: <200807251200.45321.david-b@pacbell.net>
On Fri, Jul 25, 2008 at 3:00 PM, David Brownell <david-b@pacbell.net> wrote:
> On Friday 25 July 2008, Grant Likely wrote:
>>
>> This change is in preparation for adding an OF device tree parser to
>> generate spi_devices based on data in the device tree.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>
> Given the comment fixes noted above and below:
>
>
> Acked-by: David Brownell <dbrownell@users.sourceforge.net>
Cool, I'll get all this fixed. Thanks.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC] 4xx hardware watchpoint support
From: Kumar Gala @ 2008-07-25 19:38 UTC (permalink / raw)
To: Kumar Gala; +Cc: Paul Mackerras, Christoph Hellwig, ppc-dev
In-Reply-To: <C786174A-78C1-40B6-ABC9-24E7419FA182@kernel.crashing.org>
On Jul 25, 2008, at 10:23 AM, Kumar Gala wrote:
>
> On Jul 24, 2008, at 11:00 PM, Benjamin Herrenschmidt wrote:
>
>> On Wed, 2008-07-23 at 13:10 -0300, Luis Machado wrote:
>>> On Wed, 2008-07-23 at 11:53 -0400, Josh Boyer wrote:
>>>> Shouldn't this (and other places) be:
>>>>
>>>> #if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
>>>>
>>>> if you are going to exclude 40x for now? Otherwise this is still
>>>> enabled on 405 and setting the wrong register.
>>>>
>>>> josh
>>>
>>> Yes, sorry. I wasn't aware of this specific define value. It makes
>>> things easier to support 405's later.
>>>
>>> Like so?
>>
>> Please, next time, when you re-send a patch like that, re-include
>> the full subject and patch description. You can add then blurbs after
>> the --- line if you want to add things that won't make it to git.
>>
>> I'll deal with that specific one for now.
>
> Ben, I want to make sure this works on FSL Book-E before it gets
> into tree and I need to think about what SMP issues it might have.
>
> I talked to Josh about this at OLS and if you are ok I can deal with
> acceptance of this patch via my tree.
Josh pointed out that you went ahead and merged this. Curse you :)
I've got a patch in my tree to address my initial concerns.
- k
^ permalink raw reply
* Re: [OT] write flash on MPC5200 board via jtag
From: Albrecht Dreß @ 2008-07-25 19:48 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <9e4733910807221530vd5573d3oc2b56f544ab1c1f6@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 464 bytes --]
Hi all!
Thank you so much for your tips, they were really helpful!
Am 23.07.08 00:30 schrieb(en) Jon Smirl:
> I haven't tried it, but a Macraigor USB Wiggler supports the MPC5200
> and it's $250.
[snip]
> Let me know if you try it and it works.
I guess I will try this one, and of course give you feedback. Do not
expect this too fast, though, I have to get it to Europe, and test it
with the Lite board first.
Thanks again,
Albrecht.
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: clean up the Book-E HW watchpoint support
From: Luis Machado @ 2008-07-25 19:50 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0807251425390.29749@blarg.am.freescale.net>
Works for me.
I presume you had positive results on the Book-E as well.
By the way, thanks for cleaning it up.
Luis
On Fri, 2008-07-25 at 14:27 -0500, Kumar Gala wrote:
> * CONFIG_BOOKE is selected by CONFIG_44x so we dont need both
> * Fixed a few comments
> * Go back to only using DBCR0_IDM to determine if we are using
> debug resources.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>
> Luis, can you test this on 44x. I don't expect any breakage.
>
> - k
>
> arch/powerpc/kernel/entry_32.S | 6 +++---
> arch/powerpc/kernel/process.c | 8 ++++----
> arch/powerpc/kernel/ptrace.c | 7 ++++---
> arch/powerpc/kernel/signal.c | 2 +-
> 4 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 81c8324..da52269 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -148,7 +148,7 @@ transfer_to_handler:
> /* Check to see if the dbcr0 register is set up to debug. Use the
> internal debug mode bit to do this. */
> lwz r12,THREAD_DBCR0(r12)
> - andis. r12,r12,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
> + andis. r12,r12,DBCR0_IDM@h
> beq+ 3f
> /* From user and task is ptraced - load up global dbcr0 */
> li r12,-1 /* clear all pending debug events */
> @@ -292,7 +292,7 @@ syscall_exit_cont:
> /* If the process has its own DBCR0 value, load it up. The internal
> debug mode bit tells us that dbcr0 should be loaded. */
> lwz r0,THREAD+THREAD_DBCR0(r2)
> - andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
> + andis. r10,r0,DBCR0_IDM@h
> bnel- load_dbcr0
> #endif
> #ifdef CONFIG_44x
> @@ -720,7 +720,7 @@ restore_user:
> /* Check whether this process has its own DBCR0 value. The internal
> debug mode bit tells us that dbcr0 should be loaded. */
> lwz r0,THREAD+THREAD_DBCR0(r2)
> - andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
> + andis. r10,r0,DBCR0_IDM@h
> bnel- load_dbcr0
> #endif
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index db2497c..e030f3b 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -254,7 +254,7 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> return;
>
> /* Clear the DAC and struct entries. One shot trigger */
> -#if (defined(CONFIG_44x) || defined(CONFIG_BOOKE))
> +#if defined(CONFIG_BOOKE)
> mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> | DBCR0_IDM));
> #endif
> @@ -286,7 +286,7 @@ int set_dabr(unsigned long dabr)
> mtspr(SPRN_DABR, dabr);
> #endif
>
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
> mtspr(SPRN_DAC1, dabr);
> #endif
>
> @@ -373,7 +373,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
> if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
> set_dabr(new->thread.dabr);
>
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
> /* If new thread DAC (HW breakpoint) is the same then leave it */
> if (new->thread.dabr)
> set_dabr(new->thread.dabr);
> @@ -568,7 +568,7 @@ void flush_thread(void)
> current->thread.dabr = 0;
> set_dabr(0);
>
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
> current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W);
> #endif
> }
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index a5d0e78..66204cb 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -717,7 +717,7 @@ void user_disable_single_step(struct task_struct *task)
> struct pt_regs *regs = task->thread.regs;
>
>
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
> /* If DAC then do not single step, skip */
> if (task->thread.dabr)
> return;
> @@ -744,10 +744,11 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> if (addr > 0)
> return -EINVAL;
>
> + /* The bottom 3 bits in dabr are flags */
> if ((data & ~0x7UL) >= TASK_SIZE)
> return -EIO;
>
> -#ifdef CONFIG_PPC64
> +#ifndef CONFIG_BOOKE
>
> /* For processors using DABR (i.e. 970), the bottom 3 bits are flags.
> * It was assumed, on previous implementations, that 3 bits were
> @@ -769,7 +770,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> task->thread.dabr = data;
>
> #endif
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
>
> /* As described above, it was assumed 3 bits were passed with the data
> * address, but we will assume only the mode bits will be passed
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index 7aada78..2b5eaa6 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -147,7 +147,7 @@ int do_signal(sigset_t *oldset, struct pt_regs *regs)
> */
> if (current->thread.dabr) {
> set_dabr(current->thread.dabr);
> -#if defined(CONFIG_44x) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE)
> mtspr(SPRN_DBCR0, current->thread.dbcr0);
> #endif
> }
^ permalink raw reply
* Re: [PATCH] powerpc: clean up the Book-E HW watchpoint support
From: Kumar Gala @ 2008-07-25 20:18 UTC (permalink / raw)
To: luisgpm; +Cc: linuxppc-dev
In-Reply-To: <1217015418.29012.67.camel@gargoyle>
On Jul 25, 2008, at 2:50 PM, Luis Machado wrote:
> Works for me.
>
> I presume you had positive results on the Book-E as well.
I havent had a chance to test this on our HW.
> By the way, thanks for cleaning it up.
np.
- k
^ permalink raw reply
* Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
From: Trent Piepho @ 2008-07-25 20:38 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Stephen Rothwell, linux-kernel, linuxppc-dev, Richard Purdie
In-Reply-To: <20080718100549.GA28741@polina.dev.rtsoft.ru>
Here are my patches for the OF bindings. The first is just a tiny change to
the leds code to silence a warning. The second is the real patch.
The leds-gpio driver gets two sub-options in Kconfig, one for platform
device support and one for openfirmware platform device support.
There is support for setting an LED trigger. I didn't include support for
active-low or initial brightness, but I think those would be good features
to add. See below for more on that.
Since each device node can describe multiple leds, I used "gpio-leds"
instead of "gpio-led" for the compatible property.
Examples of the dts syntax:
leds {
compatible = "gpio-leds";
hdd {
label = "IDE activity";
gpios = <&mcu_pio 0 0>;
function = "ide-disk";
};
};
run-control {
compatible = "gpio-leds";
red {
gpios = <&mpc8572 6 0>;
};
green {
gpios = <&mpc8572 7 0>;
};
}
On Fri, 18 Jul 2008, Anton Vorontsov wrote:
> Later I tried to use aliases for default-trigger.
>
> http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057244.html
I doesn't seem to me that the alias method will work very well. If I want
an LED that starts on, I need to add code to the kernel to support an
"led-on-while-kernel-boots" alias? And if I want red & green leds to flash
while booting, I need to add aliases "led-flashing-while-kernel-boots-1" and
"led-flashing-while-kernel-boots-2", since you can't assign two leds to the
same alias?
> As for linux,default-brightness... we don't need this since now we
> have default-on trigger.
Except we can't use triggers....
There is an issue with the default-on trigger, besides requiring led
triggers be turned on and the extra code that needs. It causes the led to
have a glitch in it. Suppose the LED is already on from reset or the boot
loader. When the gpio is allocated, it's allocated with an initial value of
off. Then, later, it's turned back on when the trigger runs. But say the
trigger doesn't run?
Here's a real example. I have an LED that comes on the board powers on. It
was supposed to turn off when the kernel has finished booting. But suppose
the kernel panics between the gpio getting lowered when the led is added and
the trigger turning it back on? Now the LED is off and it appears the
problem happened after the kernel finished booting, but really it happened
during the kernel boot. In an embedded system with no console, that's quite
a bit of misinformation. It can be entirely avoided by changing just three
lines of code with the leds-gpio driver to add an option to start the led
on.
It could also be the case that the gpio the led is on also triggers some
other piece of hardware. An alarm that's on the same line as a fault led
for example. The glitch created by the default-on trigger could be
unacceptable in that case.
^ permalink raw reply
* RE: 82xx performance
From: Rune Torgersen @ 2008-07-25 20:41 UTC (permalink / raw)
To: Arnd Bergmann, linuxppc-dev; +Cc: Milton Miller
In-Reply-To: <200807172332.18030.arnd@arndb.de>
> From: Arnd Bergmann [mailto:arnd@arndb.de]=20
> On Thursday 17 July 2008, Rune Torgersen wrote:
> > Arnd Bergmann wrote:
> > > So again, nothing conclusive. I'm running out of ideas.
> >=20
> > Is the syscall path different or the same on ppc and powerpc?
> > Any differences in the task switching, irq handling or page fault
> > handling?
> >=20
>=20
> It's all different in suble ways, but those changes should only
> show up in the system time accounting, not user time accounting.
I've been running the workload this board will see. On a 2.6.18 kernel
%idle is ~50% and %wa (waiting for IO) is less than 1% most of the time.
On 2.6.25, the idle% is lower (by about 10-15%) and the %wa is
consistently hovering around 20-30% sometimes spiking to 100%.
The workload involves quite a bit of socket IO (TCP, UDP, Unix Sockets
and TIPC) and disk IO.
Any easy way of finding what is causing the wait for IO?
(Ive been trying to get lttng to work, but not any good results so far).
^ permalink raw reply
* [PATCH 1/2] leds: make the default trigger name const
From: Trent Piepho @ 2008-07-25 21:01 UTC (permalink / raw)
To: linux-kernel; +Cc: Stephen Rothwell, linuxppc-dev, Richard Purdie, Trent Piepho
In-Reply-To: <Pine.LNX.4.64.0807181339500.10116@t2.domain.actdsltmp>
The default_trigger fields of struct gpio_led and thus struct led_classdev
are pretty much always assigned from a string literal, which means the
string can't be modified. Which is fine, since there is no reason to
modify the string and in fact it never is.
But they should be marked const to prevent such code from being added, to
prevent warnings if -Wwrite-strings is used and when assigned from a
constant string other than a string literal (which produces a warning under
current kernel compiler flags), and for general good coding practices.
Signed-off-by: Trent Piepho <tpiepho@freescale.com>
---
include/linux/leds.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 519df72..defe693 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -48,7 +48,7 @@ struct led_classdev {
struct device *dev;
struct list_head node; /* LED Device list */
- char *default_trigger; /* Trigger to use */
+ const char *default_trigger; /* Trigger to use */
#ifdef CONFIG_LEDS_TRIGGERS
/* Protects the trigger data below */
@@ -121,7 +121,7 @@ extern void ledtrig_ide_activity(void);
/* For the leds-gpio driver */
struct gpio_led {
const char *name;
- char *default_trigger;
+ const char *default_trigger;
unsigned gpio;
u8 active_low;
};
--
1.5.4.3
^ 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