LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] [POWERPC] Fix CONFIG_SMP=n build error on ppc64
From: Kamalesh Babulal @ 2007-11-11  4:25 UTC (permalink / raw)
  To: Olof Johansson
  Cc: linuxppc-dev, Ingo Molnar, paulus, linux-kernel, Kamalesh Babulal
In-Reply-To: <20071110205929.GA4845@lixom.net>

On Sat, Nov 10, 2007 at 02:59:29PM -0600, Olof Johansson wrote:
> [POWERPC] Fix CONFIG_SMP=n build error
> 
> The patch "KVM: fix !SMP build error" change the way smp_call_function()
> actually uses the passed in function names on non-SMP builds. So
> previously it was never caught that the function passed in was never
> actually defined.
> 
> This causes a build error on ppc64_defconfig + CONFIG_SMP=n:
> 
> arch/powerpc/mm/tlb_64.c: In function 'pgtable_free_now':
> arch/powerpc/mm/tlb_64.c:71: error: 'pte_free_smp_sync' undeclared (first use in this function)
> arch/powerpc/mm/tlb_64.c:71: error: (Each undeclared identifier is reported only once
> arch/powerpc/mm/tlb_64.c:71: error: for each function it appears in.)
> 
> So we need to define it even if CONFIG_SMP is off. Either that or ifdef
> out the smp_call_function() call, but that's ugly.
Hi,

Thanks, the patch fixes the build failure.

Tested-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Signed-off-by: Olof Johansson <olof@lixom.net>

diff --git a/arch/powerpc/mm/tlb_64.c b/arch/powerpc/mm/tlb_64.c
index eafbca5..e2d867c 100644
--- a/arch/powerpc/mm/tlb_64.c
+++ b/arch/powerpc/mm/tlb_64.c
@@ -54,12 +54,10 @@ unsigned long pte_freelist_forced_free;
      ((PAGE_SIZE - sizeof(struct pte_freelist_batch)) \ 
        / sizeof(pgtable_free_t))

-#ifdef CONFIG_SMP
 static void pte_free_smp_sync(void *arg)
 { 
      /* Do nothing, just ensure we sync with all CPUs */
 } 
-#endif

 /* This is only called when we are critically out of memory
  * (and fail to get a page in pte_free_tlb).

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.

^ permalink raw reply related

* Re: [PATCH] Avoid unpaired stwcx. on some processors
From: Benjamin Herrenschmidt @ 2007-11-11  4:46 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20071109231927.GA19415@lixom.net>


> Seems like a "better" (but still ugly) workaround would be to create a
> _new_ reservation to a RA that's unavailable to any userspace process,
> so that they could never do a successful store to it. That way you would
> have stray reservations, but never dangling stwcx:es. No?

Many processors don't compare the reservation address locally. If
there's any valid reservation held by that processor, a subsequent
stwcx. will always succeed. That would make you scheme dangerous :-)

Ben.

^ permalink raw reply

* Re: IRQs in i2c-mpc.c
From: Benjamin Herrenschmidt @ 2007-11-11  6:26 UTC (permalink / raw)
  To: Kumar Gala; +Cc: PowerPC dev list
In-Reply-To: <A4797E92-9FCF-484A-B6AA-9385913AD465@kernel.crashing.org>


On Sat, 2007-11-10 at 17:18 -0600, Kumar Gala wrote:
> On Nov 10, 2007, at 5:16 PM, Jon Smirl wrote:
> 
> > On 11/10/07, Kumar Gala <galak@kernel.crashing.org> wrote:
> >> Looking at the current driver it looks like we could get ride of if
> >> check since the previous code checked the return of  
> >> platform_get_irq().
> >
> > The code was a snippet from the larger patch that is converting i2c
> > from being a platform driver to a of_platform driver.
> >
> > The question is, what to do about a missing IRQ tag in the device tree
> > or a IRQ of zero. What is an error and what should be ignored, etc.
> 
> I think the lack of an IRQ in the device tree should be an error.  If  
> the IRQ value is zero, than its zero.

virq 0 is always illegal so if platform_get_irq() returns 0, it can be
safely treated as an error or the absence of irq, on both powerpc and
x86.

Ben.

^ permalink raw reply

* Re: IRQs in i2c-mpc.c
From: Benjamin Herrenschmidt @ 2007-11-11  6:27 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711101544v178e29cer4e53577c011e89f7@mail.gmail.com>


> irq_of_parse_and_map() returns NO_IRQ when the irq parameter is missing.
> 
> This API appears to be broken. In asm-powerpc/irq.h NO_IRQ is defined
> as (0). There is no way to tell an error in the attribute from a valid
> attribute selecting interrupt zero.

No, it is not broken. Interrupt numbers in arch/powerpc are virtually
mapped and 0 is never a legal value, thus NO_IRQ is 0. This was done in
part because Linus wanted it that way.

The hardware number can perfectly be 0 though, that's a different thing,
and has nothing to do with what you pass to request_irq().

Ben.

^ permalink raw reply

* Re: [PATCH] Avoid unpaired stwcx. on some processors
From: Olof Johansson @ 2007-11-11  7:14 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1194756365.21340.32.camel@pasglop>

On Sun, Nov 11, 2007 at 03:46:05PM +1100, Benjamin Herrenschmidt wrote:
> 
> > Seems like a "better" (but still ugly) workaround would be to create a
> > _new_ reservation to a RA that's unavailable to any userspace process,
> > so that they could never do a successful store to it. That way you would
> > have stray reservations, but never dangling stwcx:es. No?
> 
> Many processors don't compare the reservation address locally. If
> there's any valid reservation held by that processor, a subsequent
> stwcx. will always succeed. That would make you scheme dangerous :-)

Yeah, I had missed that arch detail. Becky already explained it in her
reply, but thanks for doing it again. :)


-Olof

^ permalink raw reply

* Re: IRQs in i2c-mpc.c
From: Jon Smirl @ 2007-11-11 15:34 UTC (permalink / raw)
  To: benh; +Cc: PowerPC dev list
In-Reply-To: <1194762471.21340.39.camel@pasglop>

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > irq_of_parse_and_map() returns NO_IRQ when the irq parameter is missing.
> >
> > This API appears to be broken. In asm-powerpc/irq.h NO_IRQ is defined
> > as (0). There is no way to tell an error in the attribute from a valid
> > attribute selecting interrupt zero.
>
> No, it is not broken. Interrupt numbers in arch/powerpc are virtually
> mapped and 0 is never a legal value, thus NO_IRQ is 0. This was done in
> part because Linus wanted it that way.
>
> The hardware number can perfectly be 0 though, that's a different thing,
> and has nothing to do with what you pass to request_irq().

Is the only error return from irq_of_parse_and_map() NO_IRQ? or can we
get standard negative returns? I started tracing out the returns from
irq_of_parse_and_map() but there are a lot of them.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* [PATCH] powerpc: Add EXPORT_SYMBOL for symbols required by fs_enet and cpm_uart
From: Jochen Friedrich @ 2007-11-11 17:01 UTC (permalink / raw)
  To: linuxppc-embedded@ozlabs.org; +Cc: paulus, linux-kernel

fs_enet and cpm_uart need symbols from commproc.c (for CPM1) or
cpm2_common.c. Add EXPORT_SYMBOL for cpmp, cpm_setbrg and cpm2_immr, so
the drivers can be compiled as modules.

  Building modules, stage 2.
  MODPOST 5 modules
ERROR: "cpm2_immr" [drivers/net/fs_enet/fs_enet.ko] undefined!
ERROR: "cpmp" [drivers/net/fs_enet/fs_enet.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

Signed-off-by: Jochen Friedrich <jochen@scram.de>
---
 arch/powerpc/sysdev/commproc.c    |    3 +++
 arch/powerpc/sysdev/cpm2_common.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index f6a6378..d5a0dcf 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -51,6 +51,8 @@ static void m8xx_cpm_dpinit(void);
 static uint host_buffer; /* One page of host buffer */
 static uint host_end;    /* end + 1 */
 cpm8xx_t __iomem *cpmp;  /* Pointer to comm processor space */
