* [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <1322783258-20443-1-git-send-email-paul.gortmaker@windriver.com>
There is a struct uart_8250_port that is private to 8250.c
itself, and it had a bugs field. But there is no reason to
assume that hardware bugs are unique to 8250 type UARTS.
Make the bugs field part of the globally visible struct
uart_port and remove the 8250 specific one.
The value in doing so is that it helps pave the way for
allowing arch or platform specific code to pass in information
to the specific uart drivers about uart bugs known to impact
certain platforms that would otherwise be hard to detect from
within the context of the driver itself.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/tty/serial/8250.c | 25 ++++++++++++-------------
include/linux/serial_core.h | 1 +
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index eeadf1b..7c94dbc 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -134,7 +134,6 @@ struct uart_8250_port {
struct timer_list timer; /* "no irq" timer */
struct list_head list; /* ports on this IRQ */
unsigned short capabilities; /* port capabilities */
- unsigned short bugs; /* port bugs */
unsigned int tx_loadsz; /* transmit fifo load size */
unsigned char acr;
unsigned char ier;
@@ -828,7 +827,7 @@ static void autoconfig_has_efr(struct uart_8250_port *up)
* when DLL is 0.
*/
if (id3 == 0x52 && rev == 0x01)
- up->bugs |= UART_BUG_QUOT;
+ up->port.bugs |= UART_BUG_QUOT;
return;
}
@@ -1102,7 +1101,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
spin_lock_irqsave(&up->port.lock, flags);
up->capabilities = 0;
- up->bugs = 0;
+ up->port.bugs = 0;
if (!(up->port.flags & UPF_BUGGY_UART)) {
/*
@@ -1337,7 +1336,7 @@ static void serial8250_start_tx(struct uart_port *port)
up->ier |= UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
- if (up->bugs & UART_BUG_TXEN) {
+ if (port->bugs & UART_BUG_TXEN) {
unsigned char lsr;
lsr = serial_in(up, UART_LSR);
up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
@@ -1373,7 +1372,7 @@ static void serial8250_enable_ms(struct uart_port *port)
container_of(port, struct uart_8250_port, port);
/* no MSR capabilities */
- if (up->bugs & UART_BUG_NOMSR)
+ if (port->bugs & UART_BUG_NOMSR)
return;
up->ier |= UART_IER_MSI;
@@ -2089,7 +2088,7 @@ static int serial8250_startup(struct uart_port *port)
* kick the UART on a regular basis.
*/
if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
- up->bugs |= UART_BUG_THRE;
+ port->bugs |= UART_BUG_THRE;
pr_debug("ttyS%d - using backup timer\n",
serial_index(port));
}
@@ -2099,7 +2098,7 @@ static int serial8250_startup(struct uart_port *port)
* The above check will only give an accurate result the first time
* the port is opened so this value needs to be preserved.
*/
- if (up->bugs & UART_BUG_THRE) {
+ if (port->bugs & UART_BUG_THRE) {
up->timer.function = serial8250_backup_timeout;
up->timer.data = (unsigned long)up;
mod_timer(&up->timer, jiffies +
@@ -2162,13 +2161,13 @@ static int serial8250_startup(struct uart_port *port)
serial_outp(up, UART_IER, 0);
if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
- if (!(up->bugs & UART_BUG_TXEN)) {
- up->bugs |= UART_BUG_TXEN;
+ if (!(port->bugs & UART_BUG_TXEN)) {
+ port->bugs |= UART_BUG_TXEN;
pr_debug("ttyS%d - enabling bad tx status workarounds\n",
serial_index(port));
}
} else {
- up->bugs &= ~UART_BUG_TXEN;
+ port->bugs &= ~UART_BUG_TXEN;
}
dont_test_tx_en:
@@ -2323,7 +2322,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
/*
* Oxford Semi 952 rev B workaround
*/
- if (up->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
+ if (port->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
quot++;
if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
@@ -2390,7 +2389,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
* CTS flow control flag and modem status interrupts
*/
up->ier &= ~UART_IER_MSI;
- if (!(up->bugs & UART_BUG_NOMSR) &&
+ if (!(port->bugs & UART_BUG_NOMSR) &&
UART_ENABLE_MS(&up->port, termios->c_cflag))
up->ier |= UART_IER_MSI;
if (up->capabilities & UART_CAP_UUE)
@@ -2666,7 +2665,7 @@ static void serial8250_config_port(struct uart_port *port, int flags)
/* if access method is AU, it is a 16550 with a quirk */
if (up->port.type == PORT_16550A && up->port.iotype == UPIO_AU)
- up->bugs |= UART_BUG_NOMSR;
+ port->bugs |= UART_BUG_NOMSR;
if (up->port.type != PORT_UNKNOWN && flags & UART_CONFIG_IRQ)
autoconfig_irq(up);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index eadf33d..791536c 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -323,6 +323,7 @@ struct uart_port {
unsigned int read_status_mask; /* driver specific */
unsigned int ignore_status_mask; /* driver specific */
+ unsigned short bugs; /* driver specific */
struct uart_state *state; /* pointer to parent state */
struct uart_icount icount; /* statistics */
--
1.7.7
^ permalink raw reply related
* [PATCH 0/3] RFC Fix Fsl 8250 BRK bug via letting plat code set bugs
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
This is a respin of an earlier patch[1] that enabled a workaround
via Kconfig for an errata issue when using BRK on FSL 16550 UARTs.
In this version, the 8250 specific bug field is moved to the generic
uart_port struct, since hardware bugs aren't the unique domain of
just the 8250 class of uarts.
Then the ability to set a known bugs value via the platform_device entry
point is added, so that it can be set externally vs. requiring some sort
of autodetection from the actual driver(s).
Then, the FSL errata fix is added to 8250.c by using the above support.
An entry in a DTS file is used to flag boards/platforms with the issue.
It is also added in a way that is optimized away for other architectures.
Kumar has a patch pending[2] that updates all the dts files. I've not
included it here, so that review focus can be on the implementation.
I've re-tested it on an sbc8641D, where sysrq was useless before; with
this fix, it works as expected.
Thanks,
Paul.
[1] http://patchwork.ozlabs.org/patch/46609/
[2] http://patchwork.ozlabs.org/patch/128070/
--------
Paul Gortmaker (3):
serial: make bugs field not specific to 8250 type uarts.
serial: allow passing in hardware bug info via platform device
8250: add workaround for MPC8[356]xx UART break IRQ storm
arch/powerpc/kernel/legacy_serial.c | 11 ++++++++++
drivers/tty/serial/8250.c | 37 +++++++++++++++++++++-------------
drivers/tty/serial/8250.h | 5 ----
include/linux/serial_8250.h | 11 ++++++++++
include/linux/serial_core.h | 1 +
5 files changed, 46 insertions(+), 19 deletions(-)
--
1.7.7
^ permalink raw reply
* [PATCH 2/3] serial: allow passing in hardware bug info via platform device
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <1322783258-20443-1-git-send-email-paul.gortmaker@windriver.com>
The normal arch hook into the 8250 serial world is via passing
in a plat_serial8250_port struct. However, this struct does
not have a bugs field, so there is no way to have the arch
code pass in info about known uart issues.
Add a bug field to the plat_serial8250_port struct, so that the
arch can pass in this information. Also don't do a blanket
overwrite of the bugs setting in the 8250.c driver. Finally,
relocate the known bug #define list to a globally visible header
so that the arch can assign any appropriate values from the list.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/tty/serial/8250.c | 3 ++-
drivers/tty/serial/8250.h | 5 -----
include/linux/serial_8250.h | 6 ++++++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 7c94dbc..f99f27c 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -1101,7 +1101,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
spin_lock_irqsave(&up->port.lock, flags);
up->capabilities = 0;
- up->port.bugs = 0;
if (!(up->port.flags & UPF_BUGGY_UART)) {
/*
@@ -3075,6 +3074,7 @@ static int __devinit serial8250_probe(struct platform_device *dev)
port.serial_out = p->serial_out;
port.handle_irq = p->handle_irq;
port.set_termios = p->set_termios;
+ port.bugs = p->bugs;
port.pm = p->pm;
port.dev = &dev->dev;
port.irqflags |= irqflag;
@@ -3225,6 +3225,7 @@ int serial8250_register_port(struct uart_port *port)
uart->port.regshift = port->regshift;
uart->port.iotype = port->iotype;
uart->port.flags = port->flags | UPF_BOOT_AUTOCONF;
+ uart->port.bugs = port->bugs;
uart->port.mapbase = port->mapbase;
uart->port.private_data = port->private_data;
if (port->dev)
diff --git a/drivers/tty/serial/8250.h b/drivers/tty/serial/8250.h
index 6edf4a6..caefe00 100644
--- a/drivers/tty/serial/8250.h
+++ b/drivers/tty/serial/8250.h
@@ -44,11 +44,6 @@ struct serial8250_config {
#define UART_CAP_UUE (1 << 12) /* UART needs IER bit 6 set (Xscale) */
#define UART_CAP_RTOIE (1 << 13) /* UART needs IER bit 4 set (Xscale, Tegra) */
-#define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */
-#define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */
-#define UART_BUG_NOMSR (1 << 2) /* UART has buggy MSR status bits (Au1x00) */
-#define UART_BUG_THRE (1 << 3) /* UART has buggy THRE reassertion */
-
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 1f05bbe..8c660af 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -14,6 +14,11 @@
#include <linux/serial_core.h>
#include <linux/platform_device.h>
+#define UART_BUG_QUOT (1 << 0) /* buggy quot LSB */
+#define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status */
+#define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits (Au1x00) */
+#define UART_BUG_THRE (1 << 3) /* buggy THRE reassertion */
+
/*
* This is the platform device platform_data structure
*/
@@ -30,6 +35,7 @@ struct plat_serial8250_port {
unsigned char hub6;
upf_t flags; /* UPF_* flags */
unsigned int type; /* If UPF_FIXED_TYPE */
+ unsigned short bugs; /* hardware specific bugs */
unsigned int (*serial_in)(struct uart_port *, int);
void (*serial_out)(struct uart_port *, int, int);
void (*set_termios)(struct uart_port *,
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 5/6] powerpc/boot: Add mfdcrx
From: Segher Boessenkool @ 2011-12-01 22:55 UTC (permalink / raw)
To: David Laight; +Cc: LinuxPPC-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AEF3@saturn3.aculab.com>
>>> +#define mfdcrx(rn) \
>>> + ({ \
>>> + unsigned long rval; \
>>> + asm volatile("mfdcrx %0,%1" : "=r"(rval) : "g"(rn)); \
>>> + rval; \
>>> + })
>>
>> "g" is never correct on PowerPC, you want "r" here. You can write
>> this as a static inline btw, you only need the #define stuff when
>> there is an "i" constraint involved.
>
> I think you can still use static inlines even when a
> constraint is one that requires a compile time constant.
Marking a function inline does not guarantee the compiler will
perform inline substitution on it, which you need here. I don't
think it's a problem with recent GCC in the context of the Linux
kernel though, if you use __always_inline that is.
Segher
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 22:10 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7FA38.6030606@freescale.com>
Scott Wood wrote:
>
> Does of_find_i2c_device_by_node() do what you want?
Yeah, that'll work. I wasn't thinking about looking for an i2c device. Thanks.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Scott Wood @ 2011-12-01 22:05 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F8CE.10801@freescale.com>
On 12/01/2011 03:59 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> Ideally we would have a field in struct device_node that points to
>> struct device.
>
> I don't think there's a single place in the code where the connection
> between the device and device_node is made. In fact, I think
> dev->of_node needs to be manually initialized by the driver during
> the OF probe.
The i2c core does this.
Does of_find_i2c_device_by_node() do what you want?
-Scott
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 21:59 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F72F.7050405@freescale.com>
Scott Wood wrote:
> Ideally we would have a field in struct device_node that points to
> struct device.
I don't think there's a single place in the code where the connection between the device and device_node is made. In fact, I think dev->of_node needs to be manually initialized by the driver during the OF probe.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 21:59 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, kumar.gala
In-Reply-To: <CACxGe6sZgG5q6NKgsGaWLyTzWHXNGqHke9nQMyqzw0audS3=tw@mail.gmail.com>
Grant Likely wrote:
> It is better to walk the list of i2c adapters and look for one that
> has the matching node pointer. It really isn't an expensive operation
> to do it that way.
That's what I was thinking. I can't figure out how to walk the list, though. i2c_get_adapter() takes an adapter number, but I don't think this is going to work:
unsigned int i = 0;
struct i2c_adapter adap = i2c_get_adapter(0);
while (adap) {
if (adap->nr == nr)
break;
adap = i2c_get_adapter(++i);
}
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Grant Likely @ 2011-12-01 21:55 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala, Timur Tabi
In-Reply-To: <4ED7F72F.7050405@freescale.com>
On Thu, Dec 1, 2011 at 2:52 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/01/2011 03:46 PM, Timur Tabi wrote:
>> Scott Wood wrote:
>>
>>> How is this going to interact with other i2c buses (e.g. on a board
>>> FPGA) that might have a conflicting static numbering scheme? =A0Have yo=
u
>>> ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
>>> device) can happen before the static SoC i2c buses are added?
>>
>> Hmm.... You have a point there.
>>
>>>> An alternative approach is to create a function like this:
>>>>
>>>> =A0 =A0 struct i2c_adapter *i2c_adapter_from_node(struct device_node *=
np);
>>>>
>>>> I could then just use adap->nr directly.
>>>
>>> If there isn't a way to get a "struct device" from "struct device_node"=
,
>>> we should add it.
>>
>> How do I do that? =A0Scan all the struct devices until I find one where =
dev->of_node =3D=3D np? =A0That seems really inefficient.
>
> Ideally we would have a field in struct device_node that points to
> struct device.
No. There are cases where multiple devices may reference the same node.
It is better to walk the list of i2c adapters and look for one that
has the matching node pointer. It really isn't an expensive operation
to do it that way.
g.
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Scott Wood @ 2011-12-01 21:52 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F5AA.2040909@freescale.com>
On 12/01/2011 03:46 PM, Timur Tabi wrote:
> Scott Wood wrote:
>
>> How is this going to interact with other i2c buses (e.g. on a board
>> FPGA) that might have a conflicting static numbering scheme? Have you
>> ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
>> device) can happen before the static SoC i2c buses are added?
>
> Hmm.... You have a point there.
>
>>> An alternative approach is to create a function like this:
>>>
>>> struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
>>>
>>> I could then just use adap->nr directly.
>>
>> If there isn't a way to get a "struct device" from "struct device_node",
>> we should add it.
>
> How do I do that? Scan all the struct devices until I find one where dev->of_node == np? That seems really inefficient.
Ideally we would have a field in struct device_node that points to
struct device.
-Scott
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 21:46 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F1AE.70806@freescale.com>
Scott Wood wrote:
> How is this going to interact with other i2c buses (e.g. on a board
> FPGA) that might have a conflicting static numbering scheme? Have you
> ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
> device) can happen before the static SoC i2c buses are added?
Hmm.... You have a point there.
>> An alternative approach is to create a function like this:
>>
>> struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
>>
>> I could then just use adap->nr directly.
>
> If there isn't a way to get a "struct device" from "struct device_node",
> we should add it.
How do I do that? Scan all the struct devices until I find one where dev->of_node == np? That seems really inefficient.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: ppc4xx simple vs SoC's
From: Benjamin Herrenschmidt @ 2011-12-01 21:39 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <CA+5PVA5xaCa8Tv3MMr1WzQAuLit9KenMoezOKB7ENQSt=fOSOg@mail.gmail.com>
On Thu, 2011-12-01 at 06:44 -0500, Josh Boyer wrote:
> > However, the entry for ppc44x_simple doesn't select any of these,
> > meaning for example, AFAIK, that you don't get EMAC4 etc... I'm
> > surprised things work at all !
> >
> > What am I missing ?
>
> CONFIG_PPC44x_SIMPLE is selected by the board Kconfig values, which
> also select all of the SoC Kconfig options they need as well.
> ppc44x_simple started as a way to basically avoid duplicating a bunch
> of board.c files, but it's still "driven" from the board options.
>
> So hopefully that explains how things are working today. The question
> now is whether you would like something different?
Ok, just getting my head around that stuff, I haven't really looked
since you and Grant I believe came up with the scheme :-) (Was wondering
what to do for currituck, but for now we'll keep it a separate board, at
least until I can decide how to deal with the interrupt quirk).
I just hadn't realized that while we had ppc4xx_simple, we still had
board Kconfig's to enable it.
Cheers,
Ben.
^ permalink raw reply
* Re: [BUG?]3.0-rc4+ftrace+kprobe: set kprobe at instruction 'stwu' lead to system crash/freeze
From: Benjamin Herrenschmidt @ 2011-12-01 21:37 UTC (permalink / raw)
To: tiejun.chen
Cc: Jim Keniston, Anton Blanchard, linux-kernel, Steven Rostedt,
Yong Zhang, paulus, yrl.pp-manager.tt, Masami Hiramatsu,
linuxppc-dev
In-Reply-To: <4ED75A9A.1030100@windriver.com>
On Thu, 2011-12-01 at 18:44 +0800, tiejun.chen wrote:
> Do you mean we should push the original pt_regs (or that whole exception stack)
> downwards the location the new r1 point? Then its safe to perform this real
> emulated stw instruction. At last we will reroute r1 to that copied exception
> frame to restore as normal. Right?
Right. That way we don't have to modify the (sensitive) restore path, we
only hook around the do_work branch which is a lot easier and less
risky.
> Here I suppose so, I implement this for PPC32 based on the above understanding.
> I take a validation for kprobing do_fork()/show_interrupts(), now looks fine.
> Tomorrow I will go PPC64, and hope its fine as well.
>
> If everything is good I'll send these patches to linuxppc-dev next week.
Excellent, thanks !
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Scott Wood @ 2011-12-01 21:29 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7E975.6090704@freescale.com>
On 12/01/2011 02:54 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> On 12/01/2011 11:33 AM, Timur Tabi wrote:
>>> An I2C device tree node can contain a 'cell-index' property that can be
>>> used to enumerate the I2C devices. If such a property exists, use it
>>> to specify the I2C adapter number.
>>
>> Didn't we decide a long time ago that this was a bad idea?
>
> I don't see what's wrong with it.
Obviously, since you're suggesting doing it. :-P
Did you search for the old discussions?
How is this going to interact with other i2c buses (e.g. on a board
FPGA) that might have a conflicting static numbering scheme? Have you
ensured that no dynamic bus registrations (e.g. an i2c bus on a PCI
device) can happen before the static SoC i2c buses are added?
Global numberspaces of this sort are usually the wrong answer. Look at
the mess it made with IRQ numbers, and the gymnastics we go through to
virtualize it.
>>> This feature is necessary for the Freescale PowerPC audio drivers (e.g.
>>> on the P1022DS). The "machine driver" needs to know the adapter number
>>> for each I2C adapter, but it only has access to the device tree.
>>> Previously, the I2C nodes always appeared in cell-index order, so the
>>> dynamic numbering coincided with the cell-index property. With commit
>>> ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
>>> unintentionally reversed in the device tree, and so the machine driver
>>> guesses the wrong I2C adapter number.
>>
>> What specifically do you need this number for? What does it represent?
>
> Well, I thought the above paragraph explained it pretty well.
All you said was "needs to know the adapter number", nothing about why.
> Audio drivers come in sets of four -- machine, ssi, dma, and codec.
> The machine driver needs to know which ssi is connected to which
> codec and which DMA channel(s) it needs to use. So I extract all
> this information from the device tree.
>
> The individual drivers, however, register only with their own
> subsystem. So the codec driver, for example, doesn't know anything
> about device trees -- it just registers as an i2c driver. That means
> that the machine driver has to guess what name the codec driver will
> use when it registers. In this case, that's the i2c device name,
> which include the I2C adapter number (adap->nr). That means that
> given a device tree node, I need to know what the actual bus number
> is.
>
> An alternative approach is to create a function like this:
>
> struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
>
> I could then just use adap->nr directly.
If there isn't a way to get a "struct device" from "struct device_node",
we should add it. From that you should be able to look up the sysfs
name -- no need to mess around with adap->nr as an intermediary.
-Scott
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 20:54 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7CA64.2080503@freescale.com>
Scott Wood wrote:
> On 12/01/2011 11:33 AM, Timur Tabi wrote:
>> An I2C device tree node can contain a 'cell-index' property that can be
>> used to enumerate the I2C devices. If such a property exists, use it
>> to specify the I2C adapter number.
>
> Didn't we decide a long time ago that this was a bad idea?
I don't see what's wrong with it.
>> This feature is necessary for the Freescale PowerPC audio drivers (e.g.
>> on the P1022DS). The "machine driver" needs to know the adapter number
>> for each I2C adapter, but it only has access to the device tree.
>> Previously, the I2C nodes always appeared in cell-index order, so the
>> dynamic numbering coincided with the cell-index property. With commit
>> ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
>> unintentionally reversed in the device tree, and so the machine driver
>> guesses the wrong I2C adapter number.
>
> What specifically do you need this number for? What does it represent?
Well, I thought the above paragraph explained it pretty well.
Audio drivers come in sets of four -- machine, ssi, dma, and codec. The machine driver needs to know which ssi is connected to which codec and which DMA channel(s) it needs to use. So I extract all this information from the device tree.
The individual drivers, however, register only with their own subsystem. So the codec driver, for example, doesn't know anything about device trees -- it just registers as an i2c driver. That means that the machine driver has to guess what name the codec driver will use when it registers. In this case, that's the i2c device name, which include the I2C adapter number (adap->nr). That means that given a device tree node, I need to know what the actual bus number is.
An alternative approach is to create a function like this:
struct i2c_adapter *i2c_adapter_from_node(struct device_node *np);
I could then just use adap->nr directly.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Jean-Michel Hautbois @ 2011-12-01 20:09 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Tabi Timur-B04825, linux-kernel
In-Reply-To: <40F38A2B-48A0-4A8C-B45D-579865F135CC@kernel.crashing.org>
2011/12/1 Kumar Gala <galak@kernel.crashing.org>:
>
> On Dec 1, 2011, at 11:52 AM, Jean-Michel Hautbois wrote:
>
>> 2011/12/1 Jean-Michel Hautbois <jhautbois@gmail.com>:
>>> 2011/12/1 Tabi Timur-B04825 <B04825@freescale.com>:
>>>> On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois
>>>> <jhautbois@gmail.com> wrote:
>>>>>
>>>>> Any idea on how to debug this ? I am using a 2.6.35 kernel.
>>>>
>>>> There are a ton of Kconfig options for debugging various locking bugs.
>>>> =C2=A0Try turning them on.
>>>>
>>
>> All Kconfig options do not help me with cache coherency problems, though=
.
>> Or, I didn't find any option related.
>> Any idea ?
>>
>> JM
>
> Nope, if you can't describe the code in any more detail we can't help.
>
Here is what I can tell, I hope this will help you.
I have a userland process, which reads and writes data to a character
device thanks to ioctl.
Reads and Writes are performed with this scheme :
Lock Access -> ioctl -> down_interruptible(semaphore)
Read (or Write) -> ioctl -> ioread32
Unlock Access -> ioctl -> up(semaphore)
It reads (or writes) 4 bytes by 4 bytes, in a loop in the process
which will eventually get several bytes from the PCIe device.
The read (write) ioctl is really simple (and in fact, may be rewritten) :
- kmalloc a structure, then copy_from_user
- ioread32 using adress and offset from the structure copied previously
- copy_to_user and free the structure.
The userland process is currently doing a loop in order to read or
writes with a 4bytes step.
I am neither the writer of the driver nor the userland process, this
is why I can't give you the code.
Here is what I would do, at least :
1/ rewrite the read/write part of the ioctl in order to user a per_cpu
structure and not a "dynamically allocated at each call" structure
2/ rewrite the userland part in order to ask the driver for n bytes,
leaving the loop on ioread32/iowrite32 inside the ioctl
Thanks for your help.
JM
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Kumar Gala @ 2011-12-01 19:35 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: linuxppc-dev, Tabi Timur-B04825, linux-kernel
In-Reply-To: <CAL8zT=jgZS2=t0SadUXXshabKPsxHJe_frkfOw5XyF88k3jR=g@mail.gmail.com>
On Dec 1, 2011, at 11:52 AM, Jean-Michel Hautbois wrote:
> 2011/12/1 Jean-Michel Hautbois <jhautbois@gmail.com>:
>> 2011/12/1 Tabi Timur-B04825 <B04825@freescale.com>:
>>> On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois
>>> <jhautbois@gmail.com> wrote:
>>>>
>>>> Any idea on how to debug this ? I am using a 2.6.35 kernel.
>>>
>>> There are a ton of Kconfig options for debugging various locking bugs.
>>> Try turning them on.
>>>
>
> All Kconfig options do not help me with cache coherency problems, though.
> Or, I didn't find any option related.
> Any idea ?
>
> JM
Nope, if you can't describe the code in any more detail we can't help.
- k
^ permalink raw reply
* [PATCH 6/8 v2] powerpc/ps3: Fix PS3 repository build warnings
From: Geoff Levand @ 2011-12-01 18:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: cbe-oss-dev, linuxppc-dev, Geert Uytterhoeven
In-Reply-To: <CAMuHMdUjOXLskrRXYYO1Esd75W4Z=WS3srT0+qFF66DfLeQQmA@mail.gmail.com>
Fix some PS3 repository.c build warnings when DEBUG is
defined. Also change most pr_debug calls to pr_devel calls.
Fixes warnings like these:
format '%lx' expects type 'long unsigned int', but argument 7 has type 'u64'
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
Hi Ben,
I rebased my for-powerpc branch to include this update.
-Geoff
v2: - Format u64 is using the "ll" length modifier (thanks Geert).
- Convert pr_debug to pr_devel.
arch/powerpc/platforms/ps3/repository.c | 135 ++++++++++++++++---------------
1 files changed, 68 insertions(+), 67 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
index cb68729..7bdfea3 100644
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -44,7 +44,7 @@ static void _dump_field(const char *hdr, u64 n, const char *func, int line)
s[i] = (in[i] <= 126 && in[i] >= 32) ? in[i] : '.';
s[i] = 0;
- pr_debug("%s:%d: %s%016llx : %s\n", func, line, hdr, n, s);
+ pr_devel("%s:%d: %s%016llx : %s\n", func, line, hdr, n, s);
#endif
}
@@ -53,7 +53,7 @@ static void _dump_field(const char *hdr, u64 n, const char *func, int line)
static void _dump_node_name(unsigned int lpar_id, u64 n1, u64 n2, u64 n3,
u64 n4, const char *func, int line)
{
- pr_debug("%s:%d: lpar: %u\n", func, line, lpar_id);
+ pr_devel("%s:%d: lpar: %u\n", func, line, lpar_id);
_dump_field("n1: ", n1, func, line);
_dump_field("n2: ", n2, func, line);
_dump_field("n3: ", n3, func, line);
@@ -65,13 +65,13 @@ static void _dump_node_name(unsigned int lpar_id, u64 n1, u64 n2, u64 n3,
static void _dump_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4,
u64 v1, u64 v2, const char *func, int line)
{
- pr_debug("%s:%d: lpar: %u\n", func, line, lpar_id);
+ pr_devel("%s:%d: lpar: %u\n", func, line, lpar_id);
_dump_field("n1: ", n1, func, line);
_dump_field("n2: ", n2, func, line);
_dump_field("n3: ", n3, func, line);
_dump_field("n4: ", n4, func, line);
- pr_debug("%s:%d: v1: %016llx\n", func, line, v1);
- pr_debug("%s:%d: v2: %016llx\n", func, line, v2);
+ pr_devel("%s:%d: v1: %016llx\n", func, line, v1);
+ pr_devel("%s:%d: v2: %016llx\n", func, line, v2);
}
/**
@@ -135,7 +135,7 @@ static int read_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4,
&v2);
if (result) {
- pr_debug("%s:%d: lv1_read_repository_node failed: %s\n",
+ pr_warn("%s:%d: lv1_read_repository_node failed: %s\n",
__func__, __LINE__, ps3_result(result));
dump_node_name(lpar_id, n1, n2, n3, n4);
return -ENOENT;
@@ -149,10 +149,10 @@ static int read_node(unsigned int lpar_id, u64 n1, u64 n2, u64 n3, u64 n4,
*_v2 = v2;
if (v1 && !_v1)
- pr_debug("%s:%d: warning: discarding non-zero v1: %016llx\n",
+ pr_devel("%s:%d: warning: discarding non-zero v1: %016llx\n",
__func__, __LINE__, v1);
if (v2 && !_v2)
- pr_debug("%s:%d: warning: discarding non-zero v2: %016llx\n",
+ pr_devel("%s:%d: warning: discarding non-zero v2: %016llx\n",
__func__, __LINE__, v2);
return 0;
@@ -323,16 +323,16 @@ int ps3_repository_find_device(struct ps3_repository_device *repo)
result = ps3_repository_read_bus_num_dev(tmp.bus_index, &num_dev);
if (result) {
- pr_debug("%s:%d read_bus_num_dev failed\n", __func__, __LINE__);
+ pr_devel("%s:%d read_bus_num_dev failed\n", __func__, __LINE__);
return result;
}
- pr_debug("%s:%d: bus_type %u, bus_index %u, bus_id %llu, num_dev %u\n",
+ pr_devel("%s:%d: bus_type %u, bus_index %u, bus_id %llu, num_dev %u\n",
__func__, __LINE__, tmp.bus_type, tmp.bus_index, tmp.bus_id,
num_dev);
if (tmp.dev_index >= num_dev) {
- pr_debug("%s:%d: no device found\n", __func__, __LINE__);
+ pr_devel("%s:%d: no device found\n", __func__, __LINE__);
return -ENODEV;
}
@@ -340,7 +340,7 @@ int ps3_repository_find_device(struct ps3_repository_device *repo)
&tmp.dev_type);
if (result) {
- pr_debug("%s:%d read_dev_type failed\n", __func__, __LINE__);
+ pr_devel("%s:%d read_dev_type failed\n", __func__, __LINE__);
return result;
}
@@ -348,12 +348,12 @@ int ps3_repository_find_device(struct ps3_repository_device *repo)
&tmp.dev_id);
if (result) {
- pr_debug("%s:%d ps3_repository_read_dev_id failed\n", __func__,
+ pr_devel("%s:%d ps3_repository_read_dev_id failed\n", __func__,
__LINE__);
return result;
}
- pr_debug("%s:%d: found: dev_type %u, dev_index %u, dev_id %llu\n",
+ pr_devel("%s:%d: found: dev_type %u, dev_index %u, dev_id %llu\n",
__func__, __LINE__, tmp.dev_type, tmp.dev_index, tmp.dev_id);
*repo = tmp;
@@ -367,14 +367,14 @@ int ps3_repository_find_device_by_id(struct ps3_repository_device *repo,
struct ps3_repository_device tmp;
unsigned int num_dev;
- pr_debug(" -> %s:%u: find device by id %llu:%llu\n", __func__, __LINE__,
+ pr_devel(" -> %s:%u: find device by id %llu:%llu\n", __func__, __LINE__,
bus_id, dev_id);
for (tmp.bus_index = 0; tmp.bus_index < 10; tmp.bus_index++) {
result = ps3_repository_read_bus_id(tmp.bus_index,
&tmp.bus_id);
if (result) {
- pr_debug("%s:%u read_bus_id(%u) failed\n", __func__,
+ pr_devel("%s:%u read_bus_id(%u) failed\n", __func__,
__LINE__, tmp.bus_index);
return result;
}
@@ -382,23 +382,23 @@ int ps3_repository_find_device_by_id(struct ps3_repository_device *repo,
if (tmp.bus_id == bus_id)
goto found_bus;
- pr_debug("%s:%u: skip, bus_id %llu\n", __func__, __LINE__,
+ pr_devel("%s:%u: skip, bus_id %llu\n", __func__, __LINE__,
tmp.bus_id);
}
- pr_debug(" <- %s:%u: bus not found\n", __func__, __LINE__);
+ pr_devel(" <- %s:%u: bus not found\n", __func__, __LINE__);
return result;
found_bus:
result = ps3_repository_read_bus_type(tmp.bus_index, &tmp.bus_type);
if (result) {
- pr_debug("%s:%u read_bus_type(%u) failed\n", __func__,
+ pr_devel("%s:%u read_bus_type(%u) failed\n", __func__,
__LINE__, tmp.bus_index);
return result;
}
result = ps3_repository_read_bus_num_dev(tmp.bus_index, &num_dev);
if (result) {
- pr_debug("%s:%u read_bus_num_dev failed\n", __func__,
+ pr_devel("%s:%u read_bus_num_dev failed\n", __func__,
__LINE__);
return result;
}
@@ -408,7 +408,7 @@ found_bus:
tmp.dev_index,
&tmp.dev_id);
if (result) {
- pr_debug("%s:%u read_dev_id(%u:%u) failed\n", __func__,
+ pr_devel("%s:%u read_dev_id(%u:%u) failed\n", __func__,
__LINE__, tmp.bus_index, tmp.dev_index);
return result;
}
@@ -416,21 +416,21 @@ found_bus:
if (tmp.dev_id == dev_id)
goto found_dev;
- pr_debug("%s:%u: skip, dev_id %llu\n", __func__, __LINE__,
+ pr_devel("%s:%u: skip, dev_id %llu\n", __func__, __LINE__,
tmp.dev_id);
}
- pr_debug(" <- %s:%u: dev not found\n", __func__, __LINE__);
+ pr_devel(" <- %s:%u: dev not found\n", __func__, __LINE__);
return result;
found_dev:
result = ps3_repository_read_dev_type(tmp.bus_index, tmp.dev_index,
&tmp.dev_type);
if (result) {
- pr_debug("%s:%u read_dev_type failed\n", __func__, __LINE__);
+ pr_devel("%s:%u read_dev_type failed\n", __func__, __LINE__);
return result;
}
- pr_debug(" <- %s:%u: found: type (%u:%u) index (%u:%u) id (%llu:%llu)\n",
+ pr_devel(" <- %s:%u: found: type (%u:%u) index (%u:%u) id (%llu:%llu)\n",
__func__, __LINE__, tmp.bus_type, tmp.dev_type, tmp.bus_index,
tmp.dev_index, tmp.bus_id, tmp.dev_id);
*repo = tmp;
@@ -443,18 +443,18 @@ int __devinit ps3_repository_find_devices(enum ps3_bus_type bus_type,
int result = 0;
struct ps3_repository_device repo;
- pr_debug(" -> %s:%d: find bus_type %u\n", __func__, __LINE__, bus_type);
+ pr_devel(" -> %s:%d: find bus_type %u\n", __func__, __LINE__, bus_type);
repo.bus_type = bus_type;
result = ps3_repository_find_bus(repo.bus_type, 0, &repo.bus_index);
if (result) {
- pr_debug(" <- %s:%u: bus not found\n", __func__, __LINE__);
+ pr_devel(" <- %s:%u: bus not found\n", __func__, __LINE__);
return result;
}
result = ps3_repository_read_bus_id(repo.bus_index, &repo.bus_id);
if (result) {
- pr_debug("%s:%d read_bus_id(%u) failed\n", __func__, __LINE__,
+ pr_devel("%s:%d read_bus_id(%u) failed\n", __func__, __LINE__,
repo.bus_index);
return result;
}
@@ -469,13 +469,13 @@ int __devinit ps3_repository_find_devices(enum ps3_bus_type bus_type,
result = callback(&repo);
if (result) {
- pr_debug("%s:%d: abort at callback\n", __func__,
+ pr_devel("%s:%d: abort at callback\n", __func__,
__LINE__);
break;
}
}
- pr_debug(" <- %s:%d\n", __func__, __LINE__);
+ pr_devel(" <- %s:%d\n", __func__, __LINE__);
return result;
}
@@ -489,7 +489,7 @@ int ps3_repository_find_bus(enum ps3_bus_type bus_type, unsigned int from,
for (i = from; i < 10; i++) {
error = ps3_repository_read_bus_type(i, &type);
if (error) {
- pr_debug("%s:%d read_bus_type failed\n",
+ pr_devel("%s:%d read_bus_type failed\n",
__func__, __LINE__);
*bus_index = UINT_MAX;
return error;
@@ -509,7 +509,7 @@ int ps3_repository_find_interrupt(const struct ps3_repository_device *repo,
int result = 0;
unsigned int res_index;
- pr_debug("%s:%d: find intr_type %u\n", __func__, __LINE__, intr_type);
+ pr_devel("%s:%d: find intr_type %u\n", __func__, __LINE__, intr_type);
*interrupt_id = UINT_MAX;
@@ -521,7 +521,7 @@ int ps3_repository_find_interrupt(const struct ps3_repository_device *repo,
repo->dev_index, res_index, &t, &id);
if (result) {
- pr_debug("%s:%d read_dev_intr failed\n",
+ pr_devel("%s:%d read_dev_intr failed\n",
__func__, __LINE__);
return result;
}
@@ -535,7 +535,7 @@ int ps3_repository_find_interrupt(const struct ps3_repository_device *repo,
if (res_index == 10)
return -ENODEV;
- pr_debug("%s:%d: found intr_type %u at res_index %u\n",
+ pr_devel("%s:%d: found intr_type %u at res_index %u\n",
__func__, __LINE__, intr_type, res_index);
return result;
@@ -547,7 +547,7 @@ int ps3_repository_find_reg(const struct ps3_repository_device *repo,
int result = 0;
unsigned int res_index;
- pr_debug("%s:%d: find reg_type %u\n", __func__, __LINE__, reg_type);
+ pr_devel("%s:%d: find reg_type %u\n", __func__, __LINE__, reg_type);
*bus_addr = *len = 0;
@@ -560,7 +560,7 @@ int ps3_repository_find_reg(const struct ps3_repository_device *repo,
repo->dev_index, res_index, &t, &a, &l);
if (result) {
- pr_debug("%s:%d read_dev_reg failed\n",
+ pr_devel("%s:%d read_dev_reg failed\n",
__func__, __LINE__);
return result;
}
@@ -575,7 +575,7 @@ int ps3_repository_find_reg(const struct ps3_repository_device *repo,
if (res_index == 10)
return -ENODEV;
- pr_debug("%s:%d: found reg_type %u at res_index %u\n",
+ pr_devel("%s:%d: found reg_type %u at res_index %u\n",
__func__, __LINE__, reg_type, res_index);
return result;
@@ -1009,7 +1009,7 @@ int ps3_repository_dump_resource_info(const struct ps3_repository_device *repo)
int result = 0;
unsigned int res_index;
- pr_debug(" -> %s:%d: (%u:%u)\n", __func__, __LINE__,
+ pr_devel(" -> %s:%d: (%u:%u)\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
for (res_index = 0; res_index < 10; res_index++) {
@@ -1021,13 +1021,13 @@ int ps3_repository_dump_resource_info(const struct ps3_repository_device *repo)
if (result) {
if (result != LV1_NO_ENTRY)
- pr_debug("%s:%d ps3_repository_read_dev_intr"
+ pr_devel("%s:%d ps3_repository_read_dev_intr"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
break;
}
- pr_debug("%s:%d (%u:%u) intr_type %u, interrupt_id %u\n",
+ pr_devel("%s:%d (%u:%u) intr_type %u, interrupt_id %u\n",
__func__, __LINE__, repo->bus_index, repo->dev_index,
intr_type, interrupt_id);
}
@@ -1042,18 +1042,18 @@ int ps3_repository_dump_resource_info(const struct ps3_repository_device *repo)
if (result) {
if (result != LV1_NO_ENTRY)
- pr_debug("%s:%d ps3_repository_read_dev_reg"
+ pr_devel("%s:%d ps3_repository_read_dev_reg"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
break;
}
- pr_debug("%s:%d (%u:%u) reg_type %u, bus_addr %lxh, len %lxh\n",
+ pr_devel("%s:%d (%u:%u) reg_type %u, bus_addr %llxh, len %llxh\n",
__func__, __LINE__, repo->bus_index, repo->dev_index,
reg_type, bus_addr, len);
}
- pr_debug(" <- %s:%d\n", __func__, __LINE__);
+ pr_devel(" <- %s:%d\n", __func__, __LINE__);
return result;
}
@@ -1063,22 +1063,22 @@ static int dump_stor_dev_info(struct ps3_repository_device *repo)
unsigned int num_regions, region_index;
u64 port, blk_size, num_blocks;
- pr_debug(" -> %s:%d: (%u:%u)\n", __func__, __LINE__,
+ pr_devel(" -> %s:%d: (%u:%u)\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
result = ps3_repository_read_stor_dev_info(repo->bus_index,
repo->dev_index, &port, &blk_size, &num_blocks, &num_regions);
if (result) {
- pr_debug("%s:%d ps3_repository_read_stor_dev_info"
+ pr_devel("%s:%d ps3_repository_read_stor_dev_info"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
goto out;
}
- pr_debug("%s:%d (%u:%u): port %lu, blk_size %lu, num_blocks "
- "%lu, num_regions %u\n",
- __func__, __LINE__, repo->bus_index, repo->dev_index, port,
- blk_size, num_blocks, num_regions);
+ pr_devel("%s:%d (%u:%u): port %llu, blk_size %llu, num_blocks "
+ "%llu, num_regions %u\n",
+ __func__, __LINE__, repo->bus_index, repo->dev_index,
+ port, blk_size, num_blocks, num_regions);
for (region_index = 0; region_index < num_regions; region_index++) {
unsigned int region_id;
@@ -1088,19 +1088,20 @@ static int dump_stor_dev_info(struct ps3_repository_device *repo)
repo->dev_index, region_index, ®ion_id,
®ion_start, ®ion_size);
if (result) {
- pr_debug("%s:%d ps3_repository_read_stor_dev_region"
+ pr_devel("%s:%d ps3_repository_read_stor_dev_region"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
break;
}
- pr_debug("%s:%d (%u:%u) region_id %u, start %lxh, size %lxh\n",
+ pr_devel("%s:%d (%u:%u) region_id %u, start %lxh, size %lxh\n",
__func__, __LINE__, repo->bus_index, repo->dev_index,
- region_id, region_start, region_size);
+ region_id, (unsigned long)region_start,
+ (unsigned long)region_size);
}
out:
- pr_debug(" <- %s:%d\n", __func__, __LINE__);
+ pr_devel(" <- %s:%d\n", __func__, __LINE__);
return result;
}
@@ -1109,7 +1110,7 @@ static int dump_device_info(struct ps3_repository_device *repo,
{
int result = 0;
- pr_debug(" -> %s:%d: bus_%u\n", __func__, __LINE__, repo->bus_index);
+ pr_devel(" -> %s:%d: bus_%u\n", __func__, __LINE__, repo->bus_index);
for (repo->dev_index = 0; repo->dev_index < num_dev;
repo->dev_index++) {
@@ -1118,7 +1119,7 @@ static int dump_device_info(struct ps3_repository_device *repo,
repo->dev_index, &repo->dev_type);
if (result) {
- pr_debug("%s:%d ps3_repository_read_dev_type"
+ pr_devel("%s:%d ps3_repository_read_dev_type"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
break;
@@ -1128,15 +1129,15 @@ static int dump_device_info(struct ps3_repository_device *repo,
repo->dev_index, &repo->dev_id);
if (result) {
- pr_debug("%s:%d ps3_repository_read_dev_id"
+ pr_devel("%s:%d ps3_repository_read_dev_id"
" (%u:%u) failed\n", __func__, __LINE__,
repo->bus_index, repo->dev_index);
continue;
}
- pr_debug("%s:%d (%u:%u): dev_type %u, dev_id %lu\n", __func__,
+ pr_devel("%s:%d (%u:%u): dev_type %u, dev_id %lu\n", __func__,
__LINE__, repo->bus_index, repo->dev_index,
- repo->dev_type, repo->dev_id);
+ repo->dev_type, (unsigned long)repo->dev_id);
ps3_repository_dump_resource_info(repo);
@@ -1144,7 +1145,7 @@ static int dump_device_info(struct ps3_repository_device *repo,
dump_stor_dev_info(repo);
}
- pr_debug(" <- %s:%d\n", __func__, __LINE__);
+ pr_devel(" <- %s:%d\n", __func__, __LINE__);
return result;
}
@@ -1153,7 +1154,7 @@ int ps3_repository_dump_bus_info(void)
int result = 0;
struct ps3_repository_device repo;
- pr_debug(" -> %s:%d\n", __func__, __LINE__);
+ pr_devel(" -> %s:%d\n", __func__, __LINE__);
memset(&repo, 0, sizeof(repo));
@@ -1164,7 +1165,7 @@ int ps3_repository_dump_bus_info(void)
&repo.bus_type);
if (result) {
- pr_debug("%s:%d read_bus_type(%u) failed\n",
+ pr_devel("%s:%d read_bus_type(%u) failed\n",
__func__, __LINE__, repo.bus_index);
break;
}
@@ -1173,32 +1174,32 @@ int ps3_repository_dump_bus_info(void)
&repo.bus_id);
if (result) {
- pr_debug("%s:%d read_bus_id(%u) failed\n",
+ pr_devel("%s:%d read_bus_id(%u) failed\n",
__func__, __LINE__, repo.bus_index);
continue;
}
if (repo.bus_index != repo.bus_id)
- pr_debug("%s:%d bus_index != bus_id\n",
+ pr_devel("%s:%d bus_index != bus_id\n",
__func__, __LINE__);
result = ps3_repository_read_bus_num_dev(repo.bus_index,
&num_dev);
if (result) {
- pr_debug("%s:%d read_bus_num_dev(%u) failed\n",
+ pr_devel("%s:%d read_bus_num_dev(%u) failed\n",
__func__, __LINE__, repo.bus_index);
continue;
}
- pr_debug("%s:%d bus_%u: bus_type %u, bus_id %lu, num_dev %u\n",
+ pr_devel("%s:%d bus_%u: bus_type %u, bus_id %lu, num_dev %u\n",
__func__, __LINE__, repo.bus_index, repo.bus_type,
- repo.bus_id, num_dev);
+ (unsigned long)repo.bus_id, num_dev);
dump_device_info(&repo, num_dev);
}
- pr_debug(" <- %s:%d\n", __func__, __LINE__);
+ pr_devel(" <- %s:%d\n", __func__, __LINE__);
return result;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Scott Wood @ 2011-12-01 18:41 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <1322760781-31226-1-git-send-email-timur@freescale.com>
On 12/01/2011 11:33 AM, Timur Tabi wrote:
> An I2C device tree node can contain a 'cell-index' property that can be
> used to enumerate the I2C devices. If such a property exists, use it
> to specify the I2C adapter number.
Didn't we decide a long time ago that this was a bad idea?
> This feature is necessary for the Freescale PowerPC audio drivers (e.g.
> on the P1022DS). The "machine driver" needs to know the adapter number
> for each I2C adapter, but it only has access to the device tree.
> Previously, the I2C nodes always appeared in cell-index order, so the
> dynamic numbering coincided with the cell-index property. With commit
> ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
> unintentionally reversed in the device tree, and so the machine driver
> guesses the wrong I2C adapter number.
What specifically do you need this number for? What does it represent?
-Scott
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Jean-Michel Hautbois @ 2011-12-01 17:52 UTC (permalink / raw)
To: Tabi Timur-B04825; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CAL8zT=hK2rDDC7s+SkvCA=C5LjVX+5oRQmP+KQet2mkFEnaKKw@mail.gmail.com>
2011/12/1 Jean-Michel Hautbois <jhautbois@gmail.com>:
> 2011/12/1 Tabi Timur-B04825 <B04825@freescale.com>:
>> On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois
>> <jhautbois@gmail.com> wrote:
>>>
>>> Any idea on how to debug this ? I am using a 2.6.35 kernel.
>>
>> There are a ton of Kconfig options for debugging various locking bugs.
>> =C2=A0Try turning them on.
>>
All Kconfig options do not help me with cache coherency problems, though.
Or, I didn't find any option related.
Any idea ?
JM
^ permalink raw reply
* [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 17:33 UTC (permalink / raw)
To: kumar.gala, grant.likely, linuxppc-dev
An I2C device tree node can contain a 'cell-index' property that can be
used to enumerate the I2C devices. If such a property exists, use it
to specify the I2C adapter number.
This feature is necessary for the Freescale PowerPC audio drivers (e.g.
on the P1022DS). The "machine driver" needs to know the adapter number
for each I2C adapter, but it only has access to the device tree.
Previously, the I2C nodes always appeared in cell-index order, so the
dynamic numbering coincided with the cell-index property. With commit
ab827d97 ("powerpc/85xx: Rework P1022DS device tree"), the I2C nodes are
unintentionally reversed in the device tree, and so the machine driver
guesses the wrong I2C adapter number.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/i2c/busses/i2c-mpc.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 107397a..8551c34 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -632,7 +632,19 @@ static int __devinit fsl_i2c_probe(struct platform_device *op)
i2c->adap.dev.parent = &op->dev;
i2c->adap.dev.of_node = of_node_get(op->dev.of_node);
- result = i2c_add_adapter(&i2c->adap);
+ /*
+ * If the I2C node has a "cell-index" property, use it to enumerate
+ * the I2C adapter.
+ */
+ prop = of_get_property(i2c->adap.dev.of_node, "cell-index", &plen);
+ if (prop && plen == sizeof(u32)) {
+ dev_dbg(i2c->dev, "using cell-index property %u", *prop);
+ i2c->adap.nr = *prop;
+ result = i2c_add_numbered_adapter(&i2c->adap);
+ } else {
+ result = i2c_add_adapter(&i2c->adap);
+ }
+
if (result < 0) {
dev_err(i2c->dev, "failed to add adapter\n");
goto fail_add;
--
1.7.3.4
^ permalink raw reply related
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Jean-Michel Hautbois @ 2011-12-01 16:14 UTC (permalink / raw)
To: Tabi Timur-B04825; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CAOZdJXVq_g=GAJLRsHZ+290cAJfpOx0LX3sBL3T5i0CaG+xh9Q@mail.gmail.com>
2011/12/1 Tabi Timur-B04825 <B04825@freescale.com>:
> On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois
> <jhautbois@gmail.com> wrote:
>>
>> Any idea on how to debug this ? I am using a 2.6.35 kernel.
>
> There are a ton of Kconfig options for debugging various locking bugs.
> =C2=A0Try turning them on.
>
And when I do that, it works, even in SMP... :).
JM
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Tabi Timur-B04825 @ 2011-12-01 15:56 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CAL8zT=gGfBYsLcsKobJPhfhEhNeLcrYMoWh2kv0odT0n0wnZgQ@mail.gmail.com>
On Thu, Dec 1, 2011 at 9:04 AM, Jean-Michel Hautbois
<jhautbois@gmail.com> wrote:
>
> Any idea on how to debug this ? I am using a 2.6.35 kernel.
There are a ton of Kconfig options for debugging various locking bugs.
Try turning them on.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Jean-Michel Hautbois @ 2011-12-01 15:04 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <D70E8CC4-9FA3-4DC9-B623-02F32EFBFE0F@kernel.crashing.org>
2011/12/1 Kumar Gala <galak@kernel.crashing.org>:
>
> On Dec 1, 2011, at 3:57 AM, Jean-Michel Hautbois wrote:
>
>> Hi,
>>
>> I have a P2020 CPU (powerpc) and I compiled it with two different defcon=
figs.
>> The first one is a SMP, 2 cores, launched with the "nosmp" kernel
>> parameter, the other one is an UP kernel.
>>
>> My driver behaviour is not the same whether launching one or the
>> other. It is hard to explain more precisely, as it deals only with
>> ioctl which only does ioread32/iowrite32 on a PCIe device.
>> But I can tell that it never works the same way when UP (not working
>> correctly), or SMP "nosmp" (working) or even SMP (not working).
>>
>> AFAIK, the "nosmp" parameter should tell the kernel to act the same is
>> if it is an UP kernel, and it disables IO APIC, which is not an issue
>> in my case.
>>
>> Can you think about anything that would explain it, or would help me
>> debugging it ?
>
> This is a bit odd, hard to say w/o more details on what exactly your code=
is trying to do and wait the failure is.
I understand that it is difficult to give more details, but the driver
is not mine, and I can't give all the code associated.
> The larger differences between SMP & UP build would be setting of memory =
attribute for cache coherency. =C2=A0In UP case we don't set it.
>
> Between SMP 'nosmp' and SMP 'on' cases you seem like you're hitting some =
locking condition.
>
> Guessing your running into a timing & locking issue that you're getting l=
ucky on the SMP 'nosmp' case such that it just happens to work.
Any idea on how to debug this ? I am using a 2.6.35 kernel.
Thanks,
JM
^ permalink raw reply
* Re: Different behaviour when using "nosmp" parameter on SMP and UP
From: Kumar Gala @ 2011-12-01 14:03 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CAL8zT=juU_PC92R4za31R8Lmur0PrFwog1m1fk+PEarW_0C6ng@mail.gmail.com>
On Dec 1, 2011, at 3:57 AM, Jean-Michel Hautbois wrote:
> Hi,
>=20
> I have a P2020 CPU (powerpc) and I compiled it with two different =
defconfigs.
> The first one is a SMP, 2 cores, launched with the "nosmp" kernel
> parameter, the other one is an UP kernel.
>=20
> My driver behaviour is not the same whether launching one or the
> other. It is hard to explain more precisely, as it deals only with
> ioctl which only does ioread32/iowrite32 on a PCIe device.
> But I can tell that it never works the same way when UP (not working
> correctly), or SMP "nosmp" (working) or even SMP (not working).
>=20
> AFAIK, the "nosmp" parameter should tell the kernel to act the same is
> if it is an UP kernel, and it disables IO APIC, which is not an issue
> in my case.
>=20
> Can you think about anything that would explain it, or would help me
> debugging it ?
This is a bit odd, hard to say w/o more details on what exactly your =
code is trying to do and wait the failure is.
The larger differences between SMP & UP build would be setting of memory =
attribute for cache coherency. In UP case we don't set it.
Between SMP 'nosmp' and SMP 'on' cases you seem like you're hitting some =
locking condition.
Guessing your running into a timing & locking issue that you're getting =
lucky on the SMP 'nosmp' case such that it just happens to work.
- k=
^ 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