[PATCH 14/19] powerpc: SPU support routines for Celleb

[Arnd Bergmann]

On Friday 12 January 2007 02:13, Ishizaki Kou wrote:
> SPU support routines for Celleb platform.

Mostly looks good. I'm sorry I did reply to you when you asked
for details about the new style device tree layout though.
Your spu-manage.c file basically implements what the old generation
of IBM cell blades requires, which we're now trying to phase out.

My feeling is that we should try to consolidate this again into
a common manage.c file for both celleb and ibm blades, as opposed
to the ps3 platform that needs to do this in a completely different
way.

I'll try to do a write a version that should run on both celleb
and the blades, using either version of the device tree layout,
so you can convert your device trees in future releases.

If you send a patch that leaves out manage.c in the meantime,
I'll Ack that one.

	Arnd <><

> +static int __init find_spu_node_id(struct device_node *spe)
> +{
> +	const unsigned int *id;
> +	struct device_node *cpu;
> +	cpu = spe->parent->parent;
> +	id = get_property(cpu, "node-id", NULL);
> +	return id ? *id : 0;
> +}

This breaks when the SPE is not a child of a child of a CPU
node. The new model would put it into a /be node.

The right solution is to replace this function with a call to
of_node_to_nid(), which does the right thing everywhere.

> +static u64 __init find_spu_unit_number(struct device_node *spe)
> +{
> +	const unsigned int *reg;
> +	reg = get_property(spe, "reg", NULL);
> +	return reg ? (u64)*reg : 0ul;
> +}

This is the bigger problem, since the "reg" property changed its meaning.
It should probably check the "unit-id" property first, and only
use the "reg" property if in backwards-compatible mode.

> +static int __init cell_spuprop_present(struct spu *spu, struct device_node *spe,
> +		const char *prop)
> +{
> +	static DEFINE_MUTEX(add_spumem_mutex);
> +
> +	const struct address_prop {
> +		unsigned long address;
> +		unsigned int len;
> +	} __attribute__((packed)) *p;
> +	int proplen;
> +
> +	unsigned long start_pfn, nr_pages;
> +	struct pglist_data *pgdata;
> +	struct zone *zone;
> +	int ret;
> +
> +	p = get_property(spe, prop, &proplen);
> +	WARN_ON(proplen != sizeof (*p));
> +
> +	start_pfn = p->address >> PAGE_SHIFT;
> +	nr_pages = ((unsigned long)p->len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> +
> +	pgdata = NODE_DATA(spu_get_pdata(spu)->nid);
> +	zone = pgdata->node_zones;
> +
> +	/* XXX rethink locking here */
> +	mutex_lock(&add_spumem_mutex);
> +	ret = __add_pages(zone, start_pfn, nr_pages);
> +	mutex_unlock(&add_spumem_mutex);
> +
> +	return ret;
> +}

This should be the same as the other, existing, function of the 
same name.

> +static void __iomem * __init map_spe_prop(struct spu *spu,
> +		struct device_node *n, const char *name)
> +{
> +	const struct address_prop {
> +		unsigned long address;
> +		unsigned int len;
> +	} __attribute__((packed)) *prop;
> +
> +	const void *p;
> +	int proplen;
> +	void __iomem *ret = NULL;
> +	int err = 0;
> +
> +	p = get_property(n, name, &proplen);
> +	if (proplen != sizeof (struct address_prop))
> +		return NULL;
> +
> +	prop = p;
> +
> +	err = cell_spuprop_present(spu, n, name);
> +	if (err && (err != -EEXIST))
> +		goto out;
> +
> +	ret = ioremap(prop->address, prop->len);
> +
> + out:
> +	return ret;
> +}
> +
> +static void spu_unmap_beat(struct spu *spu)
> +{
> +	iounmap(spu->priv2);
> +	iounmap(spu->problem);
> +	iounmap((__force u8 __iomem *)spu->local_store);
> +}
> +
> +static int __init spu_map_device_beat(struct spu *spu,
> +				      struct device_node *node)
> +{
> +	const char *prop;
> +	int ret;
> +
> +	ret = -ENODEV;
> +	spu->name = get_property(node, "name", NULL);
> +	if (!spu->name)
> +		goto out;
> +
> +	prop = get_property(node, "local-store", NULL);
> +	if (!prop)
> +		goto out;
> +	spu->local_store_phys = *(unsigned long *)prop;
> +
> +	/* we use local store as ram, not io memory */
> +	spu->local_store = (void __force *)
> +		map_spe_prop(spu, node, "local-store");
> +	if (!spu->local_store)
> +		goto out;
> +
> +	prop = get_property(node, "problem", NULL);
> +	if (!prop)
> +		goto out_unmap;
> +	spu->problem_phys = *(unsigned long *)prop;
> +
> +	spu->problem= map_spe_prop(spu, node, "problem");
> +	if (!spu->problem)
> +		goto out_unmap;
> +
> +	spu->priv2= map_spe_prop(spu, node, "priv2");
> +	if (!spu->priv2)
> +		goto out_unmap;
> +	ret = 0;
> +	goto out;
> +
> +out_unmap:
> +	spu_unmap_beat(spu);
> +out:
> +	return ret;
> +}

In the new model, the information comes from the 'reg' property,
instead of individual ones. "priv1" is intentionally kept last in
that list, so it can be left out for the hv case.

	Arnd <><