+EXPORT_SYMBOL(cpmp);
+
 immap_t __iomem *mpc8xx_immr;
 static cpic8xx_t __iomem *cpic_reg;
 
@@ -302,6 +304,7 @@ cpm_setbrg(uint brg, uint rate)
 		out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
 		             CPM_BRG_EN | CPM_BRG_DIV16);
 }
+EXPORT_SYMBOL(cpm_setbrg);
 
 #ifndef CONFIG_PPC_CPM_NEW_BINDING
 /*
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index 859362f..4ed5df6 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -51,11 +51,13 @@ static void cpm2_dpinit(void);
 #endif
 
 cpm_cpm2_t __iomem *cpmp; /* Pointer to comm processor space */
+EXPORT_SYMBOL(cpmp);
 
 /* We allocate this here because it is used almost exclusively for
  * the communication processor devices.
  */
 cpm2_map_t __iomem *cpm2_immr;
+EXPORT_SYMBOL(cpm2_immr);
 
 #define CPM_MAP_SIZE	(0x40000)	/* 256k - the PQ3 reserve this amount
 					   of space for CPM as it is larger
@@ -117,6 +119,7 @@ cpm_setbrg(uint brg, uint rate)
 
 	cpm2_unmap(bp);
 }
+EXPORT_SYMBOL(cpm_setbrg);
 
 /* This function is used to set high speed synchronous baud rate
  * clocks.
-- 
1.5.3.5

^ permalink raw reply related

* [PATCH] powerpc: Add support for PORTA sor and PORTB odr registers
From: Jochen Friedrich @ 2007-11-11 17:08 UTC (permalink / raw)
  To: linuxppc-embedded@ozlabs.org; +Cc: paulus, linux-kernel

PORTA has an so register and PORTB had an odr register, as well.
However, the PORTB odr register is only 16bit.

Signed-off-by: Jochen Friedrich <jochen@scram.de>
---
 arch/powerpc/sysdev/commproc.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index f6a6378..a9f5fcf 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -438,6 +438,13 @@ static void cpm1_set_pin32(int port, int pin, int flags)
 	else
 		clrbits32(&iop->par, pin);
 
+	if (port == CPM_PORTB) {
+		if (flags & CPM_PIN_OPENDRAIN)
+			setbits16(&mpc8xx_immr->im_cpm.cp_pbodr, pin);
+		else
+			clrbits16(&mpc8xx_immr->im_cpm.cp_pbodr, pin);
+	}
+
 	if (port == CPM_PORTE) {
 		if (flags & CPM_PIN_SECONDARY)
 			setbits32(&iop->sor, pin);
@@ -471,7 +478,7 @@ static void cpm1_set_pin16(int port, int pin, int flags)
 	else
 		clrbits16(&iop->par, pin);
 
-	if (port == CPM_PORTC) {
+	if ((port == CPM_PORTA) || (port == CPM_PORTC)) {
 		if (flags & CPM_PIN_SECONDARY)
 			setbits16(&iop->sor, pin);
 		else
-- 
1.5.3.5

^ permalink raw reply related

* Re: [linux-pm] [RFC] powermac: proper sleep management
From: Pavel Machek @ 2007-11-11 13:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list, linux-pm, David Woodhouse
In-Reply-To: <1194523729.6294.18.camel@johannes.berg>

Hi!

> This adds platform_suspend_ops for PMU based machines, directly in
> the PMU driver. This finally allows suspending via /sys/power/state
> on powerbooks.

Thanks for doing this!

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [RFC/PATCH] powerpc: Add support for 8xx style watchdog
From: Jochen Friedrich @ 2007-11-11 20:10 UTC (permalink / raw)
  To: linuxppc-embedded@ozlabs.org

Hi,

this is an attempt to port the 8xx watchdog driver to ARC=powerpc. As the watchdog seems to
be similar on pq1 / pq2 and pq2pro platforms (except from the divider value), one driver
should be enough to support the whole family. Am i correct with this assumption?

Thanks,
Jochen
---
 arch/powerpc/platforms/8xx/mpc885ads_setup.c |    5 +
 arch/powerpc/sysdev/Makefile                 |    3 +
 arch/powerpc/sysdev/pq_wdt.c                 |  195 ++++++++++++++++++++++
 arch/powerpc/sysdev/pq_wdt.h                 |   27 +++
 drivers/watchdog/Kconfig                     |   13 ++-
 drivers/watchdog/Makefile                    |    1 +
 drivers/watchdog/pq_wdt.c                    |  222 ++++++++++++++++++++++++++
 7 files changed, 465 insertions(+), 1 deletions(-)
 create mode 100644 arch/powerpc/sysdev/pq_wdt.c
 create mode 100644 arch/powerpc/sysdev/pq_wdt.h
 create mode 100644 drivers/watchdog/pq_wdt.c

diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
index 2cf1b6a..a686747 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
+++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
@@ -41,6 +41,7 @@
 #include <asm/udbg.h>
 
 #include <sysdev/commproc.h>
+#include <sysdev/pq_wdt.h>
 
 static u32 __iomem *bcsr, *bcsr5;
 
@@ -246,6 +247,10 @@ static void __init mpc885ads_setup_arch(void)
 	m8xx_pcmcia_ops.hw_ctrl = pcmcia_hw_setup;
 	m8xx_pcmcia_ops.voltage_set = pcmcia_set_voltage;
 #endif
+
+#if defined(CONFIG_PQ_WDT) || defined(CONFIG_PQ_WDT_MODULE)
+	pq_wdt_init();
+#endif
 }
 
 static int __init mpc885ads_probe(void)
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 99a77d7..84f190e 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -35,5 +35,8 @@ obj-$(CONFIG_CPM)		+= cpm_common.o
 obj-$(CONFIG_CPM2)		+= cpm2_common.o cpm2_pic.o
 obj-$(CONFIG_PPC_DCR)		+= dcr.o
 obj-$(CONFIG_8xx)		+= mpc8xx_pic.o commproc.o
+ifneq ($(CONFIG_PQ_WDT),)
+obj-y				+= pq_wdt.o
+endif
 obj-$(CONFIG_UCODE_PATCH)	+= micropatch.o
 endif
diff --git a/arch/powerpc/sysdev/pq_wdt.c b/arch/powerpc/sysdev/pq_wdt.c
new file mode 100644
index 0000000..10a196f
--- /dev/null
+++ b/arch/powerpc/sysdev/pq_wdt.c
@@ -0,0 +1,195 @@
+/*
+ * pq_wdt.c - Freescale PowerQUICC watchdog driver
+ *
+ * Author: Florian Schirmer <jolt@tuxbox.org>
+ *
+ * 2002 (c) Florian Schirmer <jolt@tuxbox.org> This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ * 2007 (c) Jochen Friedrich <jochen@scram.de> ported to ARCH=powerpc and
+ * extended to be useful on any Power QUICC 1/2/2pro which have the same
+ * style of watchdog.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <asm/irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+
+#include "pq_wdt.h"
+
+struct pq_wdt {
+	__be32 res0;
+	__be32 swcrr; /* System watchdog control register */
+	__be32 swcnr; /* System watchdog count register */
+	u8 res1[2];
+	__be16 swsrr; /* System watchdog service register */
+};
+
+static int wdt_timeout;
+static int wdt_freq;
+static struct pq_wdt __iomem *wdt_reg;
+static int wdt_scale;
+static int wdt_timerun;
+static DEFINE_SPINLOCK(wdt_spinlock);
+
+void pq_wdt_reset(void)
+{
+	if (!wdt_reg)
+		return;
+
+	spin_lock(&wdt_spinlock);
+	out_be16(&wdt_reg->swsrr, 0x556c);	/* write magic1 */
+	out_be16(&wdt_reg->swsrr, 0xaa39);	/* write magic2 */
+	spin_unlock(&wdt_spinlock);
+}
+EXPORT_SYMBOL(pq_wdt_reset);
+
+static void wdt_timer_func(unsigned long data);
+
+static struct timer_list wdt_timer =
+	TIMER_INITIALIZER(wdt_timer_func, 0, 0);
+
+void pq_wdt_stop_timer(void)
+{
+	spin_lock(&wdt_spinlock);
+	if (wdt_timerun)
+		del_timer(&wdt_timer);
+	wdt_timerun = 0;
+	spin_unlock(&wdt_spinlock);
+}
+EXPORT_SYMBOL(pq_wdt_stop_timer);
+
+void pq_wdt_install_timer(void)
+{
+	pq_wdt_reset();
+	spin_lock(&wdt_spinlock);
+	if (!wdt_timerun) {
+		wdt_timer.expires = jiffies + (HZ/2);
+		add_timer(&wdt_timer);
+	}
+	wdt_timerun = 1;
+	spin_unlock(&wdt_spinlock);
+}
+EXPORT_SYMBOL(pq_wdt_install_timer);
+
+static void wdt_timer_func(unsigned long data)
+{
+	pq_wdt_install_timer();
+}
+
+int pq_wdt_get_timeout(void)
+{
+	return wdt_timeout / wdt_freq;
+}
+EXPORT_SYMBOL(pq_wdt_get_timeout);
+
+static int wdt_readparam(void)
+{
+	u32 swcrr;
+
+	wdt_timeout = 0;
+
+	swcrr = in_be32(&wdt_reg->swcrr);
+
+	if (!(swcrr & SWCRR_SWEN)) {
+		printk(KERN_NOTICE "pq_wdt: wdt disabled (SWCRR: 0x%08X)\n",
+		       swcrr);
+		return -EINVAL;
+	}
+
+	pq_wdt_reset();
+
+	printk(KERN_NOTICE
+	       "pq_wdt: active wdt found (SWTC: 0x%04X, SWP: 0x%01X)\n",
+	       (swcrr >> 16), swcrr & 0x07);
+
+	wdt_timeout = (swcrr >> 16) & 0xFFFF;
+
+	if (!wdt_timeout)
+		wdt_timeout = 0xFFFF;
+
+	if (swcrr & SWCRR_SWPR)
+		wdt_timeout *= wdt_scale;
+
+	return 0;
+}
+
+int pq_wdt_setup(int value)
+{
+	if (!wdt_reg)
+		return -ENODEV;
+		
+	out_be32(&wdt_reg->swcrr, value);
+	return wdt_readparam();
+}
+EXPORT_SYMBOL(pq_wdt_setup);
+
+int __init pq_wdt_init_timer(void)
+{
+	if (wdt_reg) {
+		pq_wdt_install_timer();
+		return 0;
+	} else
+		return -ENODEV;
+}
+arch_initcall(pq_wdt_init_timer);
+
+int pq_wdt_init(void)
+{
+	struct device_node *np, *soc;
+	int ret;
+	const u32 *data;
+
+	if (wdt_reg)
+		return 0;
+
+	wdt_scale = 2048;
+	np = of_find_compatible_node(NULL, NULL, "fsl,pq1-wdt");
+	if (np == NULL)
+		np = of_find_compatible_node(NULL, NULL, "fsl,pq2-wdt");
+	if (np == NULL) {
+		np = of_find_compatible_node(NULL, NULL, "fsl,pq2pro-wdt");
+		wdt_scale = 65536;
+	}
+	if (np == NULL) {
+		printk(KERN_ERR "Could not find fsl,pq1/2/2pro-wdt node\n");
+		return -ENODEV;
+	}
+
+	soc = of_find_node_by_type(NULL, "soc");
+	if (!soc) {
+		printk(KERN_ERR "Could not find soc node\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
+	data = of_get_property(soc, "bus-frequency", NULL);
+	if (!data) {
+		of_node_put(soc);
+		printk(KERN_ERR "Could not find bus-frequency in soc node\n");
+		ret = -ENODEV;
+		goto out;
+	}
+	of_node_put(soc);
+	wdt_freq = *data;
+
+	wdt_reg = of_iomap(np, 0);
+	if (wdt_reg == NULL) {
+		printk(KERN_ERR "Could not iomap wdt\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = wdt_readparam();
+out:
+	of_node_put(np);
+	return ret;
+}
+EXPORT_SYMBOL(pq_wdt_init);
diff --git a/arch/powerpc/sysdev/pq_wdt.h b/arch/powerpc/sysdev/pq_wdt.h
new file mode 100644
index 0000000..6f9e085
--- /dev/null
+++ b/arch/powerpc/sysdev/pq_wdt.h
@@ -0,0 +1,27 @@
+/*
+ * Author: Florian Schirmer <jolt@tuxbox.org>
+ *
+ * 2002 (c) Florian Schirmer <jolt@tuxbox.org> This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ * 2007 (c) Jochen Friedrich <jochen@scram.de> ported to ARCH=powerpc and
+ * extended to be useful on any Power QUICC 1/2/2pro which have the same
+ * style of watchdog.
+ */
+#ifndef _POWERPC_SYSDEV_PQ_WDT_H
+#define _POWERPC_SYSDEV_PQ_WDT_H
+
+#define SWCRR_SWEN 0x00000004 /* Watchdog Enable bit. */
+#define SWCRR_SWRI 0x00000002 /* Software Watchdog Reset/Interrupt Select bit.*/
+#define SWCRR_SWPR 0x00000001 /* Software Watchdog Counter Prescale bit. */
+
+extern int pq_wdt_get_timeout(void);
+extern void pq_wdt_reset(void);
+extern void pq_wdt_install_timer(void);
+extern void pq_wdt_stop_timer(void);
+extern int pq_wdt_setup(int);
+extern int pq_wdt_init(void);
+
+#endif				/* _POWERPC_SYSDEV_PQ_WDT_H */
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 2792bc1..79ee351 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -626,7 +626,18 @@ config MPC5200_WDT
 
 config 8xx_WDT
 	tristate "MPC8xx Watchdog Timer"
-	depends on 8xx
+	depends on 8xx && ! OF
+
+config PQ_WDT
+	tristate "Power QUICC Watchdog Timer"
+	depends on (8xx || PPC_82xx || PPC_83xx) && OF
+	default y
+	help
+	  Watchdog driver for Power QUICC 1/2/2pro style watchdog drivers.
+	  You should really select this unless your boot loader turns
+	  off the watchdog. As the watchdog is turned on by default and
+	  can be turned on/off only once after reboot, your board won't
+	  run otherwise. Say 'M' if unsure.
 
 config 83xx_WDT
 	tristate "MPC83xx Watchdog Timer"
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 7d9e573..bdcf3f3 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_AR7_WDT) += ar7_wdt.o
 
 # POWERPC Architecture
 obj-$(CONFIG_8xx_WDT) += mpc8xx_wdt.o
+obj-$(CONFIG_PQ_WDT) += pq_wdt.o
 obj-$(CONFIG_MPC5200_WDT) += mpc5200_wdt.o
 obj-$(CONFIG_83xx_WDT) += mpc83xx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
diff --git a/drivers/watchdog/pq_wdt.c b/drivers/watchdog/pq_wdt.c
new file mode 100644
index 0000000..e03daa3
--- /dev/null
+++ b/drivers/watchdog/pq_wdt.c
@@ -0,0 +1,222 @@
+/*
+ * pq_wdt.c - Power QUICC watchdog userspace interface
+ *
+ * Author: Florian Schirmer <jolt@tuxbox.org>
+ *
+ * 2002 (c) Florian Schirmer <jolt@tuxbox.org> This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ * 2007 (c) Jochen Friedrich <jochen@scram.de> renamed to pq_wdt.c and
+ * extended to be useful on any Power QUICC 1/2/2pro which have the same
+ * style of watchdog.
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+#include <linux/of_platform.h>
+#include <asm/uaccess.h>
+#include <asm/io.h>
+#include <sysdev/pq_wdt.h>
+
+static unsigned long wdt_opened;
+static int wdt_status;
+
+static u16 timeout = 0xffff;
+module_param(timeout, ushort, 0);
+MODULE_PARM_DESC(timeout,
+	"Watchdog timeout in ticks. (0<timeout<65536, default=65535");
+
+static int reset = 1;
+module_param(reset, bool, 0);
+MODULE_PARM_DESC(reset,
+	"Watchdog Interrupt/Reset Mode. 0 = interrupt, 1 = reset (default)");
+
+static int prescale = 1;
+
+static void pq_wdt_handler_disable(void)
+{
+	pq_wdt_stop_timer();
+
+	pr_debug("pq_wdt: keep-alive handler deactivated\n");
+}
+
+static void pq_wdt_handler_enable(void)
+{
+	pq_wdt_install_timer();
+
+	pr_debug("pq_wdt: keep-alive handler activated\n");
+}
+
+static int pq_wdt_open(struct inode *inode, struct file *file)
+{
+	u32 tmp = SWCRR_SWEN;
+
+	if (test_and_set_bit(0, &wdt_opened))
+		return -EBUSY;
+
+	pq_wdt_reset();
+
+	if (prescale)
+		tmp |= SWCRR_SWPR;
+	if (reset)
+		tmp |= SWCRR_SWRI;
+
+	tmp |= timeout << 16;
+
+	if (pq_wdt_setup(tmp))
+		return -EBUSY;
+
+#if defined(CONFIG_WATCHDOG_NOWAYOUT)
+	/* Once we start the watchdog we can't stop it */
+	__module_get(THIS_MODULE);
+#endif
+
+	pq_wdt_handler_disable();
+
+	return nonseekable_open(inode, file);
+}
+
+static int pq_wdt_release(struct inode *inode, struct file *file)
+{
+	pq_wdt_reset();
+
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+	pq_wdt_handler_enable();
+#endif
+
+	clear_bit(0, &wdt_opened);
+
+	return 0;
+}
+
+static ssize_t pq_wdt_write(struct file *file, const char *data, size_t len,
+				loff_t *ppos)
+{
+	if (len)
+		pq_wdt_reset();
+
+	return len;
+}
+
+static int pq_wdt_ioctl(struct inode *inode, struct file *file,
+			    unsigned int cmd, unsigned long arg)
+{
+	int timeout;
+	static struct watchdog_info info = {
+		.options = WDIOF_KEEPALIVEPING,
+		.firmware_version = 0,
+		.identity = "PQ watchdog",
+	};
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		if (copy_to_user((void *)arg, &info, sizeof(info)))
+			return -EFAULT;
+		break;
+
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		if (put_user(wdt_status, (int *)arg))
+			return -EFAULT;
+		wdt_status &= ~WDIOF_KEEPALIVEPING;
+		break;
+
+	case WDIOC_GETTEMP:
+		return -EOPNOTSUPP;
+
+	case WDIOC_SETOPTIONS:
+		return -EOPNOTSUPP;
+
+	case WDIOC_KEEPALIVE:
+		pq_wdt_reset();
+		wdt_status |= WDIOF_KEEPALIVEPING;
+		break;
+
+	case WDIOC_SETTIMEOUT:
+		return -EOPNOTSUPP;
+
+	case WDIOC_GETTIMEOUT:
+		timeout = pq_wdt_get_timeout();
+		if (put_user(timeout, (int *)arg))
+			return -EFAULT;
+		break;
+
+	default:
+		return -ENOTTY;
+	}
+
+	return 0;
+}
+
+static const struct file_operations pq_wdt_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.write = pq_wdt_write,
+	.ioctl = pq_wdt_ioctl,
+	.open = pq_wdt_open,
+	.release = pq_wdt_release,
+};
+
+static struct miscdevice pq_wdt_miscdev = {
+	.minor = WATCHDOG_MINOR,
+	.name = "watchdog",
+	.fops = &pq_wdt_fops,
+};
+
+static int __devinit pq_wdt_probe(struct of_device *op, const struct of_device_id *match)
+{
+	int ret;
+
+	ret = pq_wdt_init();
+	if (ret)
+		return ret;
+
+	pq_wdt_handler_enable();
+
+	return misc_register(&pq_wdt_miscdev);
+}
+
+static int __devexit pq_wdt_remove(struct of_device *op)
+{
+	misc_deregister(&pq_wdt_miscdev);
+	return 0;
+}
+
+static struct of_device_id pq_wdt_match[] = {
+	{ .compatible = "fsl,pq1-wdt", },
+	{ .compatible = "fsl,pq2-wdt", },
+	{ .compatible = "fsl,pq2pro-wdt", },
+	{},
+};
+
+static struct of_platform_driver pq_wdt_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pq-wdt",
+	.match_table	= pq_wdt_match,
+	.probe		= pq_wdt_probe,
+	.remove		= pq_wdt_remove,
+};
+
+static int __init pq_wdt_moduleinit(void)
+{
+	return of_register_platform_driver(&pq_wdt_driver);
+}
+
+static void __exit pq_wdt_moduleexit(void)
+{
+	of_unregister_platform_driver(&pq_wdt_driver);
+}
+
+module_init(pq_wdt_moduleinit);
+module_exit(pq_wdt_moduleexit);
+
+MODULE_AUTHOR("Florian Schirmer <jolt@tuxbox.org>");
+MODULE_DESCRIPTION("PQ watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.5.3.5

^ permalink raw reply related

* Re: IRQs in i2c-mpc.c
From: Benjamin Herrenschmidt @ 2007-11-11 20:44 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711110734m7d36a235t72f5112e59cbe0d5@mail.gmail.com>


> Is the only error return from irq_of_parse_and_map() NO_IRQ? or can we
> get standard negative returns? I started tracing out the returns from
> irq_of_parse_and_map() but there are a lot of them.

NP_IRQ is the only error return.

Ben.

^ permalink raw reply

* Re: [RFC] PMU: remove dead code
From: Benjamin Herrenschmidt @ 2007-11-11 20:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list
In-Reply-To: <1194523239.6294.7.camel@johannes.berg>


On Thu, 2007-11-08 at 13:00 +0100, Johannes Berg wrote:
> Some code in via-pmu.c is never compiled because of "compile options"
> within the file. Remove the code completely.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  drivers/macintosh/via-pmu.c |   42 +-----------------------------------------
>  1 file changed, 1 insertion(+), 41 deletions(-)
> 
> --- everything.orig/drivers/macintosh/via-pmu.c	2007-11-08 11:47:42.332847765 +0100
> +++ everything/drivers/macintosh/via-pmu.c	2007-11-08 11:48:37.192845813 +0100
> @@ -65,9 +65,7 @@
>  #include "via-pmu-event.h"
>  
>  /* Some compile options */
> -#undef SUSPEND_USES_PMU
>  #define DEBUG_SLEEP
> -#undef HACKED_PCI_SAVE
>  
>  /* Misc minor number allocated for /dev/pmu */
>  #define PMU_MINOR		154
> @@ -1255,9 +1253,7 @@ void
>  pmu_suspend(void)
>  {
>  	unsigned long flags;
> -#ifdef SUSPEND_USES_PMU
> -	struct adb_request *req;
> -#endif
> +
>  	if (!via)
>  		return;
>  	
> @@ -1275,17 +1271,10 @@ pmu_suspend(void)
>  		via_pmu_interrupt(0, NULL);
>  		spin_lock_irqsave(&pmu_lock, flags);
>  		if (!adb_int_pending && pmu_state == idle && !req_awaiting_reply) {
> -#ifdef SUSPEND_USES_PMU
> -			pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, 0);
> -			spin_unlock_irqrestore(&pmu_lock, flags);
> -			while(!req.complete)
> -				pmu_poll();
> -#else /* SUSPEND_USES_PMU */
>  			if (gpio_irq >= 0)
>  				disable_irq_nosync(gpio_irq);
>  			out_8(&via[IER], CB1_INT | IER_CLR);
>  			spin_unlock_irqrestore(&pmu_lock, flags);
> -#endif /* SUSPEND_USES_PMU */
>  			break;
>  		}
>  	} while (1);
> @@ -1306,18 +1295,11 @@ pmu_resume(void)
>  		return;
>  	}
>  	adb_int_pending = 1;
> -#ifdef SUSPEND_USES_PMU
> -	pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, pmu_intr_mask);
> -	spin_unlock_irqrestore(&pmu_lock, flags);
> -	while(!req.complete)
> -		pmu_poll();
> -#else /* SUSPEND_USES_PMU */
>  	if (gpio_irq >= 0)
>  		enable_irq(gpio_irq);
>  	out_8(&via[IER], CB1_INT | IER_SET);
>  	spin_unlock_irqrestore(&pmu_lock, flags);
>  	pmu_poll();
> -#endif /* SUSPEND_USES_PMU */
>  }
>  
>  /* Interrupt data could be the result data from an ADB cmd */
> @@ -1743,14 +1725,10 @@ pmu_present(void)
>   * PCI devices which may get powered off when we sleep.
>   */
>  static struct pci_save {
> -#ifndef HACKED_PCI_SAVE
>  	u16	command;
>  	u16	cache_lat;
>  	u16	intr;
>  	u32	rom_address;
> -#else
> -	u32	config[16];
> -#endif	
>  } *pbook_pci_saves;
>  static int pbook_npci_saves;
>  
> @@ -1796,16 +1774,10 @@ pbook_pci_save(void)
>  			pci_dev_put(pd);
>  			return;
>  		}
> -#ifndef HACKED_PCI_SAVE
>  		pci_read_config_word(pd, PCI_COMMAND, &ps->command);
>  		pci_read_config_word(pd, PCI_CACHE_LINE_SIZE, &ps->cache_lat);
>  		pci_read_config_word(pd, PCI_INTERRUPT_LINE, &ps->intr);
>  		pci_read_config_dword(pd, PCI_ROM_ADDRESS, &ps->rom_address);
> -#else
> -		int i;
> -		for (i=1;i<16;i++)
> -			pci_read_config_dword(pd, i<<4, &ps->config[i]);
> -#endif
>  		++ps;
>  	}
>  }
> @@ -1824,17 +1796,6 @@ pbook_pci_restore(void)
>  	int j;
>  
>  	while ((pd = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pd)) != NULL) {
> -#ifdef HACKED_PCI_SAVE
> -		int i;
> -		if (npci-- == 0) {
> -			pci_dev_put(pd);
> -			return;
> -		}
> -		ps++;
> -		for (i=2;i<16;i++)
> -			pci_write_config_dword(pd, i<<4, ps->config[i]);
> -		pci_write_config_dword(pd, 4, ps->config[1]);
> -#else
>  		if (npci-- == 0)
>  			return;
>  		ps++;
> @@ -1858,7 +1819,6 @@ pbook_pci_restore(void)
>  			pci_write_config_word(pd, PCI_COMMAND, ps->command);
>  			break;
>  		}
> -#endif	
>  	}
>  }
>  
> 

