Subject: Re: [Cbe-oss-dev] [PATCH] cell: abstract spu management routines

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 08 Nov 2006 16:35:03 +1100

> > +struct platform_data {
> > +	int nid;
> > +	struct device_node *devnode;
> > +	struct spu_priv1 __iomem *priv1;
> > +};
> > +
> > +static struct platform_data *platform_data(struct spu *spu)
> > +{
> > +	BUG_ON(!spu->platform_data);
> > +	return (struct platform_data*)spu->platform_data;
> > +}
> 
> I don't see the point of this, why not just grab platform data directly?

Because it's a void * in the struct spu. This accessor casts it and BUGS
if it's not set (which is probably unnecessary).

I'd prefer however a different naming:

struct spu_pdata {
	blah
};

static struct spu_pdata *spu_get_pdata(struct spu *spu)

Either that, or you could just do something like that in spu.h :

struct spu_pdata;

struct spu
{
	.../...
	struct spu_pdata *pdata;
	../...
};

And have the various priv1 implementation define their own struct
spu_pdata. A bit sneaky but provides strong typing without needing an
accessor.

Ben.