Dear Gregory CLEMENT,

On Fri,  3 Jan 2014 10:59:44 +0100, Gregory CLEMENT wrote:

+static int __init mvebu_soc_id_init(void)
+{
+	struct device_node *np;
+	int ret = 0;
+
+	np = of_find_matching_node(NULL, mvebu_pcie_of_match_table);
+	if (np) {

Change this to:

	if (!np)
		return 0;

so that you avoid indenting the entire function code inside the if
{ ... } block.

+		void __iomem *pci_base;
+		struct clk *clk;
+		/*
+		 * ID and revision are available from any port, so we
+		 * just pick the first one
+		 */
+		struct device_node *child = of_get_next_child(np, NULL);

Maybe check that child is not NULL here?

+
+		clk = of_clk_get_by_name(child, NULL);
+		if (IS_ERR(clk)) {
+			pr_err("%s: cannot get clock\n", __func__);

I think you should rather do:

#define pr_fmt(fmt) "mvebu-soc-id: " fmt

at the beginning of your C file and get rid of the "%s" for the
__func__.

+		/* SoC ID */
+		soc_dev_id = __raw_readl(pci_base + PCIE_DEV_ID_OFF) >> 16;
+
+		/* SoC revision */
+		soc_rev = __raw_readl(pci_base + PCIE_DEV_REV_OFF)
+			& SOC_REV_MASK;

Use readl() for both of these reads. __raw_readl() will not work
properly when the system is booted big-endian.

+	return ret;
+}
+arch_initcall(mvebu_soc_id_init);

+#ifdef CONFIG_ARCH_MVEBU
+int mvebu_get_soc_id(u32 *dev, u32 *rev);
+#else
+int mvebu_get_soc_id(u32 *dev, u32 *rev)

Missing "static inline". Without these, if this header file is included
more than once, you will have two times the same symbol defined.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com