^ permalink raw reply

* Re: [RFC] PMU: don't lock_kernel()
From: Benjamin Herrenschmidt @ 2007-11-11 20:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list
In-Reply-To: <1194523283.6294.9.camel@johannes.berg>


On Thu, 2007-11-08 at 13:01 +0100, Johannes Berg wrote:
> I see nothing that this lock_kernel() actually protects against
> so remove it.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  drivers/macintosh/via-pmu.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> --- everything.orig/drivers/macintosh/via-pmu.c	2007-11-08 11:48:37.192845813 +0100
> +++ everything/drivers/macintosh/via-pmu.c	2007-11-08 11:48:48.292846681 +0100
> @@ -33,7 +33,6 @@
>  #include <linux/adb.h>
>  #include <linux/pmu.h>
>  #include <linux/cuda.h>
> -#include <linux/smp_lock.h>
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
>  #include <linux/pm.h>
> @@ -2436,7 +2435,6 @@ pmu_release(struct inode *inode, struct 
>  	struct pmu_private *pp = file->private_data;
>  	unsigned long flags;
>  
> -	lock_kernel();
>  	if (pp != 0) {
>  		file->private_data = NULL;
>  		spin_lock_irqsave(&all_pvt_lock, flags);
> @@ -2450,7 +2448,6 @@ pmu_release(struct inode *inode, struct 
>  
>  		kfree(pp);
>  	}
> -	unlock_kernel();
>  	return 0;
>  }
>  
> 

^ permalink raw reply

* Re: [RFC] PMU: replace information ioctls and schedule for removal
From: Benjamin Herrenschmidt @ 2007-11-11 20:55 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list
In-Reply-To: <1194523351.6294.11.camel@johannes.berg>


On Thu, 2007-11-08 at 13:02 +0100, Johannes Berg wrote:
> This patch adds sysfs attributes to the PMU to allow getting
> the information on the PMU type and whether adb is present from
> there instead of via the ioctl. The ioctl is made optional and
> scheduled for removal.

Acked-by:  Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

While there, maybe also expose the PMU version ?

> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  Documentation/feature-removal-schedule.txt |    8 ++++++
>  drivers/macintosh/Kconfig                  |   11 ++++++++
>  drivers/macintosh/via-pmu.c                |   36 +++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+)
> 
> --- everything.orig/drivers/macintosh/via-pmu.c	2007-11-08 11:48:48.292846681 +0100
> +++ everything/drivers/macintosh/via-pmu.c	2007-11-08 11:48:53.112861818 +0100
> @@ -2533,10 +2533,12 @@ pmu_ioctl(struct inode * inode, struct f
>  #endif /* CONFIG_INPUT_ADBHID */
>  #endif /* CONFIG_PMAC_BACKLIGHT_LEGACY */
>  
> +#ifdef CONFIG_DEPRECATED_PMU_INFO_IOCTLS
>  	case PMU_IOC_GET_MODEL:
>  	    	return put_user(pmu_kind, argp);
>  	case PMU_IOC_HAS_ADB:
>  		return put_user(pmu_has_adb, argp);
> +#endif
>  	}
>  	return error;
>  }
> @@ -2680,6 +2682,39 @@ static int pmu_sys_resume(struct sys_dev
>  
>  #endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
>  
> +static ssize_t show_kind(struct sys_device *sysdev, char *buf)
> +{
> +	return sprintf(buf, "%d\n", pmu_kind);
> +}
> +
> +static ssize_t show_has_adb(struct sys_device *sysdev, char *buf)
> +{
> +	return sprintf(buf, "%d\n", pmu_has_adb);
> +}
> +
> +static SYSDEV_ATTR(kind, 0444, show_kind, NULL);
> +static SYSDEV_ATTR(has_adb, 0444, show_has_adb, NULL);
> +
> +int pmu_sys_add(struct sys_device *sysdev)
> +{
> +	int err;
> +
> +	err = sysdev_create_file(sysdev, &attr_kind);
> +	if (err)
> +		goto out;
> +
> +	err = sysdev_create_file(sysdev, &attr_has_adb);
> +	if (err)
> +		goto out_remove_kind;
> +
> +	return 0;
> +
> + out_remove_kind:
> +	sysdev_remove_file(sysdev, &attr_kind);
> + out:
> +	return err;
> +}
> +
>  static struct sysdev_class pmu_sysclass = {
>  	set_kset_name("pmu"),
>  };
> @@ -2693,6 +2728,7 @@ static struct sysdev_driver driver_pmu =
>  	.suspend	= &pmu_sys_suspend,
>  	.resume		= &pmu_sys_resume,
>  #endif /* CONFIG_PM_SLEEP && CONFIG_PPC32 */
> +	.add		= &pmu_sys_add,
>  };
>  
>  static int __init init_pmu_sysfs(void)
> --- everything.orig/Documentation/feature-removal-schedule.txt	2007-11-08 11:47:39.502844564 +0100
> +++ everything/Documentation/feature-removal-schedule.txt	2007-11-08 11:48:53.122870607 +0100
> @@ -342,3 +342,11 @@ Why:	This driver has been marked obsolet
>  Who:	Stephen Hemminger <shemminger@linux-foundation.org>
>  
>  ---------------------------
> +
> +What:	/dev/pmu information ioctls
> +When:	January 2010
> +Files:	drivers/macintosh/via-pmu.c
> +Why:	The PMU information can be obtained from sysfs now.
> +Who:	Johannes Berg <johannes@sipsolutions.net>
> +
> +---------------------------
> --- everything.orig/drivers/macintosh/Kconfig	2007-11-08 11:47:39.422844727 +0100
> +++ everything/drivers/macintosh/Kconfig	2007-11-08 11:48:53.122870607 +0100
> @@ -87,6 +87,17 @@ config ADB_PMU
>  	  this device; you should do so if your machine is one of those
>  	  mentioned above.
>  
> +config DEPRECATED_PMU_INFO_IOCTLS
> +	bool "Support deprecated PMU information ioctl"
> +	depends on ADB_PMU
> +	default y
> +	help
> +	  The PMU ioctl supports getting information on the type of PMU and
> +	  whether an ADB is present. This information is also available in
> +	  sysfs so this ioctl is no longer needed.
> +
> +	  If in doubt, say Y even if you will not use the ioctl.
> +
>  config ADB_PMU_LED
>  	bool "Support for the Power/iBook front LED"
>  	depends on ADB_PMU
> 

