[PATCH] spufs split off platform code
Arnd Bergmann
arnd at arndb.de
Sat Jan 28 14:57:07 EST 2006
On Saturday 28 January 2006 00:51, Geoff Levand wrote:
> Index: alp-linux--dev--10/arch/powerpc/platforms/cell/spu_priv1.c
> ===================================================================
> --- alp-linux--dev--10.orig/arch/powerpc/platforms/cell/spu_priv1.c 2006-01-27 10:45:30.000000000 -0800
> +++ alp-linux--dev--10/arch/powerpc/platforms/cell/spu_priv1.c 2006-01-27 11:31:06.000000000 -0800
> @@ -5,6 +5,202 @@
>
> #include <asm/io.h>
> #include <asm/spu.h>
> +#include <asm/prom.h>
> +#include "interrupt.h"
> +
> +struct spu_priv_data {
> + u32 isrc;
> + u32 node;
> +};
> +
The spu_priv_data should be rather generic. You definitely need an
interrupt source number in order to be able to register the irq handler
correctly. It might be better to store isrc and node in a single word,
since both of them only need four bits according to the current CBE
spec.
If you only have one physical processor in the system, you don't need
the node-id property at all, as it will always be zero.
> +static int __init find_spu_node_id(struct device_node *spe)
> +{
> + unsigned int *id;
> + struct device_node *cpu;
> +
> + cpu = spe->parent->parent;
> + id = (unsigned int *)get_property(cpu, "node-id", NULL);
> +
> + return id ? *id : 0;
> +}
which also means that this function becomes generic. In retrospect,
it seems we should have used the open PAPR bindings for NUMA to
specify the node, but I'm not sure if there is a point in moving
to that now.
> +
> +static void __init get_spe_property(struct device_node *n, const char *name,
> + struct spu_phys* phys)
> +{
> + struct address_prop {
> + unsigned long address;
> + unsigned int len;
> + } __attribute__((packed)) *prop;
> +
> + void *p;
> + int proplen;
> +
> + p = get_property(n, name, &proplen);
> + if (proplen != sizeof (struct address_prop)) {
> + phys->addr = phys->size = 0;
> + return;
> + }
> + prop = p;
> +
> + phys->addr = prop->address;
> + phys->size = prop->len;
> +}
> +
> +int __init enum_and_create_spu(void)
> +{
> + struct device_node *node;
> + int ret;
> +
> + ret = -ENODEV;
> + for (node = of_find_node_by_type(NULL, "spe");
> + node; node = of_find_node_by_type(node, "spe")) {
> + ret = create_spu(node);
> + if (ret)
> + break;
> + }
> + /* in some old firmware versions, the spe is called 'spc', so we
> + look for that as well */
> + for (node = of_find_node_by_type(NULL, "spc");
> + node; node = of_find_node_by_type(node, "spc")) {
> + ret = create_spu(node);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(enum_and_create_spu);
I guess that the "spc" device type can be removed now, I don't think that
any systems are left that have not been converted.
Do you have "spe" type nodes at all? Is there anything that you need to
do different about them?
> +
> +int __init spu_setup(struct spu *spu, void *spu_setup_data)
> +{
> + struct device_node *spe = spu_setup_data;
> + char *prop;
> +
> + spu->priv_data = kmalloc(sizeof(struct spu_priv_data), GFP_KERNEL);
> + if (!spu->priv_data)
> + return -ENOMEM;
> +
> + spu->priv_data->node = find_spu_node_id(spe);
> +
> + prop = get_property(spe, "isrc", NULL);
> + if (!prop)
> + goto out_free;
> + spu->priv_data->isrc = *(unsigned int *)prop;
> +
> + spu->name = get_property(spe, "name", NULL);
> + if (!spu->name)
> + goto out_free;
> +
> + get_spe_property(spe, "local-store", &spu->local_store_phys);
> + if (!spu->local_store_phys.addr)
> + goto out_free;
> +
> + get_spe_property(spe, "problem", &spu->problem_phys);
> + if (!spu->problem_phys.addr)
> + goto out_free;
> +
> + get_spe_property(spe, "priv1", &spu->priv1_phys);
> +
> + get_spe_property(spe, "priv2", &spu->priv2_phys);
> + if (!spu->priv2_phys.addr)
> + goto out_free;
> +
> + return 0;
> +
> +out_free:
> + printk(KERN_WARNING "%s: no spu device found\n", __func__);
> + pr_debug("%s: error (%p)\n", __func__, spu);
> + kfree(spu->priv_data);
> + spu->priv_data = NULL;
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(spu_setup);
> +
> +void spu_free_priv_data(struct spu *spu)
> +{
> + if (spu->priv_data) {
> + kfree(spu->priv_data);
> + spu->priv_data = NULL;
> + }
> +}
> +EXPORT_SYMBOL_GPL(spu_free_priv_data);
> +
> +int spu_request_irqs(struct spu *spu,
> + irqreturn_t (*cls0)(int, void *, struct pt_regs *),
> + irqreturn_t (*cls1)(int, void *, struct pt_regs *),
> + irqreturn_t (*cls2)(int, void *, struct pt_regs *))
> +{
> + int ret;
> + int irq_base;
> +
> + BUG_ON(spu->priv_data);
> +
> + if(!spu->priv_data) {
> + spu->priv_data = kmalloc(sizeof(struct spu_priv_data),
> + GFP_KERNEL);
> +
> + if(!spu->priv_data) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + }
> +
> + irq_base = IIC_NODE_STRIDE * spu->priv_data->node + IIC_SPE_OFFSET;
> +
> + snprintf(spu->irq_c0, sizeof (spu->irq_c0), "spe%02d.0", spu->number);
> + ret = request_irq(irq_base + spu->priv_data->isrc,
> + cls0, 0, spu->irq_c0, spu);
> + if (ret)
> + goto out;
> +
> + snprintf(spu->irq_c1, sizeof (spu->irq_c1), "spe%02d.1", spu->number);
> + ret = request_irq(irq_base + IIC_CLASS_STRIDE + spu->priv_data->isrc,
> + cls1, 0, spu->irq_c1, spu);
> + if (ret)
> + goto out1;
> +
> + snprintf(spu->irq_c2, sizeof (spu->irq_c2), "spe%02d.2", spu->number);
> + ret = request_irq(irq_base + 2*IIC_CLASS_STRIDE + spu->priv_data->isrc,
> + cls2, 0, spu->irq_c2, spu);
> + if (ret)
> + goto out2;
> + goto out;
> +
> +out2:
> + free_irq(irq_base + IIC_CLASS_STRIDE + spu->priv_data->isrc, spu);
> +out1:
> + free_irq(irq_base + spu->priv_data->isrc, spu);
> +out:
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(spu_request_irqs);
Ok, I can see how it is not generic enough to use just assume that
'n * IIC_CLASS_STRIDE + spu->isrc' is the correct irq number.
We might be able to keep the scheme, but it should at least be
documented in some sort of generic device tree bindings file for
Cell.
> +
> +void spu_free_irqs(struct spu *spu)
> +{
> + int irq_base;
> +
> + if(!spu->priv_data) {
> + pr_debug("null priv_data in %p\n", spu);
> + return;
> + }
It may be just me, but I don't like this bit of coding style:
You are trying to deal with priv_data being either allocated
or not allocated at this point. Better make sure that you have
freed the structure before returning an error from any function
that would allocate it on success. Then get rid of the check
here.
> +
> + irq_base = IIC_NODE_STRIDE * spu->priv_data->node + IIC_SPE_OFFSET;
> +
> + free_irq(irq_base + spu->priv_data->isrc, spu);
> + free_irq(irq_base + IIC_CLASS_STRIDE + spu->priv_data->isrc, spu);
> + free_irq(irq_base + 2*IIC_CLASS_STRIDE + spu->priv_data->isrc, spu);
> +
> + kfree(spu->priv_data);
> + spu->priv_data = NULL;
> +}
> +EXPORT_SYMBOL_GPL(spu_free_irqs);
> +
> +void spu_irq_setaffinity(struct spu *spu, int cpu)
> +{
> + u64 target = iic_get_target_id(cpu);
> + u64 route = target << 48 | target << 32 | target << 16;
> + spu_int_route_set(spu, route);
> +}
> +EXPORT_SYMBOL_GPL(spu_irq_setaffinity);
>
> void spu_int_mask_and(struct spu *spu, int class, u64 mask)
> {
> Index: alp-linux--dev--10/include/asm-powerpc/spu.h
> ===================================================================
> --- alp-linux--dev--10.orig/include/asm-powerpc/spu.h 2006-01-27 10:45:30.000000000 -0800
> +++ alp-linux--dev--10/include/asm-powerpc/spu.h 2006-01-27 11:02:29.000000000 -0800
> @@ -25,6 +25,7 @@
> #include <linux/config.h>
> #include <linux/kref.h>
> #include <linux/workqueue.h>
> +#include <linux/interrupt.h>
>
> #define LS_SIZE (256 * 1024)
> #define LS_ADDR_MASK (LS_SIZE - 1)
> @@ -103,20 +104,26 @@
>
> struct spu_context;
> struct spu_runqueue;
> +struct spu_priv_data;
> +struct spu_phys {
> + unsigned long addr;
> + unsigned long size;
> +};
>
> struct spu {
> + struct spu_priv_data *priv_data; /* opaque */
> char *name;
If you want priv_data to point to different types of data structures
depending on the context, I find it easier to understand if there is
a simple void pointer and the actual struct definitions have different
type names.
> - unsigned long local_store_phys;
> + struct spu_phys local_store_phys;
> u8 *local_store;
> - unsigned long problem_phys;
> + struct spu_phys problem_phys;
> struct spu_problem __iomem *problem;
> + struct spu_phys priv1_phys;
> struct spu_priv1 __iomem *priv1;
> + struct spu_phys priv2_phys;
> struct spu_priv2 __iomem *priv2;
> struct list_head list;
> struct list_head sched_list;
> int number;
> - u32 isrc;
> - u32 node;
> u64 flags;
> u64 dar;
> u64 dsisr;
> @@ -178,7 +185,19 @@
> }
> #endif
>
> -/* access to priv1 registers */
> +/* base setup routines */
> +int create_spu(void *spu_setup_data);
> +
> +/* platform setup routines */
> +int enum_and_create_spu(void);
> +int spu_setup(struct spu *spu, void *spu_setup_data);
> +int spu_request_irqs(struct spu *spu,
> + irqreturn_t (*cls0)(int, void *, struct pt_regs *),
> + irqreturn_t (*cls1)(int, void *, struct pt_regs *),
> + irqreturn_t (*cls2)(int, void *, struct pt_regs *));
> +void spu_free_irqs(struct spu *spu);
> +
> +/* platform priv1 register access */
More information about the Linuxppc64-dev
mailing list