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...
> PPC_CPM_NEW_BINDING is default y, and will be going away as an option as
> soon as arch/ppc does. In the meantime, instead of selecting it here,
> add FS_ENET_MPC5121_FEC to the depends list of PPC_CPM_NEW_BINDING -- or
> just remove the depends list altogether.
Good idea, done that.
> > +/*
> > + * Define the buffer descriptor structure.
> > + */
> > +typedef struct bufdesc {
> > + unsigned short cbd_sc; /* Control and status info */
> > + unsigned short cbd_datlen; /* Data length */
> > + unsigned long cbd_bufaddr; /* Buffer address */
> > +} cbd_t;
>
> This can be factored out into a common header -- along with most if not
> all of the flag defines.
Ok, I added the changes to ../fec.h inside an #if-#else.
> > +#define TX_ALIGN_WORKAROUND
> > +#ifdef TX_ALIGN_WORKAROUND
> > +static struct sk_buff *aligntxskb(struct net_device *dev, struct sk_buff
> > *skb) +{
> > + struct sk_buff *skbn;
> > + skbn = dev_alloc_skb(ENET_RX_FRSIZE+0x20);
> > + if (skbn)
> > + skb_align(skbn, 0x20);
> > +
> > + if (!skbn) {
> > + printk(KERN_WARNING DRV_MODULE_NAME
> > + ": %s Memory squeeze, dropping tx packet.\n",
> > + dev->name);
> > + dev_kfree_skb_any(skb);
> > + return NULL;
> > + }
> > +
> > + skb_copy_from_linear_data(skb, skbn->data, skb->len);
> > + skb_put(skbn, skb->len);
> > + dev_kfree_skb_any(skb);
> > + return skbn;
> > +}
> > +#else
> > +#define aligntxskb(skb) skb
> > +#endif
>
> 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.
>[...]
> > + printk("<1> %s: %s (%d)\n",__FILE__,__FUNCTION__,__LINE__);
>
> Please use the KERN_ prefixes rather than hardcoding the number, and put
> spaces after commas. Of course, if this is to be here at all, this
> should be dev_dbg() rather than KERN_ALERT.
Ooops. This shouldn't be here. Removing it.
> > -#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)
> > +#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?
Best regards,
--
David Jander
Protonic Holland.
_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
