LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* cbe_cpufreq crashes my machine
From: Olof Johansson @ 2007-04-26  6:56 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Christian Krafft
In-Reply-To: <200704260147.35878.arnd@arndb.de>

On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> I can't see any check in here that finds out if you are actually running on
> the right hardware. 

Seems like this came along with our use of cbe_cpufreq.c as a basis. You
don't do it either! :-)

Enabling CONFIG_CBE_CPUFREQ makes my machine die a horrible death,
see below. I'd post a patch if I knew what machine compatible fields to
compare with, but I have no clue what's considered approprate to check
for on cell.


Unable to handle kernel paging request for data at address 0x00000888
Faulting instruction address: 0xc000000000036b14
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2
Modules linked in:
NIP: C000000000036B14 LR: C000000000036B08 CTR: C000000000036A60
REGS: c00000007fd83830 TRAP: 0300   Not tainted  (2.6.20)
MSR: 9000000000009032 <EE,ME,IR,DR>  CR: 82000022  XER: 20000000
DAR: 0000000000000888, DSISR: 0000000040000000
TASK = c00000000ffc5820[1] 'swapper' THREAD: c00000007fd80000 CPU: 0
GPR00: C00000000093F940 C00000007FD83AB0 C000000000916EC0 0000000000000000
GPR04: C00000000068343F C0000000009C1208 C00000007F8FAE00 0000000000000000
GPR08: C000000000859720 0000000000000888 C0000000009C3DE0 0000000000000000
GPR12: 0000000000000000 C000000000779600 0000000000000000 C000000000684808
GPR16: 4000000000000000 C0000000006834E0 0000000000000000 0000000000000000
GPR20: 0000000000764880 C000000000764880 C000000000764AF0 0000000000764AF0
GPR24: C000000000684308 C00000000085FAE8 C0000000025DD028 0000000000000000
GPR28: FFFFFFFFFFFFFFF4 C00000007F8FAC38 C000000000784B58 C00000007F8FAC00
NIP [C000000000036B14] .cbe_cpufreq_cpu_init+0xb4/0x12c
LR [C000000000036B08] .cbe_cpufreq_cpu_init+0xa8/0x12c
Call Trace:
[C00000007FD83AB0] [C000000000036AA8] .cbe_cpufreq_cpu_init+0x48/0x12c (unreliable)
[C00000007FD83B40] [C0000000004831FC] .cpufreq_add_dev+0x158/0x4f4
[C00000007FD83D00] [C00000000033A5E4] .sysdev_driver_register+0xbc/0x158
[C00000007FD83D90] [C000000000482FF0] .cpufreq_register_driver+0xd0/0x184
[C00000007FD83E40] [C0000000007421B4] .cbe_cpufreq_init+0x1c/0x34
[C00000007FD83EC0] [C0000000000093B4] .init+0x1c4/0x39c
[C00000007FD83F90] [C000000000022E10] .kernel_thread+0x4c/0x68
Instruction dump:
800b0000 2f80fffe 409effe4 e93e8038 380061a8 e87f000e 901f0018 f93f0030
4bffe239 60000000 39230888 7c0004ac <e9230888> 0c090000 4c00012c ebbe8008



-Olof

^ permalink raw reply

* Re: [PATCH v2] powerpc: 64K page support for kexec
From: Olof Johansson @ 2007-04-26  7:15 UTC (permalink / raw)
  To: Luke Browning; +Cc: Paul Mackerras, cbe-oss-dev, Arnd Bergmann, linuxppc-dev
In-Reply-To: <1177529739.24866.34.camel@luke-laptop>

Hi,
                                                                                                                                             
Two comments below, besides that it looks good!
                                                                                                                                             
On Wed, Apr 25, 2007 at 04:35:39PM -0300, Luke Browning wrote:

