* Re: [RFC] Optimize __arch_swab32 and __arch_swab16
From: Joakim Tjernlund @ 2011-08-11 8:51 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linuxppc-dev
In-Reply-To: <m34o1ouyix.fsf@hase.home>
Andreas Schwab <schwab@redhat.com> wrote on 2011/08/11 10:45:42:
>
> Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:
>
> > unsigned short my__arch_swab16(unsigned short value)
> > {
> > __asm__("rlwimi %0,%0,16,0x00ff0000"
> > : "+r" (value));
>
> You are creating a value that does not fit in a short.
Short is passed in a 32 bit register with the upper 16 bits cleared. I just
temporarily use the upper bits and shift it back with the next insn:
__asm__("rlwinm %0,%0,24,0x0000ffff"
: "+r"(value));
Can I not use the upper 16 bits in this manner?
Jocke
^ permalink raw reply
* Re: [RFC] Optimize __arch_swab32 and __arch_swab16
From: Andreas Schwab @ 2011-08-11 8:45 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev
In-Reply-To: <OF9F5D0B06.A12659DA-ONC12578E9.002B23BB-C12578E9.002D304C__34569.7653633126$1313050887$gmane$org@transmode.se>
Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:
> unsigned short my__arch_swab16(unsigned short value)
> {
> __asm__("rlwimi %0,%0,16,0x00ff0000"
> : "+r" (value));
You are creating a value that does not fit in a short.
Andreas.
--
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
^ permalink raw reply
* [RFC] Optimize __arch_swab32 and __arch_swab16
From: Joakim Tjernlund @ 2011-08-11 8:13 UTC (permalink / raw)
To: linuxppc-dev
PPC __arch_swab32 and __arch_swab16 generates non optimal code.
It doesn't schedule very well, need to copy its input register and
and swab16 needs an extra insn to clear its upper bits. I have improved
these functions(see my__xx). Any problem with the new asm? If
not I will send a patch.
Below some example code to illustrate:
#include <stdio.h>
unsigned long __arch_swab32(unsigned long value)
{
unsigned long result;
__asm__("rlwimi %0,%1,24,16,23\n\t"
"rlwimi %0,%1,8,8,15\n\t"
"rlwimi %0,%1,24,0,7"
: "=r" (result)
: "r" (value), "0" (value >> 24));
return result;
}
unsigned long my__arch_swab32(unsigned long value)
{
unsigned long tmp;
__asm__("rlwimi %0,%0,24,0xffffffff"
: "+r" (value));
__asm__("rlwinm %0,%1,16,0xffffffff"
: "=r" (tmp), "+r" (value));
__asm__("rlwimi %0,%1,0,0x00ff0000"
: "+r" (value), "+r" (tmp));
__asm__("rlwimi %0,%1,0,0x000000ff"
: "+r" (value), "+r" (tmp));
return value;
}
unsigned short __arch_swab16(unsigned short value)
{
unsigned short result;
__asm__("rlwimi %0,%1,8,16,23"
: "=r" (result)
: "r" (value), "0" (value >> 8));
return result;
}
unsigned short my__arch_swab16(unsigned short value)
{
__asm__("rlwimi %0,%0,16,0x00ff0000"
: "+r" (value));
__asm__("rlwinm %0,%0,24,0x0000ffff"
: "+r"(value));
return value;
}
main()
{
unsigned long x=0x12345678, y;
y = my__arch_swab32(x);
printf("swab32 x:%x, y:%x\n", x, y);
y = my__arch_swab16(x);
printf("swab16 x:%x, y:%x\n", x, y);
}
Generated asm:
.file "tst.c"
.section ".text"
.align 2
.globl __arch_swab32
.type __arch_swab32, @function
__arch_swab32:
mr %r0,%r3
srwi %r3,%r3,24
#APP
rlwimi %r3,%r0,24,16,23
rlwimi %r3,%r0,8,8,15
rlwimi %r3,%r0,24,0,7
#NO_APP
blr
.size __arch_swab32, .-__arch_swab32
.align 2
.globl my__arch_swab32
.type my__arch_swab32, @function
my__arch_swab32:
#APP
rlwimi %r3,%r3,24,0xffffffff
rlwinm %r0,%r3,16,0xffffffff
rlwimi %r3,%r0,0,0x00ff0000
rlwimi %r3,%r0,0,0x000000ff
#NO_APP
blr
.size my__arch_swab32, .-my__arch_swab32
.align 2
.globl __arch_swab16
.type __arch_swab16, @function
__arch_swab16:
mr %r0,%r3
srwi %r3,%r3,8
#APP
rlwimi %r3,%r0,8,16,23
#NO_APP
rlwinm %r3,%r3,0,0xffff
blr
.size __arch_swab16, .-__arch_swab16
.align 2
.globl my__arch_swab16
.type my__arch_swab16, @function
my__arch_swab16:
#APP
rlwimi %r3,%r3,16,0x00ff0000
rlwinm %r3,%r3,24,0x0000ffff
#NO_APP
blr
.size my__arch_swab16, .-my__arch_swab16
.section .rodata.str1.4,"aMS",@progbits,1
.align 2
.LC0:
.string "swab32 x:%x, y:%x\n"
.align 2
.LC1:
.string "swab16 x:%x, y:%x\n"
.section ".text"
.align 2
.globl main
.type main, @function
main:
mflr %r0
lis %r3,0x1234
stwu %r1,-16(%r1)
ori %r3,%r3,22136
stw %r0,20(%r1)
bl my__arch_swab32
mr %r5,%r3
lis %r4,0x1234
lis %r3,.LC0@ha
ori %r4,%r4,22136
la %r3,.LC0@l(%r3)
bl printf
li %r3,22136
bl my__arch_swab16
lis %r4,0x1234
mr %r5,%r3
lis %r3,.LC1@ha
la %r3,.LC1@l(%r3)
ori %r4,%r4,22136
bl printf
lwz %r0,20(%r1)
addi %r1,%r1,16
mtlr %r0
blr
.size main, .-main
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.6 (Gentoo 3.4.6-r2, ssp-3.4.6-1.0, pie-8.7.9)"
^ permalink raw reply
* Re: [PATCH v11 4/5] powerpc: Add flexcan device support for p1010rdb.
From: Wolfgang Grandegger @ 2011-08-11 7:35 UTC (permalink / raw)
To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list
In-Reply-To: <20110811035620.GB4926@sgi.com>
On 08/11/2011 05:56 AM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 08:16:33PM +0200, Wolfgang Grandegger wrote:
>> On 08/10/2011 07:01 PM, Kumar Gala wrote:
>>>
>>> On Aug 10, 2011, at 11:27 AM, Robin Holt wrote:
>>>
>>>> I added a simple clock source for the p1010rdb so the flexcan driver
>>>> could determine a clock frequency. The p1010 flexcan device only has
>>>> an oscillator of system bus frequency divided by 2.
>>>>
>>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
>>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
>>>> Cc: U Bhaskar-B22300 <B22300@freescale.com>
>>>> Cc: socketcan-core@lists.berlios.de,
>>>> Cc: netdev@vger.kernel.org,
>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>>> ---
>>>> arch/powerpc/platforms/85xx/Kconfig | 2 +
>>>> arch/powerpc/platforms/85xx/Makefile | 2 +
>>>> arch/powerpc/platforms/85xx/clock.c | 52 ++++++++++++++++++++++++++++++++
>>>> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++
>>>> 4 files changed, 64 insertions(+), 0 deletions(-)
>>>> create mode 100644 arch/powerpc/platforms/85xx/clock.c
>>>
>>> I dont understand how mpc85xx_clk_functions() ends up being associated with the frequency the flexcan is running at.
>>
>> The function mpc85xx_clk_get_rate() returns "fsl_get_sys_freq() / 2" for
>> Flexcan devices.
>>
>>> This either seems to global or I'm missing something.
>>
>> This patch extends the existing Flexcan platform driver for ARM for the
>> PowerPC using the device tree. Due to the nice integration of the device
>> tree (of-platform) into the platform driver and devices, the difference
>> are quite small (see patches 1..3). Apart from the endianess issue, only
>> the clock needs to be handled in a common way. As ARM already uses the
>> clk interface, we found it straight-forward to implement it for the
>> P1010, or more general for the 85xx, as well, instead of using an
>> additional helper function.
>>
>>> I still think the clk / freq info should be in the device tree and handled in the driver and NOT arch/powerpc platform code.
>>
>> If I understand you correctly, you want the boot-loader to provide the
>> relevant information by fixing up the device tree, which then can be
>> handled arch-independently by the driver, right?
>
> Marc and Wolfgang,
>
> This is a very early swag at this which I worked up while driving home from dinner
> this evening. It works with my current config, but that includes at least one
> bogus patch. I will have to do more testing tomorrow. For now, it is something
> to ponder.
Yes, that's what Kumar is proposing. Fine for me with the P1010. It just
needs some proper U-Boot implementation.
> Thanks,
> Robin
> ------------------------------------------------------------------------
>
> flexcan: Prefer device tree clock frequency if available.
>
> If our CAN device's device tree node has a clock-frequency property,
> then use that value for the can devices clock frequency. If not, fall
> back to asking the platform/mach code for the clock frequency associated
> with the flexcan device.
>
> Too-early-to-be-signed-off-by: Robin Holt <holt@sgi.com>
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 662f832..d6a87c9 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -33,6 +33,7 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
>
> #define DRV_NAME "flexcan"
> @@ -929,12 +930,25 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> void __iomem *base;
> resource_size_t mem_size;
> int err, irq;
> + u32 clock_freq = 0;
>
> - clk = clk_get(&pdev->dev, NULL);
> - if (IS_ERR(clk)) {
> - dev_err(&pdev->dev, "no clock defined\n");
> - err = PTR_ERR(clk);
> - goto failed_clock;
> + if (pdev->dev.of_node) {
> + u32 *clock_freq_p;
Should be:
const u32 *clock_freq_p;
> +
> + clk = NULL;
> + clock_freq_p = (u32 *)of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
No cast please.
> + if (clock_freq_p)
> + clock_freq = *clock_freq_p;
> + }
Wolfgang.
^ permalink raw reply
* Re: [PATCH v11 4/5] powerpc: Add flexcan device support for p1010rdb.
From: Wolfgang Grandegger @ 2011-08-11 7:26 UTC (permalink / raw)
To: Kumar Gala; +Cc: socketcan-core, netdev, U Bhaskar-B22300, PPC list
In-Reply-To: <634AB7A6-1CDA-41B3-8A5D-01F29EF01521@kernel.crashing.org>
On 08/11/2011 06:46 AM, Kumar Gala wrote:
>
> On Aug 10, 2011, at 1:16 PM, Wolfgang Grandegger wrote:
>
>> On 08/10/2011 07:01 PM, Kumar Gala wrote:
>>>
>>> On Aug 10, 2011, at 11:27 AM, Robin Holt wrote:
>>>
>>>> I added a simple clock source for the p1010rdb so the flexcan driver
>>>> could determine a clock frequency. The p1010 flexcan device only has
>>>> an oscillator of system bus frequency divided by 2.
>>>>
>>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
>>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
>>>> Cc: U Bhaskar-B22300 <B22300@freescale.com>
>>>> Cc: socketcan-core@lists.berlios.de,
>>>> Cc: netdev@vger.kernel.org,
>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>>> ---
>>>> arch/powerpc/platforms/85xx/Kconfig | 2 +
>>>> arch/powerpc/platforms/85xx/Makefile | 2 +
>>>> arch/powerpc/platforms/85xx/clock.c | 52 ++++++++++++++++++++++++++++++++
>>>> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++
>>>> 4 files changed, 64 insertions(+), 0 deletions(-)
>>>> create mode 100644 arch/powerpc/platforms/85xx/clock.c
>>>
>>> I dont understand how mpc85xx_clk_functions() ends up being associated with the frequency the flexcan is running at.
>>
>> The function mpc85xx_clk_get_rate() returns "fsl_get_sys_freq() / 2" for
>> Flexcan devices.
>>
>>> This either seems to global or I'm missing something.
>>
>> This patch extends the existing Flexcan platform driver for ARM for the
>> PowerPC using the device tree. Due to the nice integration of the device
>> tree (of-platform) into the platform driver and devices, the difference
>> are quite small (see patches 1..3). Apart from the endianess issue, only
>> the clock needs to be handled in a common way. As ARM already uses the
>> clk interface, we found it straight-forward to implement it for the
>> P1010, or more general for the 85xx, as well, instead of using an
>> additional helper function.
>
> I see, that. What concerns me is there are numerous clocks / frequencies that exist inside a MPC85xx/P1010 SOC. The code I'm seeing does NOT seem to do anything to relate this clock JUST to the flexcan.
The clk interface is not commonly used on PowerPC, I know. It's just to
provide compatibility with ARM. An alternative would be to use some
helper function.
>>> I still think the clk / freq info should be in the device tree and handled in the driver and NOT arch/powerpc platform code.
>>
>> If I understand you correctly, you want the boot-loader to provide the
>> relevant information by fixing up the device tree, which then can be
>> handled arch-independently by the driver, right?
>
> Yes, that is part of what I want.
This works fine if we just have *one* fixed clock source and frequency.
When there are choices (source and divider) it does make sense to allow
the user to select the frequency via DTS file entries for Linux. Then we
need arch-specific code anyway (to set the relevant registers).
Furthermore we rely on the boot-loader (the usual argument) which is not
a problem for new boards (with new boot-loader), of course.
Wolfgang.
^ permalink raw reply
* [PATCH 4/4] powerpc: Fix oops when echoing bad values to /sys/devices/system/memory/probe
From: Anton Blanchard @ 2011-08-11 6:44 UTC (permalink / raw)
To: benh, paulus, sfr, nfont; +Cc: linuxppc-dev
In-Reply-To: <20110811064427.515781037@samba.org>
If we echo an address the hypervisor doesn't like to
/sys/devices/system/memory/probe we oops the box:
# echo 0x10000000000 > /sys/devices/system/memory/probe
kernel BUG at arch/powerpc/mm/hash_utils_64.c:541!
The backtrace is:
create_section_mapping
arch_add_memory
add_memory
memory_probe_store
sysdev_class_store
sysfs_write_file
vfs_write
SyS_write
In create_section_mapping we BUG if htab_bolt_mapping returned
an error. A better approach is to return an error which will
propagate back to userspace.
Rerunning the test with this patch applied:
# echo 0x10000000000 > /sys/devices/system/memory/probe
-bash: echo: write error: Invalid argument
Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@kernel.org
---
Index: linux-build/arch/powerpc/mm/hash_utils_64.c
===================================================================
--- linux-build.orig/arch/powerpc/mm/hash_utils_64.c 2011-08-10 17:00:42.603007554 +1000
+++ linux-build/arch/powerpc/mm/hash_utils_64.c 2011-08-10 17:05:56.338457482 +1000
@@ -534,11 +534,11 @@ static unsigned long __init htab_get_tab
}
#ifdef CONFIG_MEMORY_HOTPLUG
-void create_section_mapping(unsigned long start, unsigned long end)
+int create_section_mapping(unsigned long start, unsigned long end)
{
- BUG_ON(htab_bolt_mapping(start, end, __pa(start),
+ return htab_bolt_mapping(start, end, __pa(start),
pgprot_val(PAGE_KERNEL), mmu_linear_psize,
- mmu_kernel_ssize));
+ mmu_kernel_ssize);
}
int remove_section_mapping(unsigned long start, unsigned long end)
Index: linux-build/arch/powerpc/include/asm/sparsemem.h
===================================================================
--- linux-build.orig/arch/powerpc/include/asm/sparsemem.h 2011-08-10 17:06:11.948728948 +1000
+++ linux-build/arch/powerpc/include/asm/sparsemem.h 2011-08-10 17:06:21.878901648 +1000
@@ -16,7 +16,7 @@
#endif /* CONFIG_SPARSEMEM */
#ifdef CONFIG_MEMORY_HOTPLUG
-extern void create_section_mapping(unsigned long start, unsigned long end);
+extern int create_section_mapping(unsigned long start, unsigned long end);
extern int remove_section_mapping(unsigned long start, unsigned long end);
#ifdef CONFIG_NUMA
extern int hot_add_scn_to_nid(unsigned long scn_addr);
Index: linux-build/arch/powerpc/mm/mem.c
===================================================================
--- linux-build.orig/arch/powerpc/mm/mem.c 2011-08-10 17:06:42.539260996 +1000
+++ linux-build/arch/powerpc/mm/mem.c 2011-08-10 17:06:45.269308484 +1000
@@ -123,7 +123,8 @@ int arch_add_memory(int nid, u64 start,
pgdata = NODE_DATA(nid);
start = (unsigned long)__va(start);
- create_section_mapping(start, start + size);
+ if (create_section_mapping(start, start + size))
+ return -EINVAL;
/* this should work for most non-highmem platforms */
zone = pgdata->node_zones;
^ permalink raw reply
* [PATCH 3/4] [PATCH] powerpc: Coding style cleanups
From: Anton Blanchard @ 2011-08-11 6:44 UTC (permalink / raw)
To: benh, paulus, sfr; +Cc: linuxppc-dev
In-Reply-To: <20110811064427.515781037@samba.org>
While converting code to use for_each_node_by_type I noticed a
number of coding style issues.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c 2011-08-10 16:19:00.629356097 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c 2011-08-10 16:21:31.642031299 +1000
@@ -281,11 +281,11 @@ static void __init initialize_cache_info
for_each_node_by_type(np, "cpu") {
num_cpus += 1;
- /* We're assuming *all* of the CPUs have the same
+ /*
+ * We're assuming *all* of the CPUs have the same
* d-cache and i-cache sizes... -Peter
*/
-
- if ( num_cpus == 1 ) {
+ if (num_cpus == 1) {
const u32 *sizep, *lsizep;
u32 size, lsize;
@@ -294,10 +294,13 @@ static void __init initialize_cache_info
sizep = of_get_property(np, "d-cache-size", NULL);
if (sizep != NULL)
size = *sizep;
- lsizep = of_get_property(np, "d-cache-block-size", NULL);
+ lsizep = of_get_property(np, "d-cache-block-size",
+ NULL);
/* fallback if block size missing */
if (lsizep == NULL)
- lsizep = of_get_property(np, "d-cache-line-size", NULL);
+ lsizep = of_get_property(np,
+ "d-cache-line-size",
+ NULL);
if (lsizep != NULL)
lsize = *lsizep;
if (sizep == 0 || lsizep == 0)
@@ -314,9 +317,12 @@ static void __init initialize_cache_info
sizep = of_get_property(np, "i-cache-size", NULL);
if (sizep != NULL)
size = *sizep;
- lsizep = of_get_property(np, "i-cache-block-size", NULL);
+ lsizep = of_get_property(np, "i-cache-block-size",
+ NULL);
if (lsizep == NULL)
- lsizep = of_get_property(np, "i-cache-line-size", NULL);
+ lsizep = of_get_property(np,
+ "i-cache-line-size",
+ NULL);
if (lsizep != NULL)
lsize = *lsizep;
if (sizep == 0 || lsizep == 0)
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-08-10 16:19:57.080356036 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 16:20:46.981240046 +1000
@@ -709,7 +709,6 @@ static void __init parse_drconf_memory(s
static int __init parse_numa_properties(void)
{
- struct device_node *cpu = NULL;
struct device_node *memory;
int default_nid = 0;
unsigned long i;
@@ -732,6 +731,7 @@ static int __init parse_numa_properties(
* each node to be onlined must have NODE_DATA etc backing it.
*/
for_each_present_cpu(i) {
+ struct device_node *cpu;
int nid;
cpu = of_get_cpu_node(i, NULL);
@@ -800,8 +800,9 @@ new_range:
}
/*
- * Now do the same thing for each MEMBLOCK listed in the ibm,dynamic-memory
- * property in the ibm,dynamic-reconfiguration-memory node.
+ * Now do the same thing for each MEMBLOCK listed in the
+ * ibm,dynamic-memory property in the
+ * ibm,dynamic-reconfiguration-memory node.
*/
memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (memory)
^ permalink raw reply
* [PATCH 2/4] [PATCH] powerpc: Use for_each_node_by_type instead of open coding it
From: Anton Blanchard @ 2011-08-11 6:44 UTC (permalink / raw)
To: benh, paulus, sfr; +Cc: linuxppc-dev
In-Reply-To: <20110811064427.515781037@samba.org>
Use for_each_node_by_type instead of open coding it.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/machine_kexec_64.c 2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/kernel/machine_kexec_64.c 2011-08-10 16:18:03.000000000 +1000
@@ -74,8 +74,7 @@ int default_machine_kexec_prepare(struct
}
/* We also should not overwrite the tce tables */
- for (node = of_find_node_by_type(NULL, "pci"); node != NULL;
- node = of_find_node_by_type(node, "pci")) {
+ for_each_node_by_type(node, "pci") {
basep = of_get_property(node, "linux,tce-base", NULL);
sizep = of_get_property(node, "linux,tce-size", NULL);
if (basep == NULL || sizep == NULL)
Index: linux-powerpc/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/kernel/setup_64.c 2011-08-10 16:17:52.518167781 +1000
+++ linux-powerpc/arch/powerpc/kernel/setup_64.c 2011-08-10 16:19:00.629356097 +1000
@@ -278,7 +278,7 @@ static void __init initialize_cache_info
DBG(" -> initialize_cache_info()\n");
- for (np = NULL; (np = of_find_node_by_type(np, "cpu"));) {
+ for_each_node_by_type(np, "cpu") {
num_cpus += 1;
/* We're assuming *all* of the CPUs have the same
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-08-10 16:17:52.508167607 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-10 16:19:57.080356036 +1000
@@ -710,7 +710,7 @@ static void __init parse_drconf_memory(s
static int __init parse_numa_properties(void)
{
struct device_node *cpu = NULL;
- struct device_node *memory = NULL;
+ struct device_node *memory;
int default_nid = 0;
unsigned long i;
@@ -750,8 +750,8 @@ static int __init parse_numa_properties(
}
get_n_mem_cells(&n_mem_addr_cells, &n_mem_size_cells);
- memory = NULL;
- while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+
+ for_each_node_by_type(memory, "memory") {
unsigned long start;
unsigned long size;
int nid;
@@ -1187,10 +1187,10 @@ static int hot_add_drconf_scn_to_nid(str
*/
int hot_add_node_scn_to_nid(unsigned long scn_addr)
{
- struct device_node *memory = NULL;
+ struct device_node *memory;
int nid = -1;
- while ((memory = of_find_node_by_type(memory, "memory")) != NULL) {
+ for_each_node_by_type(memory, "memory") {
unsigned long start, size;
int ranges;
const unsigned int *memcell_buf;
^ permalink raw reply
* [PATCH 1/4] [PATCH] powerpc: numa: Remove double of_node_put in hot_add_node_scn_to_nid
From: Anton Blanchard @ 2011-08-11 6:44 UTC (permalink / raw)
To: benh, paulus, sfr, nfont; +Cc: linuxppc-dev
During memory hotplug testing, I got the following warning:
ERROR: Bad of_node_put() on /memory@0
of_node_release
kref_put
of_node_put
of_find_node_by_type
hot_add_node_scn_to_nid
hot_add_scn_to_nid
memory_add_physaddr_to_nid
...
of_find_node_by_type() loop does the of_node_put for us so we only
need the handle the case where we terminate the loop early.
As suggested by Stephen Rothwell we can do the of_node_put
unconditionally outside of the loop since of_node_put handles a
NULL argument fine.
Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@kernel.org
---
Index: linux-powerpc/arch/powerpc/mm/numa.c
===================================================================
--- linux-powerpc.orig/arch/powerpc/mm/numa.c 2011-06-06 08:07:35.148708089 +1000
+++ linux-powerpc/arch/powerpc/mm/numa.c 2011-08-11 09:07:07.329584634 +1000
@@ -1214,11 +1214,12 @@ int hot_add_node_scn_to_nid(unsigned lon
break;
}
- of_node_put(memory);
if (nid >= 0)
break;
}
+ of_node_put(memory);
+
return nid;
}
^ permalink raw reply
* 44x/pci: Add __init annotations for *init_port_hw() functions.
From: Tony Breeds @ 2011-08-11 6:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, LinuxPPC-dev, Josh Boyer; +Cc: Stephen Rothwell
The various port_init_hw methods of ppc4xx_pciex_hwops should have been
marked __init and when I added ppc4xx_pciex_port_reset_sdr(), which is
__init. This added many section mismatch warnings like:
WARNING: arch/powerpc/sysdev/built-in.o(.text+0x5c68): Section mismatch in reference from the function ppc440spe_pciex_init_port_hw() to the function .init.text:ppc4xx_pciex_port_reset_sdr()
The function ppc440spe_pciex_init_port_hw() references
the function __init ppc4xx_pciex_port_reset_sdr().
This is often because ppc440spe_pciex_init_port_hw lacks a __init
annotation or the annotation of ppc4xx_pciex_port_reset_sdr is wrong.
Trivial patch to silence those warnings.
Reported-By: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index dbfe96b..60541a6 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -834,7 +834,7 @@ static int __init ppc440spe_pciex_core_init(struct device_node *np)
return 3;
}
-static int ppc440spe_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc440spe_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
u32 val = 1 << 24;
@@ -872,12 +872,12 @@ static int ppc440spe_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
return ppc4xx_pciex_port_reset_sdr(port);
}
-static int ppc440speA_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc440speA_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
return ppc440spe_pciex_init_port_hw(port);
}
-static int ppc440speB_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc440speB_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
int rc = ppc440spe_pciex_init_port_hw(port);
@@ -936,7 +936,7 @@ static int __init ppc460ex_pciex_core_init(struct device_node *np)
return 2;
}
-static int ppc460ex_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc460ex_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
u32 val;
u32 utlset1;
@@ -1122,7 +1122,7 @@ static int __init ppc460sx_pciex_core_init(struct device_node *np)
return 2;
}
-static int ppc460sx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc460sx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
if (port->endpoint)
@@ -1189,7 +1189,7 @@ static void ppc405ex_pcie_phy_reset(struct ppc4xx_pciex_port *port)
mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET, 0x00101000);
}
-static int ppc405ex_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+static int __init ppc405ex_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
{
u32 val;
Yours Tony
^ permalink raw reply related
* Re: [PATCH v11 4/5] powerpc: Add flexcan device support for p1010rdb.
From: Kumar Gala @ 2011-08-11 4:46 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list
In-Reply-To: <4E42CB01.7030700@grandegger.com>
On Aug 10, 2011, at 1:16 PM, Wolfgang Grandegger wrote:
> On 08/10/2011 07:01 PM, Kumar Gala wrote:
>>=20
>> On Aug 10, 2011, at 11:27 AM, Robin Holt wrote:
>>=20
>>> I added a simple clock source for the p1010rdb so the flexcan driver
>>> could determine a clock frequency. The p1010 flexcan device only =
has
>>> an oscillator of system bus frequency divided by 2.
>>>=20
>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
>>> Cc: U Bhaskar-B22300 <B22300@freescale.com>
>>> Cc: socketcan-core@lists.berlios.de,
>>> Cc: netdev@vger.kernel.org,
>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>> ---
>>> arch/powerpc/platforms/85xx/Kconfig | 2 +
>>> arch/powerpc/platforms/85xx/Makefile | 2 +
>>> arch/powerpc/platforms/85xx/clock.c | 52 =
++++++++++++++++++++++++++++++++
>>> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++
>>> 4 files changed, 64 insertions(+), 0 deletions(-)
>>> create mode 100644 arch/powerpc/platforms/85xx/clock.c
>>=20
>> I dont understand how mpc85xx_clk_functions() ends up being =
associated with the frequency the flexcan is running at.
>=20
> The function mpc85xx_clk_get_rate() returns "fsl_get_sys_freq() / 2" =
for
> Flexcan devices.
>=20
>> This either seems to global or I'm missing something.
>=20
> This patch extends the existing Flexcan platform driver for ARM for =
the
> PowerPC using the device tree. Due to the nice integration of the =
device
> tree (of-platform) into the platform driver and devices, the =
difference
> are quite small (see patches 1..3). Apart from the endianess issue, =
only
> the clock needs to be handled in a common way. As ARM already uses the
> clk interface, we found it straight-forward to implement it for the
> P1010, or more general for the 85xx, as well, instead of using an
> additional helper function.
I see, that. What concerns me is there are numerous clocks / =
frequencies that exist inside a MPC85xx/P1010 SOC. The code I'm seeing =
does NOT seem to do anything to relate this clock JUST to the flexcan.
>> I still think the clk / freq info should be in the device tree and =
handled in the driver and NOT arch/powerpc platform code.
>=20
> If I understand you correctly, you want the boot-loader to provide the
> relevant information by fixing up the device tree, which then can be
> handled arch-independently by the driver, right?
Yes, that is part of what I want.
- k=
^ permalink raw reply
* Re: [PATCH v11 4/5] powerpc: Add flexcan device support for p1010rdb.
From: Robin Holt @ 2011-08-11 3:56 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list
In-Reply-To: <4E42CB01.7030700@grandegger.com>
On Wed, Aug 10, 2011 at 08:16:33PM +0200, Wolfgang Grandegger wrote:
> On 08/10/2011 07:01 PM, Kumar Gala wrote:
> >
> > On Aug 10, 2011, at 11:27 AM, Robin Holt wrote:
> >
> >> I added a simple clock source for the p1010rdb so the flexcan driver
> >> could determine a clock frequency. The p1010 flexcan device only has
> >> an oscillator of system bus frequency divided by 2.
> >>
> >> Signed-off-by: Robin Holt <holt@sgi.com>
> >> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> >> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> >> Cc: U Bhaskar-B22300 <B22300@freescale.com>
> >> Cc: socketcan-core@lists.berlios.de,
> >> Cc: netdev@vger.kernel.org,
> >> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> >> Cc: Kumar Gala <galak@kernel.crashing.org>
> >> ---
> >> arch/powerpc/platforms/85xx/Kconfig | 2 +
> >> arch/powerpc/platforms/85xx/Makefile | 2 +
> >> arch/powerpc/platforms/85xx/clock.c | 52 ++++++++++++++++++++++++++++++++
> >> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++
> >> 4 files changed, 64 insertions(+), 0 deletions(-)
> >> create mode 100644 arch/powerpc/platforms/85xx/clock.c
> >
> > I dont understand how mpc85xx_clk_functions() ends up being associated with the frequency the flexcan is running at.
>
> The function mpc85xx_clk_get_rate() returns "fsl_get_sys_freq() / 2" for
> Flexcan devices.
>
> > This either seems to global or I'm missing something.
>
> This patch extends the existing Flexcan platform driver for ARM for the
> PowerPC using the device tree. Due to the nice integration of the device
> tree (of-platform) into the platform driver and devices, the difference
> are quite small (see patches 1..3). Apart from the endianess issue, only
> the clock needs to be handled in a common way. As ARM already uses the
> clk interface, we found it straight-forward to implement it for the
> P1010, or more general for the 85xx, as well, instead of using an
> additional helper function.
>
> > I still think the clk / freq info should be in the device tree and handled in the driver and NOT arch/powerpc platform code.
>
> If I understand you correctly, you want the boot-loader to provide the
> relevant information by fixing up the device tree, which then can be
> handled arch-independently by the driver, right?
Marc and Wolfgang,
This is a very early swag at this which I worked up while driving home from dinner
this evening. It works with my current config, but that includes at least one
bogus patch. I will have to do more testing tomorrow. For now, it is something
to ponder.
Thanks,
Robin
------------------------------------------------------------------------
flexcan: Prefer device tree clock frequency if available.
If our CAN device's device tree node has a clock-frequency property,
then use that value for the can devices clock frequency. If not, fall
back to asking the platform/mach code for the clock frequency associated
with the flexcan device.
Too-early-to-be-signed-off-by: Robin Holt <holt@sgi.com>
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 662f832..d6a87c9 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -33,6 +33,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#define DRV_NAME "flexcan"
@@ -929,12 +930,25 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
void __iomem *base;
resource_size_t mem_size;
int err, irq;
+ u32 clock_freq = 0;
- clk = clk_get(&pdev->dev, NULL);
- if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "no clock defined\n");
- err = PTR_ERR(clk);
- goto failed_clock;
+ if (pdev->dev.of_node) {
+ u32 *clock_freq_p;
+
+ clk = NULL;
+ clock_freq_p = (u32 *)of_get_property(pdev->dev.of_node, "clock-frequency", NULL);
+ if (clock_freq_p)
+ clock_freq = *clock_freq_p;
+ }
+
+ if (clock_freq) {
+ clk = clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk)) {
+ dev_err(&pdev->dev, "no clock defined\n");
+ err = PTR_ERR(clk);
+ goto failed_clock;
+ }
+ clock_freq = clk_get_rate(clk);
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -967,7 +981,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
dev->flags |= IFF_ECHO; /* we support local echo in hardware */
priv = netdev_priv(dev);
- priv->can.clock.freq = clk_get_rate(clk);
+ priv->can.clock.freq = clock_freq;
priv->can.bittiming_const = &flexcan_bittiming_const;
priv->can.do_set_mode = flexcan_set_mode;
priv->can.do_get_berr_counter = flexcan_get_berr_counter;
@@ -1002,7 +1016,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
failed_map:
release_mem_region(mem->start, mem_size);
failed_get:
- clk_put(clk);
+ if (clk)
+ clk_put(clk);
failed_clock:
return err;
}
@@ -1020,7 +1035,8 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
release_mem_region(mem->start, resource_size(mem));
- clk_put(priv->clk);
+ if (priv->clk)
+ clk_put(priv->clk);
free_candev(dev);
^ permalink raw reply related
* Re: [PATCH 1/3] KVM: PPC: Assemble book3s{, _hv}_rmhandlers.S separately
From: Alexander Graf @ 2011-08-11 1:00 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev@ozlabs.org, kvm-ppc@vger.kernel.org
In-Reply-To: <20110810235818.GC5536@bloggs.ozlabs.ibm.com>
Am 11.08.2011 um 01:58 schrieb Paul Mackerras <paulus@samba.org>:
> On Tue, Aug 02, 2011 at 02:20:28PM +0200, Alexander Graf wrote:
>> On 07/23/2011 09:41 AM, Paul Mackerras wrote:
>>> This makes arch/powerpc/kvm/book3s_rmhandlers.S and
>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S be assembled as
>>> separate compilation units rather than having them #included in
>>> arch/powerpc/kernel/exceptions-64s.S. We no longer have any
>>> conditional branches between the exception prologs in
>>> exceptions-64s.S and the KVM handlers, so there is no need to
>>> keep their contents close together in the vmlinux image.
>>>=20
>>> In their current location, they are using up part of the limited
>>> space between the first-level interrupt handlers and the firmware
>>> NMI data area at offset 0x7000, and with some kernel configurations
>>> this area will overflow (e.g. allyesconfig), leading to an
>>> "attempt to .org backwards" error when compiling exceptions-64s.S.
>>>=20
>>> Moving them out requires that we add some #includes that the
>>> book3s_{,hv_}rmhandlers.S code was previously getting implicitly
>>> via exceptions-64s.S.
>>=20
>> So what if your kernel binary is bigger than the 24 bits we can jump
>> and the KVM code happens to be at the end? Or do we just not care
>> here?
>=20
> Actually we can jump +/- 32MB (26 bits). I believe that the linker
> inserts trampolines if the branch target is more than 32MB away, so it
> should still work if the kernel is really large and the KVM code
> happens to be at the end.
>=20
> Stephen Rothwell has been asking me about this patch. He wants it in
> (or something like it) so that he can get his daily linux-next powerpc
> allyesconfig builds to stop failing.
Yup, currently running autotest with this and other patches applied. Will se=
nd out a pullreq soon.
Alex
>=20
> Paul.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] KVM: PPC: Assemble book3s{,_hv}_rmhandlers.S separately
From: Paul Mackerras @ 2011-08-10 23:58 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <4E37EB8C.8040408@suse.de>
On Tue, Aug 02, 2011 at 02:20:28PM +0200, Alexander Graf wrote:
> On 07/23/2011 09:41 AM, Paul Mackerras wrote:
> >This makes arch/powerpc/kvm/book3s_rmhandlers.S and
> >arch/powerpc/kvm/book3s_hv_rmhandlers.S be assembled as
> >separate compilation units rather than having them #included in
> >arch/powerpc/kernel/exceptions-64s.S. We no longer have any
> >conditional branches between the exception prologs in
> >exceptions-64s.S and the KVM handlers, so there is no need to
> >keep their contents close together in the vmlinux image.
> >
> >In their current location, they are using up part of the limited
> >space between the first-level interrupt handlers and the firmware
> >NMI data area at offset 0x7000, and with some kernel configurations
> >this area will overflow (e.g. allyesconfig), leading to an
> >"attempt to .org backwards" error when compiling exceptions-64s.S.
> >
> >Moving them out requires that we add some #includes that the
> >book3s_{,hv_}rmhandlers.S code was previously getting implicitly
> >via exceptions-64s.S.
>
> So what if your kernel binary is bigger than the 24 bits we can jump
> and the KVM code happens to be at the end? Or do we just not care
> here?
Actually we can jump +/- 32MB (26 bits). I believe that the linker
inserts trampolines if the branch target is more than 32MB away, so it
should still work if the kernel is really large and the KVM code
happens to be at the end.
Stephen Rothwell has been asking me about this patch. He wants it in
(or something like it) so that he can get his daily linux-next powerpc
allyesconfig builds to stop failing.
Paul.
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: fix build errors with older toolchains
From: Alexander Graf @ 2011-08-10 21:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, Marcelo Tosatti, linux-kernel, kvm-ppc, Paul Mackerras,
Avi Kivity, Nishanth Aravamudan, linuxppc-dev
In-Reply-To: <1313002585.1674.13.camel@pasglop>
On 10.08.2011, at 20:56, Benjamin Herrenschmidt wrote:
> On Wed, 2011-08-10 at 16:58 +0200, Alexander Graf wrote:
>> On 08/03/2011 08:55 PM, Nishanth Aravamudan wrote:
>>> On a box with gcc 4.3.2, I see errors like:
>>>=20
>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S:1254: Error: Unrecognized =
opcode: stxvd2x
>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S:1316: Error: Unrecognized =
opcode: lxvd2x
>>=20
>> Paul, mind to ack?
>=20
> I merged it already :-) It was trivial & annoying enough.
Alright then, I won't touch it :)
Alex
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: fix build errors with older toolchains
From: Benjamin Herrenschmidt @ 2011-08-10 18:56 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm, Marcelo Tosatti, linux-kernel, kvm-ppc, Paul Mackerras,
Avi Kivity, Nishanth Aravamudan, linuxppc-dev
In-Reply-To: <4E429CAD.5050000@suse.de>
On Wed, 2011-08-10 at 16:58 +0200, Alexander Graf wrote:
> On 08/03/2011 08:55 PM, Nishanth Aravamudan wrote:
> > On a box with gcc 4.3.2, I see errors like:
> >
> > arch/powerpc/kvm/book3s_hv_rmhandlers.S:1254: Error: Unrecognized opcode: stxvd2x
> > arch/powerpc/kvm/book3s_hv_rmhandlers.S:1316: Error: Unrecognized opcode: lxvd2x
>
> Paul, mind to ack?
I merged it already :-) It was trivial & annoying enough.
Cheers,
Ben.
>
> Alex
>
> > Signed-off-by: Nishanth Aravamudan<nacc@us.ibm.com>
> > ---
> > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > index 6dd3358..de29501 100644
> > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > @@ -1251,7 +1251,7 @@ BEGIN_FTR_SECTION
> > reg = 0
> > .rept 32
> > li r6,reg*16+VCPU_VSRS
> > - stxvd2x reg,r6,r3
> > + STXVD2X(reg,r6,r3)
> > reg = reg + 1
> > .endr
> > FTR_SECTION_ELSE
> > @@ -1313,7 +1313,7 @@ BEGIN_FTR_SECTION
> > reg = 0
> > .rept 32
> > li r7,reg*16+VCPU_VSRS
> > - lxvd2x reg,r7,r4
> > + LXVD2X(reg,r7,r4)
> > reg = reg + 1
> > .endr
> > FTR_SECTION_ELSE
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2011-08-10 18:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list
In-Reply-To: <CA+55aFxbjN3y3Wg193C2qDLbv+AeuR0=23p-r38jB19WbbRa-g@mail.gmail.com>
On Wed, 2011-08-10 at 11:07 -0700, Linus Torvalds wrote:
> On Wed, Aug 10, 2011 at 8:27 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> >
> > Here are a few fixes for powerpc. Mostly innocuous stuff, in fact most of
> > it has been in my tree for a while, I just hadn't got a chance to actually
> > send it as I was travelling.
> >
> > Cheers,
> > Ben.
> >
> > The following changes since commit 53d1e658df6e26d62500410719aaee2b82067c03:
> >
> > Merge branch 'devicetree/merge' of git://git.secretlab.ca/git/linux-2.6 (2011-08-04 06:37:07 -1000)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git ..BRANCH.NOT.VERIFIED..
>
> What branch?
Heh oops, sorry, the mirror hadn't caught up. It's in the subject of the
email tho :-) "merge" is the branch.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v11 5/5] powerpc: Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-10 18:45 UTC (permalink / raw)
To: Scott Wood
Cc: socketcan-core, netdev, devicetree-discuss, U Bhaskar-B22300,
Robin Holt, PPC list
In-Reply-To: <4E42D09E.4080405@freescale.com>
On Wed, Aug 10, 2011 at 01:40:30PM -0500, Scott Wood wrote:
> On 08/10/2011 01:30 PM, Robin Holt wrote:
> > On Wed, Aug 10, 2011 at 12:36:22PM -0500, Scott Wood wrote:
> >> On 08/10/2011 12:19 PM, Robin Holt wrote:
> >>> On Wed, Aug 10, 2011 at 11:56:28AM -0500, Scott Wood wrote:
> >>>> Also may want to list fsl,p1010-rdb as a "canonical compatible" for
> >>>> anything which is backwards compatible with p1010's implementation.
> >>>
> >>> How do I specify 'canonical compatible'?
> >>
> >> Something like:
> >>
> >> compatible: Should be "fsl,<processor>-flexcan" and "fsl,flexcan".
> >>
> >> An implementation should also claim any of the following compatibles
> >> that it is fully backwards compatible with:
> >>
> >> - fsl,p1010-rdb
>
> Gah, I don't know how "rdb" replaced "flexcan" in the above. Sorry for
> any confusion.
>
> > I am so confused. fsl,p1010-flexcan refers, in my mind at least, to
> > a particular chiplet on the p1010 freescale processor.
>
> It refers to a particular version of the flexcan logic, for which the
> hardware doc people weren't kind enough to give us a public version number.
>
> It has been common and recommended practice in such cases, when there
> are multiple chips containing the same device, to pick a canonical chip
> (such as the first one to have the device or to be supported) and have
> others claim compatibility with it.
>
> > fsl,p1010-rdb
> > would mean nothing to me as that is a p1010 processor with two flexcan
> > chiplets wired to a pair of DB-9 jacks. For the driver, what additional
> > information is being conveyed?
>
> The programming model of the flexcan chiplet.
>
> > Let's cut to the chase. Here is what I have after incorporating your
> > earlier comment about the compatible line. Please mark this up to
> > exactly what you are asking for.
> >
> > Thanks,
> > Robin
> > ------------------------------------------------------------------------
> > Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> >
> > Required properties:
> >
> > - compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
>
> An implementation should also claim any of the following compatibles
> that it is fully backwards compatible with:
>
> - fsl,p1010-flexcan
Ah, there is my confusion. I did not realize you were saying the
entire preceeding 4 lines should be included. I thought you were
making a comment which I did not understand.
Thank you for your patience with my ignorance,
Robin
^ permalink raw reply
* Re: [PATCH v11 5/5] powerpc: Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-10 18:40 UTC (permalink / raw)
To: Robin Holt
Cc: socketcan-core, netdev, devicetree-discuss, U Bhaskar-B22300,
PPC list
In-Reply-To: <20110810183016.GY4926@sgi.com>
On 08/10/2011 01:30 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 12:36:22PM -0500, Scott Wood wrote:
>> On 08/10/2011 12:19 PM, Robin Holt wrote:
>>> On Wed, Aug 10, 2011 at 11:56:28AM -0500, Scott Wood wrote:
>>>> Also may want to list fsl,p1010-rdb as a "canonical compatible" for
>>>> anything which is backwards compatible with p1010's implementation.
>>>
>>> How do I specify 'canonical compatible'?
>>
>> Something like:
>>
>> compatible: Should be "fsl,<processor>-flexcan" and "fsl,flexcan".
>>
>> An implementation should also claim any of the following compatibles
>> that it is fully backwards compatible with:
>>
>> - fsl,p1010-rdb
Gah, I don't know how "rdb" replaced "flexcan" in the above. Sorry for
any confusion.
> I am so confused. fsl,p1010-flexcan refers, in my mind at least, to
> a particular chiplet on the p1010 freescale processor.
It refers to a particular version of the flexcan logic, for which the
hardware doc people weren't kind enough to give us a public version number.
It has been common and recommended practice in such cases, when there
are multiple chips containing the same device, to pick a canonical chip
(such as the first one to have the device or to be supported) and have
others claim compatibility with it.
> fsl,p1010-rdb
> would mean nothing to me as that is a p1010 processor with two flexcan
> chiplets wired to a pair of DB-9 jacks. For the driver, what additional
> information is being conveyed?
The programming model of the flexcan chiplet.
> Let's cut to the chase. Here is what I have after incorporating your
> earlier comment about the compatible line. Please mark this up to
> exactly what you are asking for.
>
> Thanks,
> Robin
> ------------------------------------------------------------------------
> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
>
> Required properties:
>
> - compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
An implementation should also claim any of the following compatibles
that it is fully backwards compatible with:
- fsl,p1010-flexcan
> - reg : Offset and length of the register set for this device
> - interrupts : Interrupt tuple for this device
>
> Example:
>
> can@1c000 {
> compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> reg = <0x1c000 0x1000>;
> interrupts = <48 0x2>;
> interrupt-parent = <&mpic>;
> };
>
-Scott
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-10 18:39 UTC (permalink / raw)
To: Robin Holt
Cc: Wood Scott-B07421, netdev@vger.kernel.org, U Bhaskar-B22300,
socketcan-core@lists.berlios.de, PPC list
In-Reply-To: <20110810183526.GZ4926@sgi.com>
On 08/10/2011 01:35 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote:
>> "...and then associate the label with an alias."
>>
>> The alias can then be used if you want "can0" versus "can1".
>
> Does the alias get used by either the kernel or something else or is it
> just extra detail with no purpose?
It could be used by udev to produce a symlink. Currently, the major
non-human consumer of the aliases is U-Boot.
-Scott
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-10 18:35 UTC (permalink / raw)
To: Scott Wood
Cc: Wood Scott-B07421, netdev@vger.kernel.org, U Bhaskar-B22300,
socketcan-core@lists.berlios.de, Robin Holt, PPC list
In-Reply-To: <4E42CDA8.5050902@freescale.com>
On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote:
> On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote:
> > On 08/10/2011 06:00 PM, Robin Holt wrote:
> >> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> > ...
> >> It looks like the way to do that is to assign a label to those devices
> >> and then associate the label with an alias. I have no idea how that
> >> works under the hood, but it is the way other files are set up. Take a
> >> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> >> interfaces.
> >
> > With a label you mean "label:" at the beginning of a node. Such labels
> > are translated by the device tree compiler in node handles, which can be
> > referenced within nodes by using <&label>, e.g.:
> >
> > UIC0: interrupt-controller0 {
> > ...
> > };
> > UIC1: interrupt-controller1 {
> > ...
> > interrupt-parent = <&UIC0>;
> > ...
> > };
> >
> > It has nothing to do with the name of the node.
>
> "...and then associate the label with an alias."
>
> The alias can then be used if you want "can0" versus "can1".
Does the alias get used by either the kernel or something else or is it
just extra detail with no purpose?
Robin
^ permalink raw reply
* Re: [PATCH v11 5/5] powerpc: Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-10 18:30 UTC (permalink / raw)
To: Scott Wood
Cc: socketcan-core, netdev, devicetree-discuss, U Bhaskar-B22300,
Robin Holt, PPC list
In-Reply-To: <4E42C196.7030708@freescale.com>
On Wed, Aug 10, 2011 at 12:36:22PM -0500, Scott Wood wrote:
> On 08/10/2011 12:19 PM, Robin Holt wrote:
> > On Wed, Aug 10, 2011 at 11:56:28AM -0500, Scott Wood wrote:
> >> On 08/10/2011 11:27 AM, Robin Holt wrote:
> >>> -CPI Clock- Can Protocol Interface Clock
> >>> - This CLK_SRC bit of CTRL(control register) selects the clock source to
> >>> - the CAN Protocol Interface(CPI) to be either the peripheral clock
> >>> - (driven by the PLL) or the crystal oscillator clock. The selected clock
> >>> - is the one fed to the prescaler to generate the Serial Clock (Sclock).
> >>> - The PRESDIV field of CTRL(control register) controls a prescaler that
> >>> - generates the Serial Clock (Sclock), whose period defines the
> >>> - time quantum used to compose the CAN waveform.
> >>> +- compatible : Should be "fsl,flexcan" and optionally
> >>> + "fsl,flexcan-<processor>"
> >>
> >> fsl,<processor>-flexcan, and it should not be optional, and should come
> >> before "fsl,flexcan".
> >>
> >> Also may want to list fsl,p1010-rdb as a "canonical compatible" for
> >> anything which is backwards compatible with p1010's implementation.
> >
> > How do I specify 'canonical compatible'?
>
> Something like:
>
> compatible: Should be "fsl,<processor>-flexcan" and "fsl,flexcan".
>
> An implementation should also claim any of the following compatibles
> that it is fully backwards compatible with:
>
> - fsl,p1010-rdb
I am so confused. fsl,p1010-flexcan refers, in my mind at least, to
a particular chiplet on the p1010 freescale processor. fsl,p1010-rdb
would mean nothing to me as that is a p1010 processor with two flexcan
chiplets wired to a pair of DB-9 jacks. For the driver, what additional
information is being conveyed?
Let's cut to the chase. Here is what I have after incorporating your
earlier comment about the compatible line. Please mark this up to
exactly what you are asking for.
Thanks,
Robin
------------------------------------------------------------------------
Flexcan CAN contoller on Freescale's ARM and PowerPC processors
Required properties:
- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
- reg : Offset and length of the register set for this device
- interrupts : Interrupt tuple for this device
Example:
can@1c000 {
compatible = "fsl,p1010-flexcan", "fsl,flexcan";
reg = <0x1c000 0x1000>;
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
};
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-10 18:27 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Wood Scott-B07421, netdev@vger.kernel.org, U Bhaskar-B22300,
socketcan-core@lists.berlios.de, Robin Holt, PPC list
In-Reply-To: <4E42CCB0.8090803@grandegger.com>
On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote:
> On 08/10/2011 06:00 PM, Robin Holt wrote:
>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> ...
>> It looks like the way to do that is to assign a label to those devices
>> and then associate the label with an alias. I have no idea how that
>> works under the hood, but it is the way other files are set up. Take a
>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
>> interfaces.
>
> With a label you mean "label:" at the beginning of a node. Such labels
> are translated by the device tree compiler in node handles, which can be
> referenced within nodes by using <&label>, e.g.:
>
> UIC0: interrupt-controller0 {
> ...
> };
> UIC1: interrupt-controller1 {
> ...
> interrupt-parent = <&UIC0>;
> ...
> };
>
> It has nothing to do with the name of the node.
"...and then associate the label with an alias."
The alias can then be used if you want "can0" versus "can1".
Appending numbers to the node name is typically only done when there's
no unit address, and a need to disambiguate.
-Scott
^ permalink raw reply
* Please pull 'next' branch of 4xx tree
From: Josh Boyer @ 2011-08-10 18:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
Hi Ben,
Finally somewhat caught up. Now that -rc1 is out, here are some
patches for the next merge window.
josh
The following changes since commit 53d1e658df6e26d62500410719aaee2b82067c03:
Merge branch 'devicetree/merge' of
git://git.secretlab.ca/git/linux-2.6 (2011-08-04 06:37:07 -1000)
are available in the git repository at:
ssh://master.kernel.org/pub/scm/linux/kernel/git/jwboyer/powerpc-4xx.git next
Ayman El-Khashab (1):
powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx
Josh Boyer (1):
powerpc/40x: Remove obsolete HCU4 board
Mike Williams (1):
powerpc/4xx: edac: Add comma to fix build error
Stefan Roese (1):
powerpc/44x: Add NOR flash device to Yosemite dts
Suzuki Poulose (1):
powerpc/44x: Kexec support for PPC440X chipsets
arch/powerpc/Kconfig | 2 +-
arch/powerpc/boot/dts/hcu4.dts | 168 ------------------------------
arch/powerpc/boot/dts/yosemite.dts | 36 +++++++
arch/powerpc/configs/40x/hcu4_defconfig | 80 --------------
arch/powerpc/configs/ppc40x_defconfig | 1 -
arch/powerpc/include/asm/kexec.h | 2 +-
arch/powerpc/kernel/misc_32.S | 171 +++++++++++++++++++++++++++++++
arch/powerpc/platforms/40x/Kconfig | 8 --
arch/powerpc/platforms/40x/Makefile | 1 -
arch/powerpc/platforms/40x/hcu4.c | 61 -----------
arch/powerpc/sysdev/ppc4xx_pci.c | 89 +++++++++++++---
arch/powerpc/sysdev/ppc4xx_pci.h | 12 ++
drivers/edac/ppc4xx_edac.c | 2 +-
13 files changed, 294 insertions(+), 339 deletions(-)
delete mode 100644 arch/powerpc/boot/dts/hcu4.dts
delete mode 100644 arch/powerpc/configs/40x/hcu4_defconfig
delete mode 100644 arch/powerpc/platforms/40x/hcu4.c
^ permalink raw reply
* Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Wolfgang Grandegger @ 2011-08-10 18:23 UTC (permalink / raw)
To: Robin Holt
Cc: Wood Scott-B07421, netdev@vger.kernel.org, U Bhaskar-B22300,
socketcan-core@lists.berlios.de, PPC list
In-Reply-To: <20110810160054.GT4926@sgi.com>
On 08/10/2011 06:00 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
...
> It looks like the way to do that is to assign a label to those devices
> and then associate the label with an alias. I have no idea how that
> works under the hood, but it is the way other files are set up. Take a
> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> interfaces.
With a label you mean "label:" at the beginning of a node. Such labels
are translated by the device tree compiler in node handles, which can be
referenced within nodes by using <&label>, e.g.:
UIC0: interrupt-controller0 {
...
};
UIC1: interrupt-controller1 {
...
interrupt-parent = <&UIC0>;
...
};
It has nothing to do with the name of the node.
Wolfgang.
^ 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