Re: [PATCH 1/9] SOC: brcmstb: add memory API
From: Christoph Hellwig
Date: Tue Oct 17 2017 - 04:24:21 EST
> +/* Macros to help extract property data */
> +#define U8TOU32(b, offs) \
> + ((((u32)b[0 + offs] << 0) & 0x000000ff) | \
> + (((u32)b[1 + offs] << 8) & 0x0000ff00) | \
> + (((u32)b[2 + offs] << 16) & 0x00ff0000) | \
> + (((u32)b[3 + offs] << 24) & 0xff000000))
Please us helpers like get_unaligned_le32 instead opencoding them.
> +#define DT_PROP_DATA_TO_U32(b, offs) (fdt32_to_cpu(U8TOU32(b, offs)))
And together with this it looks really whacky. So you're converting
from le to native first and then do another conversion from be to cpu?
Something doesn't work out here.
> +/* -------------------- Functions -------------------- */
Please remove pointless comments like this one.
> +
> +/*
> + * If the DT nodes are handy, determine which MEMC holds the specified
> + * physical address.
> + */
> +#ifdef CONFIG_ARCH_BRCMSTB
> +int __brcmstb_memory_phys_addr_to_memc(phys_addr_t pa, void __iomem *base)
Please move this into the arm arch code.
> +#elif defined(CONFIG_MIPS)
And this into the mips arch code.
> +EXPORT_SYMBOL(brcmstb_memory_phys_addr_to_memc);
> +EXPORT_SYMBOL(brcmstb_memory_memc_size);
Why is this exported?