> +#define LP_SHIFT	12
> +#define LP_BITS		8
> +#define LP_MASK(i)	((0xFF >> (i)) << LP_SHIFT)
> +
> +static void hpte_decode(hpte_t *hpte, unsigned long slot, 
> +			int *psize, unsigned long *va)
> +{
> +	unsigned long hpte_r = hpte->r;
> +	unsigned long hpte_v = hpte->v;
> +	unsigned long avpn;
> +	int i, size, shift, penc, avpnm_bits;
> +		
> +	if (!(hpte_v & HPTE_V_LARGE))
> +		size = MMU_PAGE_4K;
> +	else {
> +		for (i = 0; i < LP_BITS; i++) {
> +			if ((hpte_r & LP_MASK(i+1)) == LP_MASK(i+1))
> +				break;
> +		}
> +		penc = LP_MASK(i+1) >> LP_SHIFT;
> +		for (size = 0; size < MMU_PAGE_COUNT - 1; size++) {

This will never consider the last page size. Either <= MMU_PAGE_COUNT-1
or < MMU_PAGE_COUNT. The latter would be more natural.

> -		pteg = slot / HPTES_PER_GROUP;
> -		if (hpte_v & HPTE_V_SECONDARY)
> -			pteg = ~pteg;
> +			/* 4K pages are not represented by LP */
> +			if (size == MMU_PAGE_4K)
> +				continue;
> +
> +			/* valid entries have a shift value */
> +			if (!mmu_psize_defs[size].shift)
> +				continue;
>  
> -		vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
> +			if (penc == mmu_psize_defs[size].penc)
> +				break;
> +		}
> +	}
>  
> -		va |= vpi << PAGE_SHIFT;
> +	/*
> +	 * FIXME, the code below works for 16M, 64K, and 4K pages as these
> +	 * fall under the p<=23 rules for calculating the virtual address.
> +	 * In the case of 16M pages, an extra bit is stolen from the AVPN
> +	 * field to achieve the requisite 24 bits. 
> +	 *
> +	 * 16G pages are not supported by the code below.
> +	 */
> +	BUG_ON(hpte_v & 0x4000000000000000UL);		/* 1T segment */
> +	BUG_ON(size == MMU_PAGE_16G);
> +	BUG_ON(size == MMU_PAGE_64K_AP);

Milton didn't like BUG_ON() here, and I think I agree after hearing his
motivations. I know I was the one suggesting them, but they're better
to keep out at the moment.

[...]



Thanks,

-Olof

^ permalink raw reply

* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Wolfgang Grandegger @ 2007-04-26  8:21 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-embedded
In-Reply-To: <462F6707.80502@ru.mvista.com>

Sergei Shtylyov wrote:
> Hello.
> 
> Wolfgang Grandegger wrote:
> 
>>>>>>> are there already some thought or even example code on how to 
>>>>>>> define Flash memory and MTD partitions in the DTS. This would 
>>>>>>> avoid the ackward MTD partitioning via static structure or boot 
>>>>>>> line argument and could be done in a generic way.
> 
>>>>>>     See Documentation/powerpc/booting-without-of.txt and 
>>>>>> drivers/mtd/physmap_of.c for what's been already done.
> 
>>>>>    I meant drivers/mtd/maps/physmap_of.c
> 
>>>> I had a closer look to this driver. Unfortunately, it can not handle 
>>>> yet  multiple FLASH banks. This could be achieved with an additional 
>>>> property "bank_count". What do you think?
> 
>   Please use hyphen, not underscore in property names ("device_type" 
> seems an only exception from this rule).

Yes, I know and I also wondered why an "_" is used for the device type.

>>>  I'm not sure what banks you're talking about -- note that this 
>>> driver is equivalen in functionality to the plain 'physmap' driver 
>>> and there's inherited "bank-width" property that handles flash banking.
> 
>> Yes, I know. Nevertheless the driver cannot handle flash regions 
>> composed of more than on FLASH bank because we need to probe every 
>> bank. Search for "BANK" in drivers/mtd/maps" to understand what I mean.
> 
>   Hm, from what I could see, those banks are separate maps registered to 
> MTD subsys separately.
> What's the point of adding a count property when we simply can define 
> multiple "rom" devices?

To complete this thread, physmap_of can map _one_ FLASH region composed 
of one or more FLASH banks. But it currently cannot map more than _one_ 
FLASH region. It would be nice to extend it for multiple FLASH 
definitions (could code it when time permits).

Wolfgang.

^ permalink raw reply

* Re: cbe_cpufreq crashes my machine
From: Benjamin Herrenschmidt @ 2007-04-26  8:39 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, Arnd Bergmann, Christian Krafft
In-Reply-To: <20070426065644.GA24799@lixom.net>

On Thu, 2007-04-26 at 01:56 -0500, Olof Johansson wrote:
> On Thu, Apr 26, 2007 at 01:47:35AM +0200, Arnd Bergmann wrote:
> > I can't see any check in here that finds out if you are actually running on
> > the right hardware. 
> 
> Seems like this came along with our use of cbe_cpufreq.c as a basis. You
> don't do it either! :-)
> 
> Enabling CONFIG_CBE_CPUFREQ makes my machine die a horrible death,
> see below. I'd post a patch if I knew what machine compatible fields to
> compare with, but I have no clue what's considered approprate to check
> for on cell.

Or machine_is(cell)

Ben.

^ permalink raw reply

* MPC8544 Watchdog Timer
From: vinay hegde @ 2007-04-26  8:43 UTC (permalink / raw)
  To: linuxppc-embedded

Hi,

I am working with Freescale MPC8544E board. I need an
information related to the functionality of hardware
watchdog provided by MPC8544 processor. 

Here is the question:

On watchdog timeout, whether the CPU can reset itself
_or_ does it assert a hardware HRESET_REQ signal to
some external hardware requesting for reset? 

Please let me know the inputs.

Thanks,
Vinay


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

^ permalink raw reply

* Re: [PATCH v2] [2.6.22] pasemi: hardware rng driver
From: Arnd Bergmann @ 2007-04-26  8:50 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, egor, mb
In-Reply-To: <20070426053725.GB23922@lixom.net>

On Thursday 26 April 2007, Olof Johansson wrote:
> Driver for the on-chip hardware random number generator on PA Semi
> PA6T-1682M.
> 
> Signed-off-by: Egor Martovetsky <egor@pasemi.com>
> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver
From: Arnd Bergmann @ 2007-04-26  8:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Olof Johansson, egor, paulus, cpufreq
In-Reply-To: <20070426053700.GA23922@lixom.net>

On Thursday 26 April 2007, Olof Johansson wrote:

> I chose not to do this as an of_platform driver since it doesn't fit
> that well with the cpufreq driver model; having 3 levels of init/probe
> functions is excessive.

<snip>

> +	dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
> +	if (!dn)
> +		goto out;
> +	err = of_address_to_resource(dn, 0, &res);
> +	of_node_put(dn);
> +	if (err)
> +		goto out;
> +	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
> +	if (!sdcasr_mapbase) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
> +	if (!dn) {
> +		err = -ENODEV;
> +		goto out_unmap_sdcasr;
> +	}
> +	err = of_address_to_resource(dn, 0, &res);
> +	of_node_put(dn);
> +	if (err)
> +		goto out_unmap_sdcasr;
> +	sdcpwr_mapbase = ioremap(res.start, 0x1000);
> +	if (!sdcpwr_mapbase) {
> +		err = -EINVAL;
> +		goto out_unmap_sdcasr;
> +	}

What are sdc and gizmo anyway? If they are both only used for cpufreq, maybe the
easiest way to do this with an of_platform_driver would be to have a single
node that has two register ranges.

	Arnd <><

^ permalink raw reply

* Re: [PATCH 9/13] powerpc: Add arch/powerpc mv64x60 I2C platform data setup
From: Arnd Bergmann @ 2007-04-26  9:00 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <20070426060855.GD2030@xyzzy.farnsworth.org>

On Thursday 26 April 2007, Dale Farnsworth wrote:
> 
> > Ok, I see your point there. But after looking at the i2c and net drivers,
> > I believe that they can easily be split into an architecture dependent
> > part that is either an of_platform_driver or a platform_driver, and
> > a common part that does not know about either of these.
> 
> Oh, it's certainly possible, but it doesn't seem desirable to me. Why
> should the drivers carry the burden of supporting both platform_driver
> and of_platform_driver interfaces?

The reason is that platform_devices are for stuff that fundamentally cannot
be probed but has to be hardcoded in some place.

The point about the of device tree is that it allows you to probe this
kind of device. This means you get automatic module loading based on the
device tree, and that the devices show up in sane locations in /sys.

	Arnd <><

^ permalink raw reply

* Re: [PATCH] Fix interrupt distribution in ppc970
From: Mohan Kumar M @ 2007-04-26  9:24 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev, Paul Mackerras, Anton Blanchard, fastboot
In-Reply-To: <080126626f9bea228426c0c3d7bf1730@bga.com>

On Fri, Apr 20, 2007 at 12:45:15AM -0500, Milton Miller wrote:
[snip]
> 
> Don't test cpus_empty twice, instead do a nested if.
> 
> Actually, looking at this a bit, you need your default_distrib or
> default_server check again.
> 
> How about reorder to be if (!cpus_equal(...ALL)) {
>     cpus_and
>     if !cpus_empty(tmp
>         return get_hard(first);
>     if (strict)
>         return -1;
> }
> 
> if (cpus_equal(online, preseent)
> return distrib
> return default_server
> 
> For that matter, can we just call first_cpu, and check its <= NUM_CPUS
> elminating the call to cpus_empty ?   Only call get_hard_cpu if its
> valid.
>

Milton, I hope this patch meets all your requirements.

Cc: Milton Miller <miltonm@bga.com>,
    Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
---
 arch/powerpc/platforms/pseries/xics.c |   39 ++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 15 deletions(-)

Index: linux-2.6.21-rc4/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.21-rc4.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6.21-rc4/arch/powerpc/platforms/pseries/xics.c
@@ -156,9 +156,9 @@ static inline void lpar_qirr_info(int n_
 
 
 #ifdef CONFIG_SMP
-static int get_irq_server(unsigned int virq)
+static int get_irq_server(unsigned int virq, unsigned int strict_check)
 {
-	unsigned int server;
+	int server;
 	/* For the moment only implement delivery to all cpus or one cpu */
 	cpumask_t cpumask = irq_desc[virq].affinity;
 	cpumask_t tmp = CPU_MASK_NONE;
@@ -166,22 +166,28 @@ static int get_irq_server(unsigned int v
 	if (!distribute_irqs)
 		return default_server;
 
-	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
-		server = default_distrib_server;
-	} else {
+	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
 		cpus_and(tmp, cpu_online_map, cpumask);
 
-		if (cpus_empty(tmp))
-			server = default_distrib_server;
+		server = first_cpu(tmp);
+
+		if (server < NR_CPUS)
+			return get_hard_smp_processor_id(server);
+		else {
+			if(strict_check)
+				return (-1);
+			else
+				return default_distrib_server;
+		}
+	} else {
+		if (cpus_equal(cpu_online_map, cpu_present_map))
+			return default_distrib_server;
 		else
-			server = get_hard_smp_processor_id(first_cpu(tmp));
+			return default_server;
 	}
-
-	return server;
-
 }
 #else
-static int get_irq_server(unsigned int virq)
+static int get_irq_server(unsigned int virq, unsigned int strict_check)
 {
 	return default_server;
 }
@@ -192,7 +198,7 @@ static void xics_unmask_irq(unsigned int
 {
 	unsigned int irq;
 	int call_status;
-	unsigned int server;
+	int server;
 
 	pr_debug("xics: unmask virq %d\n", virq);
 
@@ -201,7 +207,7 @@ static void xics_unmask_irq(unsigned int
 	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
 		return;
 
-	server = get_irq_server(virq);
+	server = get_irq_server(virq, 0);
 
 	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, irq, server,
 				DEFAULT_PRIORITY);
@@ -415,7 +421,10 @@ static void xics_set_affinity(unsigned i
 
 	/* For the moment only implement delivery to all cpus or one cpu */
 	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
-		newmask = default_distrib_server;
+		if (cpus_equal(cpu_online_map, cpu_present_map))
+			newmask = default_distrib_server;
+		else
+			newmask = default_server;
 	} else {
 		cpus_and(tmp, cpu_online_map, cpumask);
 		if (cpus_empty(tmp))


=================== 
> ...
> >@@ -415,7 +419,10 @@ static void xics_set_affinity(unsigned i
> ...
> > 	/* For the moment only implement delivery to all cpus or one cpu */
> > 	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> >-		newmask = default_distrib_server;
> >+		if (cpus_equal(cpu_online_map, cpu_present_map))
> >
> 
> this was supposed to be the call with strict = 1

Do you mean to use 'strict_check' argument in xics_set_affinity?
set_affinity call is declared in linux/irq.h, so if modifying
xics_set_affinity will affect other arch's set_affinity also.

Regards,
Mohan

^ permalink raw reply

* Re: [PATCH] [2.6.22] pasemi: hardware rng driver
From: Michael Buesch @ 2007-04-26  9:23 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Olof Johansson, linuxppc-dev, egor
In-Reply-To: <200704260138.32220.arnd@arndb.de>

On Thursday 26 April 2007 01:38:31 Arnd Bergmann wrote:
> On Wednesday 25 April 2007, Olof Johansson wrote:
> 
> > +static void __iomem *rng_regs;
> > +
> > +static int pasemi_rng_data_present(struct hwrng *rng)
> > +{
> > +	return (in_le32(rng_regs + SDCRNG_CTL_REG)
> > +		& SDCRNG_CTL_FVLD_M) ? 1 : 0;
> > +}
> 
> It would be nicer to get rid of the global rng_regs variable by sticking
> it into rng->priv.

Yeah, I think that would be better. Saves one global variable.


-- 
Greetings Michael.

^ permalink raw reply

* Re: [PATCH] [2.6.22] pasemi: hardware rng driver
From: Michael Buesch @ 2007-04-26  9:22 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, egor
In-Reply-To: <20070425204512.GB19781@lixom.net>

On Wednesday 25 April 2007 22:45:12 Olof Johansson wrote:
> +static int __devinit rng_probe(struct of_device *ofdev,
> +				     const struct of_device_id *match)
> +{
> +	struct device_node *rng_np;
> +	struct resource res;
> +	int err = 0;
> +
> +	rng_np = of_find_compatible_node(NULL, "rng", "1682m-rng");
> +	if (!rng_np)
> +		return -ENODEV;
> +
> +	err = of_address_to_resource(rng_np, 0, &res);
> +	of_node_put(rng_np);
> +
> +	if (err)
> +		return -EINVAL;

I think EINVAL is not the correct error code. I'd suggest ENODEV.

> +	if (!rng_regs)
> +		rng_regs = ioremap(res.start, 0x100);
> +
> +	if (!rng_regs)
> +		return -EPERM;

I think EPERM is not the correct error code. I'd suggest ENOMEM.

> +	printk(KERN_INFO "Registering PA Semi RNG\n");
> +
> +	return hwrng_register(&pasemi_rng);

Resource leak.
Please do something like

	err = hwrng_register(&pasemi_rng);
	if (err)
		iounmap(rng_regs);
	return err;

> +}
> +
> +static int rng_remove(struct of_device *dev)
> +{
> +	iounmap(rng_regs);
> +	hwrng_unregister(&pasemi_rng);

Swap these to prevent race conditions.

> +
> +	return 0;
> +}



-- 
Greetings Michael.

^ permalink raw reply

* [PATCH 1/2] ehea: fix for sysfs entries
From: Thomas Klein @ 2007-04-26  9:56 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, Stefan Roscher, linux-ppc, Marcus Eder

Create symbolic link from each logical port to ehea driver

Signed-off-by: Thomas Klein <tklein@de.ibm.com>
---


This patch applies on top of the netdev upstream branch for 2.6.22



diff -Nurp -X dontdiff linux-2.6.21/drivers/net/ehea/ehea.h patched_kernel/drivers/net/ehea/ehea.h
--- linux-2.6.21/drivers/net/ehea/ehea.h	2007-04-26 10:07:59.000000000 +0200
+++ patched_kernel/drivers/net/ehea/ehea.h	2007-04-26 11:15:25.000000000 +0200
@@ -39,7 +39,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"ehea"
-#define DRV_VERSION	"EHEA_0055"
+#define DRV_VERSION	"EHEA_0057"
 
 #define EHEA_MSG_DEFAULT (NETIF_MSG_LINK | NETIF_MSG_TIMER \
 	| NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR)
diff -Nurp -X dontdiff linux-2.6.21/drivers/net/ehea/ehea_main.c patched_kernel/drivers/net/ehea/ehea_main.c
--- linux-2.6.21/drivers/net/ehea/ehea_main.c	2007-04-26 10:07:59.000000000 +0200
+++ patched_kernel/drivers/net/ehea/ehea_main.c	2007-04-26 11:15:25.000000000 +0200
@@ -78,6 +78,28 @@ MODULE_PARM_DESC(sq_entries, " Number of
 		 __MODULE_STRING(EHEA_DEF_ENTRIES_SQ) ")");
 MODULE_PARM_DESC(use_mcs, " 0:NAPI, 1:Multiple receive queues, Default = 1 ");
 
+static int port_name_cnt = 0;
+
+static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
+                                        const struct of_device_id *id);
+
+static int __devexit ehea_remove(struct ibmebus_dev *dev);
+
+static struct of_device_id ehea_device_table[] = {
+	{
+		.name = "lhea",
+		.compatible = "IBM,lhea",
+	},
+	{},
+};
+
+static struct ibmebus_driver ehea_driver = {
+	.name = "ehea",
+	.id_table = ehea_device_table,
+	.probe = ehea_probe_adapter,
+	.remove = ehea_remove,
+};
+
 void ehea_dump(void *adr, int len, char *msg) {
 	int x;
 	unsigned char *deb = adr;
@@ -2361,6 +2383,34 @@ static void __devinit logical_port_relea
 	of_node_put(port->ofdev.node);
 }
 
+static int ehea_driver_sysfs_add(struct device *dev,
+                                 struct device_driver *driver)
+{
+	int ret;
+
+	ret = sysfs_create_link(&driver->kobj, &dev->kobj,
+				kobject_name(&dev->kobj));
+	if (ret == 0) {
+		ret = sysfs_create_link(&dev->kobj, &driver->kobj,
+					"driver");
+		if (ret)
+			sysfs_remove_link(&driver->kobj,
+					  kobject_name(&dev->kobj));
+	}
+	return ret;
+}
+
+static void ehea_driver_sysfs_remove(struct device *dev,
+                                     struct device_driver *driver)
+{
+	struct device_driver *drv = driver;
+
+	if (drv) {
+		sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
+		sysfs_remove_link(&dev->kobj, "driver");
+	}
+}
+
 static struct device *ehea_register_port(struct ehea_port *port,
 					 struct device_node *dn)
 {
@@ -2368,8 +2418,9 @@ static struct device *ehea_register_port
 
 	port->ofdev.node = of_node_get(dn);
 	port->ofdev.dev.parent = &port->adapter->ebus_dev->ofdev.dev;
+	port->ofdev.dev.bus = &ibmebus_bus_type;
 
-	sprintf(port->ofdev.dev.bus_id, "port%d", port->logical_port_id);
+	sprintf(port->ofdev.dev.bus_id, "port%d", port_name_cnt++);
 	port->ofdev.dev.release = logical_port_release;
 
 	ret = of_device_register(&port->ofdev);
@@ -2384,8 +2435,16 @@ static struct device *ehea_register_port
 		goto out_unreg_of_dev;
 	}
 
+	ret = ehea_driver_sysfs_add(&port->ofdev.dev, &ehea_driver.driver);
+	if (ret) {
+		ehea_error("failed to register sysfs driver link");
+		goto out_rem_dev_file;
+	}
+
 	return &port->ofdev.dev;
 
+out_rem_dev_file:
+	device_remove_file(&port->ofdev.dev, &dev_attr_log_port_id);
 out_unreg_of_dev:
 	of_device_unregister(&port->ofdev);
 out:
@@ -2394,6 +2453,7 @@ out:
 
 static void ehea_unregister_port(struct ehea_port *port)
 {
+	ehea_driver_sysfs_remove(&port->ofdev.dev, &ehea_driver.driver);
 	device_remove_file(&port->ofdev.dev, &dev_attr_log_port_id);
 	of_device_unregister(&port->ofdev);
 }
@@ -2838,21 +2898,6 @@ static int check_module_parm(void)
 	return ret;
 }
 
-static struct of_device_id ehea_device_table[] = {
-	{
-		.name = "lhea",
-		.compatible = "IBM,lhea",
-	},
-	{},
-};
-
-static struct ibmebus_driver ehea_driver = {
-	.name = "ehea",
-	.id_table = ehea_device_table,
-	.probe = ehea_probe_adapter,
-	.remove = ehea_remove,
-};
-
 int __init ehea_module_init(void)
 {
 	int ret;

^ permalink raw reply

* [PATCH 2/2] ehea: fix for dlpar support
From: Thomas Klein @ 2007-04-26  9:56 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, Stefan Roscher, linux-ppc, Marcus Eder

Certain resources may only be allocated when first logical port is available,
and must be removed when last logical port has been removed.

Signed-off-by: Thomas Klein <tklein@de.ibm.com>
---


This patch applies on top of the netdev upstream branch for 2.6.22


diff -Nurp -X dontdiff linux-2.6.21/drivers/net/ehea/ehea.h patched_kernel/drivers/net/ehea/ehea.h
--- linux-2.6.21/drivers/net/ehea/ehea.h	2007-04-26 11:21:33.000000000 +0200
+++ patched_kernel/drivers/net/ehea/ehea.h	2007-04-26 11:23:16.000000000 +0200
@@ -39,7 +39,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"ehea"
-#define DRV_VERSION	"EHEA_0057"
+#define DRV_VERSION	"EHEA_0058"
 
 #define EHEA_MSG_DEFAULT (NETIF_MSG_LINK | NETIF_MSG_TIMER \
 	| NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR)
diff -Nurp -X dontdiff linux-2.6.21/drivers/net/ehea/ehea_main.c patched_kernel/drivers/net/ehea/ehea_main.c
--- linux-2.6.21/drivers/net/ehea/ehea_main.c	2007-04-26 11:21:33.000000000 +0200
+++ patched_kernel/drivers/net/ehea/ehea_main.c	2007-04-26 11:23:16.000000000 +0200
@@ -2130,6 +2130,28 @@ static int ehea_clean_all_portres(struct
 	return ret;
 }
 
+static void ehea_remove_adapter_mr (struct ehea_adapter *adapter)
+{
+	int i;
+
+	for (i=0; i < EHEA_MAX_PORTS; i++)
+		if (adapter->port[i])
+			return;
+
+	ehea_rem_mr(&adapter->mr);
+}
+
+static int ehea_add_adapter_mr (struct ehea_adapter *adapter)
+{
+	int i;
+
+	for (i=0; i < EHEA_MAX_PORTS; i++)
+		if (adapter->port[i])
+			return 0;
+
+	return ehea_reg_kernel_mr(adapter, &adapter->mr);
+}
+
 static int ehea_up(struct net_device *dev)
 {
 	int ret, i;
@@ -2580,7 +2602,6 @@ static int ehea_setup_ports(struct ehea_
 	struct device_node *eth_dn = NULL;
 
 	u32 *dn_log_port_id;
-	int port_setup_ok = 0;
 	int i = 0;
 
 	lhea_dn = adapter->ebus_dev->ofdev.node;
@@ -2594,6 +2615,12 @@ static int ehea_setup_ports(struct ehea_
 			continue;
 		}
 
+		if (ehea_add_adapter_mr(adapter)) {
+			ehea_error("creating MR failed");
+			of_node_put(eth_dn);
+			return -EIO;
+		}
+
 		adapter->port[i] = ehea_setup_single_port(adapter,
 							  *dn_log_port_id,
 							  eth_dn);
@@ -2601,18 +2628,13 @@ static int ehea_setup_ports(struct ehea_
 			ehea_info("%s -> logical port id #%d",
 				  adapter->port[i]->netdev->name,
 				  *dn_log_port_id);
+		else
+			ehea_remove_adapter_mr(adapter);
+
 		i++;
 	};
 
-	/* Check for succesfully set up ports */
-	for (i = 0; i < EHEA_MAX_PORTS; i++)
-		if (adapter->port[i])
-			port_setup_ok++;
-
-	if (port_setup_ok)
-		return 0;	/* At least some ports are setup correctly */
-
-	return -EINVAL;
+	return 0;
 }
 
 static struct device_node *ehea_get_eth_dn(struct ehea_adapter *adapter,
@@ -2664,6 +2686,11 @@ static ssize_t ehea_probe_port(struct de
 		return -EINVAL;
 	}
 
+	if (ehea_add_adapter_mr(adapter)) {
+		ehea_error("creating MR failed");
+		return -EIO;
+	}
+
 	port = ehea_setup_single_port(adapter, logical_port_id, eth_dn);
 
 	of_node_put(eth_dn);
@@ -2677,8 +2704,10 @@ static ssize_t ehea_probe_port(struct de
 
 		ehea_info("added %s (logical port id=%d)", port->netdev->name,
 			  logical_port_id);
-	} else
+	} else {
+		ehea_remove_adapter_mr(adapter);
 		return -EIO;
+	}
 
 	return (ssize_t) count;
 }
@@ -2713,6 +2742,8 @@ static ssize_t ehea_remove_port(struct d
 		return -EINVAL;
 	}
 
+	ehea_remove_adapter_mr(adapter);
+
 	return (ssize_t) count;
 }
 
@@ -2773,18 +2804,13 @@ static int __devinit ehea_probe_adapter(
 
 	dev->ofdev.dev.driver_data = adapter;
 
-	ret = ehea_reg_kernel_mr(adapter, &adapter->mr);
-	if (ret) {
-		dev_err(&dev->ofdev.dev, "reg_mr_adapter failed\n");
-		goto out_free_ad;
-	}
 
 	/* initialize adapter and ports */
 	/* get adapter properties */
 	ret = ehea_sense_adapter_attr(adapter);
 	if (ret) {
 		dev_err(&dev->ofdev.dev, "sense_adapter_attr failed: %d", ret);
-		goto out_free_res;
+		goto out_free_ad;
 	}
 
 	adapter->neq = ehea_create_eq(adapter,
@@ -2792,7 +2818,7 @@ static int __devinit ehea_probe_adapter(
 	if (!adapter->neq) {
 		ret = -EIO;
 		dev_err(&dev->ofdev.dev, "NEQ creation failed");
-		goto out_free_res;
+		goto out_free_ad;
 	}
 
 	tasklet_init(&adapter->neq_tasklet, ehea_neq_tasklet,
@@ -2837,9 +2863,6 @@ out_free_irq:
 out_kill_eq:
 	ehea_destroy_eq(adapter->neq);
 
-out_free_res:
-	ehea_rem_mr(&adapter->mr);
-
 out_free_ad:
 	kfree(adapter);
 out:
@@ -2865,7 +2888,7 @@ static int __devexit ehea_remove(struct 
 	tasklet_kill(&adapter->neq_tasklet);
 
 	ehea_destroy_eq(adapter->neq);
-	ehea_rem_mr(&adapter->mr);
+	ehea_remove_adapter_mr(adapter);
 	kfree(adapter);
 	return 0;
 }

^ permalink raw reply

* Re: [PATCH v2] [2.6.22] pasemi: cpufreq driver
From: Johannes Berg @ 2007-04-26 10:26 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev, egor, paulus, arnd, cpufreq
In-Reply-To: <20070426053700.GA23922@lixom.net>

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, 2007-04-26 at 00:37 -0500, Olof Johansson wrote:
> 
> +       policy->cur = pas_freqs[cur_astate].frequency;
> +       policy->cpus = cpu_possible_map; 

That doesn't seem right.

Either, all your processors scale along each other in which case you
should use cpu_online_map here, or they scale each on their own in which
case you just set a single bit here.

The generic code works like this:
CPU 0 is brought online and cpufreq initialised for it
CPU 1 is brought online and cpufreq initialised for it. if cpus includes
more than a single bit, cpufreq is linked to the first other CPU in
policy->cpus and then cpufreq for CPU1 is deinitialised again.

We have the same bug on powermac but for some reason the patch to fix it
that I posted a long time ago (look for "powermac: fix G5-cpufreq for
cpu on/offline") wasn't applied yet. Look at it though, it includes a
comment on what needs to be done.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* Re: [PATCH 7/13] powerpc: Add arch/powerpc mv64x60 MPSC platform data setup
From: Arnd Bergmann @ 2007-04-26 11:24 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <20070426055716.GB2030@xyzzy.farnsworth.org>

On Thursday 26 April 2007, Dale Farnsworth wrote:
> > This looks wrong to me. See drivers/serial/of_serial.c to find how we d=
o it for
> > 8250 compatible serial ports. You should probably just add your serial =
port
> > stuff in there as well, instead of doing your own scanning of the devic=
e tree.
>=20
> Unfortunately, this hardware is very much non-8250 compatible.

That shouldn't matter much. The driver is not 8250 specific by itself, it's=
 just
that right now it doesn't know about any other chips.

If you find that there is more code that needs to be added than what is alr=
eady
in there, you could also create a new one based on of_serial for your port.

I can understand why you want to keep using the platform_device here, and i=
n the
other places, but if you do then please use platform_device_register from an
of_platform_driver probe function instead of platform_device_register_simple
so you can set the parent to the of_device.

> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0pdev =3D platform_device_register_s=
imple(MPSC_CTLR_NAME, i, r, 5);
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (IS_ERR(pdev)) {
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0err =3D PTR=
_ERR(pdev);
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0goto ret_no=
de_put;
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0}
> >=20
> > Now this really needs some explanation.
> >=20
> > Why the heck do you have a platform device that gets its resources from
> > nonstandard properties of a serial port?
>=20
> There is an existing mpsc driver usable on both MIPS and powerpc platforms
> that requires these non-standard properties.

Ok, I see where some of the limitations come from. However, instead of
introducing the new "sdma" and "brg" properties, why not just add the
register ranges to the "reg" property, like other drivers do?

	Arnd <><

^ permalink raw reply

* Re: [PATCH 10/13] powerpc: Add arch/powerpc mv64x60 PCI setup
From: Arnd Bergmann @ 2007-04-26 11:39 UTC (permalink / raw)
  To: Dale Farnsworth; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070426063315.GE2030@xyzzy.farnsworth.org>

On Thursday 26 April 2007, Dale Farnsworth wrote:
> > /* The probing of PCI controllers from of_platform is currently
> > =A0* 64 bits only, mostly due to gratuitous differences between
> > =A0* the 32 and 64 bits PCI code on PowerPC and the 32 bits one
> > =A0* lacking some bits needed here.
> > =A0*/
>=20
> Is this comment incorrect?
>=20
> I agree that this is more code duplication than I like, and we could
> benefit from some refactoring. =A0However, I find 15 other places in
> arch/powerpc that largely duplicate this pci initialization code.
> That doesn't make a 16th right, but at least I'm in good company.=20

Yes, you're right. I was assuming that the code had been ported to
32 bit already, which was incorrect.

While I would very much like you (or someone else) to make it work,
it should not be a prerequisite to get your code merged in 2.6.22.

The one remaining comment I have about this one is that IMHO it
should be a separate file, mv64x60-pci.c instead of being
in the same file as the interrupt controller and other code.

	Arnd <><

^ permalink raw reply

* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Sergei Shtylyov @ 2007-04-26 12:36 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linuxppc-embedded
In-Reply-To: <4630611C.4060902@grandegger.com>

Hello.

Wolfgang Grandegger wrote:

>>>>>>>> are there already some thought or even example code on how to 
>>>>>>>> define Flash memory and MTD partitions in the DTS. This would 
>>>>>>>> avoid the ackward MTD partitioning via static structure or boot 
>>>>>>>> line argument and could be done in a generic way.

>>>>>>>     See Documentation/powerpc/booting-without-of.txt and 
>>>>>>> drivers/mtd/physmap_of.c for what's been already done.

>>>>>>    I meant drivers/mtd/maps/physmap_of.c

>>>>> I had a closer look to this driver. Unfortunately, it can not 
>>>>> handle yet  multiple FLASH banks. This could be achieved with an 
>>>>> additional property "bank_count". What do you think?

>>   Please use hyphen, not underscore in property names ("device_type" 
>> seems an only exception from this rule).
 
> Yes, I know and I also wondered why an "_" is used for the device type.

   Ask the OF spec. developers. :-)
 
>>>>  I'm not sure what banks you're talking about -- note that this 
>>>> driver is equivalen in functionality to the plain 'physmap' driver 
>>>> and there's inherited "bank-width" property that handles flash banking.

>>> Yes, I know. Nevertheless the driver cannot handle flash regions 
>>> composed of more than on FLASH bank because we need to probe every 
>>> bank. Search for "BANK" in drivers/mtd/maps" to understand what I mean.

>>   Hm, from what I could see, those banks are separate maps registered 
>> to MTD subsys separately.
>> What's the point of adding a count property when we simply can define 
>> multiple "rom" devices?
 
> To complete this thread, physmap_of can map _one_ FLASH region composed 
> of one or more FLASH banks. But it currently cannot map more than _one_ 
> FLASH region.

   I wodner why?! :-O

> It would be nice to extend it for multiple FLASH 
> definitions (could code it when time permits).

   I'm seeing nothing that prevents it from working now -- it should register as much "rom" devices as you supply via the device tree.

> Wolfgang.

MBR, Sergei

^ permalink raw reply

* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Wolfgang Grandegger @ 2007-04-26 12:59 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-embedded
In-Reply-To: <46309CD9.1050403@ru.mvista.com>

Sergei Shtylyov wrote:
> Hello.
> 
> Wolfgang Grandegger wrote:
> 
>>>>>>>>> are there already some thought or even example code on how to 
>>>>>>>>> define Flash memory and MTD partitions in the DTS. This would 
>>>>>>>>> avoid the ackward MTD partitioning via static structure or boot 
>>>>>>>>> line argument and could be done in a generic way.
> 
>>>>>>>>     See Documentation/powerpc/booting-without-of.txt and 
>>>>>>>> drivers/mtd/physmap_of.c for what's been already done.
> 
>>>>>>>    I meant drivers/mtd/maps/physmap_of.c
> 
>>>>>> I had a closer look to this driver. Unfortunately, it can not 
>>>>>> handle yet  multiple FLASH banks. This could be achieved with an 
>>>>>> additional property "bank_count". What do you think?
> 
>>>   Please use hyphen, not underscore in property names ("device_type" 
>>> seems an only exception from this rule).
> 
>> Yes, I know and I also wondered why an "_" is used for the device type.
> 
>   Ask the OF spec. developers. :-)
> 
>>>>>  I'm not sure what banks you're talking about -- note that this 
>>>>> driver is equivalen in functionality to the plain 'physmap' driver 
>>>>> and there's inherited "bank-width" property that handles flash 
>>>>> banking.
> 
>>>> Yes, I know. Nevertheless the driver cannot handle flash regions 
>>>> composed of more than on FLASH bank because we need to probe every 
>>>> bank. Search for "BANK" in drivers/mtd/maps" to understand what I mean.
> 
>>>   Hm, from what I could see, those banks are separate maps registered 
>>> to MTD subsys separately.
>>> What's the point of adding a count property when we simply can define 
>>> multiple "rom" devices?
> 
>> To complete this thread, physmap_of can map _one_ FLASH region 
>> composed of one or more FLASH banks. But it currently cannot map more 
>> than _one_ FLASH region.
> 
>   I wodner why?! :-O

I know as well ;-).

>> It would be nice to extend it for multiple FLASH definitions (could 
>> code it when time permits).
> 
>   I'm seeing nothing that prevents it from working now -- it should 
> register as much "rom" devices as you supply via the device tree.

OK, I see, for each registered direct-mapped flash resource, 
of_physmap_probe() will be called.

Thanks.

Wolfgang.

^ permalink raw reply

* mpc5200 and spi bus
From: Asier Llano Palacios @ 2007-04-26 13:50 UTC (permalink / raw)
  To: linuxppc-embedded; +Cc: tlopez, Sylvain Munaut, Aitor Arzuaga

I've seen some comments about drivers for SPI of the MPC5200. I'm not
sure about it, but is there anything functional about it?. I want to
know if there is anything already done, in order to use it or improve
it. I need a driver of the SPI bus, so if it is not implemented at all
we should contribute it.

Thank you,
Asier Llano=20
=20
----------------------------------------- PLEASE NOTE =
-------------------------------------------
This message, along with any attachments, may be confidential or legally =
privileged.=20
It is intended only for the named person(s), who is/are the only =
authorized recipients.
If this message has reached you in error, kindly destroy it without =
review and notify the sender immediately.
Thank you for your help.
=B5SysCom uses virus scanning software but excludes any liability for =
viruses contained in any attachment.
=20
------------------------------------ ROGAMOS LEA ESTE TEXTO =
-------------------------------
Este mensaje y sus anexos pueden contener informaci=F3n confidencial y/o =
con derecho legal.=20
Est=E1 dirigido =FAnicamente a la/s persona/s o entidad/es rese=F1adas =
como =FAnico destinatario autorizado.
Si este mensaje le hubiera llegado por error, por favor elim=EDnelo sin =
revisarlo ni reenviarlo y notif=EDquelo inmediatamente al remitente. =
Gracias por su colaboraci=F3n. =20
=B5SysCom utiliza software antivirus, pero no se hace responsable de los =
virus contenidos en los ficheros anexos.

^ permalink raw reply

* Re: [PATCH 9/13] powerpc: Add arch/powerpc mv64x60 I2C platform data setup
From: Dale Farnsworth @ 2007-04-26 14:19 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <200704261100.25650.arnd@arndb.de>

On Thu, Apr 26, 2007 at 11:00:25AM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Dale Farnsworth wrote:
> > 
> > > Ok, I see your point there. But after looking at the i2c and net drivers,
> > > I believe that they can easily be split into an architecture dependent
> > > part that is either an of_platform_driver or a platform_driver, and
> > > a common part that does not know about either of these.
> > 
> > Oh, it's certainly possible, but it doesn't seem desirable to me. Why
> > should the drivers carry the burden of supporting both platform_driver
> > and of_platform_driver interfaces?
> 
> The reason is that platform_devices are for stuff that fundamentally cannot
> be probed but has to be hardcoded in some place.
> 
> The point about the of device tree is that it allows you to probe this
> kind of device. This means you get automatic module loading based on the
> device tree, and that the devices show up in sane locations in /sys.

I understand the benefits of the DT; that's not the issue.

Here we have platform devices common to MIPS and PowerPC platforms.
The drivers must continue to support the platform_driver interface
for MIPS platforms.  The question is, where should we put the glue
that transforms the DT info into the platform_driver format?

You seem to suggest putting the ethernet-related glue into
drivers/net/mv643xx_eth.c.  That's bogus, IMHO. The base driver
shouldn't have to accommodate every arch-specific interface.
(I know OF isn't strictly arch-specific, but it's far from universal.)
I put this glue into arch/powerpc/sysdev/mv64x60.c. I still don't see
the benefit of moving it into the drivers.

-Dale

^ permalink raw reply

* U-boot + Linux 2.6 + Avnet FX12 MM (based on ML403 config)
From: Guillaume Berthelom @ 2007-04-26 14:25 UTC (permalink / raw)
  To: linuxppc-embedded


Hi,

I'm trying to port Linux 2.6 on Avnet FX12 MM board with U-Boot and the help
of xilinx ML403 config.

I have generate my costum xparameters.h file with EDK 8.2i.
I use ELDK 4.1 and Linux 2.6.20.x.
I have modified ml403.h u-boot's config file to adapt it for my board and I
have no problem to boot u-boot.
I use the right board_info structure from <asm/ppcboot.h> and not the one in
xilinx_ml403.h.

When I load the zImage directly with xilinx xmd debugger, the kernel start
without problem (if I don't have KGDB or SERIAL_TEXT_DEBUG option in my
kernel config). But when I try to load the uImage with u-boot, the kernel
start but crach befor I can see something on output.

The kernel start because I debug it and the platform_init is ok, setup_arch
too and it crach after ppc4xx_setup_arch at  printk(KERN_INFO "Xilinx ML403
Reference System (Virtex-4 FX)\n") line  when the kernel access in printk.c.

I suppose the serial init have problem but I have no idea what is the cause
of the problem. I use the opb_uart16550 IP in my Xilinx design and the
associat drivers in u-boot and linux.

Thanks,
Guillaume Berthelom

-- 
View this message in context: http://www.nabble.com/U-boot-%2B-Linux-2.6-%2B-Avnet-FX12-MM-%28based-on-ML403-config%29-tf3651846.html#a10201070
Sent from the linuxppc-embedded mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH 7/13] powerpc: Add arch/powerpc mv64x60 MPSC platform data setup
From: Dale Farnsworth @ 2007-04-26 14:30 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <200704261324.20226.arnd@arndb.de>

On Thu, Apr 26, 2007 at 01:24:19PM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Dale Farnsworth wrote:
> > > This looks wrong to me. See drivers/serial/of_serial.c to find how we do it for
> > > 8250 compatible serial ports. You should probably just add your serial port
> > > stuff in there as well, instead of doing your own scanning of the device tree.
> > 
> > Unfortunately, this hardware is very much non-8250 compatible.
> 
> That shouldn't matter much. The driver is not 8250 specific by itself,
> it's just that right now it doesn't know about any other chips.

Hmm, I wouldn't call that a driver, I'd call it OF interface
glue used to register the driver.  I guess I could put the
platform_device_register call for the mpsc driver in that file.
But I think that will increase, rather than reduce complexity.

> 
> If you find that there is more code that needs to be added than what is already
> in there, you could also create a new one based on of_serial for your port.
> 
> I can understand why you want to keep using the platform_device here, and in the
> other places, but if you do then please use platform_device_register from an
> of_platform_driver probe function instead of platform_device_register_simple
> so you can set the parent to the of_device.
> 
> > > > +???????????pdev = platform_device_register_simple(MPSC_CTLR_NAME, i, r, 5);
> > > > +???????????if (IS_ERR(pdev)) {
> > > > +???????????????????err = PTR_ERR(pdev);
> > > > +???????????????????goto ret_node_put;
> > > > +???????????}

Can you coerce your mailer to stop munging tabs?

> > > Now this really needs some explanation.
> > > 
> > > Why the heck do you have a platform device that gets its resources from
> > > nonstandard properties of a serial port?
> > 
> > There is an existing mpsc driver usable on both MIPS and powerpc platforms
> > that requires these non-standard properties.
> 
> Ok, I see where some of the limitations come from. However, instead of
> introducing the new "sdma" and "brg" properties, why not just add the
> register ranges to the "reg" property, like other drivers do?

The sdma and brg are not simply properties of the serial device, these
nodes represent hardware modules that may be shared by multiple drivers.

-Dale

^ permalink raw reply

* Re: [PATCH] powerpc: Add EDAC platform devices for 85xx
From: Kumar Gala @ 2007-04-26 14:31 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linuxppc-dev, bluesmoke-devel, david
In-Reply-To: <20070426003748.GA30730@blade.az.mvista.com>


On Apr 25, 2007, at 7:37 PM, Dave Jiang wrote:

> Add memory controller and l2-cache controller entries in the dts  
> files for 85xx
> platforms. Also adding code to create the platform devices used by  
> EDAC drivers
> to claim the resources in order to access the error registers and  
> interrupts.
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>
> Removed explicit phandles as suggested by David Gibson.
>
>  arch/powerpc/boot/dts/mpc8540ads.dts |   19 ++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   19 ++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   21 ++++++-
>  arch/powerpc/sysdev/fsl_soc.c        |  115 +++++++++++++++++++++++ 
> +++++++++++
>  4 files changed, 173 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/ 
> boot/dts/mpc8540ads.dts
> index f261d64..63c274d 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";

I'm concerned this is too generic.

> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;

Why the 32-bit?

> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/ 
> boot/dts/mpc8548cds.dts
> index b2b2200..f8a9f3f 100644
> --- a/arch/powerpc/boot/dts/mpc8548cds.dts
> +++ b/arch/powerpc/boot/dts/mpc8548cds.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/ 
> boot/dts/mpc8560ads.dts
> index 1f2afe9..43ee847 100644
> --- a/arch/powerpc/boot/dts/mpc8560ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8560ads.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00000200>;
>  		bus-frequency = <13ab6680>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		mdio@24520 {
>  			device_type = "mdio";
>  			compatible = "gianfar";
> @@ -110,7 +129,7 @@
>  			#address-cells = <3>;
>  			compatible = "85xx";
>  			device_type = "pci";
> -			reg = <8000 400>;
> +			reg = <8000 1000>;
>  			clock-frequency = <3f940aa>;
>  			interrupt-map-mask = <f800 0 0 7>;
>  			interrupt-map = <
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> fsl_soc.c
> index 8a123c7..a0beb8b 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -1103,3 +1103,118 @@ err:
>  arch_initcall(cpm_smc_uart_of_init);
>
>  #endif /* CONFIG_8xx */
> +
> +/* platform device setup for EDAC */

Why not have the EDAC code for 85xx use of_device?

> +#ifdef CONFIG_PPC_85xx
> +static int __init mpc85xx_mc_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	int ret = 0;
> +
> +	memset(r, 0, sizeof(r));
> +
> +	np = of_find_compatible_node(NULL, "mem-ctrl", "85xx");
> +	if (!np)
> +		return 0;
> +
> +	ret = of_address_to_resource(np, 0, &r[0]);
> +	if (ret)
> +		goto err;
> +
> +	of_irq_to_resource(np, 0, &r[1]);
> +
> +	of_node_put(np);
> +
> +	pdev = platform_device_register_simple("mpc85xx_mc_err", 0, r, 2);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-mem-ctrl setup failed\n");
> +	return 0;
> +}
> +arch_initcall(mpc85xx_mc_err_init);
> +
> +static int __init mpc85xx_l2_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	int ret = 0;
> +
> +	memset(r, 0, sizeof(r));
> +
> +	np = of_find_compatible_node(NULL, "l2-cache", "85xx");
> +	if (!np)
> +		return 0;
> +
> +	ret = of_address_to_resource(np, 0, &r[0]);
> +	if (ret)
> +		goto err;
> +	
> +	/* we only need access to the error registers */
> +	r[0].start += 0xe00;
> +
> +	of_irq_to_resource(np, 0, &r[1]);
> +
> +	of_node_put(np);
> +
> +	pdev = platform_device_register_simple("mpc85xx_l2_err", 0, r, 2);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-l2 setup failed\n");
> +	return 0;
> +}
> +arch_initcall(mpc85xx_l2_err_init);
> +
> +static int __init mpc85xx_pci_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np = NULL;
> +	struct platform_device *pdev;
> +	int i;
> +	int ret = 0;
> +
> +	for (i = 0;
> +		(np = of_find_compatible_node(np, "pci", "85xx"));
> +		i++) {
> +		memset(r, 0, sizeof(r));
> +
> +		ret = of_address_to_resource(np, 0, &r[0]);
> +		if (ret)
> +			goto err;
> +	
> +		/* we only need access to the error registers */
> +		r[0].start += 0xe00;
> +
> +		of_irq_to_resource(np, 0, &r[1]);
> +
> +		pdev = platform_device_register_simple("mpc85xx_pci_err",
> +				i, r, 2);
> +		if (IS_ERR(pdev)) {
> +			ret = PTR_ERR(pdev);
> +			goto err;
> +		}
> +
> +	}
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-pci setup failed\n");
> +	return ret;
> +}
> +arch_initcall(mpc85xx_pci_err_init);
> +
> +#endif /* CONFIG_PPC_85xx */

^ permalink raw reply

* Re: MPC8544 Watchdog Timer
From: Kumar Gala @ 2007-04-26 14:35 UTC (permalink / raw)
  To: vinay hegde; +Cc: linuxppc-embedded
In-Reply-To: <571811.46559.qm@web8406.mail.in.yahoo.com>


On Apr 26, 2007, at 3:43 AM, vinay hegde wrote:

> Hi,
>
> I am working with Freescale MPC8544E board. I need an
> information related to the functionality of hardware
> watchdog provided by MPC8544 processor.
>
> Here is the question:
>
> On watchdog timeout, whether the CPU can reset itself
> _or_ does it assert a hardware HRESET_REQ signal to
> some external hardware requesting for reset?
>
> Please let me know the inputs.

This a question best asked to Freescale support. However, I believe  
the processor will end up asserting HRESET_REQ and expects external  
board logic to reset things.

The processor core will reset itself as part of this process.

- k

^ permalink raw reply

* Re: [PATCH 10/13] powerpc: Add arch/powerpc mv64x60 PCI setup
From: Dale Farnsworth @ 2007-04-26 14:42 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <200704261339.18640.arnd@arndb.de>

On Thu, Apr 26, 2007 at 01:39:18PM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Dale Farnsworth wrote:
> > > /* The probing of PCI controllers from of_platform is currently
> > > ?* 64 bits only, mostly due to gratuitous differences between
> > > ?* the 32 and 64 bits PCI code on PowerPC and the 32 bits one
> > > ?* lacking some bits needed here.
> > > ?*/
> > 
> > Is this comment incorrect?
> > 
> > I agree that this is more code duplication than I like, and we could
> > benefit from some refactoring. ?However, I find 15 other places in
> > arch/powerpc that largely duplicate this pci initialization code.
> > That doesn't make a 16th right, but at least I'm in good company. 
> 
> Yes, you're right. I was assuming that the code had been ported to
> 32 bit already, which was incorrect.
> 
> While I would very much like you (or someone else) to make it work,
> it should not be a prerequisite to get your code merged in 2.6.22.
> 
> The one remaining comment I have about this one is that IMHO it
> should be a separate file, mv64x60-pci.c instead of being
> in the same file as the interrupt controller and other code.

Ah, the debate continues.  I remember 4 or 5 years ago there was
campaign to consolidate the xxxx-pci.c files into the core support
files.  I have no strong preference.  More opinions are welcome.

Thanks,
-Dale

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox