[PATCH 2/2] Re-added support for FEC on MPC5121 from Freescale LTIB

[Scott Wood]

On Thu, Jun 12, 2008 at 12:33:12PM +0200, David Jander wrote:
> On Wednesday 11 June 2008 20:19:17 Scott Wood wrote:
> > Please post patches to linuxppc-dev rather than linuxppc-embedded --
> > you'll get a larger reviewing audience.
> 
> Ok, but I thought linuxppc-dev was for all non-embedded development only...

No, it's for all Linux/PowerPC development.  linuxppc-embedded is more
for user questions.

> > Can we encode the required alignment for rx and tx in the device tree?
> 
> Hmmm... This actually is a silicon-bug workaround AFAIK. 
> I agree with you that this solution is not good, but I doubt resolving this 
> in the device-tree is much better. It will add run-time overhead just because 
> right now there are some buggy chips around. I think this could better 
> be a Kconfig option, so people can have faster kernels if they want later.
> If you are concerned about multiplatform kernels, this shouldn't harm.

OK.

> > > -#define __cbd_out32(addr, x)	out_be32(addr, x)
> > > -#define __cbd_out16(addr, x)	out_be16(addr, x)
> > > -#define __cbd_in32(addr)	in_be32(addr)
> > > -#define __cbd_in16(addr)	in_be16(addr)
> > > +#define __cbd_out32(addr, x)	out_be32((volatile void __iomem *)addr, x)
> > > +#define __cbd_out16(addr, x)	out_be16((volatile void __iomem *)addr, x)
> > > +#define __cbd_in32(addr)	in_be32((volatile void __iomem *)addr)
> > > +#define __cbd_in16(addr)	in_be16((volatile void __iomem *)addr)
> >
> > NACK, please don't remove type checking.
> 
> Ok, but given the macro's further down, I'll have to cast there...
> 
> #define CBDR_BUFADDR(_cbd) __cbd_in32((volatile void __iomem *)&(_cbd)->cbd_bufaddr)

Why?  cbd should already have an __iomem annotation, and the volatile is
unnecessary.

> > > +#ifndef CONFIG_FS_ENET_MPC5121_FEC
> > >  	FW(fecp, r_hash, PKT_MAXBUF_SIZE);
> > > +#endif
> > >
> > >  	/* get physical address */
> > >  	rx_bd_base_phys = fep->ring_mem_addr;
> > > @@ -320,10 +325,17 @@ static void restart(struct net_device *dev)
> > >
> > >  	fs_init_bds(dev);
> > >
> > > +#ifndef CONFIG_FS_ENET_MPC5121_FEC
> > >  	/*
> > >  	 * Enable big endian and don't care about SDMA FC.
> > >  	 */
> > >  	FW(fecp, fun_code, 0x78000000);
> > > +#else
> > > +	/*
> > > +	 * Set DATA_BO and DESC_BO and leave the rest unchanged
> > > +	 */
> > > +	FS(fecp, dma_control, 0xc0000000);
> > > +#endif
> 
> I have tried, but it is almost impossible, since both FEC types use a struct
> named "fec_t" which very different betwen both types. That means that the code
> would have enormous amounts of "if" statements all over.
> Any suggestion?

OK, I missed that this was FEC-specific code and not general fs-enet. 

8xx/512x multiplatform is already unsupported due to core differences,
but make sure that 82xx/512x multiplatform doesn't get broken.

-Scott