LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 10/12] Uartlite: Comment block tidy
From: Grant Likely @ 2007-10-02  2:15 UTC (permalink / raw)
  To: linuxppc-dev, jwboyer
In-Reply-To: <20071002021334.9579.68179.stgit@trillian.cg.shawcable.net>

From: Grant Likely <grant.likely@secretlab.ca>

Tidy the comments to split the driver into logical section; the main driver,
the console driver, the platform bus binding, and module initialization
and teardown.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/serial/uartlite.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index c00a627..ed13b9f 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -23,9 +23,13 @@
 #define ULITE_MINOR		187
 #define ULITE_NR_UARTS		4
 
-/* For register details see datasheet:
-   http://www.xilinx.com/bvdocs/ipcenter/data_sheet/opb_uartlite.pdf
-*/
+/* ---------------------------------------------------------------------
+ * Register definitions
+ *
+ * For register details see datasheet:
+ * http://www.xilinx.com/bvdocs/ipcenter/data_sheet/opb_uartlite.pdf
+ */
+
 #define ULITE_RX		0x00
 #define ULITE_TX		0x04
 #define ULITE_STATUS		0x08
@@ -49,6 +53,10 @@
 
 static struct uart_port ulite_ports[ULITE_NR_UARTS];
 
+/* ---------------------------------------------------------------------
+ * Core UART driver operations
+ */
+
 static int ulite_receive(struct uart_port *port, int stat)
 {
 	struct tty_struct *tty = port->info->tty;
@@ -308,6 +316,10 @@ static struct uart_ops ulite_ops = {
 	.verify_port	= ulite_verify_port
 };
 
+/* ---------------------------------------------------------------------
+ * Console driver operations
+ */
+
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
 static void ulite_console_wait_tx(struct uart_port *port)
 {
@@ -413,6 +425,19 @@ static struct uart_driver ulite_uart_driver = {
 #endif
 };
 
+/* ---------------------------------------------------------------------
+ * Port assignment functions (mapping devices to uart_port structures)
+ */
+
+/** ulite_assign: register a uartlite device with the driver
+ *
+ * @dev: pointer to device structure
+ * @id: requested id number.  Pass -1 for automatic port assignment
+ * @base: base address of uartlite registers
+ * @irq: irq number for uartlite
+ *
+ * Returns: 0 on success, <0 otherwise
+ */
 static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 {
 	struct uart_port *port;
@@ -465,6 +490,10 @@ static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 	return 0;
 }
 
+/** ulite_release: register a uartlite device with the driver
+ *
+ * @dev: pointer to device structure
+ */
 static int __devinit ulite_release(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
@@ -479,6 +508,10 @@ static int __devinit ulite_release(struct device *dev)
 	return rc;
 }
 
+/* ---------------------------------------------------------------------
+ * Platform bus binding
+ */
+
 static int __devinit ulite_probe(struct platform_device *pdev)
 {
 	struct resource *res, *res2;
@@ -508,6 +541,10 @@ static struct platform_driver ulite_platform_driver = {
 		   },
 };
 
+/* ---------------------------------------------------------------------
+ * Module setup/teardown
+ */
+
 int __init ulite_init(void)
 {
 	int ret;

^ permalink raw reply related

* Please pull powerpc.git merge branch
From: Paul Mackerras @ 2007-10-02  3:15 UTC (permalink / raw)
  To: torvalds; +Cc: linuxppc-dev

Linus,

Please do

git pull \
git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git merge

to get a bug-fix from Anton Blanchard for 2.6.23.

Thanks,
Paul.

 arch/powerpc/platforms/pseries/xics.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

commit e48395f1753cab0fde6c03f1db833cece9ef2ba0
Author: Anton Blanchard <anton@samba.org>
Date:   Mon Oct 1 07:45:55 2007 +1000

    [POWERPC] Fix xics set_affinity code
    
    On a POWER6 machine running 2.6.23-rc8 I sometimes see the following error:
    
    xics_set_affinity: No online cpus in the mask 00000000,00000000,00000000,00000001 for irq 20
    
    In a desperate attempt to get a changelog entry in 2.6.23, I took a look
    into it.
    
    It turns out we are passing a real and not a virtual irq into
    get_irq_server.  This works for the case where hwirq < NR_IRQS and we
    set virq = hwirq.  In my case however hwirq = 590082 and we try and
    access irq_desc[590082], slightly past the end at 512 entries.
    
    Lucky we ship lots of memory with our machines.
    
    Signed-off-by: Anton Blanchard <anton@samba.org>
    Signed-off-by: Paul Mackerras <paulus@samba.org>

^ permalink raw reply

* Re: [PATCH] powerpc: fix pci domain detection
From: Benjamin Herrenschmidt @ 2007-10-02  3:16 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, linux-pci, paulus, linux-kernel
In-Reply-To: <200709261602.06214.arnd@arndb.de>


On Wed, 2007-09-26 at 16:02 +0200, Arnd Bergmann wrote:
> The /proc/bus/pci/* files list PCI domain numbers only for
> devices that claim to be on a multi-domain system. The check
> for this is broken on powerpc, because the buid value is
> truncated to 32 bits.
> 
> There is at least one machine (IBM QS21) that only uses
> the high-order bits of the buid, so the return value
> of pci_proc_domain() ends up being always zero, which
> makes /proc/bus/pci useless.
> 
> Change the logic to always return '1' for a nonzero
> buid value.
> 
> Signed-off-by: Arnd Bergmann <arnd.bergmann@de.ibm.com>

Good catch !

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

> 
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 291ffbc..9f63bdc 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -588,7 +588,7 @@ int pci_proc_domain(struct pci_bus *bus)
>  		return 0;
>  	else {
>  		struct pci_controller *hose = pci_bus_to_host(bus);
> -		return hose->buid;
> +		return hose->buid != 0;
>  	}
>  }
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* [PATCH] [POWERPC] Limit range of __init_ref_ok somewhat
From: Stephen Rothwell @ 2007-10-02  3:37 UTC (permalink / raw)
  To: paulus; +Cc: ppc-dev

This patch introduces zalloc_maybe_bootmem and uses it so that we don;t
have to mark a whole (largish) routine as __init_ref_ok.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/kernel/irq.c    |   10 ++--------
 arch/powerpc/lib/alloc.c     |   15 +++++++++++++++
 include/asm-powerpc/system.h |    1 +
 3 files changed, 18 insertions(+), 8 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 0e47c8c..151b131 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -424,7 +424,7 @@ static int default_irq_host_match(struct irq_host *h, struct device_node *np)
 	return h->of_node != NULL && h->of_node == np;
 }
 
-__init_refok struct irq_host *irq_alloc_host(struct device_node *of_node,
+struct irq_host *irq_alloc_host(struct device_node *of_node,
 				unsigned int revmap_type,
 				unsigned int revmap_arg,
 				struct irq_host_ops *ops,
@@ -439,13 +439,7 @@ __init_refok struct irq_host *irq_alloc_host(struct device_node *of_node,
 	/* Allocate structure and revmap table if using linear mapping */
 	if (revmap_type == IRQ_HOST_MAP_LINEAR)
 		size += revmap_arg * sizeof(unsigned int);
-	if (mem_init_done)
-		host = kzalloc(size, GFP_KERNEL);
-	else {
-		host = alloc_bootmem(size);
-		if (host)
-			memset(host, 0, size);
-	}
+	host = zalloc_maybe_bootmem(size, GFP_KERNEL);
 	if (host == NULL)
 		return NULL;
 
diff --git a/arch/powerpc/lib/alloc.c b/arch/powerpc/lib/alloc.c
index e58c805..f53e09c 100644
--- a/arch/powerpc/lib/alloc.c
+++ b/arch/powerpc/lib/alloc.c
@@ -2,6 +2,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/bootmem.h>
+#include <linux/string.h>
 
 #include <asm/system.h>
 
@@ -12,3 +13,17 @@ void * __init_refok alloc_maybe_bootmem(size_t size, gfp_t mask)
 	else
 		return alloc_bootmem(size);
 }
+
+void * __init_refok zalloc_maybe_bootmem(size_t size, gfp_t mask)
+{
+	void *p;
+
+	if (mem_init_done)
+		p = kzalloc(size, mask);
+	else {
+		p = alloc_bootmem(size);
+		if (p)
+			memset(p, 0, size);
+	}
+	return p;
+}
diff --git a/include/asm-powerpc/system.h b/include/asm-powerpc/system.h
index f7879fc..d10e99b 100644
--- a/include/asm-powerpc/system.h
+++ b/include/asm-powerpc/system.h
@@ -190,6 +190,7 @@ extern unsigned long memory_limit;
 extern unsigned long klimit;
 
 extern void *alloc_maybe_bootmem(size_t size, gfp_t mask);
+extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 
 extern int powersave_nap;	/* set if nap mode can be used in idle loop */
 
-- 
1.5.3.2

^ permalink raw reply related

* Re: [PATCH] Make sure to of_node_get() the result of pci_device_to_OF_node()
From: Benjamin Herrenschmidt @ 2007-10-02  5:09 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <8183225ab4b14684bb0939e0c5258caaf5b8102f.1190008974.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:03 +1000, Michael Ellerman wrote:
> pci_device_to_OF_node() returns the device node attached to a PCI device,
> but doesn't actually grab a reference - we need to do it ourselves.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/platforms/cell/axon_msi.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 57a6149..2b2dfcc 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -119,7 +119,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev *dev)
>  	const phandle *ph;
>  	struct axon_msic *msic = NULL;
>  
> -	dn = pci_device_to_OF_node(dev);
> +	dn = of_node_get(pci_device_to_OF_node(dev));
>  	if (!dn) {
>  		dev_dbg(&dev->dev, "axon_msi: no pci_dn found\n");
>  		return NULL;
> @@ -176,7 +176,7 @@ static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
>  	int len;
>  	const u32 *prop;
>  
> -	dn = pci_device_to_OF_node(dev);
> +	dn = of_node_get(pci_device_to_OF_node(dev));
>  	if (!dn) {
>  		dev_dbg(&dev->dev, "axon_msi: no pci_dn found\n");
>  		return -ENODEV;

^ permalink raw reply

* Re: [PATCH 1/7] Store the base address in dcr_host_t
From: Benjamin Herrenschmidt @ 2007-10-02  5:10 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <449a181e8169af98984bb2d61a759c23e55d19b4.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> In its current form, dcr_map() doesn't remember the base address you passed
> it, which means you need to store it somewhere else. Rather than adding the
> base to another struct it seems simpler to store it in the dcr_host_t.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/sysdev/dcr.c        |    2 +-
>  include/asm-powerpc/dcr-mmio.h   |    6 +++++-
>  include/asm-powerpc/dcr-native.h |    6 ++++--
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index e82d54d..ab11c0b 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -104,7 +104,7 @@ u64 of_translate_dcr_address(struct device_node *dev,
>  dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
>  		   unsigned int dcr_c)
>  {
> -	dcr_host_t ret = { .token = NULL, .stride = 0 };
> +	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
>  	u64 addr;
>  
>  	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
> index 5dbfca8..6b82c3b 100644
> --- a/include/asm-powerpc/dcr-mmio.h
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -23,7 +23,11 @@
>  
>  #include <asm/io.h>
>  
> -typedef struct { void __iomem *token; unsigned int stride; } dcr_host_t;
> +typedef struct {
> +	void __iomem *token;
> +	unsigned int stride;
> +	unsigned int base;
> +} dcr_host_t;
>  
>  #define DCR_MAP_OK(host)	((host).token != NULL)
>  
> diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
> index 05af081..f41058c 100644
> --- a/include/asm-powerpc/dcr-native.h
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -22,11 +22,13 @@
>  #ifdef __KERNEL__
>  #ifndef __ASSEMBLY__
>  
> -typedef struct {} dcr_host_t;
> +typedef struct {
> +	unsigned int base;
> +} dcr_host_t;
>  
>  #define DCR_MAP_OK(host)	(1)
>  
> -#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){})
> +#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
>  #define dcr_unmap(host, dcr_n, dcr_c)	do {} while (0)
>  #define dcr_read(host, dcr_n)		mfdcr(dcr_n)
>  #define dcr_write(host, dcr_n, value)	mtdcr(dcr_n, value)

^ permalink raw reply

* Re: [PATCH 2/7] Update mpic to use dcr_host_t.base
From: Benjamin Herrenschmidt @ 2007-10-02  5:12 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <59ba66469e13b37eddf4e2423ed0540cce2e1079.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the mpic
> code, rather than storing it separately.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/sysdev/mpic.c |   28 +++++++++++-----------------
>  include/asm-powerpc/mpic.h |    6 ------
>  2 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 8de29f2..16b1f4b 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -156,8 +156,7 @@ static inline u32 _mpic_read(enum mpic_reg_type type,
>  	switch(type) {
>  #ifdef CONFIG_PPC_DCR
>  	case mpic_access_dcr:
> -		return dcr_read(rb->dhost,
> -				rb->dbase + reg + rb->doff);
> +		return dcr_read(rb->dhost, rb->dhost.base + reg);
>  #endif
>  	case mpic_access_mmio_be:
>  		return in_be32(rb->base + (reg >> 2));
> @@ -174,8 +173,7 @@ static inline void _mpic_write(enum mpic_reg_type type,
>  	switch(type) {
>  #ifdef CONFIG_PPC_DCR
>  	case mpic_access_dcr:
> -		return dcr_write(rb->dhost,
> -				 rb->dbase + reg + rb->doff, value);
> +		return dcr_write(rb->dhost, rb->dhost.base + reg, value);
>  #endif
>  	case mpic_access_mmio_be:
>  		return out_be32(rb->base + (reg >> 2), value);
> @@ -279,9 +277,11 @@ static void _mpic_map_mmio(struct mpic *mpic, unsigned long phys_addr,
>  static void _mpic_map_dcr(struct mpic *mpic, struct mpic_reg_bank *rb,
>  			  unsigned int offset, unsigned int size)
>  {
> -	rb->dbase = mpic->dcr_base;
> -	rb->doff = offset;
> -	rb->dhost = dcr_map(mpic->irqhost->of_node, rb->dbase + rb->doff, size);
> +	const u32 *dbasep;
> +
> +	dbasep = of_get_property(mpic->irqhost->of_node, "dcr-reg", NULL);
> +
> +	rb->dhost = dcr_map(mpic->irqhost->of_node, *dbasep + offset, size);
>  	BUG_ON(!DCR_MAP_OK(rb->dhost));
>  }
>  
> @@ -1075,20 +1075,14 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>  	BUG_ON(paddr == 0 && node == NULL);
>  
>  	/* If no physical address passed in, check if it's dcr based */
> -	if (paddr == 0 && of_get_property(node, "dcr-reg", NULL) != NULL)
> -		mpic->flags |= MPIC_USES_DCR;
> -
> +	if (paddr == 0 && of_get_property(node, "dcr-reg", NULL) != NULL) {
>  #ifdef CONFIG_PPC_DCR
> -	if (mpic->flags & MPIC_USES_DCR) {
> -		const u32 *dbasep;
> -		dbasep = of_get_property(node, "dcr-reg", NULL);
> -		BUG_ON(dbasep == NULL);
> -		mpic->dcr_base = *dbasep;
> +		mpic->flags |= MPIC_USES_DCR;
>  		mpic->reg_type = mpic_access_dcr;
> -	}
>  #else
> -	BUG_ON (mpic->flags & MPIC_USES_DCR);
> +		BUG();
>  #endif /* CONFIG_PPC_DCR */
> +	}
>  
>  	/* If the MPIC is not DCR based, and no physical address was passed
>  	 * in, try to obtain one
> diff --git a/include/asm-powerpc/mpic.h b/include/asm-powerpc/mpic.h
> index edb4a7c..ae84dde 100644
> --- a/include/asm-powerpc/mpic.h
> +++ b/include/asm-powerpc/mpic.h
> @@ -224,8 +224,6 @@ struct mpic_reg_bank {
>  	u32 __iomem	*base;
>  #ifdef CONFIG_PPC_DCR
>  	dcr_host_t	dhost;
> -	unsigned int	dbase;
> -	unsigned int	doff;
>  #endif /* CONFIG_PPC_DCR */
>  };
>  
> @@ -289,10 +287,6 @@ struct mpic
>  	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
>  	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
>  
> -#ifdef CONFIG_PPC_DCR
> -	unsigned int		dcr_base;
> -#endif
> -
>  	/* Protected sources */
>  	unsigned long		*protected;
>  

^ permalink raw reply

* Re: [PATCH 3/7] Use dcr_host_t.base in ibm_emac_mal
From: Benjamin Herrenschmidt @ 2007-10-02  5:13 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <5022a72815fe6334b5255dbe1288036131c2c961.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> This requires us to do a sort-of fake dcr_map(), so that base is set
> properly. This will be fixed/removed when the device-tree-aware emac driver
> is merged.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

(Have you actually tested btw ? :-)

> ---
>  drivers/net/ibm_emac/ibm_emac_mal.c |    5 ++++-
>  drivers/net/ibm_emac/ibm_emac_mal.h |    5 ++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.c b/drivers/net/ibm_emac/ibm_emac_mal.c
> index cabd984..b08da96 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.c
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.c
> @@ -421,7 +421,10 @@ static int __init mal_probe(struct ocp_device *ocpdev)
>  		       ocpdev->def->index);
>  		return -ENOMEM;
>  	}
> -	mal->dcrbase = maldata->dcr_base;
> +
> +	/* XXX This only works for native dcr for now */
> +	mal->dcrhost = dcr_map(NULL, maldata->dcr_base, 0);
> +
>  	mal->def = ocpdev->def;
>  
>  	INIT_LIST_HEAD(&mal->poll_list);
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
> index 64bc338..6b1fbeb 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.h
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.h
> @@ -191,7 +191,6 @@ struct mal_commac {
>  };
>  
>  struct ibm_ocp_mal {
> -	int			dcrbase;
>  	dcr_host_t		dcrhost;
>  
>  	struct list_head	poll_list;
> @@ -209,12 +208,12 @@ struct ibm_ocp_mal {
>  
>  static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
>  {
> -	return dcr_read(mal->dcrhost, mal->dcrbase + reg);
> +	return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
>  }
>  
>  static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
>  {
> -	dcr_write(mal->dcrhost, mal->dcrbase + reg, val);
> +	dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
>  }
>  
>  /* Register MAL devices */

^ permalink raw reply

* Re: [PATCH 4/7] Update axon_msi to use dcr_host_t.base
From: Benjamin Herrenschmidt @ 2007-10-02  5:14 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <2a6b3b82775119b4a1e1fd9b290051b1fef66d55.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the
> axon_msi code, rather than storing it separately.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/platforms/cell/axon_msi.c |   13 ++++++-------
>  1 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 74407af..23e039a 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -69,7 +69,6 @@ struct axon_msic {
>  	dcr_host_t dcr_host;
>  	struct list_head list;
>  	u32 read_offset;
> -	u32 dcr_base;
>  };
>  
>  static LIST_HEAD(axon_msic_list);
> @@ -78,12 +77,12 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
>  {
>  	pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
>  
> -	dcr_write(msic->dcr_host, msic->dcr_base + dcr_n, val);
> +	dcr_write(msic->dcr_host, msic->dcr_host.base + dcr_n, val);
>  }
>  
>  static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
>  {
> -	return dcr_read(msic->dcr_host, msic->dcr_base + dcr_n);
> +	return dcr_read(msic->dcr_host, msic->dcr_host.base + dcr_n);
>  }
>  
>  static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
> @@ -324,7 +323,7 @@ static int axon_msi_setup_one(struct device_node *dn)
>  	struct page *page;
>  	struct axon_msic *msic;
>  	unsigned int virq;
> -	int dcr_len;
> +	int dcr_base, dcr_len;
>  
>  	pr_debug("axon_msi: setting up dn %s\n", dn->full_name);
>  
> @@ -335,17 +334,17 @@ static int axon_msi_setup_one(struct device_node *dn)
>  		goto out;
>  	}
>  
> -	msic->dcr_base = dcr_resource_start(dn, 0);
> +	dcr_base = dcr_resource_start(dn, 0);
>  	dcr_len = dcr_resource_len(dn, 0);
>  
> -	if (msic->dcr_base == 0 || dcr_len == 0) {
> +	if (dcr_base == 0 || dcr_len == 0) {
>  		printk(KERN_ERR
>  		       "axon_msi: couldn't parse dcr properties on %s\n",
>  			dn->full_name);
>  		goto out;
>  	}
>  
> -	msic->dcr_host = dcr_map(dn, msic->dcr_base, dcr_len);
> +	msic->dcr_host = dcr_map(dn, dcr_base, dcr_len);
>  	if (!DCR_MAP_OK(msic->dcr_host)) {
>  		printk(KERN_ERR "axon_msi: dcr_map failed for %s\n",
>  		       dn->full_name);

^ permalink raw reply

* Re: [PATCH 5/7] Add dcr_host_t.base in dcr_read()/dcr_write()
From: Benjamin Herrenschmidt @ 2007-10-02  5:17 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <0caf686b20c5a22172d41e6c77b5b0bb3c429534.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that all users of dcr_read()/dcr_write() add the dcr_host_t.base, we can
> save them the trouble and do it in dcr_read()/dcr_write().
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Please, fixup the changeset comment to be more exlicit or provide some
Documentation/powerpc/dcr.txt explaning some of the discussions we had
about why this is actually a good idea :-)

Among others:

 - Initially, the goal was to operate like mfdcr/mtdcr who take absolute
DCR numbers. The reason is that on 4xx hardware, indirect DCR access is
a pain (goes through a table of instructions) and it's useful to have
the compiler resolve an absolute DCR inline.

 - We decided that wasn't worth the API bastardisation since most cases
where absolute DCR values are used are low level 4xx-only code which may
as well continue using mfdcr/mtdcr, while the new API is designed for
device "instances" that can exist on 4xx and Axon type platforms and may
be located at variable DCR offsets.

Something around those lines...

Appart from that, patch is fine, I'll ack with the new comment :-)

Ben.

> ---
>  arch/powerpc/platforms/cell/axon_msi.c |    4 ++--
>  arch/powerpc/sysdev/mpic.c             |    4 ++--
>  drivers/net/ibm_emac/ibm_emac_mal.h    |    4 ++--
>  include/asm-powerpc/dcr-mmio.h         |    4 ++--
>  include/asm-powerpc/dcr-native.h       |    4 ++--
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 23e039a..26a5e88 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -77,12 +77,12 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
>  {
>  	pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
>  
> -	dcr_write(msic->dcr_host, msic->dcr_host.base + dcr_n, val);
> +	dcr_write(msic->dcr_host, dcr_n, val);
>  }
>  
>  static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
>  {
> -	return dcr_read(msic->dcr_host, msic->dcr_host.base + dcr_n);
> +	return dcr_read(msic->dcr_host, dcr_n);
>  }
>  
>  static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 16b1f4b..61f5730 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -156,7 +156,7 @@ static inline u32 _mpic_read(enum mpic_reg_type type,
>  	switch(type) {
>  #ifdef CONFIG_PPC_DCR
>  	case mpic_access_dcr:
> -		return dcr_read(rb->dhost, rb->dhost.base + reg);
> +		return dcr_read(rb->dhost, reg);
>  #endif
>  	case mpic_access_mmio_be:
>  		return in_be32(rb->base + (reg >> 2));
> @@ -173,7 +173,7 @@ static inline void _mpic_write(enum mpic_reg_type type,
>  	switch(type) {
>  #ifdef CONFIG_PPC_DCR
>  	case mpic_access_dcr:
> -		return dcr_write(rb->dhost, rb->dhost.base + reg, value);
> +		return dcr_write(rb->dhost, reg, value);
>  #endif
>  	case mpic_access_mmio_be:
>  		return out_be32(rb->base + (reg >> 2), value);
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
> index 6b1fbeb..10dc978 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.h
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.h
> @@ -208,12 +208,12 @@ struct ibm_ocp_mal {
>  
>  static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
>  {
> -	return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
> +	return dcr_read(mal->dcrhost, reg);
>  }
>  
>  static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
>  {
> -	dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
> +	dcr_write(mal->dcrhost, reg, val);
>  }
>  
>  /* Register MAL devices */
> diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
> index 6b82c3b..a7d9eaf 100644
> --- a/include/asm-powerpc/dcr-mmio.h
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -37,12 +37,12 @@ extern void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c);
>  
>  static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
>  {
> -	return in_be32(host.token + dcr_n * host.stride);
> +	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
>  }
>  
>  static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
>  {
> -	out_be32(host.token + dcr_n * host.stride, value);
> +	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
>  }
>  
>  extern u64 of_translate_dcr_address(struct device_node *dev,
> diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
> index f41058c..3bc780f 100644
> --- a/include/asm-powerpc/dcr-native.h
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -30,8 +30,8 @@ typedef struct {
>  
>  #define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
>  #define dcr_unmap(host, dcr_n, dcr_c)	do {} while (0)
> -#define dcr_read(host, dcr_n)		mfdcr(dcr_n)
> -#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n, value)
> +#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
> +#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
>  
>  /* Device Control Registers */
>  void __mtdcr(int reg, unsigned int val);
> -- 
> 1.5.1.3.g7a33b
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Benjamin Herrenschmidt @ 2007-10-02  5:19 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <60649f36e785af905b8cb1fd12847d7630e7371a.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Add a helper routine to map dcr's based on the "dcr-reg" property of
> a device node.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
even better name, but dcr_map_reg really sucks :-)

Ben.

> ---
>  arch/powerpc/sysdev/dcr.c |   17 +++++++++++++++++
>  include/asm-powerpc/dcr.h |    1 +
>  2 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index ab11c0b..da4f9c6 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -126,6 +126,23 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
>  }
>  EXPORT_SYMBOL_GPL(dcr_map);
>  
> +dcr_host_t dcr_map_reg(struct device_node *dev, unsigned int index)
> +{
> +	dcr_host_t ret = { .token = NULL };
> +
> +	unsigned int dcr_n, dcr_c;
> +
> +	dcr_n = dcr_resource_start(dev, index);
> +	if (!dcr_n)
> +		return ret;
> +
> +	dcr_c = dcr_resource_len(dev, index);
> +	if (!dcr_c)
> +		return ret;
> +
> +	return dcr_map(dev, dcr_n, dcr_c);
> +}
> +
>  void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c)
>  {
>  	dcr_host_t h = host;
> diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> index 9338d50..4d42f01 100644
> --- a/include/asm-powerpc/dcr.h
> +++ b/include/asm-powerpc/dcr.h
> @@ -38,6 +38,7 @@ extern unsigned int dcr_resource_start(struct device_node *np,
>  				       unsigned int index);
>  extern unsigned int dcr_resource_len(struct device_node *np,
>  				     unsigned int index);
> +extern dcr_host_t dcr_map_reg(struct device_node *np, unsigned int index);
>  #endif /* CONFIG_PPC_MERGE */
>  
>  #endif /* CONFIG_PPC_DCR */

^ permalink raw reply

* Re: [PATCH 7/7] Remove msic_dcr_read() and use dcr_map_reg() in axon_msi.c
From: Benjamin Herrenschmidt @ 2007-10-02  5:20 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <f2d1c6be4dd138a847c2573ae1cde5c2ac253dfa.1190009070.git.michael@ellerman.id.au>


On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> msic_dcr_read() doesn't really do anything useful, just replace it with
> direct calls to dcr_read().
> 
> Use dcr_map_reg() in the axon_msi setup code, rather than essentially doing
> it by hand.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/platforms/cell/axon_msi.c |   22 +++-------------------
>  1 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 26a5e88..57a6149 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -80,18 +80,13 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
>  	dcr_write(msic->dcr_host, dcr_n, val);
>  }
>  
> -static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
> -{
> -	return dcr_read(msic->dcr_host, dcr_n);
> -}
> -
>  static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
>  {
>  	struct axon_msic *msic = get_irq_data(irq);
>  	u32 write_offset, msi;
>  	int idx;
>  
> -	write_offset = msic_dcr_read(msic, MSIC_WRITE_OFFSET_REG);
> +	write_offset = dcr_read(msic->dcr_host, MSIC_WRITE_OFFSET_REG);
>  	pr_debug("axon_msi: original write_offset 0x%x\n", write_offset);
>  
>  	/* write_offset doesn't wrap properly, so we have to mask it */
> @@ -306,7 +301,7 @@ static int axon_msi_notify_reboot(struct notifier_block *nb,
>  	list_for_each_entry(msic, &axon_msic_list, list) {
>  		pr_debug("axon_msi: disabling %s\n",
>  			  msic->irq_host->of_node->full_name);
> -		tmp  = msic_dcr_read(msic, MSIC_CTRL_REG);
> +		tmp  = dcr_read(msic->dcr_host, MSIC_CTRL_REG);
>  		tmp &= ~MSIC_CTRL_ENABLE & ~MSIC_CTRL_IRQ_ENABLE;
>  		msic_dcr_write(msic, MSIC_CTRL_REG, tmp);
>  	}
> @@ -323,7 +318,6 @@ static int axon_msi_setup_one(struct device_node *dn)
>  	struct page *page;
>  	struct axon_msic *msic;
>  	unsigned int virq;
> -	int dcr_base, dcr_len;
>  
>  	pr_debug("axon_msi: setting up dn %s\n", dn->full_name);
>  
> @@ -334,17 +328,7 @@ static int axon_msi_setup_one(struct device_node *dn)
>  		goto out;
>  	}
>  
> -	dcr_base = dcr_resource_start(dn, 0);
> -	dcr_len = dcr_resource_len(dn, 0);
> -
> -	if (dcr_base == 0 || dcr_len == 0) {
> -		printk(KERN_ERR
> -		       "axon_msi: couldn't parse dcr properties on %s\n",
> -			dn->full_name);
> -		goto out;
> -	}
> -
> -	msic->dcr_host = dcr_map(dn, dcr_base, dcr_len);
> +	msic->dcr_host = dcr_map_reg(dn, 0);
>  	if (!DCR_MAP_OK(msic->dcr_host)) {
>  		printk(KERN_ERR "axon_msi: dcr_map failed for %s\n",
>  		       dn->full_name);

^ permalink raw reply

* Re: [PATCH 1/4] Simplify error logic in u3msi_setup_msi_irqs()
From: Benjamin Herrenschmidt @ 2007-10-02  5:21 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <3034ec8fd939bd5cfcdb7ac65206ae2771dc9b2c.1190270165.git.michael@ellerman.id.au>


On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> u3msi_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/sysdev/mpic_u3msi.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 305b864..4e50d1c 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -124,7 +124,6 @@ static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
>  static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  {
>  	irq_hw_number_t hwirq;
> -	int rc;
>  	unsigned int virq;
>  	struct msi_desc *entry;
>  	struct msi_msg msg;
> @@ -132,17 +131,15 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
>  		if (hwirq < 0) {
> -			rc = hwirq;
>  			pr_debug("u3msi: failed allocating hwirq\n");
> -			goto out_free;
> +			return hwirq;
>  		}
>  
>  		virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
>  		if (virq == NO_IRQ) {
>  			pr_debug("u3msi: failed mapping hwirq 0x%lx\n", hwirq);
>  			mpic_msi_free_hwirqs(msi_mpic, hwirq, 1);
> -			rc = -ENOSPC;
> -			goto out_free;
> +			return -ENOSPC;
>  		}
>  
>  		set_irq_msi(virq, entry);
> @@ -156,10 +153,6 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	}
>  
>  	return 0;
> -
> - out_free:
> -	u3msi_teardown_msi_irqs(pdev);
> -	return rc;
>  }
>  
>  int mpic_u3msi_init(struct mpic *mpic)

^ permalink raw reply

* Re: [PATCH 2/4] Simplify error logic in rtas_setup_msi_irqs()
From: Benjamin Herrenschmidt @ 2007-10-02  5:21 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <9a0ebd42c2e31f6a99a1028dd370f9ee36c771f3.1190270165.git.michael@ellerman.id.au>


On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> rtas_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/platforms/pseries/msi.c |   17 +++--------------
>  1 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 6063ea2..9c3bcfe 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -189,29 +189,22 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  
>  	if (rc != nvec) {
>  		pr_debug("rtas_msi: rtas_change_msi() failed\n");
> -
> -		/*
> -		 * In case of an error it's not clear whether the device is
> -		 * left with MSI enabled or not, so we explicitly disable.
> -		 */
> -		goto out_free;
> +		return rc;
>  	}
>  
>  	i = 0;
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = rtas_query_irq_number(pdn, i);
>  		if (hwirq < 0) {
> -			rc = hwirq;
>  			pr_debug("rtas_msi: error (%d) getting hwirq\n", rc);
> -			goto out_free;
> +			return hwirq;
>  		}
>  
>  		virq = irq_create_mapping(NULL, hwirq);
>  
>  		if (virq == NO_IRQ) {
>  			pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
> -			rc = -ENOSPC;
> -			goto out_free;
> +			return -ENOSPC;
>  		}
>  
>  		dev_dbg(&pdev->dev, "rtas_msi: allocated virq %d\n", virq);
> @@ -220,10 +213,6 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	}
>  
>  	return 0;
> -
> - out_free:
> -	rtas_teardown_msi_irqs(pdev);
> -	return rc;
>  }
>  
>  static void rtas_msi_pci_irq_fixup(struct pci_dev *pdev)

^ permalink raw reply

* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Benjamin Herrenschmidt @ 2007-10-02  5:23 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <fbd88f287ba9d9ca41b5c8da734afd595ae25922.1190270165.git.michael@ellerman.id.au>


On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> Currently rtas_change_msi() returns either the error code from RTAS, or if
> the RTAS call succeeded the number of irqs that were configured by RTAS.
> This makes checking the return value more complicated than it needs to be.
> 
> Instead, have rtas_change_msi() check that the number of irqs configured by
> RTAS is equal to what we requested - and return an error otherwise. This makes
> the return semantics match the usual 0 for success, something else for error.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Looks allright, just a question tho... what do we do if it fails ? Do we
try to fallback to a lower number of MSIs ? Or what ? Dead device ?

Ben.

> ---
>  arch/powerpc/platforms/pseries/msi.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 9c3bcfe..2793a1b 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -70,11 +70,15 @@ static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
>  		seq_num = rtas_ret[1];
>  	} while (rtas_busy_delay(rc));
>  
> -	if (rc == 0) /* Success */
> -		rc = rtas_ret[0];
> +	/*
> +	 * If the RTAS call succeeded, check the number of irqs is actually
> +	 * what we asked for. If not, return an error.
> +	 */
> +	if (rc == 0 && rtas_ret[0] != num_irqs)
> +		rc = -ENOSPC;
>  
> -	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d) = (%d)\n",
> -		 func, num_irqs, rc);
> +	pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d), got %d rc = %d\n",
> +		 func, num_irqs, rtas_ret[0], rc);
>  
>  	return rc;
>  }
> @@ -87,7 +91,7 @@ static void rtas_disable_msi(struct pci_dev *pdev)
>  	if (!pdn)
>  		return;
>  
> -	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
> +	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0))
>  		pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
>  }
>  
> @@ -180,14 +184,14 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	if (type == PCI_CAP_ID_MSI) {
>  		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
>  
> -		if (rc != nvec) {
> +		if (rc) {
>  			pr_debug("rtas_msi: trying the old firmware call.\n");
>  			rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
>  		}
>  	} else
>  		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
>  
> -	if (rc != nvec) {
> +	if (rc) {
>  		pr_debug("rtas_msi: rtas_change_msi() failed\n");
>  		return rc;
>  	}

^ permalink raw reply

* Re: [PATCH 4/4] Inline u3msi_compose_msi_msg()
From: Benjamin Herrenschmidt @ 2007-10-02  5:24 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <19ac8b97e98274582fe5bde914883c3585f20082.1190270165.git.michael@ellerman.id.au>


On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> In the MPIC U3 MSI code, we call u3msi_compose_msi_msg() once for each MSI.
> This is overkill, as the address is per pci device, not per MSI. So setup
> the address once, and just set the data per MSI.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

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

> ---
>  arch/powerpc/sysdev/mpic_u3msi.c |   24 +++++++++---------------
>  1 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 4e50d1c..027fe01 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -107,26 +107,17 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
>  	return;
>  }
>  
> -static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
> -				  struct msi_msg *msg)
> -{
> -	u64 addr;
> -
> -	addr = find_ht_magic_addr(pdev);
> -	msg->address_lo = addr & 0xFFFFFFFF;
> -	msg->address_hi = addr >> 32;
> -	msg->data = virq_to_hw(virq);
> -
> -	pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) at address 0x%lx\n",
> -		 virq, virq_to_hw(virq), addr);
> -}
> -
>  static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  {
>  	irq_hw_number_t hwirq;
>  	unsigned int virq;
>  	struct msi_desc *entry;
>  	struct msi_msg msg;
> +	u64 addr;
> +
> +	addr = find_ht_magic_addr(pdev);
> +	msg.address_lo = addr & 0xFFFFFFFF;
> +	msg.address_hi = addr >> 32;
>  
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
>  		hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
> @@ -146,7 +137,10 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  		set_irq_chip(virq, &mpic_u3msi_chip);
>  		set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
>  
> -		u3msi_compose_msi_msg(pdev, virq, &msg);
> +		pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) addr 0x%lx\n",
> +			  virq, hwirq, addr);
> +
> +		msg.data = hwirq;
>  		write_msi_msg(virq, &msg);
>  
>  		hwirq++;

^ permalink raw reply

* [PATCH v2] powerpc: Implement logging of unhandled signals
From: Olof Johansson @ 2007-10-02  5:35 UTC (permalink / raw)
  To: paulus; +Cc: linuxppc-dev
In-Reply-To: <20071001203242.GA14091@lixom.net>

Implement show_unhandled_signals sysctl + support to print when a process
is killed due to unhandled signals just as i386 and x86_64 does.

Signed-off-by: Olof Johansson <olof@lixom.net>


---

Fixed the const char definitions, plus the two warnings that the change
brought (printing a ptr as %lx without cast).

Index: 2.6.23/arch/powerpc/kernel/traps.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/traps.c
+++ 2.6.23/arch/powerpc/kernel/traps.c
@@ -172,11 +172,21 @@ int die(const char *str, struct pt_regs 
 void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
 {
 	siginfo_t info;
+	const char fmt32[] = KERN_INFO "%s[%d]: unhandled signal %d " \
+			"at %08lx nip %08lx lr %08lx code %x\n";
+	const char fmt64[] = KERN_INFO "%s[%d]: unhandled signal %d " \
+			"at %016lx nip %016lx lr %016lx code %x\n";
 
 	if (!user_mode(regs)) {
 		if (die("Exception in kernel mode", regs, signr))
 			return;
-	}
+	} else if (show_unhandled_signals &&
+		    unhandled_signal(current, signr) &&
+		    printk_ratelimit()) {
+			printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+				current->comm, current->pid, signr,
+				addr, regs->nip, regs->link, code);
+		}
 
 	memset(&info, 0, sizeof(info));
 	info.si_signo = signr;
Index: 2.6.23/arch/powerpc/kernel/signal_64.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal_64.c
+++ 2.6.23/arch/powerpc/kernel/signal_64.c
@@ -64,6 +64,11 @@ struct rt_sigframe {
 	char abigap[288];
 } __attribute__ ((aligned (16)));
 
+static const char fmt32[] = KERN_INFO \
+	"%s[%d]: bad frame in %s: %08lx nip %08lx lr %08lx\n";
+static const char fmt64[] = KERN_INFO \
+	"%s[%d]: bad frame in %s: %016lx nip %016lx lr %016lx\n";
+
 /*
  * Set up the sigcontext for the signal frame.
  */
@@ -315,6 +320,11 @@ badframe:
 	printk("badframe in sys_rt_sigreturn, regs=%p uc=%p &uc->uc_mcontext=%p\n",
 	       regs, uc, &uc->uc_mcontext);
 #endif
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+			current->comm, current->pid, "rt_sigreturn",
+			(long)uc, regs->nip, regs->link);
+
 	force_sig(SIGSEGV, current);
 	return 0;
 }
@@ -398,6 +408,11 @@ badframe:
 	printk("badframe in setup_rt_frame, regs=%p frame=%p newsp=%lx\n",
 	       regs, frame, newsp);
 #endif
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+			current->comm, current->pid, "setup_rt_frame",
+			(long)frame, regs->nip, regs->link);
+
 	force_sigsegv(signr, current);
 	return 0;
 }
Index: 2.6.23/kernel/sysctl.c
===================================================================
--- 2.6.23.orig/kernel/sysctl.c
+++ 2.6.23/kernel/sysctl.c
@@ -1221,7 +1221,7 @@ static ctl_table fs_table[] = {
 };
 
 static ctl_table debug_table[] = {
-#ifdef CONFIG_X86
+#if defined(CONFIG_X86) || defined(CONFIG_PPC)
 	{
 		.ctl_name	= CTL_UNNUMBERED,
 		.procname	= "exception-trace",
Index: 2.6.23/arch/powerpc/kernel/signal_32.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal_32.c
+++ 2.6.23/arch/powerpc/kernel/signal_32.c
@@ -705,11 +705,13 @@ int handle_rt_signal32(unsigned long sig
 {
 	struct rt_sigframe __user *rt_sf;
 	struct mcontext __user *frame;
+	void __user *addr;
 	unsigned long newsp = 0;
 
 	/* Set up Signal Frame */
 	/* Put a Real Time Context onto stack */
 	rt_sf = get_sigframe(ka, regs, sizeof(*rt_sf));
+	addr = rt_sf;
 	if (unlikely(rt_sf == NULL))
 		goto badframe;
 
@@ -728,6 +730,7 @@ int handle_rt_signal32(unsigned long sig
 
 	/* Save user registers on the stack */
 	frame = &rt_sf->uc.uc_mcontext;
+	addr = frame;
 	if (vdso32_rt_sigtramp && current->mm->context.vdso_base) {
 		if (save_user_regs(regs, frame, 0))
 			goto badframe;
@@ -742,6 +745,7 @@ int handle_rt_signal32(unsigned long sig
 
 	/* create a stack frame for the caller of the handler */
 	newsp = ((unsigned long)rt_sf) - (__SIGNAL_FRAMESIZE + 16);
+	addr = (void __user *)regs->gpr[1];
 	if (put_user(regs->gpr[1], (u32 __user *)newsp))
 		goto badframe;
 
@@ -762,6 +766,12 @@ badframe:
 	printk("badframe in handle_rt_signal, regs=%p frame=%p newsp=%lx\n",
 	       regs, frame, newsp);
 #endif
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(KERN_INFO "%s[%d]: bad frame in handle_rt_signal32: "
+			"%p nip %08lx lr %08lx\n",
+			current->comm, current->pid,
+			addr, regs->nip, regs->link);
+
 	force_sigsegv(sig, current);
 	return 0;
 }
@@ -886,6 +896,12 @@ long sys_rt_sigreturn(int r3, int r4, in
 	return 0;
 
  bad:
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(KERN_INFO "%s[%d]: bad frame in sys_rt_sigreturn: "
+			"%p nip %08lx lr %08lx\n",
+			current->comm, current->pid,
+			rt_sf, regs->nip, regs->link);
+
 	force_sig(SIGSEGV, current);
 	return 0;
 }
@@ -967,6 +983,13 @@ int sys_debug_setcontext(struct ucontext
 	 * We kill the task with a SIGSEGV in this situation.
 	 */
 	if (do_setcontext(ctx, regs, 1)) {
+		if (show_unhandled_signals && printk_ratelimit())
+			printk(KERN_INFO "%s[%d]: bad frame in "
+				"sys_debug_setcontext: %p nip %08lx "
+				"lr %08lx\n",
+				current->comm, current->pid,
+				ctx, regs->nip, regs->link);
+
 		force_sig(SIGSEGV, current);
 		goto out;
 	}
@@ -1048,6 +1071,12 @@ badframe:
 	printk("badframe in handle_signal, regs=%p frame=%p newsp=%lx\n",
 	       regs, frame, newsp);
 #endif
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(KERN_INFO "%s[%d]: bad frame in handle_signal32: "
+			"%p nip %08lx lr %08lx\n",
+			current->comm, current->pid,
+			frame, regs->nip, regs->link);
+
 	force_sigsegv(sig, current);
 	return 0;
 }
@@ -1061,12 +1090,14 @@ long sys_sigreturn(int r3, int r4, int r
 	struct sigcontext __user *sc;
 	struct sigcontext sigctx;
 	struct mcontext __user *sr;
+	void __user *addr;
 	sigset_t set;
 
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
 
 	sc = (struct sigcontext __user *)(regs->gpr[1] + __SIGNAL_FRAMESIZE);
+	addr = sc;
 	if (copy_from_user(&sigctx, sc, sizeof(sigctx)))
 		goto badframe;
 
@@ -1083,6 +1114,7 @@ long sys_sigreturn(int r3, int r4, int r
 	restore_sigmask(&set);
 
 	sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
+	addr = sr;
 	if (!access_ok(VERIFY_READ, sr, sizeof(*sr))
 	    || restore_user_regs(regs, sr, 1))
 		goto badframe;
@@ -1091,6 +1123,12 @@ long sys_sigreturn(int r3, int r4, int r
 	return 0;
 
 badframe:
+	if (show_unhandled_signals && printk_ratelimit())
+		printk(KERN_INFO "%s[%d]: bad frame in sys_sigreturn: "
+			"%p nip %08lx lr %08lx\n",
+			current->comm, current->pid,
+			addr, regs->nip, regs->link);
+
 	force_sig(SIGSEGV, current);
 	return 0;
 }
Index: 2.6.23/arch/powerpc/kernel/signal.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal.c
+++ 2.6.23/arch/powerpc/kernel/signal.c
@@ -16,6 +16,12 @@
 
 #include "signal.h"
 
+/* Log an error when sending an unhandled signal to a process. Controlled
+ * through debug.exception-trace sysctl.
+ */
+
+int show_unhandled_signals = 1;
+
 /*
  * Allocate space for the signal frame
  */

^ permalink raw reply

* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Michael Ellerman @ 2007-10-02  5:51 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302382.6310.80.camel@pasglop>

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

On Tue, 2007-10-02 at 15:19 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > Add a helper routine to map dcr's based on the "dcr-reg" property of
> > a device node.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
> even better name, but dcr_map_reg really sucks :-)

That would give us dcr_map(), dcr_unmap() and of_map_dcr() - which
doesn't strike me as more consistent.

I don't particularly like dcr_map_reg(), but I think it's at least
obvious that it's a variant of dcr_map() and that it has something to do
with a "reg" .. maybe even a "dcr-reg" :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

^ permalink raw reply

* Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding
From: Benjamin Herrenschmidt @ 2007-10-02  5:53 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <20070930224208.1871.2913.stgit@trillian.cg.shawcable.net>


On Sun, 2007-09-30 at 16:42 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Add of_platform bus binding so this driver can be used with arch/powerpc

Another option is to have a "constructor" in the platform code that
generates the platform device from the DT. It might even become the
preferred way one of these days, I'm not too sure at this stage. Anyway,
do what you prefer.

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  drivers/serial/uartlite.c |  101 +++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 93 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
> index ed13b9f..8752fac 100644
> --- a/drivers/serial/uartlite.c
> +++ b/drivers/serial/uartlite.c
> @@ -1,7 +1,8 @@
>  /*
>   * uartlite.c: Serial driver for Xilinx uartlite serial controller
>   *
> - * Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2006 Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2007 Secret Lab Technologies Ltd.
>   *
>   * 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
> @@ -17,6 +18,10 @@
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <asm/io.h>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#endif
>  
>  #define ULITE_NAME		"ttyUL"
>  #define ULITE_MAJOR		204
> @@ -382,8 +387,10 @@ static int __init ulite_console_setup(struct console *co, char *options)
>  	port = &ulite_ports[co->index];
>  
>  	/* not initialized yet? */
> -	if (!port->membase)
> +	if (!port->membase) {
> +		pr_debug("console on ttyUL%i not initialized\n", co->index);
>  		return -ENODEV;
> +	}
>  
>  	if (options)
>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -542,6 +549,72 @@ static struct platform_driver ulite_platform_driver = {
>  };
>  
>  /* ---------------------------------------------------------------------
> + * OF bus bindings
> + */
> +#if defined(CONFIG_OF)
> +static int __devinit
> +ulite_of_probe(struct of_device *op, const struct of_device_id *match)
> +{
> +	struct resource res;
> +	const unsigned int *id;
> +	int irq, rc;
> +
> +	dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
> +
> +	rc = of_address_to_resource(op->node, 0, &res);
> +	if (rc) {
> +		dev_err(&op->dev, "invalide address\n");
> +		return rc;
> +	}
> +
> +	irq = irq_of_parse_and_map(op->node, 0);
> +
> +	id = of_get_property(op->node, "port-number", NULL);
> +
> +	return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
> +}
> +
> +static int __devexit ulite_of_remove(struct of_device *op)
> +{
> +	return ulite_release(&op->dev);
> +}
> +
> +/* Match table for of_platform binding */
> +static struct of_device_id __devinit ulite_of_match[] = {
> +	{ .type = "serial", .compatible = "xilinx,uartlite", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ulite_of_match);
> +
> +static struct of_platform_driver ulite_of_driver = {
> +	.owner = THIS_MODULE,
> +	.name = "uartlite",
> +	.match_table = ulite_of_match,
> +	.probe = ulite_of_probe,
> +	.remove = __devexit_p(ulite_of_remove),
> +	.driver = {
> +		.name = "uartlite",
> +	},
> +};
> +
> +/* Registration helpers to keep the number of #ifdefs to a minimum */
> +static inline int __init ulite_of_register(void)
> +{
> +	pr_debug("uartlite: calling of_register_platform_driver()\n");
> +	return of_register_platform_driver(&ulite_of_driver);
> +}
> +
> +static inline void __exit ulite_of_unregister(void)
> +{
> +	of_unregister_platform_driver(&ulite_of_driver);
> +}
> +#else /* CONFIG_OF */
> +/* CONFIG_OF not enabled; do nothing helpers */
> +static inline int __init ulite_of_register(void) { return 0; }
> +static inline void __exit ulite_of_unregister(void) { }
> +#endif /* CONFIG_OF */
> +
> +/* ---------------------------------------------------------------------
>   * Module setup/teardown
>   */
>  
> @@ -549,20 +622,32 @@ int __init ulite_init(void)
>  {
>  	int ret;
>  
> -	ret = uart_register_driver(&ulite_uart_driver);
> -	if (ret)
> -		return ret;
> +	pr_debug("uartlite: calling uart_register_driver()\n");
> +	if ((ret = uart_register_driver(&ulite_uart_driver)) != 0)
> +		goto err_uart;
>  
> -	ret = platform_driver_register(&ulite_platform_driver);
> -	if (ret)
> -		uart_unregister_driver(&ulite_uart_driver);
> +	if ((ret = ulite_of_register()) != 0)
> +		goto err_of;
>  
> +	pr_debug("uartlite: calling platform_driver_register()\n");
> +	if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
> +		goto err_plat;
> +
> +	return 0;
> +
> +err_plat:
> +	ulite_of_unregister();
> +err_of:
> +	uart_unregister_driver(&ulite_uart_driver);
> +err_uart:
> +	printk(KERN_ERR "registering uartlite driver failed: err=%i", ret);
>  	return ret;
>  }
>  
>  void __exit ulite_exit(void)
>  {
>  	platform_driver_unregister(&ulite_platform_driver);
> +	ulite_of_unregister();
>  	uart_unregister_driver(&ulite_uart_driver);
>  }
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v2 5/6] Sysace: Move IRQ handler registration to occur after FSM is initialized
From: Benjamin Herrenschmidt @ 2007-10-02  5:55 UTC (permalink / raw)
  To: Grant Likely; +Cc: axboe, linuxppc-dev, paulus, linux-kernel
In-Reply-To: <20070930225726.2476.44211.stgit@trillian.cg.shawcable.net>


On Sun, 2007-09-30 at 16:57 -0600, Grant Likely wrote:
>         val |= ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ;
>         ace_out(ace, ACE_CTRL, val);
>  
> +       /* Now we can hook up the irq handler */
> +       if (ace->irq != NO_IRQ) {
> +               rc = request_irq(ace->irq, ace_interrupt, 0,
> "systemace", ace);
> +               if (rc) {
> +                       /* Failure - fall back to polled mode */
> +                       dev_err(ace->dev, "request_irq failed\n");
> +                       ace->irq = NO_IRQ;
> +               }
> +       }
> +

I don't know the HW but from the above, it looks like you enable
interrupt emission on the HW before you register the handler, which is
wrong. You should make sure on the contrary that IRQs on the HW are
disabled until after you have registered a handler.

Only really a problem if you have shared interrupts but still...

Ben.

^ permalink raw reply

* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Michael Ellerman @ 2007-10-02  5:58 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302603.6310.88.camel@pasglop>

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

On Tue, 2007-10-02 at 15:23 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> > Currently rtas_change_msi() returns either the error code from RTAS, or if
> > the RTAS call succeeded the number of irqs that were configured by RTAS.
> > This makes checking the return value more complicated than it needs to be.
> > 
> > Instead, have rtas_change_msi() check that the number of irqs configured by
> > RTAS is equal to what we requested - and return an error otherwise. This makes
> > the return semantics match the usual 0 for success, something else for error.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Looks allright, just a question tho... what do we do if it fails ? Do we
> try to fallback to a lower number of MSIs ? Or what ? Dead device ?

That's all up to the device driver. In theory the driver could try again
with a lower count - but that might require extra logic in the driver to
handle shared irq handlers etc. In practice I think the current drivers
will just fail.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

^ permalink raw reply

* Re: [PATCH v2 2/6] Sysace: Use the established platform bus api
From: Benjamin Herrenschmidt @ 2007-10-02  5:58 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, paulus, linux-kernel, linuxppc-dev
In-Reply-To: <20071001115954.GD5303@kernel.dk>


On Mon, 2007-10-01 at 13:59 +0200, Jens Axboe wrote:
> On Sun, Sep 30 2007, Grant Likely wrote:
> > On 9/30/07, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Sun, Sep 30, 2007 at 04:57:09PM -0600, Grant Likely wrote:
> > > > +     if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
> > > > +             goto err_plat;
> > >
> > >         rc = platform_driver_register(&ace_platform_driver);
> > >         if (rc)
> > >                  goto err_plat;
> > >
> > > please.
> > 
> > Okay, will do.
> > 
> > >
> > > > +      err_plat:
> > > > +     unregister_blkdev(ace_major, "xsysace");
> > > > +      err_blk:
> > >
> > > labels should be indented zero or one space, but not more.
> > 
> > scripts/Lindent does this.  Originally, I *didn't* have my labels
> > indented.  :-)   Does Lindent need to be fixed?
> 
> Seems so, if it idents labels.
> 
> Just send a fixup patch for that, I'll add your series to the block tree
> for 2.6.24.

It's actually better off living in the powerpc tree I think as it's
really about adding support for a new powerpc platform and somewhat
needs to sync with other things in there. Unless you really want the
whole thing in your tree :-)

Cheers
Ben.

^ permalink raw reply

* Re: nap/dfs on 7448
From: Benjamin Herrenschmidt @ 2007-10-02  6:01 UTC (permalink / raw)
  To: Leisner, Martin; +Cc: linuxppc-dev
In-Reply-To: <556445368AFA1C438794ABDA8901891C0738E1E8@USA0300MS03.na.xerox.net>


On Mon, 2007-10-01 at 17:32 -0400, Leisner, Martin wrote:
> I asked this on linuxppc-embedded a week ago (I didn't even know this
> list existed until last week -- another reason to get rid of
> linuxpcc-embedded).
> 
> Has anyone gotten NAP/DFS to reliably work on a 7448?
> 
> I'm seeing strange problems with peripherals...(using a ram disk
> works fine).

Could it be that your host bridge isn't properly waking up the CPU to
DOZE state for snooping DMA ? (It might require some delays on QACK in
some cases, I know Apple had workarounds in those areas, maybe something
along those lines need to be configured in the chipset).

Ben.

^ permalink raw reply

* Re: [PATCH 3/7] Use dcr_host_t.base in ibm_emac_mal
From: Michael Ellerman @ 2007-10-02  6:06 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302025.6310.70.camel@pasglop>

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

On Tue, 2007-10-02 at 15:13 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > This requires us to do a sort-of fake dcr_map(), so that base is set
> > properly. This will be fixed/removed when the device-tree-aware emac driver
> > is merged.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> (Have you actually tested btw ? :-)

Hmm, I wrote these in mid June, so to be honest I don't remember. I
don't think I boot tested it on anything, but I think I would have built
it.

I see now that the newemac driver is in Jeff's 24 tree, so it's probably
best to wait for Linus to merge that and then we can fix up it as well
in the one series.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

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

^ permalink raw reply

* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Benjamin Herrenschmidt @ 2007-10-02  6:22 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev
In-Reply-To: <1191304300.6593.2.camel@concordia>


On Tue, 2007-10-02 at 15:51 +1000, Michael Ellerman wrote:
> On Tue, 2007-10-02 at 15:19 +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > > Add a helper routine to map dcr's based on the "dcr-reg" property of
> > > a device node.
> > > 
> > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > 
> > Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
> > even better name, but dcr_map_reg really sucks :-)
> 
> That would give us dcr_map(), dcr_unmap() and of_map_dcr() - which
> doesn't strike me as more consistent.
> 
> I don't particularly like dcr_map_reg(), but I think it's at least
> obvious that it's a variant of dcr_map() and that it has something to do
> with a "reg" .. maybe even a "dcr-reg" :)

dcr_map_node maybe ? dcr_map_device ? dcr_map_property ? hrm...
allright, keep it that way if you want but heh, at least see if you can
come up with something better before it gets merged :-)

Ben

^ 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