Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On Thu, 2008-05-01 at 01:01 -0500, Kumar Gala wrote: > On Apr 30, 2008, at 6:47 PM, Paul Mackerras wrote: > > Kumar Gala writes: > > > >> If we don't handle reschedule or signal will we actually not function > >> properly? I assume reschedule isn't an issue, but could we lose a > >> signal? > > > > It depends on whether a critical or machine check handler can ever do > > anything to generate a signal or a reschedule. If they can't, then > > there is no problem. > > They can if the come from user space. I'm question what it means to > send a signal based on receiving an async exception. For "normal" async exceptions (ie. external interrupts), it could be anything... EE -> network rx -> SIGIO, or you get out of EE, hit the softirq path, figure out there was something there waiting processing (ie, some tty task) and that results in a signal being sent... > So we have 4 actual exceptions: > * CriticalInput (some external device signaled this. There are two > concepts of critical. One is error the other is high priority) > However this would have the same caveats as any ExternalInput handler. No, it's worse. It can interrupt code that normally has local_irq_disabled() and thus doesn't expect to be interrupted. That means that everything becomes unsafe including locks etc Note that driver that want to make active use of that probably want some explicit local_crit_irq_disable/enable functions to be able to implement some sort of synchronization. > * Watchdog - pretty severe if this fires. > > * Debug - user space debug is pretty straight forward. However we > have features like kprobes that require kernel level support. Which means we have to be extra careful, in fact, I consider it a design bug of BookE to have made debug be a critical interrupt... > * MachineCheck - pretty serve if this fires. > > So I'm not if there is any good way to preclude the handlers > associated with these exceptions from doing the things you listed. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] i2c: Add support for device alias names
Hi Kay,
On Mon, 28 Apr 2008 18:16:17 +0200, Kay Sievers wrote:
> On Mon, 2008-04-28 at 17:40 +0200, Jean Delvare wrote:
> > Why would i2c device modaliases ever contain multiple strings? A device
> > can't have multiple names, can it?
>
> Like ACPI/PNP devices, which can have several compat id's, which means
> that a single device can have "multiple names":
> $ cat /sys/bus/pnp/devices/00:09/id
> IBM0057
> PNP0f13
Ah, I didn't know about this. Now I'm curious how it can work. Does it
mean that several drivers attempt to bind to this device?
> > Adding a ":" at the end of the i2c device names solves the problem I
> > was mentioning, sure, but why don't we simply remove the trailing "*",
> > instead of trying to work around it? A trailing "*" simply makes no
> > sense for aliases which are simple device names.
>
> Sure, if there is only one single string, it's not useful.
>
> > This is not only i2c
> > devices, but also platform devices, acpi, dmi, pnp...
>
> ACPI, DMI, PNP (PNP does not do modalias) needs to be able to match only
> one string in a given list, so the trailing "*" is needed.
OK, I get the idea.
> > Can't we just stop handle_moddevtable() from adding a tailing "*"
> > automatically, and just let the device types which need it, add it on
> > their own?
>
> For a lot subsystems it's fine to have it appended, as there is a
> defined list of identifiers, which must appear in the same order, and
> new identifiers are appended to the end. So the "*" still matches
> modules with possibly extended modalias strings.
I understand the logic, however I am skeptical how useful it is in
practice. If we add an identifier to the device aliases, then we also
update the corresponding modalias, so no in-tree driver can break. The
only case where it makes a difference, as far as I can see, is for
out-of-tree drivers. Am I correct? On top of that, I doubt that we
actually add new identifiers that frequently, do we?
> We would also need to review all buses which export modalias, if they
> need the "*" or not, and add them by hand, if needed.
>
> I guess, it's easier to introduce an additional parameter to
> file2alias::do_table() and suppress the trailing "*" for i2c?
That's one possibility, but I had a slightly different approach, which
is to just let the type-specific handlers add the trailing "*" by
themselves if they need it. This allows for optimization in a few
cases.
Subject: modpost: i2c aliases need no trailing wildcard
Not all device type aliases need a trailing wildcard, in particular
the i2c aliases don't. Don't add a wildcard by default in do_table(),
instead let each device type handler add it if needed.
Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
---
scripts/mod/file2alias.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
--- linux-2.6.26-rc0.orig/scripts/mod/file2alias.c 2008-05-01
07:56:14.0 +0200
+++ linux-2.6.26-rc0/scripts/mod/file2alias.c 2008-05-01 09:39:37.0
+0200
@@ -51,6 +51,15 @@ do {
sprintf(str + strlen(str), "*");\
} while(0)
+/* Always end in a wildcard, for future extension */
+static inline void add_wildcard(char *str)
+{
+ int len = strlen(str);
+
+ if (str[len - 1] != '*')
+ strcat(str + len, "*");
+}
+
unsigned int cross_build = 0;
/**
* Check that sizeof(device_id type) are consistent with size of section
@@ -133,9 +142,7 @@ static void do_usb_entry(struct usb_devi
id->match_flags&USB_DEVICE_ID_MATCH_INT_PROTOCOL,
id->bInterfaceProtocol);
- /* Always end in a wildcard, for future extension */
- if (alias[strlen(alias)-1] != '*')
- strcat(alias, "*");
+ add_wildcard(alias);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"%s\");\n", alias);
}
@@ -219,6 +226,7 @@ static int do_ieee1394_entry(const char
ADD(alias, "ver", id->match_flags & IEEE1394_MATCH_VERSION,
id->version);
+ add_wildcard(alias);
return 1;
}
@@ -261,6 +269,7 @@ static int do_pci_entry(const char *file
ADD(alias, "bc", baseclass_mask == 0xFF, baseclass);
ADD(alias, "sc", subclass_mask == 0xFF, subclass);
ADD(alias, "i", interface_mask == 0xFF, interface);
+ add_wildcard(alias);
return 1;
}
@@ -283,6 +292,7 @@ static int do_ccw_entry(const char *file
id->dev_type);
ADD(alias, "dm", id->match_flags&CCW_DEVICE_ID_MATCH_DEVICE_MODEL,
id->dev_model);
+ add_wildcard(alias);
return 1;
}
@@ -290,7 +300,7 @@ static int do_ccw_entry(const char *file
static int do_ap_entry(const char *filename,
struct ap_device_id *id, char *alias)
{
- sprintf(alias, "ap:t%02X", id->dev_type);
+ sprintf(alias, "ap:t%02X*", id->dev_type);
return 1;
}
@@ -309,6 +319,7 @@ static int do_serio
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
Kumar Gala writes: > > It depends on whether a critical or machine check handler can ever do > > anything to generate a signal or a reschedule. If they can't, then > > there is no problem. > > They can if the come from user space. I'm question what it means to > send a signal based on receiving an async exception. The most common cases are (a) something that ultimately generates input on a tty (e.g. a character arriving on a serial port) and that input turns out to be a ^C or similar, or (b) something that signals I/O completion and the program doing the I/O has requested notification by a SIGIO. But in general any driver code can send a signal to userspace if it wants. > > If they can, then we have to be very careful. If a critical or > > machine check happens at a point where normal interrupts are disabled > > then we have to be extremely careful not to do anything that the code > > we've interrupted assumes can't happen - so we'd better not try to > > take any spinlocks, for example. That severely limits what the > > handler can do. It probably shouldn't even call printk, for > > instance, or wake any process up, and definitely shouldn't call > > schedule (or schedule_preempt) on the way out. > > how do we provide someone stick a kprobe on such code today? -ENOPARSE > So I'm not if there is any good way to preclude the handlers > associated with these exceptions from doing the things you listed. In that case, you'd better expect to see system freezes, memory corruption and general instability. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/6] [POWERPC] Move device_to_mask() to dma-mapping.h
Move device_to_mask() to dma-mapping.h because we need to use it from outside dma_64.c in a later patch. Why does this need to become an inline function? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [patch 2.6.25-git] spi_mpc83xx much improved driver
> -Original Message- > From: David Brownell [mailto:[EMAIL PROTECTED] > Sent: den 1 maj 2008 00:56 > To: Andrew Morton > Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; linuxppc-dev@ozlabs.org; > [EMAIL PROTECTED] > Subject: Re: [patch 2.6.25-git] spi_mpc83xx much improved driver > > On Wednesday 30 April 2008, Andrew Morton wrote: > > > + spin_lock_irq(&mpc83xx_spi->lock); > > > > irq-safe. > > > > > ... > > > > > > + spin_lock(&mpc83xx_spi->lock); > > > > not irq-safe. > > > > Deliberate? > > That latter one is inside an #if 0/#endif block, so it won't matter. > The #if 0 block bothered me. Maybe Joakim can remove it. The non irq-safe spin_lock was there before my patch and now it is removed with the #if 0/#endif block. I don't need this code but I left it there disabled to make it clear for everyone else that it was removed. I can remove it but I rather leave it there for one release so everyone will have a chance to see it and object if they feel differently. > > (By the way, I'd feel safer about this patch if there were an > ack or two from more PPC folk...) Yeah, that would be good. Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/6] [POWERPC] sysdev: implement FSL GTM support
On Wed, Apr 30, 2008 at 11:00:42PM -0500, Kumar Gala wrote:
>
> On Apr 29, 2008, at 2:00 PM, Anton Vorontsov wrote:
>> GTM stands for General-purpose Timers Module and able to generate
>> timer{1,2,3,4} interrupts. These timers are used by the drivers that
>> need time precise interrupts (like for USB transactions scheduling for
>> the Freescale USB Host controller as found in some QE and CPM chips),
>> or these timers could be used as wakeup events from the CPU deep-sleep
>> mode.
>>
>> Things unimplemented:
>> 1. Cascaded (32 bit) timers (1-2, 3-4).
>> This is straightforward to implement when needed, two timers should
>> be marked as "requested" and configured as appropriate.
>> 2. Super-cascaded (64 bit) timers (1-2-3-4).
>> This is also straightforward to implement when needed, all timers
>> should be marked as "requested" and configured as appropriate.
>>
>> Signed-off-by: Anton Vorontsov <[EMAIL PROTECTED]>
>> ---
>> Documentation/powerpc/booting-without-of.txt | 37 +++-
>> arch/powerpc/Kconfig |5 +
>> arch/powerpc/sysdev/Makefile |1 +
>> arch/powerpc/sysdev/fsl_gtm.c| 424 +
>> +
>> include/asm-powerpc/fsl_gtm.h| 47 +++
>> 5 files changed, 513 insertions(+), 1 deletions(-)
>> create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
>> create mode 100644 include/asm-powerpc/fsl_gtm.h
>>
>>
>
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 4e40c12..4070a78 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -538,6 +538,11 @@ config FSL_LBC
>> help
>>Freescale Localbus support
>>
>> +config FSL_GTM
>> +bool
>> +help
>> + Freescale General-purpose Timers support
>> +
>
> what chips actually use this? just QE or 83xx?
83xx/85xx, QE, CPM2 and CPM1 (no support).
Probably some more old and new chips.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On May 1, 2008, at 3:24 AM, Paul Mackerras wrote: Kumar Gala writes: It depends on whether a critical or machine check handler can ever do anything to generate a signal or a reschedule. If they can't, then there is no problem. They can if the come from user space. I'm question what it means to send a signal based on receiving an async exception. The most common cases are (a) something that ultimately generates input on a tty (e.g. a character arriving on a serial port) and that input turns out to be a ^C or similar, or (b) something that signals I/O completion and the program doing the I/O has requested notification by a SIGIO. But in general any driver code can send a signal to userspace if it wants. ok. Was just wondering how the async exception know that the signal it wanted to send belonged to the particular process that is running. But I guess there are cases that the signal is really intended for who ever is currently running? If they can, then we have to be very careful. If a critical or machine check happens at a point where normal interrupts are disabled then we have to be extremely careful not to do anything that the code we've interrupted assumes can't happen - so we'd better not try to take any spinlocks, for example. That severely limits what the handler can do. It probably shouldn't even call printk, for instance, or wake any process up, and definitely shouldn't call schedule (or schedule_preempt) on the way out. Do we ensure that synchronous exceptions will not occur in these cases that kernel code things interrupts are disabled in? how do we provide someone stick a kprobe on such code today? -ENOPARSE I was asking how we prevent the cases you were describing working w/ kprobes today. Since it ends up single stepping in kernel codes its possible that someone sets a kprobe in code that shouldn't be interrupted, yet we'd cause a SingleStep Exception. So I'm not if there is any good way to preclude the handlers associated with these exceptions from doing the things you listed. In that case, you'd better expect to see system freezes, memory corruption and general instability. So the case I'm trying to make work is debug and kprobes. This case seems like we have pretty good control over what the "handler" does. Are there checks we can add to BUG_ON() so we are at least aware of the code attempts to do something it shouldnt? - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
So we have 4 actual exceptions: * CriticalInput (some external device signaled this. There are two concepts of critical. One is error the other is high priority) However this would have the same caveats as any ExternalInput handler. No, it's worse. It can interrupt code that normally has local_irq_disabled() and thus doesn't expect to be interrupted. That means that everything becomes unsafe including locks etc Fair. Should local_irq_disable() clear MSR_SE & MSR_BE on classic parts? Note that driver that want to make active use of that probably want some explicit local_crit_irq_disable/enable functions to be able to implement some sort of synchronization. Or we could just have local_irq_disable -- clear MSR_EE and MSR_CE * Watchdog - pretty severe if this fires. * Debug - user space debug is pretty straight forward. However we have features like kprobes that require kernel level support. Which means we have to be extra careful, in fact, I consider it a design bug of BookE to have made debug be a critical interrupt... I consider the whole BookE debug arch a design bug :) - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On Thu, 2008-05-01 at 08:22 -0500, Kumar Gala wrote: > >> So we have 4 actual exceptions: > >> * CriticalInput (some external device signaled this. There are two > >> concepts of critical. One is error the other is high priority) > >> However this would have the same caveats as any ExternalInput > >> handler. > > > > No, it's worse. It can interrupt code that normally has > > local_irq_disabled() and thus doesn't expect to be interrupted. That > > means that everything becomes unsafe including locks etc > > Fair. Should local_irq_disable() clear MSR_SE & MSR_BE on classic > parts? > > > Note that driver that want to make active use of that probably want > > some > > explicit local_crit_irq_disable/enable functions to be able to > > implement > > some sort of synchronization. > > Or we could just have local_irq_disable -- clear MSR_EE and MSR_CE That sort of defeats the purpose of using critical interrupts in the "higher priority" sense, doesn't it? Seems it would effectively eliminate that level altogether. And it would also mask off the watchdog interrupt, which I don't think you want to do. Ben suggested local_crit_irq_disable. That would suffice for drivers that know they are configured as CE and need to do locking primitives, but that isn't very common. On 4xx there isn't even an interface for a driver to request it's interrupts be set to CE and we init all UICs to have UIC_CR be 0. So for 4xx at least, most of this discussion is theoretical for now. However, I can see value in having CEs being used for certain devices, etc so it would be nice to figure it out in the long run. josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] mpc5200: Allow for fixed speed MII configurations
On Tue, Apr 29, 2008 at 5:06 PM, Grant Likely <[EMAIL PROTECTED]> wrote:
> From: Grant Likely <[EMAIL PROTECTED]>
>
> Various improvements for configuring the MPC5200 MII link from the
> device tree:
> * Look for 'current-speed' property for fixed speed MII links
> * Look for 'fsl,7-wire-mode' property for boards using the 7 wire mode
> * move definition of private data structure out of the header file
>
> Signed-off-by: Grant Likely <[EMAIL PROTECTED]>
Any more comments on this patch? I want to push it to Paulus, but I'd
like to have someone ack it first.
Wolfgang, you used the previous version of this patch. Does this one
work for you?
Cheers,
g.
> ---
>
> .../powerpc/mpc52xx-device-tree-bindings.txt | 11 ++
> drivers/net/fec_mpc52xx.c | 96
> +++-
> drivers/net/fec_mpc52xx.h | 19
> 3 files changed, 85 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
> b/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
> index cda7a7d..6f12f1c 100644
> --- a/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
> +++ b/Documentation/powerpc/mpc52xx-device-tree-bindings.txt
> @@ -237,6 +237,17 @@ Each GPIO controller node should have the empty
> property gpio-controller and
> according to the bit numbers in the GPIO control registers. The second cell
> is for flags which is currently unsused.
>
> +8) FEC nodes
> +The FEC node can specify one of the following properties to configure
> +the MII link:
> +"fsl,7-wire-mode" - An empty property that specifies the link uses 7-wire
> +mode instead of MII
> +"current-speed" - Specifies that the MII should be configured for a fixed
> +speed. This property should contain two cells. The
> +first cell specifies the speed in Mbps and the second
> +should be '0' for half duplex and '1' for full duplex
> +"phy-handle" - Contains a phandle to an Ethernet PHY.
> +
> IV - Extra Notes
>
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index d21b7ab..eeb4433 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -43,6 +43,29 @@
>
> #define DRIVER_NAME "mpc52xx-fec"
>
> +#define FEC5200_PHYADDR_NONE (-1)
> +#define FEC5200_PHYADDR_7WIRE (-2)
> +
> +/* Private driver data structure */
> +struct mpc52xx_fec_priv {
> + int duplex;
> + int speed;
> + int r_irq;
> + int t_irq;
> + struct mpc52xx_fec __iomem *fec;
> + struct bcom_task *rx_dmatsk;
> + struct bcom_task *tx_dmatsk;
> + spinlock_t lock;
> + int msg_enable;
> +
> + /* MDIO link details */
> + int phy_addr;
> + unsigned int phy_speed;
> + struct phy_device *phydev;
> + enum phy_state link;
> +};
> +
> +
> static irqreturn_t mpc52xx_fec_interrupt(int, void *);
> static irqreturn_t mpc52xx_fec_rx_interrupt(int, void *);
> static irqreturn_t mpc52xx_fec_tx_interrupt(int, void *);
> @@ -223,7 +246,7 @@ static int mpc52xx_fec_phy_start(struct net_device *dev)
> struct mpc52xx_fec_priv *priv = netdev_priv(dev);
> int err;
>
> - if (!priv->has_phy)
> + if (priv->phy_addr < 0)
> return 0;
>
> err = mpc52xx_fec_init_phy(dev);
> @@ -243,7 +266,7 @@ static void mpc52xx_fec_phy_stop(struct net_device *dev)
> {
> struct mpc52xx_fec_priv *priv = netdev_priv(dev);
>
> - if (!priv->has_phy)
> + if (!priv->phydev)
> return;
>
> phy_disconnect(priv->phydev);
> @@ -255,7 +278,7 @@ static void mpc52xx_fec_phy_stop(struct net_device *dev)
> static int mpc52xx_fec_phy_mii_ioctl(struct mpc52xx_fec_priv *priv,
> struct mii_ioctl_data *mii_data, int cmd)
> {
> - if (!priv->has_phy)
> + if (!priv->phydev)
> return -ENOTSUPP;
>
> return phy_mii_ioctl(priv->phydev, mii_data, cmd);
> @@ -265,7 +288,7 @@ static void mpc52xx_fec_phy_hw_init(struct
> mpc52xx_fec_priv *priv)
> {
> struct mpc52xx_fec __iomem *fec = priv->fec;
>
> - if (!priv->has_phy)
> + if (priv->phydev)
> return;
>
> out_be32(&fec->mii_speed, priv->phy_speed);
> @@ -704,7 +727,7 @@ static void mpc52xx_fec_start(struct net_device *dev)
> rcntrl = FEC_RX_BUFFER_SIZE << 16; /* max frame length */
> rcntrl |= FEC_RCNTRL_FCE;
>
> - if (priv->has_phy)
> + if (priv->phy_addr != FEC5200_PHYADDR_7WIRE)
> rcntrl |= FEC_RCNTRL_MII_MODE;
>
> if (priv->duplex == DUPLEX_FULL)
> @@ -864,7 +887,10 @@ mpc52xx_fec_probe(struct of_device *op, const struct
> of_device_id *match)
> struct net_device *ndev;
> struct mpc52xx_fec_priv *priv = NULL;
> struct resource mem;
>
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote: > No. The spec indeed says that 0 for low, non-0 for high. Fair enough. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote: > This is needed to access QE GPIOs via Linux GPIO API. > > Signed-off-by: Anton Vorontsov <[EMAIL PROTECTED]> Acked-By: Timur Tabi <[EMAIL PROTECTED]> -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On May 1, 2008, at 9:26 AM, Josh Boyer wrote: On Thu, 2008-05-01 at 08:22 -0500, Kumar Gala wrote: So we have 4 actual exceptions: * CriticalInput (some external device signaled this. There are two concepts of critical. One is error the other is high priority) However this would have the same caveats as any ExternalInput handler. No, it's worse. It can interrupt code that normally has local_irq_disabled() and thus doesn't expect to be interrupted. That means that everything becomes unsafe including locks etc Fair. Should local_irq_disable() clear MSR_SE & MSR_BE on classic parts? Note that driver that want to make active use of that probably want some explicit local_crit_irq_disable/enable functions to be able to implement some sort of synchronization. Or we could just have local_irq_disable -- clear MSR_EE and MSR_CE That sort of defeats the purpose of using critical interrupts in the "higher priority" sense, doesn't it? Seems it would effectively eliminate that level altogether. And it would also mask off the watchdog interrupt, which I don't think you want to do. Ben suggested local_crit_irq_disable. That would suffice for drivers that know they are configured as CE and need to do locking primitives, but that isn't very common. On 4xx there isn't even an interface for a driver to request it's interrupts be set to CE and we init all UICs to have UIC_CR be 0. So for 4xx at least, most of this discussion is theoretical for now. However, I can see value in having CEs being used for certain devices, etc so it would be nice to figure it out in the long run. I agree. my first priority is to get debug interrupts cleaned up so we can kprobe and do other debug things in the kernel. I'm only aware of one case of someone asking for critical inputs on our parts w/linux. - k - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] Updated: Reworked Cell OProfile: SPU mutex lock fix
Sorry, looks like my mailer mangled the file.
This is a reworked patch to fix the SPU data storage. Currently, the
SPU escape sequences and program counter data is being added directly
into the kernel buffer without holding the buffer_mutex lock. This
patch changes how the data is stored. A new function,
oprofile_add_value, is added into the oprofile driver to allow adding
generic data to the per cpu buffers. This enables a series of calls
to the oprofile_add_value to enter the needed SPU escape sequences
and SPU program data into the kernel buffer via the per cpu buffers
without any additional processing. The oprofile_add_value function is
generic so it could be used by other architecures as well provided
the needed postprocessing was added to opreport.
Finally, this patch backs out the changes previously added to the
oprofile generic code for handling the architecture specific
ops.sync_start and ops.sync_stop that allowed the architecture
to skip the per CPU buffer creation.
Signed-off-by: Carl Love <[EMAIL PROTECTED]>
Index: Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/pr_util.h
===
--- Cell_kernel_4_15_2008.orig/arch/powerpc/oprofile/cell/pr_util.h
+++ Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/pr_util.h
@@ -20,11 +20,6 @@
#include
#include
-/* Defines used for sync_start */
-#define SKIP_GENERIC_SYNC 0
-#define SYNC_START_ERROR -1
-#define DO_GENERIC_SYNC 1
-
struct spu_overlay_info { /* map of sections within an SPU overlay */
unsigned int vma; /* SPU virtual memory address from elf */
unsigned int size; /* size of section from elf */
@@ -85,7 +80,7 @@ void stop_spu_profiling(void);
int spu_sync_start(void);
/* remove the hooks */
-int spu_sync_stop(void);
+void spu_sync_stop(void);
/* Record SPU program counter samples to the oprofile event buffer. */
void spu_sync_buffer(int spu_num, unsigned int *samples,
Index: Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/spu_task_sync.c
===
--- Cell_kernel_4_15_2008.orig/arch/powerpc/oprofile/cell/spu_task_sync.c
+++ Cell_kernel_4_15_2008/arch/powerpc/oprofile/cell/spu_task_sync.c
@@ -31,11 +31,12 @@
#define RELEASE_ALL
-static DEFINE_SPINLOCK(buffer_lock);
+static DEFINE_SPINLOCK(add_value_lock);
static DEFINE_SPINLOCK(cache_lock);
static int num_spu_nodes;
int spu_prof_num_nodes;
int last_guard_val[MAX_NUMNODES * 8];
+static int spu_ctx_sw_seen[MAX_NUMNODES * 8];
/* Container for caching information about an active SPU task. */
struct cached_info {
@@ -289,6 +290,7 @@ static int process_context_switch(struct
int retval;
unsigned int offset = 0;
unsigned long spu_cookie = 0, app_dcookie;
+ int cpu_buf;
retval = prepare_cached_spu_info(spu, objectId);
if (retval)
@@ -303,17 +305,28 @@ static int process_context_switch(struct
goto out;
}
- /* Record context info in event buffer */
- spin_lock_irqsave(&buffer_lock, flags);
- add_event_entry(ESCAPE_CODE);
- add_event_entry(SPU_CTX_SWITCH_CODE);
- add_event_entry(spu->number);
- add_event_entry(spu->pid);
- add_event_entry(spu->tgid);
- add_event_entry(app_dcookie);
- add_event_entry(spu_cookie);
- add_event_entry(offset);
- spin_unlock_irqrestore(&buffer_lock, flags);
+ /* Record context info in event buffer. Note, there are 4x more
+* SPUs then CPUs. Map the SPU events/data for a given SPU to
+* the same CPU buffer. Need to ensure the cntxt switch data and
+* samples stay in order.
+*/
+ cpu_buf = spu->number >> 2;
+ spin_lock_irqsave(&add_value_lock, flags);
+ oprofile_add_value(ESCAPE_CODE, cpu_buf);
+ oprofile_add_value(SPU_CTX_SWITCH_CODE, cpu_buf);
+ oprofile_add_value(spu->number, cpu_buf);
+ oprofile_add_value(spu->pid, cpu_buf);
+ oprofile_add_value(spu->tgid, cpu_buf);
+ oprofile_add_value(app_dcookie, cpu_buf);
+ oprofile_add_value(spu_cookie, cpu_buf);
+ oprofile_add_value(offset, cpu_buf);
+
+ /* Set flag to indicate SPU PC data can now be written out. If
+* the SPU program counter data is seen before an SPU context
+* record is seen, the postprocessing will fail.
+*/
+ spu_ctx_sw_seen[spu->number] = 1;
+ spin_unlock_irqrestore(&add_value_lock, flags);
smp_wmb(); /* insure spu event buffer updates are written */
/* don't want entries intermingled... */
out:
@@ -333,7 +346,6 @@ static int spu_active_notify(struct noti
unsigned long flags;
struct spu *the_spu = data;
- pr_debug("SPU event notification arrived\n");
if (!val) {
spin_lock_irqsave(&cache_lock, flags);
retval = release_cached_info(the_spu->number)
Re: powerpc boot regression
On Thu, 2008-05-01 at 15:13 +1000, Tony Breeds wrote: > On Tue, Apr 29, 2008 at 11:12:41PM -0700, David Miller wrote: > > > > This commit causes bootup failures on sparc64: > > > > commit 86f6dae1377523689bd8468fed2f2dd180fc0560 > > Author: Yasunori Goto <[EMAIL PROTECTED]> > > Date: Mon Apr 28 02:13:33 2008 -0700 > > > > memory hotplug: allocate usemap on the section with pgdat > > > > > We're seeing a boot failure on powerpc. git bisect points the problem > at this commit. However reverting just this one comitt doesn't fix the > regression. I also needed to revert > 04753278769f3b6c3b79a080edb52f21d83bf6e2 (memory hotplug: register > section/node id to free") > > Problem seen on power4, power5 and ps3. I don't see the problem on my power5 machine. Can you send the config ? Is CONFIG_SPARSEMEM_VMEMMAP enabled ? Please let me know. Thanks, Badari > > If the fix isn't trivial, can we get that patch series reverted? > > FWIW a boot looks like: > zImage starting: loaded at 0x0040 (sp: 0x02f5fea0) > Allocating 0xa2d4c8 bytes for kernel ... > OF version = 'IBM,SF240_332' > gunzipping (0x0140 <- 0x00407000:0x0079aa5d)...done 0x9a8ee0 bytes > > Linux/PowerPC load: root=/dev/sdc3 ro > Finalizing device tree... using OF tree (promptr=02039a68) > OF stdout device is: /vdevice/[EMAIL PROTECTED] > Hypertas detected, assuming LPAR ! > command line: root=/dev/sdc3 ro > memory layout at init: > alloc_bottom : 01e32000 > alloc_top: 0800 > alloc_top_hi : 8000 > rmo_top : 0800 > ram_top : 8000 > Looking for displays > instantiating rtas at 0x076a1000 ... done > : boot cpu > copying OF device tree ... > Building dt strings... > Building dt structure... > Device tree strings 0x01e33000 -> 0x01e3417c > Device tree struct 0x01e35000 -> 0x01e3d000 > Calling quiesce ... > returning from prom_init > Crash kernel location must be 0x200 > Reserving 0MB of memory at 32MB for crashkernel (System RAM: 2048MB) > Using pSeries machine description > console [udbg0] enabled > Partition configured for 20 cpus. > CPU maps initialized for 2 threads per core > Starting Linux PPC64 #52 SMP Thu May 1 14:04:46 EST 2008 > - > ppc64_pft_size= 0x19 > physicalMemorySize= 0x8000 > htab_hash_mask= 0x3 > - > Initializing cgroup subsys cpuset > Linux version 2.6.25 ([EMAIL PROTECTED]) (gcc version 4.0.4 20060507 > (prerelease) (Debian 4.0.3-3)) #52 SMP Thu May 1 14:04:46 EST 2008 > [boot]0012 Setup Arch > sparse_early_usemap_alloc: allocation failed > > EEH: No capable adapters found > PPC64 nvram contains 7168 bytes > Zone PFN ranges: > DMA 0 -> 524288 > Normal 524288 -> 524288 > Movable zone start PFN for each node > early_node_map[1] active PFN ranges > 0:0 -> 524288 > [boot]0015 Setup Done > Built 1 zonelists in Zone order, mobility grouping on. Total pages: 517120 > Kernel command line: root=/dev/sdc3 ro > [boot]0020 XICS Init > [boot]0021 XICS Done > PID hash table entries: 4096 (order: 12, 32768 bytes) > clocksource: timebase mult[1545815] shift[22] registered > Console: colour dummy device 80x25 > console handover: boot [udbg0] -> real [hvc0] > Dentry cache hash table entries: 262144 (order: 9, 2097152 bytes) > Inode-cache hash table entries: 131072 (order: 8, 1048576 bytes) > Unable to handle kernel paging request for data at address 0xcf030858 > Faulting instruction address: 0xc00c4530 > Oops: Kernel access of bad area, sig: 11 [#1] > SMP NR_CPUS=32 pSeries > Modules linked in: > NIP: c00c4530 LR: c07e8790 CTR: 801af404 > REGS: c0987ae0 TRAP: 0300 Not tainted (2.6.25) > MSR: 80009032 CR: 2488 XER: 2002 > DAR: cf030858, DSISR: 4001 > TASK = c0862650[0] 'swapper' THREAD: c0984000 CPU: 0 > GPR00: 2000 c0987d60 c0981f18 cf030858 > GPR04: 0001 c09f2a68 > GPR08: c09f2a58 01b8 cf030858 > GPR12: 4000 c09ae400 > GPR16: 0001 > GPR20: c0007ffe8000 0008 0001 c09f2848 > GPR24: cf030200 4000 0dc0 001d > GPR28: e000 cf030890 c08e5088 0dde > NIP [c00c4530] .__free_pages_bootmem+0xc/0xa8 > LR [c07e8790] .free_all_bootmem_core+0x140/0x218 > Call Trace: > [c0987d60] [01c0a438] 0x1c0a438 (unreliable) > [c0987e40] [c07d29c8] .mem_init+0x68/0x1b0 > [c000
Re: [PATCH 1/2] i2c: Add support for device alias names
On Thu, 2008-05-01 at 10:04 +0200, Jean Delvare wrote: > On Mon, 28 Apr 2008 18:16:17 +0200, Kay Sievers wrote: > > On Mon, 2008-04-28 at 17:40 +0200, Jean Delvare wrote: > > > Why would i2c device modaliases ever contain multiple strings? A device > > > can't have multiple names, can it? > > > > Like ACPI/PNP devices, which can have several compat id's, which means > > that a single device can have "multiple names": > > $ cat /sys/bus/pnp/devices/00:09/id > > IBM0057 > > PNP0f13 > > Ah, I didn't know about this. Now I'm curious how it can work. Does it > mean that several drivers attempt to bind to this device? They are usually all id's for the same type of device, and don't describe multiple functions at the same time. In most cases the vendor id's, like "IBM0057" here, do not match anything. > > > Can't we just stop handle_moddevtable() from adding a tailing "*" > > > automatically, and just let the device types which need it, add it on > > > their own? > > > > For a lot subsystems it's fine to have it appended, as there is a > > defined list of identifiers, which must appear in the same order, and > > new identifiers are appended to the end. So the "*" still matches > > modules with possibly extended modalias strings. > > I understand the logic, however I am skeptical how useful it is in > practice. If we add an identifier to the device aliases, then we also > update the corresponding modalias, so no in-tree driver can break. The > only case where it makes a difference, as far as I can see, is for > out-of-tree drivers. Am I correct? That sounds correct, yes. > On top of that, I doubt that we > actually add new identifiers that frequently, do we? Not that I know. > > We would also need to review all buses which export modalias, if they > > need the "*" or not, and add them by hand, if needed. > > > > I guess, it's easier to introduce an additional parameter to > > file2alias::do_table() and suppress the trailing "*" for i2c? > > That's one possibility, but I had a slightly different approach, which > is to just let the type-specific handlers add the trailing "*" by > themselves if they need it. This allows for optimization in a few > cases. > > Subject: modpost: i2c aliases need no trailing wildcard > > Not all device type aliases need a trailing wildcard, in particular > the i2c aliases don't. Don't add a wildcard by default in do_table(), > instead let each device type handler add it if needed. ... > The patch only changes the i2c aliases, all the rest is the same as > before (unless I messed up somewhere, that is.) Do you think this would > be acceptable for upstream? If you think it's better to add a parameter > to do_table() and let it add the "*" as it did so far, that's also fine > with me, I can update the patch to do that. Looks fine to me. If the content of: /lib/modules/$(uname -r)/modules.alias looks correct after the patch, this should go in, I think. Thanks, Kay ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On Thu, May 01, 2008 at 08:17:07AM -0500, Kumar Gala wrote: > On May 1, 2008, at 3:24 AM, Paul Mackerras wrote: > >The most common cases are (a) something that ultimately generates > >input on a tty (e.g. a character arriving on a serial port) and that > >input turns out to be a ^C or similar, or (b) something that signals > >I/O completion and the program doing the I/O has requested > >notification by a SIGIO. But in general any driver code can send a > >signal to userspace if it wants. And, of course, SIGALRM and similar timer mechanisms. > ok. Was just wondering how the async exception know that the signal > it wanted to send belonged to the particular process that is running. > But I guess there are cases that the signal is really intended for who > ever is currently running? No, it knows based on its own data structures who it's intended for -- and sometimes that happens to be the currently running process. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On Thu, May 01, 2008 at 11:14:51AM -0500, Scott Wood wrote: > On Thu, May 01, 2008 at 08:17:07AM -0500, Kumar Gala wrote: > > ok. Was just wondering how the async exception know that the signal > > it wanted to send belonged to the particular process that is running. > > But I guess there are cases that the signal is really intended for who > > ever is currently running? > > No, it knows based on its own data structures who it's intended for -- > and sometimes that happens to be the currently running process. Oh, except for things like SIGXCPU and SIGVTALRM, which do target the currently running process. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Updated: Reworked Cell OProfile: SPU mutex lock fix
Carl Love wrote: Sorry, looks like my mailer mangled the file. This is a reworked patch to fix the SPU data storage. Currently, the SPU escape sequences and program counter data is being added directly into the kernel buffer without holding the buffer_mutex lock. This patch changes how the data is stored. A new function, oprofile_add_value, is added into the oprofile driver to allow adding generic data to the per cpu buffers. This enables a series of calls to the oprofile_add_value to enter the needed SPU escape sequences and SPU program data into the kernel buffer via the per cpu buffers without any additional processing. The oprofile_add_value function is generic so it could be used by other architecures as well provided the needed postprocessing was added to opreport. Finally, this patch backs out the changes previously added to the oprofile generic code for handling the architecture specific ops.sync_start and ops.sync_stop that allowed the architecture to skip the per CPU buffer creation. Signed-off-by: Carl Love <[EMAIL PROTECTED]> Looks fine to me. All of my previous review comments have been addressed. -Maynard [snip] ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On May 1, 2008, at 11:14 AM, Scott Wood wrote: On Thu, May 01, 2008 at 08:17:07AM -0500, Kumar Gala wrote: On May 1, 2008, at 3:24 AM, Paul Mackerras wrote: The most common cases are (a) something that ultimately generates input on a tty (e.g. a character arriving on a serial port) and that input turns out to be a ^C or similar, or (b) something that signals I/O completion and the program doing the I/O has requested notification by a SIGIO. But in general any driver code can send a signal to userspace if it wants. And, of course, SIGALRM and similar timer mechanisms. ok. Was just wondering how the async exception know that the signal it wanted to send belonged to the particular process that is running. But I guess there are cases that the signal is really intended for who ever is currently running? No, it knows based on its own data structures who it's intended for -- and sometimes that happens to be the currently running process. Let me ask the question differently. Are there cases that some event occurs in the system and a signal is delivered to the current process regardless of what that process is. I'm guessing so, based on the tty example. So for the specific case I'm looking at (kprobes & debug exceptions from kernel space), I think its reasonable to BUG_ON() if thread_info- >flags changes such that TIF_SIGPENDING or TIF_NEED_RESCHED get set we aren't from user-space. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] mpc5200: Allow for fixed speed MII configurations
Hi Grant,
Grant Likely wrote:
> On Tue, Apr 29, 2008 at 5:06 PM, Grant Likely <[EMAIL PROTECTED]> wrote:
>> From: Grant Likely <[EMAIL PROTECTED]>
>>
>> Various improvements for configuring the MPC5200 MII link from the
>> device tree:
>> * Look for 'current-speed' property for fixed speed MII links
>> * Look for 'fsl,7-wire-mode' property for boards using the 7 wire mode
>> * move definition of private data structure out of the header file
>>
>> Signed-off-by: Grant Likely <[EMAIL PROTECTED]>
>
> Any more comments on this patch? I want to push it to Paulus, but I'd
> like to have someone ack it first.
>
> Wolfgang, you used the previous version of this patch. Does this one
> work for you?
Sorry for the late answer. The patch works fine (under Linux 2.6.24) on
my board with a 3-port Micrel ethernet switch. There is still a minor
issue, though:
[...deletions...]
>> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
>> index d21b7ab..eeb4433 100644
>> --- a/drivers/net/fec_mpc52xx.c
>> +++ b/drivers/net/fec_mpc52xx.c
[...deletions...]
>> @@ -950,24 +976,36 @@ mpc52xx_fec_probe(struct of_device *op, const struct
>> of_device_id *match)
>> priv->msg_enable = netif_msg_init(debug, MPC52xx_MESSAGES_DEFAULT);
>> priv->duplex = DUPLEX_FULL;
More concerning this line below.
>>
>> - /* is the phy present in device tree? */
>> - ph = of_get_property(op->node, "phy-handle", NULL);
>> - if (ph) {
>> - const unsigned int *prop;
>> - struct device_node *phy_dn;
>> - priv->has_phy = 1;
>> -
>> - phy_dn = of_find_node_by_phandle(*ph);
>> - prop = of_get_property(phy_dn, "reg", NULL);
>> - priv->phy_addr = *prop;
>> + /*
>> +* Link mode configuration
>> +*/
>>
>> - of_node_put(phy_dn);
>> + /* Start with safe defaults for link connection */
>> + priv->phy_addr = FEC5200_PHYADDR_NONE;
>> + priv->speed = 100;
>> + priv->duplex = 0;
priv->duplex is re-defined here. And instead of "0" we should use
DUPLEX_HALF.
Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On Thu, May 01, 2008 at 11:33:34AM -0500, Kumar Gala wrote: > Let me ask the question differently. Are there cases that some event > occurs in the system and a signal is delivered to the current process > regardless of what that process is. Yes, timers that expire based on CPU usage. > I'm guessing so, based on the tty example. I don't think tty interrupts would -- it'd go to the process group associated with the tty. > So for the specific case I'm looking at (kprobes & debug exceptions > from kernel space), I think its reasonable to BUG_ON() if thread_info- > >flags changes such that TIF_SIGPENDING or TIF_NEED_RESCHED get set > we aren't from user-space. Why? It may not happen currently, but it seems more future-proof to just copy the flags. And it's certainly not reasonable for normal interrupts. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] mpc5200: Allow for fixed speed MII configurations
On Thu, May 1, 2008 at 10:38 AM, Wolfgang Grandegger <[EMAIL PROTECTED]> wrote: > Hi Grant, > > > Grant Likely wrote: > > On Tue, Apr 29, 2008 at 5:06 PM, Grant Likely <[EMAIL PROTECTED]> wrote: > >> From: Grant Likely <[EMAIL PROTECTED]> > >> > >> Various improvements for configuring the MPC5200 MII link from the > >> device tree: > >> * Look for 'current-speed' property for fixed speed MII links > >> * Look for 'fsl,7-wire-mode' property for boards using the 7 wire mode > >> * move definition of private data structure out of the header file > >> > >> Signed-off-by: Grant Likely <[EMAIL PROTECTED]> > > > > Any more comments on this patch? I want to push it to Paulus, but I'd > > like to have someone ack it first. > > > > Wolfgang, you used the previous version of this patch. Does this one > > work for you? > > Sorry for the late answer. The patch works fine (under Linux 2.6.24) on > my board with a 3-port Micrel ethernet switch. There is still a minor > issue, though: > > >> - of_node_put(phy_dn); > >> + /* Start with safe defaults for link connection */ > >> + priv->phy_addr = FEC5200_PHYADDR_NONE; > >> + priv->speed = 100; > >> + priv->duplex = 0; > > priv->duplex is re-defined here. And instead of "0" we should use > DUPLEX_HALF. Oops, Fixed. If you reply with your 'acked-by' line, then I'll push this one to Paul so it can get into .26 Thanks, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] mpc5200: Allow for fixed speed MII configurations
Grant Likely wrote: > On Thu, May 1, 2008 at 10:38 AM, Wolfgang Grandegger <[EMAIL PROTECTED]> > wrote: >> Hi Grant, >> >> >> Grant Likely wrote: >> > On Tue, Apr 29, 2008 at 5:06 PM, Grant Likely <[EMAIL PROTECTED]> wrote: >> >> From: Grant Likely <[EMAIL PROTECTED]> >> >> >> >> Various improvements for configuring the MPC5200 MII link from the >> >> device tree: >> >> * Look for 'current-speed' property for fixed speed MII links >> >> * Look for 'fsl,7-wire-mode' property for boards using the 7 wire mode >> >> * move definition of private data structure out of the header file >> >> >> >> Signed-off-by: Grant Likely <[EMAIL PROTECTED]> >> > >> > Any more comments on this patch? I want to push it to Paulus, but I'd >> > like to have someone ack it first. >> > >> > Wolfgang, you used the previous version of this patch. Does this one >> > work for you? >> >> Sorry for the late answer. The patch works fine (under Linux 2.6.24) on >> my board with a 3-port Micrel ethernet switch. There is still a minor >> issue, though: >> >> >> - of_node_put(phy_dn); >> >> + /* Start with safe defaults for link connection */ >> >> + priv->phy_addr = FEC5200_PHYADDR_NONE; >> >> + priv->speed = 100; >> >> + priv->duplex = 0; >> >> priv->duplex is re-defined here. And instead of "0" we should use >> DUPLEX_HALF. > > Oops, > > Fixed. > > If you reply with your 'acked-by' line, then I'll push this one to > Paul so it can get into .26 Yes, of course. Add Acked-by: Wolfgang Grandegger <[EMAIL PROTECTED]> please. Wolfgang. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On May 1, 2008, at 11:42 AM, Scott Wood wrote: On Thu, May 01, 2008 at 11:33:34AM -0500, Kumar Gala wrote: Let me ask the question differently. Are there cases that some event occurs in the system and a signal is delivered to the current process regardless of what that process is. Yes, timers that expire based on CPU usage. I'm guessing so, based on the tty example. I don't think tty interrupts would -- it'd go to the process group associated with the tty. So for the specific case I'm looking at (kprobes & debug exceptions from kernel space), I think its reasonable to BUG_ON() if thread_info- flags changes such that TIF_SIGPENDING or TIF_NEED_RESCHED get set we aren't from user-space. Why? It may not happen currently, but it seems more future-proof to just copy the flags. And it's certainly not reasonable for normal interrupts. copying the flags isn't the issue. Its acting on the flags thats the problem. I'm not 100% sure the C code that might clear the flags is consistent on how it access them. So if one bit of code clears task_struct->stack->thread_info->flags and other clears thread_info(STACK)->flags we get into an issue on how to merge after that. I'd rather be safe for now since we don't need to send a signal in the debug from kernel case. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Please pull linux-2.6-mpc52xx.git
Paul, here's a couple more for 2.6.26. One bug fix and one minor change that I've had on the back burner for a month or so. This should be the last of any non-bugfix changes through my tree. Cheers, g. git-request-pull powerpc git://git.secretlab.ca/git/linux-2.6-mpc52xx.git The following changes since commit eabd90944b3a00766e84da3d117ea0f3e0a3b1a3: Michael Ellerman (1): [POWERPC] Fix crashkernel= handling when no crashkernel= specified are available in the git repository at: git://git.secretlab.ca/git/linux-2.6-mpc52xx.git for-2.6.26 Andrew Liu (1): Fix a potential issue in mpc52xx uart driver Grant Likely (1): [POWERPC] mpc5200: Allow for fixed speed MII configurations .../powerpc/mpc52xx-device-tree-bindings.txt | 11 ++ drivers/net/fec_mpc52xx.c | 97 +++- drivers/net/fec_mpc52xx.h | 19 drivers/serial/mpc52xx_uart.c |2 + 4 files changed, 87 insertions(+), 42 deletions(-) -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Fix a potential issue in mpc52xx uart driver
On Tue, Apr 29, 2008 at 1:36 AM, Andrew Liu <[EMAIL PROTECTED]> wrote: > mpc52xx_uart_int and __uart_put_char both try to acquire the > port->lock. Therefore the function sequence of: > > mpc52xx_uart_int--> ...-->flush_to_ldisc-->...-->__uart_put_char > > can potentially trigger a deadlock. To avoid this deadlock a fix > similar to that found in the 8250.c serial driver is applied. The > deadlock is avoided by releasing the lock before pushing a buffer > and reacquiring it when completed. > > Signed-off-by: Andrew Liu <[EMAIL PROTECTED]> Your patch is whitespace damaged and does not apply (tabs have been converted to spaces). It was trivial so I've applied it manually, but please be careful when posting future patches. Thanks, g. > > diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c > index d93b357..5f95e53 100644 > --- a/drivers/serial/mpc52xx_uart.c > +++ b/drivers/serial/mpc52xx_uart.c > @@ -783,7 +783,9 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port) > } > } > > + spin_unlock(&port->lock); > tty_flip_buffer_push(tty); > + spin_lock(&port->lock); > > return psc_ops->raw_rx_rdy(port); > } > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
NAND command line partiions
I am using u-boot-1.3.1 in order to boot linux 2.6.25 on a custom MPC8313 based board. In past versions of the kernel I was able to pass the NAND partition information to the kernel with this line: root=/dev/mtdblock2 rootfstype=jffs2 rw console=ttyS0,115200 mtdparts=nand0:1M(u-boot),3M(kernel),-(jffs2). In this latest version the kernel will not mount the partitions because the name that is parsed from the command line (nand0) is different from that of the name of the NAND that comes from ./drivers/mtd/nand/nand_ids.c (NAND 256MiB 3,3V 8-bit). I have tried to change the name of the NAND that is passed in via the command line, but u-boot does not seem to like the format of the name that linux compares against. It doesn't like the ,(comma) in 3,3V. Does anyone have any suggestions? Ron Madrid _ Make i'm yours. Create a custom banner to support your cause. http://im.live.com/Messenger/IM/Contribute/Default.aspx?source=TXT_TAGHM_MSN_Make_IM_Yours___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: NAND command line partiions
On Thu, May 01, 2008 at 11:24:35AM -0700, Ronald Madrid wrote: > I am using u-boot-1.3.1 in order to boot linux 2.6.25 on a custom MPC8313 > based board. In past versions of the kernel I was able to pass the NAND > partition information to the kernel with this line: root=/dev/mtdblock2 > rootfstype=jffs2 rw console=ttyS0,115200 > mtdparts=nand0:1M(u-boot),3M(kernel),-(jffs2). In this latest version the > kernel will not mount the partitions because the name that is parsed from > the command line (nand0) is different from that of the name of the NAND > that comes from ./drivers/mtd/nand/nand_ids.c (NAND 256MiB 3,3V 8-bit). This is fixed in the current mtd tree (commit 9ebed3e60f9, fsl_elbc_nand: fix mtd name). > I have tried to change the name of the NAND that is passed in via the > command line, but u-boot does not seem to like the format of the name that > linux compares against. It doesn't like the ,(comma) in 3,3V. Why would u-boot care about the format of the kernel command line? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: powerpc boot regression
Badari Pulavarty wrote: > On Thu, 2008-05-01 at 15:13 +1000, Tony Breeds wrote: >> On Tue, Apr 29, 2008 at 11:12:41PM -0700, David Miller wrote: >> > >> > This commit causes bootup failures on sparc64: >> > >> > commit 86f6dae1377523689bd8468fed2f2dd180fc0560 >> > Author: Yasunori Goto <[EMAIL PROTECTED]> >> > Date: Mon Apr 28 02:13:33 2008 -0700 >> > >> > memory hotplug: allocate usemap on the section with pgdat >> >> >> >> >> We're seeing a boot failure on powerpc. git bisect points the problem >> at this commit. However reverting just this one comitt doesn't fix the >> regression. I also needed to revert >> 04753278769f3b6c3b79a080edb52f21d83bf6e2 (memory hotplug: register >> section/node id to free") >> >> Problem seen on power4, power5 and ps3. > > I don't see the problem on my power5 machine. Can you send the > config ? Is CONFIG_SPARSEMEM_VMEMMAP enabled ? The bug is hit with ps3_defconfig. It does not have CONFIG_SPARSEMEM_VMEMMAP set. -Geoff ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
Kumar Gala wrote: copying the flags isn't the issue. Its acting on the flags thats the problem. I'm not 100% sure the C code that might clear the flags is consistent on how it access them. Actually *delivering* the signal should never be done except when returning to user. That's different from sending the signal, though. BTW, it doesn't seem all that unreasonable for a kernel profiling/tracing exception to signal a process that, for example, an event buffer is over a certain threshold. > So if one bit of code clears > task_struct->stack->thread_info->flags and other clears > thread_info(STACK)->flags we get into an issue on how to merge after > that. It appears that TIF_SIGPENDING is always accessed through the task struct, though not so for TIF_NEED_RESCHED. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: SKB corruption on heavy traffic
We have experience a very similar problem using a 2.4.18 kernel on an 8260 ppc processor. We have a telecomunication product that for some time only used the fec for TCP/IP ethernet traffic only and worked just fine. After we upgraded our product to implement TDM data over IP we started to notice an occasional kernel oops. We began to evaluate all of our products and determined that only some of the units exhibted this behaviour at various rates of occurrence. Further evaluation revealed that the pointers located in dpram pointing to the fec's buffer descriptors were some how getting corrupted. Note that the 8260 has 4 internal scc/fccs and we use all four for various aspects of our application and each shares dpram for pointers to buffer descriptors that reside in sdram. However, only the fec that is used for IP experiences this buffer descriptor corruption and, then, apparently, only under heavy traffic load. We spent about six months evaluating this problem including contacting freescale, but never found a solution. We finally, decided to use an external ethernet chip on a daughter card for our IP channel. It is, however, our belief that our problem relates to a possible bug in the 8260 CPM, but have yet to absolutely prove this. If we are experiencing the same problem (and potentially others) and there is a solution we would be very interested, as, we are not happy about the daughter card solution. Myron L. Dixon Sr. Software Engineer L-3 Communications, GNS 1519 Grundy's Lane Bristol, PA 19007 Phone: 215 957 3739 Fax: 215 957 3790 email: [EMAIL PROTECTED] -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Franca, Jose (NSN - PT/Portugal - MiniMD) Sent: Wednesday, April 30, 2008 5:07 AM To: linuxppc-dev@ozlabs.org; [EMAIL PROTECTED] Subject: FW: SKB corruption on heavy traffic >From our latest debugs we found that the problem occurs mainly on skbuff code. >After some variable time kfree or kalloc result in kernel oops. -Original Message- From: Franca, Jose (NSN - PT/Portugal - MiniMD) Sent: quarta-feira, 30 de Abril de 2008 9:44 To: 'ext Scott Wood' Cc: Subject: RE: SKB corruption on heavy traffic Hello! Thank you for replying! It't quite dificult to say if the problem exists without our changes, since the all software is dependent on this changes so to work with the hardware. I can't answer to that right now on that, but I forgot to add one thing: we have ring buffer full problems on our fcc_enet driver from time to time. So, I think the problem could be on linux configurations (related to hw) because there is a lot of posts on the web related to problems similar to this (none of them has really solved the bottom problem). Regards, Filipe -Original Message- From: ext Scott Wood [mailto:[EMAIL PROTECTED] Sent: terça-feira, 29 de Abril de 2008 20:15 To: Franca, Jose (NSN - PT/Portugal - MiniMD) Cc: linuxppc-dev@ozlabs.org; [EMAIL PROTECTED] Subject: Re: SKB corruption on heavy traffic On Tue, Apr 29, 2008 at 07:39:07PM +0100, Franca, Jose (NSN - PT/Portugal - MiniMD) wrote: > We are developing a MPC8247 based telecom board (512MB), using linux > 2.4 with some proprietary changes on IP stack and we are facing some > problems when we have heavy traffic on our Ethernet interfaces... Do you see these problems without the proprietary changes, and with a current kernel? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] [PATCH] vmemmap fixes to use smaller pages
Benjamin Herrenschmidt wrote: > This patch changes vmemmap to use a different region (region 0xf) of the > address space whose page size can be dynamically configured at boot. > > The problem with the current approach of always using 16M pages is that > it's not well suited to machines that have small amounts of memory such > as small partitions on pseries, or PS3's. > > In fact, on the PS3, failure to allocate the 16M page backing vmmemmap > tends to prevent hotplugging the HV's "additional" memory, thus limiting > the available memory even more, from my experience down to something > like 80M total, which makes it really not very useable. > > The logic used by my match to choose the vmemmap page size is: > > - If 16M pages are available and there's 1G or more RAM at boot, use that > size. > - Else if 64K pages are available, use that > - Else use 4K pages It doesn't seem to cause problems on PS3, and I added it into ps3-linux.git as other/powerpc-vmemmap-variable-page-size.diff, but I couldn't get it to fail without the patch... Could you send me your kernel .config? -Geoff ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Watchdog on MPC85xx SMP system
On Tue, 29 Apr 2008 16:42:05 +0800
Chen Gong <[EMAIL PROTECTED]> wrote:
> On Book-E SMP systems each core has its own private watchdog.
> If only one watchdog is enabled, when the core that doesn't
> enable the watchdog is hung, system can't reset because no
> watchdog is running on it. That's bad. It means we must
> enable watchdogs on both cores.
>
> We can use smp_call_function() to send appropriate messages to
> all the other cores to enable and update the watchdog.
>
> Signed-off-by: Chen Gong <[EMAIL PROTECTED]>
> ---
> Now Tested on MPC8572DS platform.
>
> drivers/watchdog/booke_wdt.c | 31 +++
> 1 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
> index d362f5b..8a4e7f0 100644
> --- a/drivers/watchdog/booke_wdt.c
> +++ b/drivers/watchdog/booke_wdt.c
> @@ -1,12 +1,12 @@
> /*
> - * drivers/char/watchdog/booke_wdt.c
> + * drivers/watchdog/booke_wdt.c
> *
> * Watchdog timer for PowerPC Book-E systems
> *
> * Author: Matthew McClintock
> * Maintainer: Kumar Gala <[EMAIL PROTECTED]>
> *
> - * Copyright 2005 Freescale Semiconductor Inc.
> + * Copyright 2005, 2008 Freescale Semiconductor Inc.
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License as published by the
> @@ -16,6 +16,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -47,23 +48,31 @@ u32 booke_wdt_period = WDT_PERIOD_DEFAULT;
> #define WDTP(x) (TCR_WP(x))
> #endif
>
> +static DEFINE_SPINLOCK(booke_wdt_lock);
> +
> +static void __booke_wdt_ping(void *data)
> +{
> + mtspr(SPRN_TSR, TSR_ENW|TSR_WIS);
> +}
> +
> /*
> * booke_wdt_ping:
> */
> static __inline__ void booke_wdt_ping(void)
> {
> - mtspr(SPRN_TSR, TSR_ENW|TSR_WIS);
> + smp_call_function(__booke_wdt_ping, NULL, 0, 0);
> + __booke_wdt_ping(NULL);
> }
>
> /*
> - * booke_wdt_enable:
> + * __booke_wdt_enable:
> */
> -static __inline__ void booke_wdt_enable(void)
> +static inline void __booke_wdt_enable(void *data)
> {
> u32 val;
>
> /* clear status before enabling watchdog */
> - booke_wdt_ping();
> + __booke_wdt_ping(NULL);
> val = mfspr(SPRN_TCR);
> val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period));
>
> @@ -137,12 +146,15 @@ static int booke_wdt_ioctl (struct inode *inode, struct
> file *file,
> */
> static int booke_wdt_open (struct inode *inode, struct file *file)
> {
> + spin_lock(&booke_wdt_lock);
> if (booke_wdt_enabled == 0) {
> booke_wdt_enabled = 1;
> - booke_wdt_enable();
> + __booke_wdt_enable(NULL);
> + smp_call_function(__booke_wdt_enable, NULL, 0, 0);
It's nonsensical to call an inlined function via smp_call_function(). What
we're asking the compiler to do here is to generate both an out-of-line
copy and an inlined copy.
Also, the above is an open-coded version of on_each_cpu().
so... something like the below should tidy that up, along with numerous
other things. Can you please check it over?
diff -puN
drivers/watchdog/booke_wdt.c~watchdog-fix-booke_wdtc-on-mpc85xx-smp-system-fix
drivers/watchdog/booke_wdt.c
---
a/drivers/watchdog/booke_wdt.c~watchdog-fix-booke_wdtc-on-mpc85xx-smp-system-fix
+++ a/drivers/watchdog/booke_wdt.c
@@ -1,6 +1,4 @@
/*
- * drivers/watchdog/booke_wdt.c
- *
* Watchdog timer for PowerPC Book-E systems
*
* Author: Matthew McClintock
@@ -39,7 +37,7 @@
#define WDT_PERIOD_DEFAULT 3 /* Refer to the PPC40x and PPC4xx manuals */
#endif /* for timing information */
-u32 booke_wdt_enabled = 0;
+u32 booke_wdt_enabled;
u32 booke_wdt_period = WDT_PERIOD_DEFAULT;
#ifdef CONFIG_FSL_BOOKE
@@ -55,19 +53,12 @@ static void __booke_wdt_ping(void *data)
mtspr(SPRN_TSR, TSR_ENW|TSR_WIS);
}
-/*
- * booke_wdt_ping:
- */
-static __inline__ void booke_wdt_ping(void)
+static void booke_wdt_ping(void)
{
- smp_call_function(__booke_wdt_ping, NULL, 0, 0);
- __booke_wdt_ping(NULL);
+ on_each_cpu(__booke_wdt_ping, NULL, 0, 0);
}
-/*
- * __booke_wdt_enable:
- */
-static inline void __booke_wdt_enable(void *data)
+static void __booke_wdt_enable(void *data)
{
u32 val;
@@ -79,10 +70,7 @@ static inline void __booke_wdt_enable(vo
mtspr(SPRN_TCR, val);
}
-/*
- * booke_wdt_write:
- */
-static ssize_t booke_wdt_write (struct file *file, const char __user *buf,
+static ssize_t booke_wdt_write(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
booke_wdt_ping();
@@ -90,15 +78,11 @@ static ssize_t booke_wdt_write (struct f
}
static struct watchdog_info ident = {
- .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
- .firmware_version = 0,
- .identity = "PowerPC Book-E Watchdog",
+ .options = WDIOF_SETTIMEOUT | WD
Re: [RFC] [PATCH] vmemmap fixes to use smaller pages
On Thu, 2008-05-01 at 14:46 -0700, Geoff Levand wrote: > > It doesn't seem to cause problems on PS3, and I added it into > ps3-linux.git > as other/powerpc-vmemmap-variable-page-size.diff, but I couldn't get > it to > fail without the patch... > > Could you send me your kernel .config? ps3_defconfig with added vmmemap (which is disabled by default). Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: powerpc boot regression
On Thu, May 01, 2008 at 07:51:47AM -0700, Badari Pulavarty wrote: > I don't see the problem on my power5 machine. Can you send the > config ? Is CONFIG_SPARSEMEM_VMEMMAP enabled ? Yes. I'm just using ppc64_defconfig. Yours Tony linux.conf.auhttp://www.marchsouth.org/ Jan 19 - 24 2009 The Australian Linux Technical Conference! ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] Squash build warning for print of resource_size_t in fsl_soc.c
When resource_size_t is larger than an int, the current code generates a build warning. Kill it. Signed-off-by: Becky Bruce <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/fsl_soc.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c index 7b45670..b594087 100644 --- a/arch/powerpc/sysdev/fsl_soc.c +++ b/arch/powerpc/sysdev/fsl_soc.c @@ -389,8 +389,8 @@ static int __init gfar_of_init(void) } gfar_data.phy_id = *id; - snprintf(gfar_data.bus_id, MII_BUS_ID_SIZE, "%x", - res.start); + snprintf(gfar_data.bus_id, MII_BUS_ID_SIZE, "%llx", +(unsigned long long)res.start); of_node_put(phy); of_node_put(mdio); -- 1.5.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
Kumar Gala writes: > ok. Was just wondering how the async exception know that the signal > it wanted to send belonged to the particular process that is running. Usually the driver would have a reference to the task_struct of the task that should get the signal, and it would send the signal to that task, rather than the current task. That task could be the current task, of course, but it might not be. I can't think of a case where an asynchronous interrupt would always result in a signal being sent to the current task. > >> how do we provide someone stick a kprobe on such code today? > > > > -ENOPARSE > > I was asking how we prevent the cases you were describing working w/ > kprobes today. Since it ends up single stepping in kernel codes its > possible that someone sets a kprobe in code that shouldn't be > interrupted, yet we'd cause a SingleStep Exception. I'd have to look more closely at the kprobes code to answer that in detail. I assume the kprobes exception handlers are sufficiently careful about what they do because they are aware they could have interrupted interrupt-disabled code. > >> So I'm not if there is any good way to preclude the handlers > >> associated with these exceptions from doing the things you listed. > > > > In that case, you'd better expect to see system freezes, memory > > corruption and general instability. > > So the case I'm trying to make work is debug and kprobes. This case > seems like we have pretty good control over what the "handler" does. > Are there checks we can add to BUG_ON() so we are at least aware of > the code attempts to do something it shouldnt? Well, there's in_interrupt(), and there's the __kprobes marking, which is used to mark functions where kprobes must not put a breakpoint. Apart from that I would need to read through the kprobes code to comment further... Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/6] [POWERPC] Move device_to_mask() to dma-mapping.h
On Thu, 1 May 2008 07:04:30 pm Segher Boessenkool wrote: > > Move device_to_mask() to dma-mapping.h because we need to use it from > > outside dma_64.c in a later patch. > > Why does this need to become an inline function? > > > Segher > I'm not sure exactly what you mean - it was inline before the move. But, I honestly didn't think about it too much (although I possibly should have)... I figured that it was static inline before my patch so I should leave it as such. But if everybody thinks it would be better to leave it in dma_64.c and just expose it for use outside, I'm fine with that. Thanks! Mark. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: overloading existing PTRACE_GET_DEBUGREG/PTRACE_SET_DEBUGREG commands
For the short answer, no, I'd like not to extend PTRACE_SET_DEBUGREG in
the simple fashion. The direction I want to move with this is away from
providing the hardware register formats directly as the user ABI.
What I mean is resurrecting, finishing, and porting the "hw_breakpoint"
work that Alan Stern <[EMAIL PROTECTED]> started, but which was
never finished or merged. This is an in-kernel interface handling the
hardware details and multiplexing in-kernel and user uses, with a
higher-level API that is mostly machine-independent. The idea is that
future user-level extensions for watchpoint-like features would be based
on that, not on exposing hardware features directly as such.
I think someone at IBM told me they were going to look at picking that
work up, but I'll have to remember who and track them down.
> Also, what functionality can GDB take advantage of today? Does it
> comprehend the idea of breakpoints or watchpoints that cover a range?
I'm not the expert on that, but probably the answer is "sort of".
The low-level feature on s390 is range-based, and it supports that.
The hook for machine support uses "addr+len", though on most machines
len has to be 8 or has to be {1,2,4,8} and addr has to be aligned to len.
I'm not sure that high-level watchpoints ever translate into big ranges
in practice.
Understanding the details of all the hardware features in the powerpc
variants would inform ironing out the hw_breakpoint interface details.
Thanks,
Roland
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/6] [POWERPC] Move device_to_mask() to dma-mapping.h
I'm not sure exactly what you mean - it was inline before the move. Heh, I missed that. But if everybody thinks it would be better to leave it in dma_64.c and just expose it for use outside, I'm fine with that. That's what I meant, yes. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
how to check for "optional" ppc chip features (MSR_BE)
I've been looking at PowerISA_Public.pdf that I downloaded from some ppc site. It describes various things as "need not be supported on all implementations", for example the MSR_BE bit. Is there a generic way to detect if such a feature is supported, or a known table of models that support features, or what? Right now I'm considering MSR_BE (branch tracing). I have a patch to use this (arch_has_block_step, enabling a PTRACE_SINGLEBLOCK). The only machine handy to test is a Mac G5 (PPC970FX, 3.0 (pvr 003c 0300)). I know this chip supports MSR_BE. But that's only because I wrote an affirmative test case and tried it and saw it work right. Before submitting the kernel changes, I want to get the CPU model conditionalization correct (a runtime check on some feature bit mask is fine here, if CONFIG_* alone does not indicate for sure). Thanks, Roland ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/6] [POWERPC] Move device_to_mask() to dma-mapping.h
On Fri, 2 May 2008 10:32:35 am Segher Boessenkool wrote:
> > I'm not sure exactly what you mean - it was inline before the move.
>
> Heh, I missed that.
>
> > But if everybody thinks it would be better to leave it in dma_64.c and
> > just
> > expose it for use outside, I'm fine with that.
>
> That's what I meant, yes.
Thought so. How's this:
---
arch/powerpc/kernel/dma_64.c |2 +-
include/asm-powerpc/dma-mapping.h |3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
Index: upstream/arch/powerpc/kernel/dma_64.c
===
--- upstream.orig/arch/powerpc/kernel/dma_64.c
+++ upstream/arch/powerpc/kernel/dma_64.c
@@ -15,7 +15,7 @@
* Generic iommu implementation
*/
-static inline unsigned long device_to_mask(struct device *dev)
+unsigned long device_to_mask(struct device *dev)
{
if (dev->dma_mask && *dev->dma_mask)
return *dev->dma_mask;
Index: upstream/include/asm-powerpc/dma-mapping.h
===
--- upstream.orig/include/asm-powerpc/dma-mapping.h
+++ upstream/include/asm-powerpc/dma-mapping.h
@@ -45,6 +45,9 @@ extern void __dma_sync_page(struct page
#endif /* ! CONFIG_NOT_COHERENT_CACHE */
#ifdef CONFIG_PPC64
+
+extern unsigned long device_to_mask(struct device *dev);
+
/*
* DMA operations are abstracted for G5 vs. i/pSeries, PCI vs. VIO
*/
Thanks!
Mark.
>
>
> Segher
>
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: how to check for "optional" ppc chip features (MSR_BE)
On May 1, 2008, at 8:21 PM, Roland McGrath wrote: I've been looking at PowerISA_Public.pdf that I downloaded from some ppc site. It describes various things as "need not be supported on all implementations", for example the MSR_BE bit. Is there a generic way to detect if such a feature is supported, or a known table of models that support features, or what? Right now I'm considering MSR_BE (branch tracing). I have a patch to use this (arch_has_block_step, enabling a PTRACE_SINGLEBLOCK). The only machine handy to test is a Mac G5 (PPC970FX, 3.0 (pvr 003c 0300)). I know this chip supports MSR_BE. But that's only because I wrote an affirmative test case and tried it and saw it work right. Look at arch/powerpc/kernel/cputable.c to see how we handle issues like this. Before submitting the kernel changes, I want to get the CPU model conditionalization correct (a runtime check on some feature bit mask is fine here, if CONFIG_* alone does not indicate for sure). I believe all PPC that implement MSR[SE] also support MSR[BE]. The "Book-E" class PPCs have a different mechanism to single branch completion (these are embedded PPC products). I've been working on changes related to the Book-E class machines and would be happy to try and port what you're looking over. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Rework EXC_LEVEL_EXCEPTION_PROLOG code
On May 1, 2008, at 6:34 PM, Paul Mackerras wrote: Kumar Gala writes: ok. Was just wondering how the async exception know that the signal it wanted to send belonged to the particular process that is running. Usually the driver would have a reference to the task_struct of the task that should get the signal, and it would send the signal to that task, rather than the current task. That task could be the current task, of course, but it might not be. I can't think of a case where an asynchronous interrupt would always result in a signal being sent to the current task. how do we provide someone stick a kprobe on such code today? -ENOPARSE I was asking how we prevent the cases you were describing working w/ kprobes today. Since it ends up single stepping in kernel codes its possible that someone sets a kprobe in code that shouldn't be interrupted, yet we'd cause a SingleStep Exception. I'd have to look more closely at the kprobes code to answer that in detail. I assume the kprobes exception handlers are sufficiently careful about what they do because they are aware they could have interrupted interrupt-disabled code. Can you be a bit more specific about what we have to be careful about? Also, how does this work in a hypervisor world that has no idea about when it might interrupt a guest. So I'm not if there is any good way to preclude the handlers associated with these exceptions from doing the things you listed. In that case, you'd better expect to see system freezes, memory corruption and general instability. So the case I'm trying to make work is debug and kprobes. This case seems like we have pretty good control over what the "handler" does. Are there checks we can add to BUG_ON() so we are at least aware of the code attempts to do something it shouldnt? Well, there's in_interrupt(), and there's the __kprobes marking, which is used to mark functions where kprobes must not put a breakpoint. Apart from that I would need to read through the kprobes code to comment further... I would appreciate a review here to make sure we are ok. I'm assuming if the current code is already safe that calling into it from a separate exception stack isn't going to make any difference. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Squash build warning for print of resource_size_t in fsl_soc.c
On May 1, 2008, at 6:15 PM, Becky Bruce wrote: When resource_size_t is larger than an int, the current code generates a build warning. Kill it. Signed-off-by: Becky Bruce <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/fsl_soc.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) applied. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Please pull from 'powerpc-next' branch
Please pull from 'powerpc-next' branch of master.kernel.org:/pub/scm/linux/kernel/git/galak/powerpc.git powerpc-next I dropped '[POWERPC] Set lower flag bits..' patch and will get it in for the next release. -k to receive the following updates: arch/powerpc/boot/dts/mpc8610_hpcd.dts | 23 ++- arch/powerpc/sysdev/fsl_rio.c |9 +++-- arch/powerpc/sysdev/fsl_soc.c |4 ++-- arch/powerpc/sysdev/xilinx_intc.c |2 +- drivers/char/xilinx_hwicap/xilinx_hwicap.c |6 +++--- include/linux/rio.h|2 ++ 6 files changed, 37 insertions(+), 9 deletions(-) Anton Vorontsov (1): [POWERPC] 86xx: mpc8610_hpcd: add support for PCI Express x8 slot Becky Bruce (1): [POWERPC] Squash build warning for print of resource_size_t in fsl_soc.c Jason Jin (1): [POWERPC] 86xx: Fix the wrong serial1 interrupt for 8610 board Kumar Gala (1): [POWERPC] Xilinx: Fix compile warnings Randy Dunlap (1): [RAPIDIO] fix current kernel-doc notation ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[POWERPC][v2] Bolt in SLB entry for kernel stack on secondary cpus
This fixes a regression reported by Kamalesh Bulabel where a POWER4
machine would crash because of an SLB miss at a point where the SLB
miss exception was unrecoverable. This regression is tracked at:
http://bugzilla.kernel.org/show_bug.cgi?id=10082
SLB misses at such points shouldn't happen because the kernel stack is
the only memory accessed other than things in the first segment of the
linear mapping (which is mapped at all times by entry 0 of the SLB).
The context switch code ensures that SLB entry 2 covers the kernel
stack, if it is not already covered by entry 0. None of entries 0
to 2 are ever replaced by the SLB miss handler.
Where this went wrong is that the context switch code assumes it
doesn't have to write to SLB entry 2 if the new kernel stack is in the
same segment as the old kernel stack, since entry 2 should already be
correct. However, when we start up a secondary cpu, it calls
slb_initialize, which doesn't set up entry 2. This is correct for
the boot cpu, where we will be using a stack in the kernel BSS at this
point (i.e. init_thread_union), but not necessarily for secondary
cpus, whose initial stack can be allocated anywhere. This doesn't
cause any immediate problem since the SLB miss handler will just
create an SLB entry somewhere else to cover the initial stack.
In fact it's possible for the cpu to go quite a long time without SLB
entry 2 being valid. Eventually, though, the entry created by the SLB
miss handler will get overwritten by some other entry, and if the next
access to the stack is at an unrecoverable point, we get the crash.
This fixes the problem by making slb_initialize create a suitable
entry for the kernel stack, if we are on a secondary cpu and the stack
isn't covered by SLB entry 0. This requires initializing the
get_paca()->kstack field earlier, so I do that in smp_create_idle
where the current field is initialized. This also abstracts a bit of
the computation that mk_esid_data in slb.c does so that it can be used
in slb_initialize.
Signed-off-by: Paul Mackerras <[EMAIL PROTECTED]>
---
Michael Ellerman pointed out that I should be comparing
raw_smp_processor_id() with boot_cpuid rather than with 0.
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index be35ffa..1457aa0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -386,6 +386,8 @@ static void __init smp_create_idle(unsigned int cpu)
panic("failed fork for CPU %u: %li", cpu, PTR_ERR(p));
#ifdef CONFIG_PPC64
paca[cpu].__current = p;
+ paca[cpu].kstack = (unsigned long) task_thread_info(p)
+ + THREAD_SIZE - STACK_FRAME_OVERHEAD;
#endif
current_set[cpu] = task_thread_info(p);
task_thread_info(p)->cpu = cpu;
diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index 906daed..b2c43d0 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -44,13 +44,13 @@ static void slb_allocate(unsigned long ea)
slb_allocate_realmode(ea);
}
+#define slb_esid_mask(ssize) \
+ (((ssize) == MMU_SEGSIZE_256M)? ESID_MASK: ESID_MASK_1T)
+
static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
unsigned long slot)
{
- unsigned long mask;
-
- mask = (ssize == MMU_SEGSIZE_256M)? ESID_MASK: ESID_MASK_1T;
- return (ea & mask) | SLB_ESID_V | slot;
+ return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
}
#define slb_vsid_shift(ssize) \
@@ -301,11 +301,16 @@ void slb_initialize(void)
create_shadowed_slbe(VMALLOC_START, mmu_kernel_ssize, vflags, 1);
+ /* For the boot cpu, we're running on the stack in init_thread_union,
+* which is in the first segment of the linear mapping, and also
+* get_paca()->kstack hasn't been initialized yet.
+* For secondary cpus, we need to bolt the kernel stack entry now.
+*/
slb_shadow_clear(2);
+ if (raw_smp_processor_id() != boot_cpuid &&
+ (get_paca()->kstack & slb_esid_mask(mmu_kernel_ssize)) >
PAGE_OFFSET)
+ create_shadowed_slbe(get_paca()->kstack,
+mmu_kernel_ssize, lflags, 2);
- /* We don't bolt the stack for the time being - we're in boot,
-* so the stack is in the bolted segment. By the time it goes
-* elsewhere, we'll call _switch() which will bolt in the new
-* one. */
asm volatile("isync":::"memory");
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[git pull] Please pull powerpc.git master branch
Linus, Please do: git pull \ git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git master With the exception of one commit from Grant Likely, these are all fixes for various bugs and compile problems, or documentation updates. The one from Grant is something that has been around for a month or so and only affects MPC5200-based embedded platforms. Thanks, Paul. .../powerpc/mpc52xx-device-tree-bindings.txt | 11 ++ arch/powerpc/boot/dts/mpc8610_hpcd.dts | 23 +++ arch/powerpc/configs/ps3_defconfig | 132 +--- arch/powerpc/kernel/smp.c |2 arch/powerpc/mm/slb.c | 27 ++-- arch/powerpc/platforms/ps3/interrupt.c |6 - arch/powerpc/sysdev/fsl_rio.c |9 + arch/powerpc/sysdev/fsl_soc.c |4 - arch/powerpc/sysdev/xilinx_intc.c |2 drivers/char/xilinx_hwicap/xilinx_hwicap.c |6 - drivers/net/fec_mpc52xx.c | 97 +++ drivers/net/fec_mpc52xx.h | 19 --- drivers/ps3/ps3-lpm.c |1 drivers/ps3/ps3-sys-manager.c |7 - drivers/serial/mpc52xx_uart.c |2 include/asm-powerpc/ps3.h |3 include/linux/rio.h|2 17 files changed, 231 insertions(+), 122 deletions(-) Andrew Liu (1): Fix a potential issue in mpc52xx uart driver Anton Vorontsov (1): [POWERPC] 86xx: mpc8610_hpcd: add support for PCI Express x8 slot Becky Bruce (1): [POWERPC] Squash build warning for print of resource_size_t in fsl_soc.c FUJITA Tomonori (1): [POWERPC] PS3: Add time include to lpm Geert Uytterhoeven (1): [POWERPC] PS3: Make ps3_virq_setup and ps3_virq_destroy static Geoff Levand (3): [POWERPC] Fix slb.c compile warnings [POWERPC] PS3: Remove unsupported wakeup sources [POWERPC] PS3: Update ps3_defconfig Grant Likely (1): [POWERPC] mpc5200: Allow for fixed speed MII configurations Jason Jin (1): [POWERPC] 86xx: Fix the wrong serial1 interrupt for 8610 board Kumar Gala (1): [POWERPC] Xilinx: Fix compile warnings Paul Mackerras (1): [POWERPC] Bolt in SLB entry for kernel stack on secondary cpus Randy Dunlap (1): [RAPIDIO] fix current kernel-doc notation ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [POWERPC][v2] Bolt in SLB entry for kernel stack on secondary cpus
On Fri, May 02, 2008 at 02:29:12PM +1000, Paul Mackerras wrote: > This fixes a regression reported by Kamalesh Bulabel where a POWER4 > machine would crash because of an SLB miss at a point where the SLB > miss exception was unrecoverable. This regression is tracked at: > > http://bugzilla.kernel.org/show_bug.cgi?id=10082 > > SLB misses at such points shouldn't happen because the kernel stack is > the only memory accessed other than things in the first segment of the > linear mapping (which is mapped at all times by entry 0 of the SLB). > The context switch code ensures that SLB entry 2 covers the kernel > stack, if it is not already covered by entry 0. None of entries 0 > to 2 are ever replaced by the SLB miss handler. > > Where this went wrong is that the context switch code assumes it > doesn't have to write to SLB entry 2 if the new kernel stack is in the > same segment as the old kernel stack, since entry 2 should already be > correct. However, when we start up a secondary cpu, it calls > slb_initialize, which doesn't set up entry 2. This is correct for > the boot cpu, where we will be using a stack in the kernel BSS at this > point (i.e. init_thread_union), but not necessarily for secondary > cpus, whose initial stack can be allocated anywhere. This doesn't > cause any immediate problem since the SLB miss handler will just > create an SLB entry somewhere else to cover the initial stack. > > In fact it's possible for the cpu to go quite a long time without SLB > entry 2 being valid. Eventually, though, the entry created by the SLB > miss handler will get overwritten by some other entry, and if the next > access to the stack is at an unrecoverable point, we get the crash. > > This fixes the problem by making slb_initialize create a suitable > entry for the kernel stack, if we are on a secondary cpu and the stack > isn't covered by SLB entry 0. This requires initializing the > get_paca()->kstack field earlier, so I do that in smp_create_idle > where the current field is initialized. This also abstracts a bit of > the computation that mk_esid_data in slb.c does so that it can be used > in slb_initialize. > > Signed-off-by: Paul Mackerras <[EMAIL PROTECTED]> > --- > Michael Ellerman pointed out that I should be comparing > raw_smp_processor_id() with boot_cpuid rather than with 0. Do you even need the processor ID test at all? The boot processor should always have its stack covered by SLB entry 0 when we come through here, shouldn't it? -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [git pull] Please pull powerpc.git master branch
Paul, Any plans to start applying some patches to powerpc-next (for .27)? For example, your NAP patch? the devres_ioremap_prot patch. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc ptrace block-step
I didn't test the booke variant of this (I don't have that hardware).
I only tested it on a Mac G5 (ppc64, 970FX).
There is a test program at:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/block-step.c?cvsroot=systemtap
To build that program for powerpc, remove the && 0 from this line:
# elif defined __powerpc__ && 0/* XXX not upstream yet */
and compile with -D_GNU_SOURCE.
Thanks,
Roland
---
[PATCH] powerpc ptrace block-step
This adds block-step support on powerpc,
including a PTRACE_SINGLEBLOCK request for ptrace.
Signed-off-by: Roland McGrath <[EMAIL PROTECTED]>
---
arch/powerpc/kernel/ptrace.c | 19 ++-
include/asm-powerpc/ptrace.h |4
2 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 2a9fe97..91ee077 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -619,12 +619,29 @@ void user_enable_single_step(struct task_struct *task)
task->thread.dbcr0 = DBCR0_IDM | DBCR0_IC;
regs->msr |= MSR_DE;
#else
+ regs->msr &= ~MSR_BE;
regs->msr |= MSR_SE;
#endif
}
set_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void user_enable_block_step(struct task_struct *task)
+{
+ struct pt_regs *regs = task->thread.regs;
+
+ if (regs != NULL) {
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ task->thread.dbcr0 = DBCR0_IDM | DBCR0_BT;
+ regs->msr |= MSR_DE;
+#else
+ regs->msr &= ~MSR_SE;
+ regs->msr |= MSR_BE;
+#endif
+ }
+ set_tsk_thread_flag(task, TIF_SINGLESTEP);
+}
+
void user_disable_single_step(struct task_struct *task)
{
struct pt_regs *regs = task->thread.regs;
@@ -634,7 +651,7 @@ void user_disable_single_step(struct task_struct *task)
task->thread.dbcr0 = 0;
regs->msr &= ~MSR_DE;
#else
- regs->msr &= ~MSR_SE;
+ regs->msr &= ~(MSR_SE | MSR_BE);
#endif
}
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
diff --git a/include/asm-powerpc/ptrace.h b/include/asm-powerpc/ptrace.h
index 39023dd..6a3892d 100644
--- a/include/asm-powerpc/ptrace.h
+++ b/include/asm-powerpc/ptrace.h
@@ -135,7 +135,9 @@ do {
\
* These are defined as per linux/ptrace.h, which see.
*/
#define arch_has_single_step() (1)
+#define arch_has_block_step() (1)
extern void user_enable_single_step(struct task_struct *);
+extern void user_enable_block_step(struct task_struct *);
extern void user_disable_single_step(struct task_struct *);
#endif /* __ASSEMBLY__ */
@@ -276,4 +278,6 @@ extern void user_disable_single_step(struct task_struct *);
#define PPC_PTRACE_PEEKUSR_3264 0x91
#define PPC_PTRACE_POKEUSR_3264 0x90
+#define PTRACE_SINGLEBLOCK 0x100 /* resume execution until next branch */
+
#endif /* _ASM_POWERPC_PTRACE_H */
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
