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
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