^ permalink raw reply

* Re: [RFC] powermac: proper sleep management
From: Benjamin Herrenschmidt @ 2007-11-11 20:59 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linuxppc-dev list, linux-pm, David Woodhouse, Paul Mackerras
In-Reply-To: <1194523729.6294.18.camel@johannes.berg>


> Just thought I'd send out the patch again, it has changed quite a bit
> since last time because I cleaned up the code, made it depend on
> CONFIG_SUSPEND, made the ioctl backward compatibility optional and some
> other bits.
> 
> The feared freezer vs. fuse sync deadlock is no longer present in 2.6.24
> because the sync is done before the freezer so maybe the patch stands a
> chance now.
> 
> Scott: FYI, here's the use case for the ppc_md irq suspend/resume hooks.

Looks good to me, +/- a couple of things:

 - We _REALLY_ want the freezer to be optional and not enabled by
default on PowerPC. Maybe make it a compile option ?

 - Don't remove the pmu_polled_request() debug code. It's very useful
for debugging and I don't want to have to re-invent it.

Plus I also need to test it on some exotic hardware :-)

Cheers,
Ben.

^ permalink raw reply

* Gianfar ethernet device
From: Jon Smirl @ 2007-11-11 23:11 UTC (permalink / raw)
  To: PowerPC dev list, Kumar Gala

