* Re: [alsa-devel] [PATCH] aoa: remove driver_data direct access of struct device
From: Greg KH @ 2009-05-12 23:05 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linuxppc-dev, johannes, Roel Kluin, alsa-devel, lkml
In-Reply-To: <s5hoctyu6m4.wl%tiwai@suse.de>
On Tue, May 12, 2009 at 09:23:47AM +0200, Takashi Iwai wrote:
> At Mon, 11 May 2009 23:57:43 -0700,
> Greg KH wrote:
> >
> > On Tue, May 12, 2009 at 08:40:05AM +0200, Takashi Iwai wrote:
> > > At Mon, 11 May 2009 21:54:51 +0200,
> > > Roel Kluin wrote:
> > > >
> > > > To avoid direct access to the driver_data pointer in struct device, the
> > > > functions dev_get_drvdata() and dev_set_drvdata() should be used.
> > > >
> > > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > >
> > > The same fix has been already in Greg's tree (and thus in linux-next).
> > >
> > > Greg, is it OK that I take over your patch to sound git tree?
> >
> > Yes, please do, no objection from me at all.
>
> OK, will do.
>
> BTW, should it be pushed to 2.6.30? If "the near future" you
> mentioned in the patch means 2.6.31, it'll be better to merge that fix
> now. Otherwise, I'm going to apply the patch as a 2.6.31 material.
.31 is fine, that is what I was meaning by with "near future" :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH] leds: Add options to have GPIO LEDs start on or keep their state
From: Trent Piepho @ 2009-05-12 22:33 UTC (permalink / raw)
To: linuxppc-dev, Richard Purdie; +Cc: Trent Piepho, Sean MacLennan
In-Reply-To: <fa686aa40904172207sc0f923eoefe8fb0a2703d548@mail.gmail.com>
There already is a "default-on" trigger but there are problems with it.
For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.
But the real reason is that is produces a glitch on the LED. The GPIO is
allocate with the LED *off*, then *later* when the trigger runs it is
turned back on. If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on. While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.
One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.
Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state. This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.
We also let GPIO LEDs get their initial value from whatever the current
state of the GPIO line is. On some systems the LEDs are put into some
state by the firmware or hardware before Linux boots, and it is desired to
have them keep this state which is otherwise unknown to Linux.
This requires that the underlying GPIO driver support reading the value of
output GPIOs. Some drivers support this and some do not.
The platform device binding gains a field in the platform data
"default_state" that controls this. There are three constants defined to
select from on, off, or keeping the current state. The OpenFirmware
binding uses a property named "default-state" that can be set to "on",
"off", or "keep". The default if the property isn't present is off.
Signed-off-by: Trent Piepho <xyzzy@speakeasy.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 17 ++++++++++++++++-
drivers/leds/leds-gpio.c | 21 ++++++++++++++++++---
include/linux/leds.h | 9 +++++++--
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 4fe14de..064db92 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -16,10 +16,17 @@ LED sub-node properties:
string defining the trigger assigned to the LED. Current triggers are:
"backlight" - LED will act as a back-light, controlled by the framebuffer
system
- "default-on" - LED will turn on
+ "default-on" - LED will turn on, but see "default-state" below
"heartbeat" - LED "double" flashes at a load average based rate
"ide-disk" - LED indicates disk activity
"timer" - LED flashes at a fixed, configurable rate
+- default-state: (optional) The initial state of the LED. Valid
+ values are "on", "off", and "keep". If the LED is already on or off
+ and the default-state property is set the to same value, then no
+ glitch should be produced where the LED momentarily turns off (or
+ on). The "keep" setting will keep the LED at whatever its current
+ state is, without producing a glitch. The default is off if this
+ property is not present.
Examples:
@@ -30,14 +37,22 @@ leds {
gpios = <&mcu_pio 0 1>; /* Active low */
linux,default-trigger = "ide-disk";
};
+
+ fault {
+ gpios = <&mcu_pio 1 0>;
+ /* Keep LED on if BIOS detected hardware fault */
+ default-state = "keep";
+ };
};
run-control {
compatible = "gpio-leds";
red {
gpios = <&mpc8572 6 0>;
+ default-state = "off";
};
green {
gpios = <&mpc8572 7 0>;
+ default-state = "on";
};
}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index d210905..b5fd008 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led *template,
struct gpio_led_data *led_dat, struct device *parent,
int (*blink_set)(unsigned, unsigned long *, unsigned long *))
{
- int ret;
+ int ret, state;
/* skip leds that aren't available */
if (!gpio_is_valid(template->gpio)) {
@@ -99,11 +99,15 @@ static int __devinit create_gpio_led(const struct gpio_led *template,
led_dat->cdev.blink_set = gpio_blink_set;
}
led_dat->cdev.brightness_set = gpio_led_set;
- led_dat->cdev.brightness = LED_OFF;
+ if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP)
+ state = !!gpio_get_value(led_dat->gpio) ^ led_dat->active_low;
+ else
+ state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
+ led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
if (!template->retain_state_suspended)
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
- ret = gpio_direction_output(led_dat->gpio, led_dat->active_low);
+ ret = gpio_direction_output(led_dat->gpio, led_dat->active_low ^ state);
if (ret < 0)
goto err;
@@ -223,12 +227,23 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
memset(&led, 0, sizeof(led));
for_each_child_of_node(np, child) {
enum of_gpio_flags flags;
+ const char *state;
led.gpio = of_get_gpio_flags(child, 0, &flags);
led.active_low = flags & OF_GPIO_ACTIVE_LOW;
led.name = of_get_property(child, "label", NULL) ? : child->name;
led.default_trigger =
of_get_property(child, "linux,default-trigger", NULL);
+ state = of_get_property(child, "default-state", NULL);
+ if (state) {
+ if (!strcmp(state, "keep")) {
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ } else if(!strcmp(state, "on")) {
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ } else {
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+ }
ret = create_gpio_led(&led, &pdata->led_data[pdata->num_leds++],
&ofdev->dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 376fe07..66e7d75 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -141,9 +141,14 @@ struct gpio_led {
const char *name;
const char *default_trigger;
unsigned gpio;
- u8 active_low : 1;
- u8 retain_state_suspended : 1;
+ unsigned active_low : 1;
+ unsigned retain_state_suspended : 1;
+ unsigned default_state : 2;
+ /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
};
+#define LEDS_GPIO_DEFSTATE_OFF 0
+#define LEDS_GPIO_DEFSTATE_ON 1
+#define LEDS_GPIO_DEFSTATE_KEEP 2
struct gpio_led_platform_data {
int num_leds;
--
1.5.4.1
^ permalink raw reply related
* Re: [PATCH 5/6] rio: warn_unused_result warnings fix
From: Andrew Morton @ 2009-05-12 22:11 UTC (permalink / raw)
To: Li Yang; +Cc: netdev, linux-kernel, linuxppc-dev, leoli, davem
In-Reply-To: <1242117363-14949-5-git-send-email-leoli@freescale.com>
On Tue, 12 May 2009 16:36:02 +0800
Li Yang <leoli@freescale.com> wrote:
> + if (rswitch) {
> + if (rswitch->route_table)
this `if' is unneeded.
> + kfree(rswitch->route_table);
> + kfree(rswitch);
> + }
^ permalink raw reply
* Re: [PATCH 4/6] rionet: add memory access to simulated Ethernet over rapidio
From: Andrew Morton @ 2009-05-12 22:10 UTC (permalink / raw)
To: Li Yang; +Cc: zw, netdev, linux-kernel, linuxppc-dev, leoli, davem
In-Reply-To: <1242117363-14949-4-git-send-email-leoli@freescale.com>
On Tue, 12 May 2009 16:36:01 +0800
Li Yang <leoli@freescale.com> wrote:
> Through the newly added IO memory access of RapidIO, sender can
> write directly to recipient's rx buffer, either by cpu or DMA engine.
>
> ...
>
> +/* Definitions for rionet memory map driver */
> +#define RIONET_DRVID 0x101
> +#define RIONET_MAX_SK_DATA_SIZE 0x1000
> +#define RIONET_MEM_RIO_BASE 0x10000000
> +#define RIONET_TX_RX_BUFF_SIZE (0x1000 * (128 + 128))
> +#define RIONET_QUEUE_NEXT(x) (((x) < 127) ? ((x) + 1) : 0)
References its arg multiple times, hence is buggy or inefficient when
passed an expression with side-effects.
static inline int rionet_queue_next(int x)
would be better. Assuming that some sane identifier is used instead of
"x".
> +#define RIONET_QUEUE_INC(x) (x = RIONET_QUEUE_NEXT(x))
It's pretty ugly to hide an assignment inside a macro like this. Why
not do
foo = rionet_queue_inc(foo);
at the callsites? It makes it much clearer for the reader.
>
> ...
>
> +#ifdef CONFIG_RIONET_MEMMAP
> +static int rio_send_mem(struct sk_buff *skb,
> + struct net_device *ndev, struct rio_dev *rdev)
> +{
> + struct rionet_private *rnet = netdev_priv(ndev);
> + int enqueue, dequeue;
> +
> + if (!rdev)
> + return -EFAULT;
Is that an appropriate error code?
>
> ...
>
^ permalink raw reply
* Re: [PATCH 3/6] powerpc: add memory map support to Freescale RapidIO block
From: Andrew Morton @ 2009-05-12 22:05 UTC (permalink / raw)
To: Li Yang; +Cc: zw, netdev, linux-kernel, linuxppc-dev, leoli, davem
In-Reply-To: <1242117363-14949-3-git-send-email-leoli@freescale.com>
On Tue, 12 May 2009 16:36:00 +0800
Li Yang <leoli@freescale.com> wrote:
> + align = (size < 0x1000) ? 0x1000 : 1 << (__ilog2(size - 1) + 1);
> +
> + /* Align the size */
> + if ((lstart + size) > (_ALIGN_DOWN(lstart, align) + align)) {
__ilog2() and _ALIGN_DOWN() are powerpc-specific functions. It would
be preferable to use more general helpers if possible. ALIGN() and ilog2()
might suit here.
^ permalink raw reply
* [PATCH] powerpc/8xxx: Update PCI outbound window addresses for 36-bit configs
From: Kumar Gala @ 2009-05-12 22:07 UTC (permalink / raw)
To: linuxppc-dev
In these configuration we expect to have large amounts of memory (> 4G)
and thus will bounce via swiotlb some region of PCI address space.
The outbound windows were wasting 512M of address space by leaving a
gap between the top of the outbound window and the 4G boundary. By
moving the top of the outbound window up to the 4G boundary we can
reclaim the vast majority of the 512M (minus space needed for PEXCSRBAR)
and thus reduces the amount of memory we have to bounce.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
arch/powerpc/boot/dts/mpc8572ds_36b.dts | 22 +++++++++++-----------
arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts | 16 ++++++++--------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8572ds_36b.dts b/arch/powerpc/boot/dts/mpc8572ds_36b.dts
index 1197965..f6365db 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_36b.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_36b.dts
@@ -533,7 +533,7 @@
#address-cells = <3>;
reg = <0xf 0xffe08000 0 0x1000>;
bus-range = <0 255>;
- ranges = <0x2000000 0x0 0xc0000000 0xc 0x00000000 0x0 0x20000000
+ ranges = <0x2000000 0x0 0xe0000000 0xc 0x00000000 0x0 0x20000000
0x1000000 0x0 0x00000000 0xf 0xffc00000 0x0 0x00010000>;
clock-frequency = <33333333>;
interrupt-parent = <&mpic>;
@@ -660,8 +660,8 @@
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
- ranges = <0x2000000 0x0 0xc0000000
- 0x2000000 0x0 0xc0000000
+ ranges = <0x2000000 0x0 0xe0000000
+ 0x2000000 0x0 0xe0000000
0x0 0x20000000
0x1000000 0x0 0x0
@@ -671,8 +671,8 @@
reg = <0x0 0x0 0x0 0x0 0x0>;
#size-cells = <2>;
#address-cells = <3>;
- ranges = <0x2000000 0x0 0xc0000000
- 0x2000000 0x0 0xc0000000
+ ranges = <0x2000000 0x0 0xe0000000
+ 0x2000000 0x0 0xe0000000
0x0 0x20000000
0x1000000 0x0 0x0
@@ -742,7 +742,7 @@
#address-cells = <3>;
reg = <0xf 0xffe09000 0 0x1000>;
bus-range = <0 255>;
- ranges = <0x2000000 0x0 0xc0000000 0xc 0x20000000 0x0 0x20000000
+ ranges = <0x2000000 0x0 0xe0000000 0xc 0x20000000 0x0 0x20000000
0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x00010000>;
clock-frequency = <33333333>;
interrupt-parent = <&mpic>;
@@ -760,8 +760,8 @@
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
- ranges = <0x2000000 0x0 0xc0000000
- 0x2000000 0x0 0xc0000000
+ ranges = <0x2000000 0x0 0xe0000000
+ 0x2000000 0x0 0xe0000000
0x0 0x20000000
0x1000000 0x0 0x0
@@ -778,7 +778,7 @@
#address-cells = <3>;
reg = <0xf 0xffe0a000 0 0x1000>;
bus-range = <0 255>;
- ranges = <0x2000000 0x0 0xc0000000 0xc 0x40000000 0x0 0x20000000
+ ranges = <0x2000000 0x0 0xe0000000 0xc 0x40000000 0x0 0x20000000
0x1000000 0x0 0x00000000 0xf 0xffc20000 0x0 0x00010000>;
clock-frequency = <33333333>;
interrupt-parent = <&mpic>;
@@ -796,8 +796,8 @@
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
- ranges = <0x2000000 0x0 0xc0000000
- 0x2000000 0x0 0xc0000000
+ ranges = <0x2000000 0x0 0xe0000000
+ 0x2000000 0x0 0xe0000000
0x0 0x20000000
0x1000000 0x0 0x0
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts b/arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts
index 65893b9..8be8e70 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn_36b.dts
@@ -370,7 +370,7 @@
#address-cells = <3>;
reg = <0x0f 0xffe08000 0x0 0x1000>;
bus-range = <0x0 0xff>;
- ranges = <0x02000000 0x0 0xc0000000 0x0c 0x00000000 0x0 0x20000000
+ ranges = <0x02000000 0x0 0xe0000000 0x0c 0x00000000 0x0 0x20000000
0x01000000 0x0 0x00000000 0x0f 0xffc00000 0x0 0x00010000>;
clock-frequency = <33333333>;
interrupt-parent = <&mpic>;
@@ -496,8 +496,8 @@
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
- ranges = <0x02000000 0x0 0xc0000000
- 0x02000000 0x0 0xc0000000
+ ranges = <0x02000000 0x0 0xe0000000
+ 0x02000000 0x0 0xe0000000
0x0 0x20000000
0x01000000 0x0 0x00000000
@@ -507,8 +507,8 @@
reg = <0 0 0 0 0>;
#size-cells = <2>;
#address-cells = <3>;
- ranges = <0x02000000 0x0 0xc0000000
- 0x02000000 0x0 0xc0000000
+ ranges = <0x02000000 0x0 0xe0000000
+ 0x02000000 0x0 0xe0000000
0x0 0x20000000
0x01000000 0x0 0x00000000
0x01000000 0x0 0x00000000
@@ -579,7 +579,7 @@
#address-cells = <3>;
reg = <0x0f 0xffe09000 0x0 0x1000>;
bus-range = <0x0 0xff>;
- ranges = <0x02000000 0x0 0xc0000000 0x0c 0x20000000 0x0 0x20000000
+ ranges = <0x02000000 0x0 0xe0000000 0x0c 0x20000000 0x0 0x20000000
0x01000000 0x0 0x00000000 0x0f 0xffc10000 0x0 0x00010000>;
clock-frequency = <33333333>;
interrupt-parent = <&mpic>;
@@ -597,8 +597,8 @@
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
- ranges = <0x02000000 0x0 0xc0000000
- 0x02000000 0x0 0xc0000000
+ ranges = <0x02000000 0x0 0xe0000000
+ 0x02000000 0x0 0xe0000000
0x0 0x20000000
0x01000000 0x0 0x00000000
--
1.6.0.6
^ permalink raw reply related
* [PATCH] powerpc/fsl: Support unique MSI addresses per PCIe Root Complex
From: Kumar Gala @ 2009-05-12 21:27 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1242163655-880-1-git-send-email-galak@kernel.crashing.org>
Its feasible based on how the PCI address map is setup that the region
of PCI address space used for MSIs differs for each PHB on the same SoC.
Instead of assuming that the address mappes to CCSRBAR 1:1 we read
PEXCSRBAR (BAR0) for the PHB that the given pci_dev is on.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
arch/powerpc/sysdev/fsl_msi.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index f25ce81..da38a1f 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -113,8 +113,13 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
struct msi_msg *msg)
{
struct fsl_msi *msi_data = fsl_msi;
+ struct pci_controller *hose = pci_bus_to_host(pdev->bus);
+ u32 base = 0;
- msg->address_lo = msi_data->msi_addr_lo;
+ pci_bus_read_config_dword(hose->bus,
+ PCI_DEVFN(0, 0), PCI_BASE_ADDRESS_0, &base);
+
+ msg->address_lo = msi_data->msi_addr_lo + base;
msg->address_hi = msi_data->msi_addr_hi;
msg->data = hwirq;
@@ -271,7 +276,7 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
msi->irqhost->host_data = msi;
msi->msi_addr_hi = 0x0;
- msi->msi_addr_lo = res.start + features->msiir_offset;
+ msi->msi_addr_lo = features->msiir_offset + (res.start & 0xfffff);
rc = fsl_msi_init_allocator(msi);
if (rc) {
--
1.6.0.6
^ permalink raw reply related
* [PATCH] powerpc/fsl: Setup PCI inbound window based on actual amount of memory
From: Kumar Gala @ 2009-05-12 21:27 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1242163655-880-2-git-send-email-galak@kernel.crashing.org>
Previouslly we just always set the inbound window to 2G. This was
broken for systems with >2G. If a system has >=4G we will need
SWIOTLB support to handle that case.
We now allocate PCICSRBAR/PEXCSRBAR right below the lowest PCI outbound
address for MMIO or the 4G boundary (if the lowest PCI address is above
4G).
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
arch/powerpc/sysdev/fsl_pci.c | 122 ++++++++++++++++++++++++++++++++++-------
arch/powerpc/sysdev/fsl_pci.h | 6 ++-
2 files changed, 107 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 94d8b3f..9b49ecc 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -23,6 +23,8 @@
#include <linux/string.h>
#include <linux/init.h>
#include <linux/bootmem.h>
+#include <linux/lmb.h>
+#include <linux/log2.h>
#include <asm/io.h>
#include <asm/prom.h>
@@ -96,7 +98,13 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
struct resource *rsrc)
{
struct ccsr_pci __iomem *pci;
- int i, j, n;
+ int i, j, n, mem_log;
+ u64 mem, sz, paddr_hi = 0;
+ u64 paddr_lo = ULLONG_MAX;
+ u32 pcicsrbar = 0, pcicsrbar_sz;
+ u32 piwar = PIWAR_EN | PIWAR_PF | PIWAR_TGI_LOCAL |
+ PIWAR_READ_SNOOP | PIWAR_WRITE_SNOOP;
+ char *name = hose->dn->full_name;
pr_debug("PCI memory map start 0x%016llx, size 0x%016llx\n",
(u64)rsrc->start, (u64)rsrc->end - (u64)rsrc->start + 1);
@@ -117,6 +125,9 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
if (!(hose->mem_resources[i].flags & IORESOURCE_MEM))
continue;
+ paddr_lo = min(paddr_lo, (u64)hose->mem_resources[i].start);
+ paddr_hi = max(paddr_hi, (u64)hose->mem_resources[i].end);
+
n = setup_one_atmu(pci, j, &hose->mem_resources[i],
hose->pci_mem_offset);
@@ -147,14 +158,97 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
}
}
- /* Setup 2G inbound Memory Window @ 1 */
- out_be32(&pci->piw[2].pitar, 0x00000000);
- out_be32(&pci->piw[2].piwbar,0x00000000);
- out_be32(&pci->piw[2].piwar, PIWAR_2G);
+ /* convert to pci address space */
+ paddr_hi -= hose->pci_mem_offset;
+ paddr_lo -= hose->pci_mem_offset;
+
+ if (paddr_hi == paddr_lo) {
+ pr_err("%s: No outbound window space\n", name);
+ return ;
+ }
+
+ if (paddr_lo == 0) {
+ pr_err("%s: No space for inbound window\n", name);
+ return ;
+ }
+
+ /* setup PCSRBAR/PEXCSRBAR */
+ early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, 0xffffffff);
+ early_read_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, &pcicsrbar_sz);
+ pcicsrbar_sz = ~pcicsrbar_sz + 1;
+
+ if (paddr_hi < (0x100000000ull - pcicsrbar_sz) ||
+ (paddr_lo > 0x100000000ull))
+ pcicsrbar = 0x100000000ull - pcicsrbar_sz;
+ else
+ pcicsrbar = (paddr_lo - pcicsrbar_sz) & -pcicsrbar_sz;
+ early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, pcicsrbar);
+
+ paddr_lo = min(paddr_lo, (u64)pcicsrbar);
+
+ pr_info("%s: PCICSRBAR @ 0x%x\n", name, pcicsrbar);
+
+ /* Setup inbound mem window */
+ mem = lmb_end_of_DRAM();
+ sz = min(mem, paddr_lo);
+ mem_log = __ilog2_u64(sz);
+
+ /* PCIe can overmap inbound & outbound since RX & TX are seperated */
+ if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
+ /* Size window to exact size if power-of-two or one size up */
+ if ((1ull << mem_log) != mem) {
+ pr_info("%s: Setting PCI inbound window greater "
+ "than memory size\n", name);
+ mem_log++;
+ }
+
+ piwar |= (mem_log - 1);
+
+ /* Setup inbound memory window */
+ out_be32(&pci->piw[2].pitar, 0x00000000);
+ out_be32(&pci->piw[2].piwbar, 0x00000000);
+ out_be32(&pci->piw[2].piwar, piwar);
+
+ hose->dma_window_base_cur = 0x00000000;
+ hose->dma_window_size = (resource_size_t)sz;
+ } else {
+ u64 paddr = 0;
+
+ /* Setup inbound memory window */
+ out_be32(&pci->piw[2].pitar, paddr >> 12);
+ out_be32(&pci->piw[2].piwbar, paddr >> 12);
+ out_be32(&pci->piw[2].piwar, (piwar | (mem_log - 1)));
+
+ paddr += 1ull << mem_log;
+ sz -= 1ull << mem_log;
+
+ if (sz) {
+ mem_log = __ilog2_u64(sz);
+
+ out_be32(&pci->piw[1].pitar, paddr >> 12);
+ out_be32(&pci->piw[1].piwbar, paddr >> 12);
+ out_be32(&pci->piw[1].piwar, (piwar | (mem_log - 1)));
+
+ paddr += 1ull << mem_log;
+ }
+
+ hose->dma_window_base_cur = 0x00000000;
+ hose->dma_window_size = (resource_size_t)paddr;
+ }
+
+ if (hose->dma_window_size < mem) {
+#ifndef CONFIG_SWIOTLB
+ pr_warning("%s: Amount of memory exceeds our ability to map. "
+ "Look at enabling CONFIG_SWIOTLB\n", name);
+#endif
+ /* adjusting outbound windows could reclaim space in mem map */
+ if (paddr_hi < 0x100000000ull)
+ pr_warning("%s: Outbound window cfg leaves gaps in "
+ "memory map.\n", name);
- /* Save the base address and size covered by inbound window mappings */
- hose->dma_window_base_cur = 0x00000000;
- hose->dma_window_size = 0x80000000;
+ pr_info("%s: DMA window size is 0x%llx\n", name,
+ (u64)hose->dma_window_size);
+ }
iounmap(pci);
}
@@ -180,16 +274,6 @@ static void __init setup_pci_cmd(struct pci_controller *hose)
}
}
-static void __init setup_pci_pcsrbar(struct pci_controller *hose)
-{
-#ifdef CONFIG_PCI_MSI
- phys_addr_t immr_base;
-
- immr_base = get_immrbase();
- early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, immr_base);
-#endif
-}
-
void fsl_pcibios_fixup_bus(struct pci_bus *bus)
{
struct pci_controller *hose = (struct pci_controller *) bus->sysdata;
@@ -273,8 +357,6 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
/* Setup PEX window registers */
setup_pci_atmu(hose, &rsrc);
- /* Setup PEXCSRBAR */
- setup_pci_pcsrbar(hose);
return 0;
}
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index 13f30c2..a9d8bbe 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -16,7 +16,11 @@
#define PCIE_LTSSM 0x0404 /* PCIE Link Training and Status */
#define PCIE_LTSSM_L0 0x16 /* L0 state */
-#define PIWAR_2G 0xa0f5501e /* Enable, Prefetch, Local Mem, Snoop R/W, 2G */
+#define PIWAR_EN 0x80000000 /* Enable */
+#define PIWAR_PF 0x20000000 /* prefetch */
+#define PIWAR_TGI_LOCAL 0x00f00000 /* target - local memory */
+#define PIWAR_READ_SNOOP 0x00050000
+#define PIWAR_WRITE_SNOOP 0x00005000
/* PCI/PCI Express outbound window reg */
struct pci_outbound_window_regs {
--
1.6.0.6
^ permalink raw reply related
* [PATCH] powerpc/85xx: Add MSI nodes for MPC8568/9 MDS systems
From: Kumar Gala @ 2009-05-12 21:27 UTC (permalink / raw)
To: linuxppc-dev
The MPC8568/9 chips support MSIs on PCIe so no reason not to enable them.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
arch/powerpc/boot/dts/mpc8568mds.dts | 16 ++++++++++++++++
arch/powerpc/boot/dts/mpc8569mds.dts | 16 ++++++++++++++++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 038913d..00c2bbd 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -288,6 +288,22 @@
device_type = "open-pic";
};
+ msi@41600 {
+ compatible = "fsl,mpc8568-msi", "fsl,mpic-msi";
+ reg = <0x41600 0x80>;
+ msi-available-ranges = <0 0x100>;
+ interrupts = <
+ 0xe0 0
+ 0xe1 0
+ 0xe2 0
+ 0xe3 0
+ 0xe4 0
+ 0xe5 0
+ 0xe6 0
+ 0xe7 0>;
+ interrupt-parent = <&mpic>;
+ };
+
par_io@e0100 {
reg = <0xe0100 0x100>;
device_type = "par_io";
diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
index 23a102e..39c2927 100644
--- a/arch/powerpc/boot/dts/mpc8569mds.dts
+++ b/arch/powerpc/boot/dts/mpc8569mds.dts
@@ -252,6 +252,22 @@
device_type = "open-pic";
};
+ msi@41600 {
+ compatible = "fsl,mpc8568-msi", "fsl,mpic-msi";
+ reg = <0x41600 0x80>;
+ msi-available-ranges = <0 0x100>;
+ interrupts = <
+ 0xe0 0
+ 0xe1 0
+ 0xe2 0
+ 0xe3 0
+ 0xe4 0
+ 0xe5 0
+ 0xe6 0
+ 0xe7 0>;
+ interrupt-parent = <&mpic>;
+ };
+
global-utilities@e0000 {
compatible = "fsl,mpc8569-guts";
reg = <0xe0000 0x1000>;
--
1.6.0.6
^ permalink raw reply related
* Having a hell of a time with spidev and spi_mpc8313
From: Joaquin Luna @ 2009-05-12 20:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <8A71B368A89016469F72CD08050AD33402D5760B@maui.asicdesigners.com>
I have a device that I need to access via SPI. I am running 2.6.27 on
the MPC8313. I have configured the kernel with both CONFIG_SPI_MPC83xx
and CONFIG_SPI_SPIDEV. I hand made a character device with a major
number of 153 and a minor number of 0.
I need some guidance on how to make the SPI slave table, and where to
put the code that connects this to my device file. Also, I am not
seeing the "%s: MPC83xx SPI Controller driver" print out at boot up, is
this a problem?
- Joaquin Luna
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: K.Prasad @ 2009-05-12 20:28 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090512164738.GA1173@b07421-ec1.am.freescale.net>
On Tue, May 12, 2009 at 11:47:38AM -0500, Scott Wood wrote:
> On Tue, May 12, 2009 at 07:51:49AM -0400, Josh Boyer wrote:
> > On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
> > >- The patch is currently implemented only for PPC64 architecture. Other
> > > architectures (especially Book-E implementations are expected to
> > > happen in due course).
> >
> > Does this mean you will work on transitioning Book-E implementations, or that
> > you expect the Book-E maintainers to? I'm just curious. The code as written
> > relies heavily on the DABR/MSR setup that ppc64 has and Book-E unfortunately
> > doesn't follow that at all.
>
> And since there will eventually be 64-bit book E chips, we need to use
> something more specific than CONFIG_PPC64 in the #ifdef (easier now than
> figuring out which ones are breakpoint-related later).
>
> -Scott
Sure. I will be glad to receive & incorporate suggestions in this regard
from PowerPC experts (or will 64-bit Book-E implementations be called by
a different name altogether?)
Thanks,
K.Prasad
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: K.Prasad @ 2009-05-12 20:25 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090512115149.GA1885@yoda.jdub.homelinux.org>
On Tue, May 12, 2009 at 07:51:49AM -0400, Josh Boyer wrote:
> On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
> >Hi PPC Dev folks,
> > Please find a patch below that implements the proposed Hardware
> >Breakpoint interfaces for PPC64 architecture.
> >
> >As a brief introduction, the proposed Hardware Breakpoint
> >infrastructure provides generic in-kernel interfaces to which users
> >from kernel- and user-space can request for breakpoint registers. An
> >ftrace plugin that can trace accesses to data variables is also part
> >of the generic HW Breakpoint interface patchset. The latest submission
> >for this patchset along with an x86 implementation can be found here:
> >http://lkml.org/lkml/2009/5/11/159.
> >
> >The following are the salient features of the PPC64 patch.
> >
> >- Arch-specific definitions for kernel and user-space requests are
> > defined in arch/powerpc/kernel/hw_breakpoint.c
> >- Ptrace is converted to use the HW Breakpoint interfaces
> >- The ftrace plugin called ksym_tracer is tested to work fine. For
> > instance when tracing pid_max kernel variable, the following was
> > obtained as output
> >
> ># cat trace
> ># tracer: ksym_tracer
> >#
> ># TASK-PID CPU# Symbol Type Function
> ># | | | | |
> >bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0x78/0x10c
> >bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0xa0/0x10c
> >bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
> >
> >There are however a few limitations/caveats of the patch as identified
> >below:
> >
> >- The patch is currently implemented only for PPC64 architecture. Other
> > architectures (especially Book-E implementations are expected to
> > happen in due course).
>
> Does this mean you will work on transitioning Book-E implementations, or that
> you expect the Book-E maintainers to? I'm just curious. The code as written
> relies heavily on the DABR/MSR setup that ppc64 has and Book-E unfortunately
> doesn't follow that at all.
>
> Book-E also allows for more than one HW breakpoint, which means you're growing
> the thread_struct by 32-bytes to support 4 of them. 64-bytes if this ever
> supports DAC events. Have you thought at all about a way to support this
> without carrying around the data in the thread struct?
>
> <snip>
>
The idea behind embedding the physical debug register values in
thread_struct is to use its synchronisation mechanisms for HW Breakpoint
related fields too. These values were originally maintained in a
separate structure whose pointer lay in thread_struct but was modified
based on a suggestion from Ingo Molnar (here:
http://lkml.org/lkml/2009/3/10/210).
I do see that Book-E processors will have severe memory footprint
constraints (in embedded environment) and if the maintainers carry a
different perspective (than the one cited above), the relevant fields
can be migrated to a new structure whose pointer will be embedded in
task_struct. The generic code may have to carry some #ifdefs though.
> >Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> >===================================================================
> >--- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
> >+++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> >@@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> > error_code &= 0x48200000;
> > else
> > is_write = error_code & DSISR_ISSTORE;
> >+
> >+ if (error_code & DSISR_DABRMATCH) {
> >+ /* DABR match */
> >+ do_dabr(regs, address, error_code);
> >+ return 0;
> >+ }
> > #else
> > is_write = error_code & ESR_DST;
> > #endif /* CONFIG_4xx || CONFIG_BOOKE */
> >@@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> > if (!user_mode(regs) && (address >= TASK_SIZE))
> > return SIGSEGV;
> >
> >-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> >- if (error_code & DSISR_DABRMATCH) {
> >- /* DABR match */
> >- do_dabr(regs, address, error_code);
> >- return 0;
> >- }
> >-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
> >-
> > if (in_atomic() || mm == NULL) {
> > if (!user_mode(regs))
> > return SIGSEGV;
>
>
> I don't understand why this was changed, and the changelog doesn't highlight it.
>
> josh
The intention is to capture the exception much before kprobes and xmon
do. The HW Breakpoint exception handler will return NOTIFY_DONE if the
exception doesn't belong to it and doesn't harm the rest. kprobes has
been tested to work fine alongwith HW Breakpoints.
I will add a description about this change in the next iteration of the
patch.
Thanks,
K.Prasad
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: K.Prasad @ 2009-05-12 20:01 UTC (permalink / raw)
To: Michael Neuling; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <4234.1242089764@neuling.org>
On Tue, May 12, 2009 at 10:56:04AM +1000, Michael Neuling wrote:
> > Hi PPC Dev folks,
> > Please find a patch below that implements the proposed Hardware
> > Breakpoint interfaces for PPC64 architecture.
> >
> > As a brief introduction, the proposed Hardware Breakpoint
> > infrastructure provides generic in-kernel interfaces to which users
> > from kernel- and user-space can request for breakpoint registers. An
> > ftrace plugin that can trace accesses to data variables is also part
> > of the generic HW Breakpoint interface patchset. The latest submission
> > for this patchset along with an x86 implementation can be found here:
> > http://lkml.org/lkml/2009/5/11/159.
> >
> > The following are the salient features of the PPC64 patch.
> >
> > - Arch-specific definitions for kernel and user-space requests are
> > defined in arch/powerpc/kernel/hw_breakpoint.c
> > - Ptrace is converted to use the HW Breakpoint interfaces
>
> Will we fall back to use the old method if more than one address is
> being watched?
>
Assuming that you mean HW Breakpoints being used by multiple processes
when you say "more than one address is being watched" - the proposed
infrastructure can take care of such requests. During context switching
in __switch_to() the DABR values of incoming process is restored and is
valid until another process using DABR is scheduled again.
> > - The ftrace plugin called ksym_tracer is tested to work fine. For
> > instance when tracing pid_max kernel variable, the following was
> > obtained as output
>
> Could you split the patch into a few pieces which implement these
> different parts. The smaller logical chucks will make it easier to
> review?
>
Sure. I will slice the patches in the next iteration of posting.
> >
> > # cat trace
> > # tracer: ksym_tracer
> > #
> > # TASK-PID CPU# Symbol Type Function
> > # | | | | |
> > bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
> conv+0x78/0x10c
> > bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
> conv+0xa0/0x10c
> > bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
> >
> > There are however a few limitations/caveats of the patch as identified
> > below:
> >
> > - The patch is currently implemented only for PPC64 architecture. Other
> > architectures (especially Book-E implementations are expected to
> > happen in due course).
> >
> > - HW Breakpoints over data addresses through Xmon (using "bd" command)
> > and the proposed HW Breakpoint interfaces can now operate in a
> > mutually exclusive manner. Xmon's integration is pending and is
> > dependant on successful triggering of breakpoints through "bd<ops>".
> > (Note: On a Power5 machine running 2.6.29, Xmon could not trigger HW
> > Breakpoints when tested).
> >
> > Kindly let me know your comments.
> >
> > Thanks,
> > K.Prasad
> >
> >
> > Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/Kconfig | 1
> > arch/powerpc/include/asm/hw_breakpoint.h | 52 +++++
> > arch/powerpc/include/asm/processor.h | 1
> > arch/powerpc/include/asm/reg.h | 2
> > arch/powerpc/include/asm/thread_info.h | 2
> > arch/powerpc/kernel/Makefile | 2
> > arch/powerpc/kernel/hw_breakpoint.c | 271 ++++++++++++++++++++++++++++
> +++
> > arch/powerpc/kernel/process.c | 18 ++
> > arch/powerpc/kernel/ptrace.c | 48 +++++
> > arch/powerpc/mm/fault.c | 14 -
> > samples/hw_breakpoint/data_breakpoint.c | 4
> > 12 files changed, 423 insertions(+), 9 deletions(-)
>
> You've not touched prace32.c. Can we use this for 32 bit apps on a 64
> bit kernel?
>
Given that PTRACE_SET_DEBUGREG invokes arch_ptrace() in ptrace32.c, I
don't see why it cannot work (although I haven't tested it yet).
> >
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/Kconfig
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> > @@ -125,6 +125,7 @@ config PPC
> > select USE_GENERIC_SMP_HELPERS if SMP
> > select HAVE_OPROFILE
> > select HAVE_SYSCALL_WRAPPERS if PPC64
> > + select HAVE_HW_BREAKPOINT if PPC64
> >
> > config EARLY_PRINTK
> > bool
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/Makefile
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> > @@ -33,7 +33,7 @@ obj-$(CONFIG_PPC64) += setup_64.o sys_p
> > signal_64.o ptrace32.o \
> > paca.o cpu_setup_ppc970.o \
> > cpu_setup_pa6t.o \
> > - firmware.o nvram_64.o
> > + firmware.o nvram_64.o hw_breakpoint.o
> > obj64-$(CONFIG_RELOCATABLE) += reloc_64.o
> > obj-$(CONFIG_PPC64) += vdso64/
> > obj-$(CONFIG_ALTIVEC) += vecemu.o vector.o
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> > @@ -0,0 +1,271 @@
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
> .
> > + *
> > + * Copyright (C) 2009 IBM Corporation
> > + */
> > +
> > +/*
> > + * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
> > + * using the CPU's debug registers.
> > + */
> > +
> > +#include <linux/notifier.h>
> > +#include <linux/kallsyms.h>
> > +#include <linux/kprobes.h>
> > +#include <linux/percpu.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/init.h>
> > +#include <linux/smp.h>
> > +
> > +#include <asm/hw_breakpoint.h>
> > +#include <asm/processor.h>
> > +#include <asm/sstep.h>
> > +
> > +/* Store the kernel-space breakpoint address value */
> > +static unsigned long kdabr;
> > +
> > +/*
> > + * Temporarily stores address for DABR before it is written by the
> > + * single-step handler routine
> > + */
> > +static DEFINE_PER_CPU(unsigned long, dabr_data);
> > +
> > +void arch_update_kernel_hw_breakpoint(void *unused)
> > +{
> > + struct hw_breakpoint *bp;
> > +
> > + /* Check if there is nothing to update */
> > + if (hbp_kernel_pos == HB_NUM)
> > + return;
> > + bp = hbp_kernel[hbp_kernel_pos];
> > + if (bp == NULL)
> > + kdabr = 0;
> > + else
> > + kdabr = bp->info.address | bp->info.type | DABR_TRANSLATION;
> > + set_dabr(kdabr);
> > +}
> > +
> > +/*
> > + * Install the thread breakpoints in their debug registers.
> > + */
> > +void arch_install_thread_hw_breakpoint(struct task_struct *tsk)
> > +{
> > + set_dabr(tsk->thread.dabr);
> > +}
> > +
> > +/*
> > + * Install the debug register values for just the kernel, no thread.
> > + */
> > +void arch_uninstall_thread_hw_breakpoint()
> > +{
> > + set_dabr(0);
> > +}
> > +
> > +/*
> > + * Check for virtual address in user space.
> > + */
> > +int arch_check_va_in_userspace(unsigned long va, u8 hbp_len)
> > +{
> > + return (va <= TASK_SIZE - HW_BREAKPOINT_LEN);
> > +}
>
> You pass ing hbp_len, but then use HW_BREAKPOINT_LEN? Is that right?
>
The 'hbp_len' parameter is useful only when the host processor can watch
for varying range of addresses, which is not the case for PPC64. I guess
I should remove the parameter (although it may be required for other
PowerPC implementations).
> > +
> > +/*
> > + * Check for virtual address in kernel space.
> > + */
> > +int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len)
> > +{
> > + return (va >= TASK_SIZE) && ((va + HW_BREAKPOINT_LEN - 1) >= TASK_SIZE)
> ;
>
> Can you use is_kernel_addr() here?
>
The above routine does more checks than is_kernel_addr() i.e. ensures
that the last address (basically DABR address + address range monitored)
also lies in kernel-space.
However I agree that it isn't significant in PPC64 which has an
alignment requirement that is greater than the range of addresses
monitored. I will use arch_check_va_in_kernelspace() as a wrapper around
is_kernel_addr() for now.
> > +}
> > +
> > +/*
> > + * Store a breakpoint's encoded address, length, and type.
> > + */
> > +int arch_store_info(struct hw_breakpoint *bp)
> > +{
> > + /*
> > + * User-space requests will always have the address field populated
> > + * For kernel-addresses, either the address or symbol name can be
> > + * specified.
> > + */
> > + if (bp->info.name)
> > + bp->info.address = (unsigned long)
> > + kallsyms_lookup_name(bp->info.name);
> > + if (bp->info.address)
> > + return 0;
> > + return -EINVAL;
> > +}
> > +
> > +/*
> > + * Validate the arch-specific HW Breakpoint register settings
> > + */
> > +int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> > + struct task_struct *tsk)
> > +{
> > + int ret = -EINVAL;
> > +
> > + if (!bp)
> > + return ret;
> > +
> > + switch (bp->info.type) {
> > + case DABR_DATA_READ:
> > + break;
> > + case DABR_DATA_WRITE:
> > + break;
> > + case DABR_DATA_RW:
> > + break;
> > + default:
> > + return ret;
> > + }
> > +
> > + if (bp->triggered)
> > + ret = arch_store_info(bp);
> > +
> > + /* Check for double word alignment - 8 bytes */
> > + if (bp->info.address & HW_BREAKPOINT_ALIGN)
> > + return -EINVAL;
> > + return ret;
> > +}
> > +
> > +void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk)
> > +{
> > + struct thread_struct *thread = &(tsk->thread);
> > + struct hw_breakpoint *bp = thread->hbp[0];
> > +
> > + if (bp)
> > + thread->dabr = bp->info.address | bp->info.type |
> > + DABR_TRANSLATION;
> > + else
> > + thread->dabr = 0;
> > +}
> > +
> > +void arch_flush_thread_hw_breakpoint(struct task_struct *tsk)
> > +{
> > + struct thread_struct *thread = &(tsk->thread);
> > +
> > + thread->dabr = 0;
> > +}
> > +
> > +/*
> > + * Handle debug exception notifications.
> > + */
> > +int __kprobes hw_breakpoint_handler(struct die_args *args)
> > +{
> > + int rc = NOTIFY_STOP;
> > + struct hw_breakpoint *bp;
> > + struct pt_regs *regs = args->regs;
> > + unsigned long dar;
> > + int cpu, stepped, is_kernel;
> > +
> > + /* Disable breakpoints during exception handling */
> > + set_dabr(0);
> > +
> > + dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> > + is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
> > +
> > + if (is_kernel)
> > + bp = hbp_kernel[0];
> > + else {
> > + bp = current->thread.hbp[0];
> > + /* Lazy debug register switching */
> > + if (!bp)
> > + return rc;
> > + rc = NOTIFY_DONE;
> > + }
> > +
> > + (bp->triggered)(bp, regs);
> > +
> > + cpu = get_cpu();
> > + if (is_kernel)
> > + per_cpu(dabr_data, cpu) = kdabr;
> > + else
> > + per_cpu(dabr_data, cpu) = current->thread.dabr;
> > +
> > + stepped = emulate_step(regs, regs->nip);
> > + /*
> > + * Single-step the causative instruction manually if
> > + * emulate_step() could not execute it
> > + */
> > + if (stepped == 0) {
> > + regs->msr |= MSR_SE;
> > + goto out;
> > + }
>
> This is where we backout to single step mode?
>
Yes, if emulate_step() has failed us we do manual single-stepping.
> > +
> > + set_dabr(per_cpu(dabr_data, cpu));
> > +out:
> > + /* Enable pre-emption only if single-stepping is finished */
> > + if (stepped)
> > + put_cpu_no_resched();
> > + return rc;
> > +}
> > +
> +/*
> > + * Handle single-step exceptions following a DABR hit.
> > + */
> > +int __kprobes single_step_dabr_instruction(struct die_args *args)
> > +{
> > + struct pt_regs *regs = args->regs;
> > + int cpu = get_cpu();
> > + int ret = NOTIFY_DONE;
> > + siginfo_t info;
> > + unsigned long this_dabr_data = per_cpu(dabr_data, cpu);
> > +
> > + /*
> > + * Check if we are single-stepping as a result of a
> > + * previous HW Breakpoint exception
> > + */
> > + if (this_dabr_data == 0)
> > + goto out;
> > +
> > + regs->msr &= ~MSR_SE;
> > + /* Deliver signal to user-space */
> > + if (this_dabr_data < TASK_SIZE) {
> > + info.si_signo = SIGTRAP;
> > + info.si_errno = 0;
> > + info.si_code = TRAP_HWBKPT;
> > + info.si_addr = (void __user *)(per_cpu(dabr_data, cpu));
> > + force_sig_info(SIGTRAP, &info, current);
> > + }
> > +
> > + set_dabr(this_dabr_data);
> > + per_cpu(dabr_data, cpu) = 0;
> > + ret = NOTIFY_STOP;
> > + put_cpu_no_resched();
> > +
> > +out:
> > + put_cpu_no_resched();
>
> This looks wrong. put_cpu_no_resched() twice?
>
single_step_dabr_instruction() is interested in notifications from
notify_die when DIE_SSTEP is the reason. This means it is invoked when
single stepping occurs due to other requests (such as kprobes/xmon) and
not just after hw_breakpoint_handler(). So, a pair of
"int cpu = get_cpu();" and "put_cpu_no_resched()" are meant for the
above case.
Secondly, we want to atomically single-step the causative instruction after
a HW Breakpoint exception and hence put_cpu_no_resched() is invoked only in
single_step_dabr_instruction() and not in hw_breakpoint_handler() when
single-stepping manually.
Alternatively, smp_processor_id() can be used if preemption is already
disabled and avoid a get_cpu() call but that just adds more code without
any apparent benefit. Hence the put_cpu_no_resched() twice.
Let me know if you can think of something better.
> > + return ret;
> > +}
> > +
> > +/*
> > + * Handle debug exception notifications.
> > + */
> > +int __kprobes hw_breakpoint_exceptions_notify(
> > + struct notifier_block *unused, unsigned long val, void *data)
> > +{
> > + int ret = NOTIFY_DONE;
> > +
> > + switch (val) {
> > + case DIE_DABR_MATCH:
> > + ret = hw_breakpoint_handler(data);
> > + break;
> > + case DIE_SSTEP:
> > + ret = single_step_dabr_instruction(data);
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> > @@ -0,0 +1,52 @@
> > +#ifndef _PPC64_HW_BREAKPOINT_H
> > +#define _PPC64_HW_BREAKPOINT_H
> > +
> > +#ifdef __KERNEL__
> > +#define __ARCH_HW_BREAKPOINT_H
> > +
> > +struct arch_hw_breakpoint {
> > + char *name; /* Contains name of the symbol to set bkpt */
> > + unsigned long address;
> > + u8 type;
> > +};
>
> Can you reorder this to pack the struct better (ie. put the unsigned
> long first).
>
> > +
> > +#include <linux/kdebug.h>
> > +#include <asm/reg.h>
> > +#include <asm-generic/hw_breakpoint.h>
> > +
> > +#define HW_BREAKPOINT_READ DABR_DATA_READ
> > +#define HW_BREAKPOINT_WRITE DABR_DATA_WRITE
> > +#define HW_BREAKPOINT_RW DABR_DATA_RW
> > +
> > +#define HW_BREAKPOINT_ALIGN 0x7
> > +#define HW_BREAKPOINT_LEN 4
>
> What is HW_BREAKPOINT_LEN?
>
> Obviouslt all instructions on ppc64 are 4 bytes. This seems to be
> defined in a few places, like here and MCOUNT_INSN_SIZE (ftrace.h). Can
> you create a #define for this generically and get all refers to use this
> instead of #define-ing 4 everywhere.
>
Agreed. A macro definition should have ideally existed in
"arch/powerpc/include/asm/reg.h" already but I found none.
Through a separate patch, I will define INSTRUCTION_LEN in reg.h and
define HW_BREAKPOINT_LEN to INSTRUCTION_LEN (because the former gels
well with the surrounding HW Breakpoint related name-space).
> > +
> > +extern struct hw_breakpoint *hbp_kernel[HB_NUM];
> > +extern unsigned int hbp_user_refcount[HB_NUM];
> > +
> > +/*
> > + * Ptrace support: breakpoint trigger routine.
> > + */
> > +extern int __modify_user_hw_breakpoint(int pos, struct task_struct *tsk,
> > + struct hw_breakpoint *bp);
> > +
> > +extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
> > +extern void arch_uninstall_thread_hw_breakpoint(void);
> > +extern int arch_check_va_in_userspace(unsigned long va, u8 hbp_len);
> > +extern int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len);
> > +extern int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> > + struct task_struct *tsk);
> > +extern void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk)
> ;
> > +extern void arch_flush_thread_hw_breakpoint(struct task_struct *tsk);
> > +extern void arch_update_kernel_hw_breakpoint(void *);
> > +extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> > + unsigned long val, void *data);
> > +
> > +extern void flush_thread_hw_breakpoint(struct task_struct *tsk);
> > +extern int copy_thread_hw_breakpoint(struct task_struct *tsk,
> > + struct task_struct *child, unsigned long clone_flags);
> > +extern void switch_to_thread_hw_breakpoint(struct task_struct *tsk);
> > +
> > +#endif /* __KERNEL__ */
> > +#endif /* _PPC64_HW_BREAKPOINT_H */
> > +
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/processor.h
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> > @@ -177,6 +177,7 @@ struct thread_struct {
> > #ifdef CONFIG_PPC64
> > unsigned long start_tb; /* Start purr when proc switched in */
> > unsigned long accum_tb; /* Total accumilated purr for process *
> /
> > + struct hw_breakpoint *hbp[HB_NUM];
> > #endif
> > unsigned long dabr; /* Data address breakpoint register */
> > #ifdef CONFIG_ALTIVEC
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> > @@ -37,6 +37,9 @@
> > #include <asm/page.h>
> > #include <asm/pgtable.h>
> > #include <asm/system.h>
> > +#ifdef CONFIG_PPC64
> > +#include <asm/hw_breakpoint.h>
> > +#endif
> >
> > /*
> > * does not yet catch signals sent when the child dies.
> > @@ -735,9 +738,22 @@ void user_disable_single_step(struct tas
> > clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> > }
> >
> > +static void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> > +{
> > + /*
> > + * The SIGTRAP signal is generated automatically for us in do_dabr().
> > + * We don't have to do anything here
> > + */
> > +}
> > +
> > int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> > unsigned long data)
> > {
> > +#ifdef CONFIG_PPC64
> > + struct thread_struct *thread = &(task->thread);
> > + struct hw_breakpoint *bp;
> > + int ret;
> > +#endif
> > /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> > * For embedded processors we support one DAC and no IAC's at the
> > * moment.
> > @@ -767,6 +783,38 @@ int ptrace_set_debugreg(struct task_stru
> > if (data && !(data & DABR_TRANSLATION))
> > return -EIO;
> >
> > +#ifdef CONFIG_PPC64
> > + bp = thread->hbp[0];
> > + if ((data & ~0x7UL) == 0) {
>
> Should use HW_BREAKPOINT_ALIGN here instead of 0x7? Is 0 some special
> command? Seems to be lots of special numbers here related to using data
> (later you mask is 0x3). What is happening here? What is contained in
> the data parameter? It overloads the type and the address?
>
I agree that HW_BREAKPOINT_ALIGN must have been used here and will
eliminate 0x3 using DABR_DATA_RW.
'data' variable is a composite of address | translation_enabled | type.
> > + if (bp) {
> > + unregister_user_hw_breakpoint(task, bp);
> > + kfree(bp);
> > + thread->hbp[0] = NULL;
> > + }
> > + return 0;
> > + }
> > +
> > + if (bp) {
> > + bp->info.type = data & 0x3UL;
> > + task->thread.dabr = bp->info.address =
> > + (data & ~HW_BREAKPOINT_ALIGN);
> > + return __modify_user_hw_breakpoint(0, task, bp);
> > + }
> > + bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
>
> When a processes ends, will this be correctly freed? Should it be freed
> in arch_flush_thread_hw_breakpoint?
>
It is freed in flush_thread_hw_breakpoint() which is a part of the
proposed kernel/hw_breakpoint.c (http://lkml.org/lkml/2009/5/11/159).
> > + if (!bp)
> > + return -ENOMEM;
> > +
> > + /* Store the type of breakpoint */
> > + bp->info.type = data & 0x3UL;
> > + bp->triggered = ptrace_triggered;
> > + task->thread.dabr = bp->info.address = (data & ~HW_BREAKPOINT_ALIGN);
> > +
> > + ret = register_user_hw_breakpoint(task, bp);
> > + if (ret)
> > + return ret;
> > + set_tsk_thread_flag(task, TIF_DEBUG);
> > +#endif /* CONFIG_PPC64 */
> > +
> > /* Move contents to the DABR register */
> > task->thread.dabr = data;
> >
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/process.c
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> > @@ -50,6 +50,7 @@
> > #include <asm/syscalls.h>
> > #ifdef CONFIG_PPC64
> > #include <asm/firmware.h>
> > +#include <asm/hw_breakpoint.h>
> > #endif
> > #include <linux/kprobes.h>
> > #include <linux/kdebug.h>
> > @@ -254,8 +255,10 @@ void do_dabr(struct pt_regs *regs, unsig
> > 11, SIGSEGV) == NOTIFY_STOP)
> > return;
> >
> > +#ifndef CONFIG_PPC64
> > if (debugger_dabr_match(regs))
> > return;
> > +#endif
> >
> > /* Clear the DAC and struct entries. One shot trigger */
> > #if defined(CONFIG_BOOKE)
> > @@ -372,8 +375,13 @@ struct task_struct *__switch_to(struct t
> >
> > #endif /* CONFIG_SMP */
> >
> > +#ifdef CONFIG_PPC64
> > + if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
> > + switch_to_thread_hw_breakpoint(new);
> > +#else
> > if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
> > set_dabr(new->thread.dabr);
> > +#endif /* CONFIG_PPC64 */
> >
> > #if defined(CONFIG_BOOKE)
> > /* If new thread DAC (HW breakpoint) is the same then leave it */
> > @@ -550,6 +558,10 @@ void show_regs(struct pt_regs * regs)
> > void exit_thread(void)
> > {
> > discard_lazy_cpu_state();
> > +#ifdef CONFIG_PPC64
> > + if (unlikely(test_tsk_thread_flag(current, TIF_DEBUG)))
> > + flush_thread_hw_breakpoint(current);
> > +#endif /* CONFIG_PPC64 */
> > }
> >
> > void flush_thread(void)
> > @@ -605,6 +617,9 @@ int copy_thread(unsigned long clone_flag
> > struct pt_regs *childregs, *kregs;
> > extern void ret_from_fork(void);
> > unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
> > +#ifdef CONFIG_PPC64
> > + struct task_struct *tsk = current;
> > +#endif
> >
> > CHECK_FULL_REGS(regs);
> > /* Copy registers */
> > @@ -672,6 +687,9 @@ int copy_thread(unsigned long clone_flag
> > * function.
> > */
> > kregs->nip = *((unsigned long *)ret_from_fork);
> > +
> > + if (unlikely(test_tsk_thread_flag(tsk, TIF_DEBUG)))
> > + copy_thread_hw_breakpoint(tsk, p, clone_flags);
> > #else
> > kregs->nip = (unsigned long)ret_from_fork;
> > #endif
> > Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c
> > +++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> > @@ -54,6 +54,10 @@ static int __init hw_break_module_init(v
> > sample_hbp.info.type = HW_BREAKPOINT_WRITE;
> > sample_hbp.info.len = HW_BREAKPOINT_LEN_4;
> > #endif /* CONFIG_X86 */
> > +#ifdef CONFIG_PPC64
> > + sample_hbp.info.name = ksym_name;
> > + sample_hbp.info.type = DABR_DATA_WRITE;
> > +#endif /* CONFIG_PPC64 */
> >
> > sample_hbp.triggered = (void *)sample_hbp_handler;
> >
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/thread_info.h
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> > @@ -114,6 +114,7 @@ static inline struct thread_info *curren
> > #define TIF_FREEZE 14 /* Freezing for suspend */
> > #define TIF_RUNLATCH 15 /* Is the runlatch enabled? */
> > #define TIF_ABI_PENDING 16 /* 32/64 bit switch needed */
> > +#define TIF_DEBUG 17 /* uses debug registers */
> >
> > /* as above, but as bit values */
> > #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
> > @@ -132,6 +133,7 @@ static inline struct thread_info *curren
> > #define _TIF_FREEZE (1<<TIF_FREEZE)
> > #define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
> > #define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING)
> > +#define _TIF_DEBUG (1<<TIF_DEBUG)
> > #define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SEC
> COMP)
> >
> > #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/reg.h
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> > @@ -184,9 +184,11 @@
> > #define CTRL_TE 0x00c00000 /* thread enable */
> > #define CTRL_RUNLATCH 0x1
> > #define SPRN_DABR 0x3F5 /* Data Address Breakpoint Register */
> > +#define HB_NUM 1 /* Number of physical HW breakpoint registers *
> /
>
> You've shortedned "hardware breakpoint" to "hbp" elsewhere. Can you be
> consistent here so change this to HBP_NUM?
>
Thanks for pointing it out! I will change.
> > #define DABR_TRANSLATION (1UL << 2)
> > #define DABR_DATA_WRITE (1UL << 1)
> > #define DABR_DATA_READ (1UL << 0)
> > +#define DABR_DATA_RW (3UL << 0)
> > #define SPRN_DABR2 0x13D /* e300 */
> > #define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension *
> /
> > #define DABRX_USER (1UL << 0)
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> > @@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> > error_code &= 0x48200000;
> > else
> > is_write = error_code & DSISR_ISSTORE;
> > +
> > + if (error_code & DSISR_DABRMATCH) {
> > + /* DABR match */
> > + do_dabr(regs, address, error_code);
> > + return 0;
> > + }
> > #else
> > is_write = error_code & ESR_DST;
> > #endif /* CONFIG_4xx || CONFIG_BOOKE */
> > @@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> > if (!user_mode(regs) && (address >= TASK_SIZE))
> > return SIGSEGV;
> >
> > -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> > - if (error_code & DSISR_DABRMATCH) {
> > - /* DABR match */
> > - do_dabr(regs, address, error_code);
> > - return 0;
> > - }
> > -#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
> > -
> > if (in_atomic() || mm == NULL) {
> > if (!user_mode(regs))
> > return SIGSEGV;
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> >
Thanks for the code-review. I will send out the revised patchset with
the changes agreed above.
-- K.Prasad
^ permalink raw reply
* [PATCH] [PowerPC] MPC8272ADS: fix device tree for 8 MB flash size
From: Wolfgang Denk @ 2009-05-12 19:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Scott Wood, linux-kernel, Wolfgang Denk
The current device tree for the MPC8272ADS assumes a mapping of 32 MB
of NOR flash at 0xFE00.0000, while there are actually only 8 MB on
the boards, mapped at 0xFF80.0000. When booting an uImage with such a
device tree, the kernel crashes because 0xFE00.0000 is not mapped.
Also introduce aliases for serial[01] and ethernet[01].
Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
cc: Kumar Gala <galak@kernel.crashing.org>
---
I am not really sure what the typical flash size on MPC8272ADS boards
is. The board I used for testing is marked as "Prototype", so it may
not be the release configuration. On the other hand, the manual also
says 8 MB, Vitaly Borduk confirms 8 MB on his board, too, and Scott
Wood eventually tested only with cuImage which fixes up the localbus
mappings, thus eventually concealing the issue.
I'm posting this as reference in case the 8 MB should turn out to be
correct. -- wd
arch/powerpc/boot/dts/mpc8272ads.dts | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/boot/dts/mpc8272ads.dts
index 2a1929a..60f3327 100644
--- a/arch/powerpc/boot/dts/mpc8272ads.dts
+++ b/arch/powerpc/boot/dts/mpc8272ads.dts
@@ -17,6 +17,13 @@
#address-cells = <1>;
#size-cells = <1>;
+ aliases {
+ ethernet0 = ð0;
+ ethernet1 = ð1;
+ serial0 = &scc1;
+ serial1 = &scc4;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -46,13 +53,13 @@
#size-cells = <1>;
reg = <0xf0010100 0x40>;
- ranges = <0x0 0x0 0xfe000000 0x2000000
+ ranges = <0x0 0x0 0xff800000 0x00800000
0x1 0x0 0xf4500000 0x8000
0x3 0x0 0xf8200000 0x8000>;
flash@0,0 {
compatible = "jedec-flash";
- reg = <0x0 0x0 0x2000000>;
+ reg = <0x0 0x0 0x00800000>;
bank-width = <4>;
device-width = <1>;
};
@@ -144,7 +151,7 @@
reg = <0x119f0 0x10 0x115f0 0x10>;
};
- serial@11a00 {
+ scc1: serial@11a00 {
device_type = "serial";
compatible = "fsl,mpc8272-scc-uart",
"fsl,cpm2-scc-uart";
@@ -155,7 +162,7 @@
fsl,cpm-command = <0x800000>;
};
- serial@11a60 {
+ scc4: serial@11a60 {
device_type = "serial";
compatible = "fsl,mpc8272-scc-uart",
"fsl,cpm2-scc-uart";
@@ -192,7 +199,7 @@
};
};
- ethernet@11300 {
+ eth0: ethernet@11300 {
device_type = "network";
compatible = "fsl,mpc8272-fcc-enet",
"fsl,cpm2-fcc-enet";
@@ -205,7 +212,7 @@
fsl,cpm-command = <0x12000300>;
};
- ethernet@11320 {
+ eth1: ethernet@11320 {
device_type = "network";
compatible = "fsl,mpc8272-fcc-enet",
"fsl,cpm2-fcc-enet";
--
1.6.0.6
^ permalink raw reply related
* RE: Unable to boot 2.6.29 (and tot kernel) with 256k page size on katmai.
From: Shubhada Pugaonkar @ 2009-05-12 17:44 UTC (permalink / raw)
To: Shubhada Pugaonkar, Wolfgang Denk; +Cc: linuxppc-dev
This issue was resolved by flashing tot u-boot on the board. An engineer
at AMCC helped me solve this problem. It looks like even though we
bought the board very recently, somehow the u-boot version on the board
was much older.=20
Thanks
Shubhada
-----Original Message-----
From: Shubhada Pugaonkar=20
Sent: Wednesday, May 06, 2009 3:54 PM
To: 'Wolfgang Denk'
Cc: linuxppc-dev@ozlabs.org
Subject: RE: Unable to boot 2.6.29 (and tot kernel) with 256k page size
on katmai.
I am using ELDK4.2 that came with the katmai board.=20
Yes that is all I get. If I use 64k page size then I get following log
if that helps determine anything.=20
=3D> tftp 1000000 uImage-2.6.29-denx
Waiting for PHY auto negotiation to complete.. done
ENET Speed is 100 Mbps - FULL duplex connection (EMAC0)
Using ppc_4xx_eth0 device
TFTP from server 10.192.165.106; our IP address is 10.192.164.166
Filename 'uImage-2.6.29-denx'.
Load address: 0x1000000
Loading:
#################################################################
############################################################
done
Bytes transferred =3D 1822193 (1bcdf1 hex)
=3D> tftp 4000000 katmai.dts.256k
Using ppc_4xx_eth0 device
TFTP from server 10.192.165.106; our IP address is 10.192.164.166
Filename 'katmai.dts.256k'.
Load address: 0x4000000
Loading: ##
done
Bytes transferred =3D 16384 (4000 hex)
=3D> bootm 1000000 - 4000000
## Booting image at 01000000 ...
Image Name: Linux-2.6.29-rc8-01447-g3bf8ce5-
Created: 2009-05-06 0:34:46 UTC
Image Type: PowerPC Linux Kernel Image (gzip compressed)
Data Size: 1822129 Bytes =3D 1.7 MB
Load Address: 00000000
Entry Point: 00000000
Verifying Checksum ... OK
Uncompressing Kernel Image ... OK
Booting using the fdt at 0x4000000
Loading Device Tree to 007fb000, end 007fefff ... OK
Using PowerPC 44x Platform machine description
Linux version 2.6.29-rc8-01447-g3bf8ce5-dirty (root@california) (gcc
version 4.2.2) #2 Tue May 5 17:34:41 PDT 2009
console [udbg0] enabled
setup_arch: bootmem
arch: exit
Zone PFN ranges:
DMA 0x00000000 -> 0x00003000
Normal 0x00003000 -> 0x00003000
HighMem 0x00003000 -> 0x00008000
Movable zone start PFN for each node
early_node_map[1] active PFN ranges
0: 0x00000000 -> 0x00008000
MMU: Allocated 1088 bytes of context maps for 255 contexts
Built 1 zonelists in Zone order, mobility grouping on. Total pages:
32752
Kernel command line:
UIC0 (32 IRQ sources) at DCR 0xc0
UIC1 (32 IRQ sources) at DCR 0xd0
UIC2 (32 IRQ sources) at DCR 0xe0
UIC3 (32 IRQ sources) at DCR 0xf0
PID hash table entries: 4096 (order: 12, 16384 bytes)
clocksource: timebase mult[500000] shift[22] registered
Dentry cache hash table entries: 131072 (order: 3, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 2, 262144 bytes)
Memory: 2089472k/2097152k available (3776k kernel code, 7040k reserved,
192k data, 736k bss, 384k init)
SLUB: Genslabs=3D14, HWalign=3D32, Order=3D0-3, MinObjects=3D0, =
CPUs=3D1, Nodes=3D1
Calibrating delay loop... 1597.44 BogoMIPS (lpj=3D3194880)
Mount-cache hash table entries: 8192
net_namespace: 296 bytes
xor: measuring software checksum speed
8regs : 112.000 MB/sec
8regs_prefetch: 144.000 MB/sec
32regs : 112.000 MB/sec
32regs_prefetch: 144.000 MB/sec
xor: using function: 32regs_prefetch (144.000 MB/sec)
NET: Registered protocol family 16
PCIE0: Checking link...
PCIE0: Device detected, waiting for link...
PCIE0: link is up !
PCI host bridge /plb/pciex@d00000000 (primary) ranges:
MEM 0x0000000e00000000..0x0000000e7fffffff -> 0x0000000080000000
IO 0x0000000f80000000..0x0000000f8000ffff -> 0x0000000000000000
4xx PCI DMA offset set to 0x00000000
PCIE0: successfully set as root-complex
PCIE1: Checking link...
PCIE1: Device detected, waiting for link...
PCIE1: link is up !
PCI host bridge /plb/pciex@d20000000 (primary) ranges:
MEM 0x0000000e80000000..0x0000000effffffff -> 0x0000000080000000
IO 0x0000000f80010000..0x0000000f8001ffff -> 0x0000000000000000
4xx PCI DMA offset set to 0x00000000
PCIE1: successfully set as root-complex
PCIE2: Checking link...
PCIE2: Device detected, waiting for link...
PCIE2: link is up !
PCI host bridge /plb/pciex@d40000000 (primary) ranges:
MEM 0x0000000f00000000..0x0000000f7fffffff -> 0x0000000080000000
IO 0x0000000f80020000..0x0000000f8002ffff -> 0x0000000000000000
4xx PCI DMA offset set to 0x00000000
PCIE2: successfully set as root-complex
PCI host bridge /plb/pci@c0ec00000 (primary) ranges:
MEM 0x0000000d80000000..0x0000000dffffffff -> 0x0000000080000000
IO 0x0000000c08000000..0x0000000c0800ffff -> 0x0000000000000000
4xx PCI DMA offset set to 0x00000000
PCI: Probing PCI hardware
PCI: Hiding 4xx host bridge resources 0000:10:00.0
pci 0000:11:00.0: PME# supported from D0 D3hot
pci 0000:11:00.0: PME# disabled
PCI: Hiding 4xx host bridge resources 0001:20:00.0
PCI: Hiding 4xx host bridge resources 0002:30:00.0
pci 0000:10:00.0: PCI bridge, secondary bus 0000:11
pci 0000:10:00.0: IO window: disabled
pci 0000:10:00.0: MEM window: 0x80000000-0x80bfffff
pci 0000:10:00.0: PREFETCH window: 0x00000080c00000-0x00000080cfffff
pci 0001:20:00.0: PCI bridge, secondary bus 0001:21
pci 0001:20:00.0: IO window: 0x1000-0x1fff
pci 0001:20:00.0: MEM window: 0x80200000-0x802fffff
pci 0001:20:00.0: PREFETCH window: 0x00000080000000-0x000000801fffff
pci 0002:30:00.0: PCI bridge, secondary bus 0002:31
pci 0002:30:00.0: IO window: 0x1000-0x1fff
pci 0002:30:00.0: MEM window: 0x80200000-0x802fffff
pci 0002:30:00.0: PREFETCH window: 0x00000080000000-0x000000801fffff
bio: create slab <bio-0> at 0
SCSI subsystem initialized
NET: Registered protocol family 2
Switched to NOHz mode on CPU #0
IP route cache hash table entries: 16384 (order: 0, 65536 bytes)
TCP established hash table entries: 32768 (order: 2, 262144 bytes)
TCP bind hash table entries: 32768 (order: 1, 131072 bytes)
TCP: Hash tables configured (established 32768 bind 32768)
TCP reno registered
NET: Registered protocol family 1
highmem bounce pool size: 64 pages
msgmni has been set to 1520
alg: No test for stdrng (krng)
async_tx: api initialized (sync-only)
io scheduler noop registered
io scheduler anticipatory registered (default)
io scheduler deadline registered
io scheduler cfq registered
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
serial8250.0: ttyS0 at MMIO 0x4f0000200 (irq =3D 21) is a 16550A
console
handover: boot [udbg0] -> real [ttyS0]
serial8250.0: ttyS1 at MMIO 0x4f0000300 (irq =3D 22) is a 16550A
serial8250.0: ttyS2 at MMIO 0x4f0000600 (irq =3D 23) is a 16550A
4f0000200.serial: ttyS0 at MMIO 0x4f0000200 (irq =3D 21) is a 16550A
4f0000300.serial: ttyS1 at MMIO 0x4f0000300 (irq =3D 22) is a 16550A
4f0000600.serial: ttyS2 at MMIO 0x4f0000600 (irq =3D 23) is a 16550A
brd: module loaded
Xilinx SystemACE device driver, major=3D253
PPC 4xx OCP EMAC driver, version 3.54
MAL v2 /plb/mcmal, 2 TX channels, 1 RX channels
eth0 (emac): not using net_device_ops yet
eth0: EMAC-0 /plb/opb/ethernet@10000800, MAC 00:01:73:77:56:64
eth0: found Generic MII PHY (0x01)
Driver 'sd' needs updating - please use bus_type methods
Fusion MPT base driver 3.04.07
Copyright (c) 1999-2008 LSI Corporation
Fusion MPT SAS Host driver 3.04.07
mptsas 0001:21:00.0: enabling device (0000 -> 0002)
mptbase: ioc0: Initiating bringup
ioc0: LSISAS1068E B3: Capabilities=3D{Initiator}
scsi0 : ioc0: LSISAS1068E B3, FwRev=3D01170200h, Ports=3D1, MaxQ=3D286, =
IRQ=3D19
scsi 0:0:0:0: Direct-Access SEAGATE ST336754SS S410 PQ: 0
ANSI: 5
sd 0:0:0:0: [sda] 71132959 512-byte hardware sectors: (36.4 GB/33.9 GiB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, supports
DPO and FUA
sd 0:0:0:0: [sda] 71132959 512-byte hardware sectors: (36.4 GB/33.9 GiB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, supports
DPO and FUA
sda: sda1
.
.
.
=20
Thanks
Shubhada
-----Original Message-----
From: Wolfgang Denk [mailto:wd@denx.de]=20
Sent: Wednesday, May 06, 2009 3:34 PM
To: Shubhada Pugaonkar
Cc: linuxppc-dev@ozlabs.org
Subject: Re: Unable to boot 2.6.29 (and tot kernel) with 256k page size
on katmai.
Dear Shubhada,
In message
<8A71B368A89016469F72CD08050AD33402D575FA@maui.asicdesigners.com> you
wrote:
>=20
> I am unable to boot the 2.6.29-rc8 denx kernel (and 2.6.30-rc4 tot
> kernel) when I enable the 256k page size. My config file is attached.
> The *same* config works fine with 64k page size. I have 2GB memory on
> this board.=20
Which root file system are you using?
> ## Booting image at 01000000 ...
> Image Name: Linux-2.6.29-rc8-01447-g3bf8ce5-
> Created: 2009-05-05 0:31:36 UTC
> Image Type: PowerPC Linux Kernel Image (gzip compressed)
> Data Size: 2138773 Bytes =3D 2 MB
> Load Address: 00000000
> Entry Point: 00000000
> Verifying Checksum ... OK
> Uncompressing Kernel Image ... OK
> Booting using the fdt at 0x4000000
> Loading Device Tree to 007fb000, end 007fefff ... OK
Is this all you get? Nothing else?
Best regards,
Wolfgang Denk
--=20
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"Nobody will ever need more than 640k RAM!" -- Bill Gates, 1981
"Windows 95 needs at least 8 MB RAM." -- Bill Gates, 1996
"Nobody will ever need Windows 95." -- logical conclusion
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: Scott Wood @ 2009-05-12 16:47 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Benjamin Herrenschmidt, K.Prasad, paulus
In-Reply-To: <20090512115149.GA1885@yoda.jdub.homelinux.org>
On Tue, May 12, 2009 at 07:51:49AM -0400, Josh Boyer wrote:
> On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
> >- The patch is currently implemented only for PPC64 architecture. Other
> > architectures (especially Book-E implementations are expected to
> > happen in due course).
>
> Does this mean you will work on transitioning Book-E implementations, or that
> you expect the Book-E maintainers to? I'm just curious. The code as written
> relies heavily on the DABR/MSR setup that ppc64 has and Book-E unfortunately
> doesn't follow that at all.
And since there will eventually be 64-bit book E chips, we need to use
something more specific than CONFIG_PPC64 in the #ifdef (easier now than
figuring out which ones are breakpoint-related later).
-Scott
^ permalink raw reply
* Re: question about softirqs
From: Chris Friesen @ 2009-05-12 15:18 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, netdev, Steven Rostedt, linuxppc-dev, paulus,
Thomas Gleixner, David Miller
In-Reply-To: <20090512081237.GA16403@elte.hu>
Ingo Molnar wrote:
> * Chris Friesen <cfriesen@nortel.com> wrote:
>>I think I see a possible problem with this. Suppose I have a
>>SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT set. Under
>>the scenario above, schedule() would re-run the spinning task
>>rather than ksoftirqd, thus preventing any incoming packets from
>>being sent up the stack until we get a real hardware
>>interrupt--which could be a whole jiffy if interrupt mitigation is
>>enabled in the net device.
>>DaveM pointed out that if we're doing transmits we're likely to
>>hit local_bh_enable(), which would process the softirq work.
>>However, I think we may still have a problem in the above rx-only
>>scenario--or is it too contrived to matter?
> This could occur, and the problem is really that task priorities do
> not extend across softirq work processing.
>
> This could occur in ordinary SCHED_OTHER tasks as well, if the
> softirq is bounced to ksoftirqd - which it only should be if there's
> serious softirq overload - or, as you describe it above, if the
> softirq is raised in process context:
One of the reasons I brought up this issue is that there is a lot of
documentation out there that says "softirqs will be processed on return
from a syscall". The fact that it actually depends on the scheduler
parameters of the task issuing the syscall isn't ever mentioned.
In fact, "Documentation/DocBook/kernel-hacking.tmpl" in the kernel
source still has the following:
Whenever a system call is about to return to userspace, or a
hardware interrupt handler exits, any 'software interrupts'
which are marked pending (usually by hardware interrupts) are
run (<filename>kernel/softirq.c</filename>).
If anyone is looking at changing this code, it might be good to ensure
that at least the kernel docs are updated.
Chris
^ permalink raw reply
* Re: Xilinx FIFO TEMAC !?
From: Grant Likely @ 2009-05-12 14:41 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A09234E.4020401@dlasys.net>
On Tue, May 12, 2009 at 1:20 AM, David H. Lynch Jr. <dhlii@dlasys.net> wrot=
e:
> =A0 =A0 =A0 Anyway is there any interest in an LL FIFO TEMAC driver ?
> =A0 =A0 =A0 Or adding the FIFO code to the SDMA TEMAC code ? It only adds=
a
> couple of dozen lines to the driver.
yes to both.
BTW, when discussing the lltemac driver, you should cc: both the
linuxppc-dev and the netdev mailing lists. You should also cc: me,
otherwise I might not see it.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: howto disable dcache (on a MPC8313)
From: Norbert van Bolhuis @ 2009-05-12 14:10 UTC (permalink / raw)
Cc: linuxppc-dev
In-Reply-To: <4A02F905.6080904@aimvalley.nl>
I hate open ends, so for those interested.
I made a small change in u-boot and kernel and it works now.
btw. it makes a huge performance difference. The kernel boots
much slower and my user-space calculation now runs in 44 secs
(no data-cache) instead of 2.8 secs (with data-cache).
in u-boot the data-cache is initially used for data and stack.
Once RAM is available and the u-boot relocation has been done
the data cache can be disabled by clearing DCE in HID0. This
must be done after flushing the cache.
diff -C 5 -r1.3 start.S
*** start.S 2 Apr 2009 10:36:46 -0000 1.3
--- start.S 8 May 2009 13:44:38 -0000
***************
*** 928,937 ****
--- 928,949 ----
5: dcbst 0,r4
add r4,r4,r6
cmplw r4,r5
blt 5b
sync /* Wait for all dcbst to complete on bus */
+
+ /* disable data-cache (TEST) */
+ mfspr r20, HID0
+ li r21, HID0_DCE|HID0_DLOCK
+ andc r20, r20, r21
+ ori r21, r20, HID0_DCFI
+ sync
+ mtspr HID0, r21 /* sets invalidate, clears enable and lock */
+ sync
+ mtspr HID0, r20 /* clears invalidate */
+
+
mr r4,r3
6: icbi 0,r4
add r4,r4,r6
cmplw r4,r5
blt 6b
The linux kernel enables the cache through
__setup_cpu_603 -> setup_common_caches
this function gets called from call_setup_cpu which has nothing
to do with CONFIG_(HAVE_)OPROFILE.
It's easy to modify this function not to set the HID0_DCE (without
caring much about the assembly).
diff -C 5 -r1.1.1.1 cpu_setup_6xx.S
*** arch/powerpc/kernel/cpu_setup_6xx.S 5 Jan 2009 10:55:25 -0000 1.1.1.1
--- arch/powerpc/kernel/cpu_setup_6xx.S 8 May 2009 13:53:31 -0000
***************
*** 79,90 ****
blr
/* Enable caches for 603's, 604, 750 & 7400 */
setup_common_caches:
mfspr r11,SPRN_HID0
! andi. r0,r11,HID0_DCE
! ori r11,r11,HID0_ICE|HID0_DCE
ori r8,r11,HID0_ICFI
bne 1f /* don't invalidate the D-cache */
ori r8,r8,HID0_DCI /* unless it wasn't enabled */
1: sync
mtspr SPRN_HID0,r8 /* enable and invalidate caches */
--- 79,90 ----
blr
/* Enable caches for 603's, 604, 750 & 7400 */
setup_common_caches:
mfspr r11,SPRN_HID0
! andi. r0,r11,(0<<14)
! ori r11,r11,HID0_ICE|(0<<14)
ori r8,r11,HID0_ICFI
bne 1f /* don't invalidate the D-cache */
ori r8,r8,HID0_DCI /* unless it wasn't enabled */
1: sync
mtspr SPRN_HID0,r8 /* enable and invalidate caches */
^ permalink raw reply
* Re: question about softirqs
From: Steven Rostedt @ 2009-05-12 12:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: netdev, David Miller, linuxppc-dev, paulus, Ingo Molnar,
Thomas Gleixner
In-Reply-To: <1242120761.11251.324.camel@twins>
On Tue, 12 May 2009, Peter Zijlstra wrote:
> On Tue, 2009-05-12 at 11:23 +0200, Ingo Molnar wrote:
> >
> > Yeah, that would be "nice". A single IRQ thread plus the process
> > context(s) doing networking might perform well.
> >
> > Multiple IRQ threads (softirq and hardirq threads mixed) i'm not so
> > sure about - it's extra context-switching cost.
>
> Sure, that was implied by the getting rid of softirqs ;-), on -rt we
> currently suffer this hardirq/softirq thread ping-pong, it sucks.
I'm going to be playing around with bypassing the net-rx/tx with my
network drivers. I'm going to add threaded irqs for my network cards and
have the driver threads do the work to get through the tcp/ip stack.
I'll still keep the softirqs for other cards, but I want to see how fast
it speeds things up if I have the driver thread do it.
-- Steve
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: Josh Boyer @ 2009-05-12 11:51 UTC (permalink / raw)
To: K.Prasad; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090511200355.GA17988@in.ibm.com>
On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
>Hi PPC Dev folks,
> Please find a patch below that implements the proposed Hardware
>Breakpoint interfaces for PPC64 architecture.
>
>As a brief introduction, the proposed Hardware Breakpoint
>infrastructure provides generic in-kernel interfaces to which users
>from kernel- and user-space can request for breakpoint registers. An
>ftrace plugin that can trace accesses to data variables is also part
>of the generic HW Breakpoint interface patchset. The latest submission
>for this patchset along with an x86 implementation can be found here:
>http://lkml.org/lkml/2009/5/11/159.
>
>The following are the salient features of the PPC64 patch.
>
>- Arch-specific definitions for kernel and user-space requests are
> defined in arch/powerpc/kernel/hw_breakpoint.c
>- Ptrace is converted to use the HW Breakpoint interfaces
>- The ftrace plugin called ksym_tracer is tested to work fine. For
> instance when tracing pid_max kernel variable, the following was
> obtained as output
>
># cat trace
># tracer: ksym_tracer
>#
># TASK-PID CPU# Symbol Type Function
># | | | | |
>bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0x78/0x10c
>bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0xa0/0x10c
>bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
>
>There are however a few limitations/caveats of the patch as identified
>below:
>
>- The patch is currently implemented only for PPC64 architecture. Other
> architectures (especially Book-E implementations are expected to
> happen in due course).
Does this mean you will work on transitioning Book-E implementations, or that
you expect the Book-E maintainers to? I'm just curious. The code as written
relies heavily on the DABR/MSR setup that ppc64 has and Book-E unfortunately
doesn't follow that at all.
Book-E also allows for more than one HW breakpoint, which means you're growing
the thread_struct by 32-bytes to support 4 of them. 64-bytes if this ever
supports DAC events. Have you thought at all about a way to support this
without carrying around the data in the thread struct?
<snip>
>Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
>===================================================================
>--- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
>+++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
>@@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> error_code &= 0x48200000;
> else
> is_write = error_code & DSISR_ISSTORE;
>+
>+ if (error_code & DSISR_DABRMATCH) {
>+ /* DABR match */
>+ do_dabr(regs, address, error_code);
>+ return 0;
>+ }
> #else
> is_write = error_code & ESR_DST;
> #endif /* CONFIG_4xx || CONFIG_BOOKE */
>@@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> if (!user_mode(regs) && (address >= TASK_SIZE))
> return SIGSEGV;
>
>-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
>- if (error_code & DSISR_DABRMATCH) {
>- /* DABR match */
>- do_dabr(regs, address, error_code);
>- return 0;
>- }
>-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
>-
> if (in_atomic() || mm == NULL) {
> if (!user_mode(regs))
> return SIGSEGV;
I don't understand why this was changed, and the changelog doesn't highlight it.
josh
^ permalink raw reply
* Re: [PowerPC] Next May 8 boot failure: OOPS during ibmveth moduleinit
From: Wei Yongjun @ 2009-05-12 8:17 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Jiri Pirko, netdev, linuxppc-dev, linux-next, David Miller
In-Reply-To: <20090512174451.eeed4126.sfr@canb.auug.org.au>
Stephen Rothwell wrote:
> Hi Dave,
>
> This fixes it (I wonder if this bug is lurking in any other drivers):
>
Yes, there are some other exists. This spatch script can help to found this.
(http://www.emn.fr/x-info/coccinelle/)
# cat netdev_dev_addr.cocci
@@
struct net_device *dev;
@@
memcpy(
- &dev->dev_addr
+ dev->dev_addr
, ...);
@@
struct net_device *dev;
expression E;
@@
memcpy(E,
- &dev->dev_addr
+ dev->dev_addr
, ...);
@@
expression E;
@@
- &E->dev_addr
+ E->dev_addr
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 12 May 2009 17:24:02 +1000
> Subject: [PATCH] net/ibmveth: fix panic in probe
>
> netdev->dev_addr changed from being an array to being a pointer, so we
> should not take its address for memcpy().
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/net/ibmveth.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ibmveth.c b/drivers/net/ibmveth.c
> index 7902e5e..8daffad 100644
> --- a/drivers/net/ibmveth.c
> +++ b/drivers/net/ibmveth.c
> @@ -1285,7 +1285,7 @@ static int __devinit ibmveth_probe(struct vio_dev *dev, const struct vio_device_
> netdev->features |= NETIF_F_LLTX;
> spin_lock_init(&adapter->stats_lock);
>
> - memcpy(&netdev->dev_addr, &adapter->mac_addr, netdev->addr_len);
> + memcpy(netdev->dev_addr, &adapter->mac_addr, netdev->addr_len);
>
> for(i = 0; i<IbmVethNumBufferPools; i++) {
> struct kobject *kobj = &adapter->rx_buff_pool[i].kobj;
>
maybe this line should be fix too.
@@ -1368,7 +1368,7 @@ static void ibmveth_proc_unregister_driv
static int ibmveth_show(struct seq_file *seq, void *v)
{
struct ibmveth_adapter *adapter = seq->private;
- char *current_mac = ((char*) &adapter->netdev->dev_addr);
+ char *current_mac = ((char*) adapter->netdev->dev_addr);
char *firmware_mac = ((char*) &adapter->mac_addr) ;
seq_printf(seq, "%s %s\n\n", ibmveth_driver_string, ibmveth_driver_version);
^ permalink raw reply
* Re: question about softirqs
From: Peter Zijlstra @ 2009-05-12 9:32 UTC (permalink / raw)
To: Ingo Molnar
Cc: linuxppc-dev, netdev, Steven Rostedt, paulus, Thomas Gleixner,
David Miller
In-Reply-To: <20090512092348.GA29796@elte.hu>
On Tue, 2009-05-12 at 11:23 +0200, Ingo Molnar wrote:
>
> Yeah, that would be "nice". A single IRQ thread plus the process
> context(s) doing networking might perform well.
>
> Multiple IRQ threads (softirq and hardirq threads mixed) i'm not so
> sure about - it's extra context-switching cost.
Sure, that was implied by the getting rid of softirqs ;-), on -rt we
currently suffer this hardirq/softirq thread ping-pong, it sucks.
^ permalink raw reply
* Re: question about softirqs
From: Peter Zijlstra @ 2009-05-12 9:12 UTC (permalink / raw)
To: Ingo Molnar
Cc: linuxppc-dev, netdev, Steven Rostedt, paulus, Thomas Gleixner,
David Miller
In-Reply-To: <20090512081237.GA16403@elte.hu>
On Tue, 2009-05-12 at 10:12 +0200, Ingo Molnar wrote:
> * Chris Friesen <cfriesen@nortel.com> wrote:
>
> > This started out as a thread on the ppc list, but on the
> > suggestion of DaveM and Paul Mackerras I'm expanding the receiver
> > list a bit.
> >
> > Currently, if a softirq is raised in process context the
> > TIF_RESCHED_PENDING flag gets set and on return to userspace we
> > run the scheduler, expecting it to switch to ksoftirqd to handle
> > the softirqd processing.
> >
> > I think I see a possible problem with this. Suppose I have a
> > SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT set. Under
> > the scenario above, schedule() would re-run the spinning task
> > rather than ksoftirqd, thus preventing any incoming packets from
> > being sent up the stack until we get a real hardware
> > interrupt--which could be a whole jiffy if interrupt mitigation is
> > enabled in the net device.
>
> TIF_RESCHED_PENDING will not be set if a SCHED_FIFO task wakes up a
> SCHED_OTHER ksoftirqd task. But starvation of ksoftirqd processing
> will occur.
>
> > DaveM pointed out that if we're doing transmits we're likely to
> > hit local_bh_enable(), which would process the softirq work.
> > However, I think we may still have a problem in the above rx-only
> > scenario--or is it too contrived to matter?
>
> This could occur, and the problem is really that task priorities do
> not extend across softirq work processing.
>
> This could occur in ordinary SCHED_OTHER tasks as well, if the
> softirq is bounced to ksoftirqd - which it only should be if there's
> serious softirq overload - or, as you describe it above, if the
> softirq is raised in process context:
>
> if (!in_interrupt())
> wakeup_softirqd();
>
> that's not really clean. We look into eliminating process context
> use of raise_softirq_irqsoff(). Such code sequence:
>
> local_irq_save(flags);
> ...
> raise_softirq_irqsoff(nr);
> ...
> local_irq_restore(flags);
>
> should be converted to something like:
>
> local_irq_save(flags);
> ...
> raise_softirq_irqsoff(nr);
> ...
> local_irq_restore(flags);
> recheck_softirqs();
>
> If someone does not do proper local_bh_disable()/enable() sequences
> for micro-optimization reasons, then push the check to after the
> critcal section - and dont cause extra reschedules by waking up
> ksoftirqd. raise_softirq_irqsoff() will also be faster.
Wouldn't the even better solution be to get rid of softirqs
all-together?
I see the recent work by Thomas to get threaded interrupts upstream as a
good first step towards that goal, once the RX processing is moved to a
thread (or multiple threads) one can priorize them in the regular
sys_sched_setscheduler() way and its obvious that a FIFO task above the
priority of the network tasks will have network starvation issues.
^ permalink raw reply
* Re: question about softirqs
From: Ingo Molnar @ 2009-05-12 9:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linuxppc-dev, netdev, Steven Rostedt, paulus, Thomas Gleixner,
David Miller
In-Reply-To: <1242119578.11251.321.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Tue, 2009-05-12 at 10:12 +0200, Ingo Molnar wrote:
> > * Chris Friesen <cfriesen@nortel.com> wrote:
> >
> > > This started out as a thread on the ppc list, but on the
> > > suggestion of DaveM and Paul Mackerras I'm expanding the receiver
> > > list a bit.
> > >
> > > Currently, if a softirq is raised in process context the
> > > TIF_RESCHED_PENDING flag gets set and on return to userspace we
> > > run the scheduler, expecting it to switch to ksoftirqd to handle
> > > the softirqd processing.
> > >
> > > I think I see a possible problem with this. Suppose I have a
> > > SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT set. Under
> > > the scenario above, schedule() would re-run the spinning task
> > > rather than ksoftirqd, thus preventing any incoming packets from
> > > being sent up the stack until we get a real hardware
> > > interrupt--which could be a whole jiffy if interrupt mitigation is
> > > enabled in the net device.
> >
> > TIF_RESCHED_PENDING will not be set if a SCHED_FIFO task wakes up a
> > SCHED_OTHER ksoftirqd task. But starvation of ksoftirqd processing
> > will occur.
> >
> > > DaveM pointed out that if we're doing transmits we're likely to
> > > hit local_bh_enable(), which would process the softirq work.
> > > However, I think we may still have a problem in the above rx-only
> > > scenario--or is it too contrived to matter?
> >
> > This could occur, and the problem is really that task priorities do
> > not extend across softirq work processing.
> >
> > This could occur in ordinary SCHED_OTHER tasks as well, if the
> > softirq is bounced to ksoftirqd - which it only should be if there's
> > serious softirq overload - or, as you describe it above, if the
> > softirq is raised in process context:
> >
> > if (!in_interrupt())
> > wakeup_softirqd();
> >
> > that's not really clean. We look into eliminating process context
> > use of raise_softirq_irqsoff(). Such code sequence:
> >
> > local_irq_save(flags);
> > ...
> > raise_softirq_irqsoff(nr);
> > ...
> > local_irq_restore(flags);
> >
> > should be converted to something like:
> >
> > local_irq_save(flags);
> > ...
> > raise_softirq_irqsoff(nr);
> > ...
> > local_irq_restore(flags);
> > recheck_softirqs();
> >
> > If someone does not do proper local_bh_disable()/enable() sequences
> > for micro-optimization reasons, then push the check to after the
> > critcal section - and dont cause extra reschedules by waking up
> > ksoftirqd. raise_softirq_irqsoff() will also be faster.
>
>
> Wouldn't the even better solution be to get rid of softirqs
> all-together?
>
> I see the recent work by Thomas to get threaded interrupts
> upstream as a good first step towards that goal, once the RX
> processing is moved to a thread (or multiple threads) one can
> priorize them in the regular sys_sched_setscheduler() way and its
> obvious that a FIFO task above the priority of the network tasks
> will have network starvation issues.
Yeah, that would be "nice". A single IRQ thread plus the process
context(s) doing networking might perform well.
Multiple IRQ threads (softirq and hardirq threads mixed) i'm not so
sure about - it's extra context-switching cost.
Btw, i noticed that using scheduling for work (packet, etc.) flow
distribution standardizes and evens out the behavior of workloads.
Softirq scheduling is really quite random currently. We have a
random processing loop-limit in the core code and various batching
and work-limit controls at individual usage sites. We sometimes
piggyback to ksoftirqd. It's far easier to keep performance in check
when things are more predictable.
But this is not an easy endevour, and performance regressions have
to be expected and addressed if they occur. There can be random
packet queuing details in networking drivers that just happen to
work fine now, and might work worse with a kernel thread in place.
So there has to be broad buy-in for the concept, and a concerted
effort to eliminate softirq processing and most of hardirq
processing by pushing those two elements into a single hardirq
thread (and the rest into process context).
Not for the faint hearted. Nor is it recommended to be done without
a good layer of asbestos.
Ingo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox