[U-Boot] [PATCH] net: axi_ethernet: Add driver to u-boot

[Marek Vasut]

On Thursday, September 01, 2011 10:55:31 AM Michal Simek wrote:

[...]

> >> I am fine to use 1 << n solution but definitely in our repo I will use
> >> in way I like.
> > 
> > Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same
> > thing. On the other note, it's hard to count the zeroes in there AND you
> > can mistake 0 and 8 in a huge series of those.
> > 
> > Also, you can have whatever you want in your repo if you seriously care
> > to invest the energy into maintaining it just because you need to be
> > stubborn. But it'd really be great if you invested that energy in a more
> > productive manner ;-)
> 
> There are two points of view. And both have con & pro.
> I don't want to argue. Net custodian should decided if is OK or not.
> 
> Look at tsec.h and probably others.

That something's in mainline doesn't mean it's obviously correct !

[...]

> >>> 
> >>> "currently", so there's possibility, in future this won't hold?
> >> 
> >> BTW: I am also sharing rx/tx buffer descriptors for dma.
> >> 
> >> When do you expect that u-boot will be able to use several MACs in one
> >> time?
> > 
> > It's not a matter of when, but -- write a correct code, it's much less
> > burden to fix it later.
> 
> Agree in general.
> It is always question of when. You can always do it in better way. The
> question is if someone will pay you for doing it in better way.

And if the code isn't accepted, they won't pay you ;-)

> If this
> feature is not important for us, make no sense to invest our time/money to
> it.

I guess having the code mainline is important ?

> 
> >>>>>> +	/* Write new speed setting out to Axi Ethernet */
> >>>>>> +	aximac_out32(dev->iobase, XAE_EMMC_OFFSET, emmc_reg);
> >>>>> 
> >>>>> Use clrsetbits() here.
> >>>> 
> >>>> Not defined for Microblaze - just for ARM/PPC.
> >>>> Not going to use it.
> >>> 
> >>> Please fix then. You're the microblaze maintainer, right?
> >> 
> >> Custodian.
> > 
> > Oh come on ...
> > 
> >> But I won't do that.
> > 
> > I think you should.
> > 
> >> If you think that all archs should have it then move it to generic
> >> location which clean code duplication and I will include it.
> > 
> > That's not the point, it's platform specific.
> 
> Just compare ppc/arm implementation and they are the same. It is even pure
> generic code. Adding it to microblaze is just code duplication which is
> also not good way to go. Then in future someone will move it to generic
> location.
> There were a lot of examples in linux kernel and includes.

Ok, if it's a generic code, please submit a patch putting it into a generic 
place and the fix this driver to use it.

> 
> + network driver should be platform independent which is exactly how it is
> written. Writing in the pure C should be fine.

Well there's the issue with register access and endianity. Even Linux has 
trouble with that.

Cheers

> 
> Thanks,
> Michal