Which platforms have the Gianfar ethernet device? The gfar code in
fsl_soc.c is getting built on the mpc5200 and it doesn't have the
device.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Gianfar ethernet device
From: Jon Smirl @ 2007-11-11 23:30 UTC (permalink / raw)
  To: PowerPC dev list, Kumar Gala
In-Reply-To: <9e4733910711111511v36aa3504s3b55438698a4b4a8@mail.gmail.com>

Does this patch add the right ifdefs in fsl_soc.c to make these
drivers build on their proper platforms? As an experiment I have
disabled platform bus on the mpc5200 and I'm only using of_platform
bus. Turning off platform bus is exposing a lot of code that is
getting built into my mpc5200 kernel that is supporting devices not
available on the platform.

A few more fixups in some other drivers and I should have mpc5200
working without platform bus.

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index d6ef264..7017510 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -130,6 +130,7 @@ u32 get_baudrate(void)
 EXPORT_SYMBOL(get_baudrate);
 #endif /* CONFIG_CPM2 */

+#if defined(CONFIG_PPC_83xx) || defined(CONFIG_PPC_85xx) ||
defined(CONFIG_PPC_86xx)
 static int __init gfar_mdio_of_init(void)
 {
 	struct device_node *np;
@@ -317,6 +318,7 @@ err:
 }

 arch_initcall(gfar_of_init);
+#endif

 #ifdef CONFIG_PPC_83xx
 static int __init mpc83xx_wdt_init(void)
@@ -379,7 +381,7 @@ nodev:
 }

 arch_initcall(mpc83xx_wdt_init);
-#endif
+

 static enum fsl_usb2_phy_modes determine_usb_phy(const char *phy_type)
 {
@@ -542,6 +544,7 @@ err:
 }

 arch_initcall(fsl_usb_of_init);
+#endif

 #ifndef CONFIG_PPC_CPM_NEW_BINDING
 #ifdef CONFIG_CPM2
@@ -1085,6 +1088,7 @@ arch_initcall(cpm_smc_uart_of_init);
 #endif /* CONFIG_8xx */
 #endif /* CONFIG_PPC_CPM_NEW_BINDING */

+#if defined(CONFIG_PPC_83xx)
 int __init fsl_spi_init(struct spi_board_info *board_infos,
 			unsigned int num_board_infos,
 			void (*activate_cs)(u8 cs, u8 polarity),
@@ -1174,6 +1178,7 @@ err:

 	return spi_register_board_info(board_infos, num_board_infos);
 }
+#endif

 #if defined(CONFIG_PPC_85xx) || defined(CONFIG_PPC_86xx)
 static __be32 __iomem *rstcr;

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply related

