Re: [PATCH 2/3] net: mv643xx: add driver support

[Michael Grzeschik <mgr@pengutronix.de>]

Hi Sebastian,

On Thu, Jan 23, 2014 at 11:51:39PM +0100, Sebastian Hesselbarth wrote:
> On 01/23/2014 08:23 PM, Michael Grzeschik wrote:
> We really all hate "mv643xx" because it is a pain to say and write it.
> I guess barebox will never be run on systems with mv64xxx controllers
> but only Marvell Orion SoC.
> 
> I'd be *very* happy if you do s/mv643xx/orion/g

I was not sure how the naming of the driver was chosen. I thought it was
ment to describe the SoCs IP-Core. But as mv64xxx is the SoCs prefix, I
totaly agree naming it orion. It actually would even be better to give
the driver the read IP-Core name.

> 
> [...]
> >diff --git a/drivers/net/mv643xx.c b/drivers/net/mv643xx.c
> >new file mode 100644
> >index 0000000..3d0bfdc
> >--- /dev/null
> >+++ b/drivers/net/mv643xx.c
> >@@ -0,0 +1,714 @@
> >+/*
> [...]
> >+ */
> >+
> >+#include <common.h>
> >+#include <malloc.h>
> >+#include <net.h>
> >+#include <init.h>
> >+#include <driver.h>
> >+#include <io.h>
> >+#include <clock.h>
> >+#include <xfuncs.h>
> >+#include <linux/phy.h>
> >+#include <linux/clk.h>
> >+#include <linux/err.h>
> >+#include <of_net.h>
> >+#include <mach/dove-regs.h>
> 
> Please don't. The same driver will be used on Kirkwood and possibly
> orion5x, mv78x00 if they get supported.
> 
> Have every register offset defined in here or "mv643xx.h" and get
> rid of the above. If you need some callback for memory windows, let's
> get it on now and create it in a way it is compatible with using this
> driver on the other SoCs.

The register offsets are in mv643xx.h. The mach/dove-regs.h is
only needed to be able to use dove_memory_find. See patch:
[Patch 1/3] ARM: mvebu: make dove_memory_find reachable for drivers

> Also, we really have no plans for Dove, Kirkwood or any other Marvell
> SoC with !CONFIG_OF, so feel free to remove any reference to non-DT
> usage.

That is a good idea. Will do that.

> BTW, how about sorting the #includes alphabetically?

Will do that.

> For the rest, I'll give it a go on Dove ASAP.

It would be nice to see if it is also working if barebox running as
first stage bootloader. Can you try to get the UART-Image [1] via xmodem
transfer running?

[1] images/barebox-solidrun-cubox-uart.img

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox