Hi Arnd,<br><br><div class="gmail_quote">On Wed, Jul 18, 2012 at 7:42 AM, Arnd Bergmann <span dir="ltr"><<a href="mailto:arnd@arndb.de" target="_blank">arnd@arndb.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Wednesday 18 July 2012, Pavel Machek wrote:<br>
> From: Dinh Nguyen <<a href="mailto:dinguyen@altera.com">dinguyen@altera.com</a>><br>
><br>
> Adding core definitions for Altera's SOCFPGA ARM platform.<br>
><br>
> Signed-off-by: Dinh Nguyen <<a href="mailto:dinguyen@altera.com">dinguyen@altera.com</a>><br>
> Reviewed-by: Pavel Machek <<a href="mailto:pavel@denx.de">pavel@denx.de</a>><br>
> Reviewed-by: Rob Herring <<a href="mailto:rob.herring@calxeda.com">rob.herring@calxeda.com</a>><br>
> Signed-off-by: Pavel Machek <<a href="mailto:pavel@denx.de">pavel@denx.de</a>><br>
><br>
> ---<br>
> Changes from V3: merged into single patch (it is now small enough),<br>
> moved dts pieces around (thanks to Thomas Petazzoni), removed clock<br>
> support (not neccessary with u-boot, needs to be redone).<br>
<br>
Looks good, I'll apply it to the next/newsoc branch for 3.6<br>
when I get the ok from Dinh. I'm not quite sure in what way you<br>
work together, but the patch lists only him in the MAINTAINERS<br>
file, so I'm making sure I'm not sidestepping him when I take<br>
the patch from you instead.<br></blockquote><div><br>I'll send you a fresh V4 today with yours and Thomas's recommended changes to the clk file.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
There are two small details I noticed. You can either send<br>
a replacement patch or fixups on top of this one.<br>
<br>
> + gmac0: stmmac@ff700000 {<br>
> + compatible = "st,spear600-gmac";<br>
> + reg = <0xff700000 0x2000>;<br>
> + interrupts = <0 115 4>;<br>
> + interrupt-names = "macirq";<br>
> + mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */<br>
> + phy-mode = "gmii";<br>
> + };<br>
<br>
We've just discussed the stmmac driver in the spear1340 update<br>
series. My understanding is that there are multiple variants of this<br>
that we may need to distintinguish. I think it's better if you<br>
make the compatible string something that does not reference the<br>
spear600 specific variant, unless you are certain that it is in fact<br>
completely identical to that one.<br>
<br>
Ideally you would have the version of the stmmac macro encoded<br>
in there, in addition to a name identifying altera as the company<br>
who added this one, ordered from most specific to least specific.<br>
<br>
For instance, this could be<br>
<br>
compatible = "altr,socfpga-stmmac", "st,stmmac-v12.3.45", "st,stmmac";<br>
<br>
Looking at the driver, it seems that this should actually not be called<br>
stmmac in the devicetree but rather dwmac, so better make it<br>
<br>
compatible = "altr,socfpga-stmmac", "snps,dwmac-4.567", "snps,dwmac";<br></blockquote><div><br>I'll figure out what we have. thanks for the comment.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> diff --git a/arch/arm/mach-socfpga/common.h b/arch/arm/mach-socfpga/common.h<br>
> new file mode 100644<br>
> index 0000000..ba90e7a<br>
> --- /dev/null<br>
> +++ b/arch/arm/mach-socfpga/common.h<br>
> @@ -0,0 +1,23 @@<br>
> +<br>
> +#ifndef __MACH_SOCFPGA_COMMON_H<br>
> +#define __MACH_SOCFPGA_COMMON_H<br>
> +<br>
> +extern struct sys_timer dw_apb_timer;<br>
> +<br>
> +#endif<br>
<br>
This header file is rather pointless when it is located in<br>
arch/arm/mach-socfpga where the driver cannot see it, so there<br>
is no guarantee that you actually use the same type in the<br>
declaration and the definition. I think it's better to also drop<br>
this header file and move the declaration into<br>
include/linux/dw_apb_timer.h<br>
<span class="HOEnZb"><font color="#888888"><br>
Arnd<br>
</font></span></blockquote></div><br>