* pcspkr device, pnpPNP,100
From: Jon Smirl @ 2007-11-11 23:35 UTC (permalink / raw)
  To: PowerPC dev list

Which platform does pcspkr device, pnpPNP,100 belong to, amiga? I'd
like to IFDEF add_pcspkr(void) in setup-common.c for the right
platform.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: pcspkr device, pnpPNP,100
From: Benjamin Herrenschmidt @ 2007-11-12  0:03 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711111535w6ffdfa97x4bed07eb84d4ab13@mail.gmail.com>


On Sun, 2007-11-11 at 18:35 -0500, Jon Smirl wrote:
> Which platform does pcspkr device, pnpPNP,100 belong to, amiga? I'd
> like to IFDEF add_pcspkr(void) in setup-common.c for the right
> platform.

#ifdef is evil. What about multiplatform kernels ? Just test for the
presence of the device in the device-tree (use a platform driver maybe)

Ben.

^ permalink raw reply

* Re: Gianfar ethernet device
From: Benjamin Herrenschmidt @ 2007-11-12  0:04 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711111530g1f50d6e6hfaa76b4f051b5887@mail.gmail.com>


On Sun, 2007-11-11 at 18:30 -0500, Jon Smirl wrote:
> Does this patch add the right ifdefs in fsl_soc.c to make these
> drivers build on their proper platforms? As an experiment I have
> disabled platform bus on the mpc5200 and I'm only using of_platform
> bus. Turning off platform bus is exposing a lot of code that is
> getting built into my mpc5200 kernel that is supporting devices not
> available on the platform.
> 
> A few more fixups in some other drivers and I should have mpc5200
> working without platform bus.

That sort of per-platform ifdefs is just WRONG.

On the other hand, if you feel that such common code deserves not being
built on all platforms, then what you can do is define a Kconfig option,
such as CONFIG_PPC_CAN_HAVE_GIANFAR, that gets select'ed by the
platforms that can have a Gianfar and use -that- for the ifdef.

Ben.

^ permalink raw reply

* Re: pcspkr device, pnpPNP,100
From: Jon Smirl @ 2007-11-12  0:07 UTC (permalink / raw)
  To: benh; +Cc: PowerPC dev list
In-Reply-To: <1194825784.18185.0.camel@pasglop>

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Sun, 2007-11-11 at 18:35 -0500, Jon Smirl wrote:
> > Which platform does pcspkr device, pnpPNP,100 belong to, amiga? I'd
> > like to IFDEF add_pcspkr(void) in setup-common.c for the right
> > platform.
>
> #ifdef is evil. What about multiplatform kernels ? Just test for the
> presence of the device in the device-tree (use a platform driver maybe)

This code should be in a device driver so that it can be selected with
Kconfig. Can we #ifdef it now as a hint? I could make it a device
driver but I don't have any hardware to test it on.

I'm trying to get the mpc5200 kernel to build with platform bus turned
off (only of_platform bus). I'm almost there, just a couple more odd
places to patch up.


>
> Ben.
>
>
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: pcspkr device, pnpPNP,100
From: Benjamin Herrenschmidt @ 2007-11-12  0:09 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711111607i35294d3ja43e1c616a3427bd@mail.gmail.com>


On Sun, 2007-11-11 at 19:07 -0500, Jon Smirl wrote:
> 
> This code should be in a device driver so that it can be selected with
> Kconfig. Can we #ifdef it now as a hint? I could make it a device
> driver but I don't have any hardware to test it on.
> 
> I'm trying to get the mpc5200 kernel to build with platform bus turned
> off (only of_platform bus). I'm almost there, just a couple more odd
> places to patch up.

Just don't do platform ifdef's, ever.

What you can do however is Kconfig options that get selected by
platforms that need a given service, and then use that as an ifdef for
that service.

A bit like how we select what interrupt controller drivers to built-in
for example.

Ben.

^ permalink raw reply

* Re: pcspkr device, pnpPNP,100
From: Jon Smirl @ 2007-11-12  0:18 UTC (permalink / raw)
  To: benh; +Cc: PowerPC dev list
In-Reply-To: <1194826191.18185.6.camel@pasglop>

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Sun, 2007-11-11 at 19:07 -0500, Jon Smirl wrote:
> >
> > This code should be in a device driver so that it can be selected with
> > Kconfig. Can we #ifdef it now as a hint? I could make it a device
> > driver but I don't have any hardware to test it on.
> >
> > I'm trying to get the mpc5200 kernel to build with platform bus turned
> > off (only of_platform bus). I'm almost there, just a couple more odd
> > places to patch up.
>
> Just don't do platform ifdef's, ever.
>
> What you can do however is Kconfig options that get selected by
> platforms that need a given service, and then use that as an ifdef for
> that service.
>
> A bit like how we select what interrupt controller drivers to built-in
> for example.

Using this scheme, which platforms should select the pcspkr hardware?

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Gianfar ethernet device
From: Jon Smirl @ 2007-11-12  0:22 UTC (permalink / raw)
  To: benh; +Cc: PowerPC dev list
In-Reply-To: <1194825876.18185.3.camel@pasglop>

On 11/11/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Sun, 2007-11-11 at 18:30 -0500, Jon Smirl wrote:
> > Does this patch add the right ifdefs in fsl_soc.c to make these
> > drivers build on their proper platforms? As an experiment I have
> > disabled platform bus on the mpc5200 and I'm only using of_platform
> > bus. Turning off platform bus is exposing a lot of code that is
> > getting built into my mpc5200 kernel that is supporting devices not
> > available on the platform.
> >
> > A few more fixups in some other drivers and I should have mpc5200
> > working without platform bus.
>
> That sort of per-platform ifdefs is just WRONG.
>
> On the other hand, if you feel that such common code deserves not being
> built on all platforms, then what you can do is define a Kconfig option,
> such as CONFIG_PPC_CAN_HAVE_GIANFAR, that gets select'ed by the
> platforms that can have a Gianfar and use -that- for the ifdef.

The real solution is that gianfar support belongs in a device driver,
not in a common file. That whole fsl_soc.c file is a catch-all of
things that belong in device drivers. I haven't looked at every line
in it, but 90%+ of the code should be moved into device drivers.

I'm preparing a patch that moves the i2c driver out of fsl_soc.c and
into i2c_mpc.c.


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Gianfar ethernet device
From: Jon Smirl @ 2007-11-12  0:30 UTC (permalink / raw)
  To: benh; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711111622v5ad6fdd2jd30989819d05615d@mail.gmail.com>

BTW, this exercise of disabling platform bus is turning up some real bugs.

There are several tests in the kernel like this:

        if (!shost->shost_gendev.parent)
                shost->shost_gendev.parent = dev ? dev : &platform_bus;

	if (adap->dev.parent == NULL) {
		adap->dev.parent = &platform_bus;
		pr_debug("I2C adapter driver [%s] forgot to specify "
			 "physical device\n", adap->name);
	}

This doesn't do the right thing when the device is on of_platform_bus
instead of platform_bus. What's the right fix for these? Shouldn't the
call error out instead of trying to fix up the parent? Then fix the
code to set the parent right when the device was created.

-- 
Jon Smirl
jonsmirl@gmail.com

^ 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