Re: [PATCH 1/9] SOC: brcmstb: add memory API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/17/2017 01:24 AM, Christoph Hellwig wrote:
>> +/* 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.

No, this needs to work on both ARM and ARM64, hence the reason why this
is in a reasonably architecture neutral place.

> 
>> +#elif defined(CONFIG_MIPS)
> 
> And this into the mips arch code.

Same reason as above.

> 
>> +EXPORT_SYMBOL(brcmstb_memory_phys_addr_to_memc);
> 
>> +EXPORT_SYMBOL(brcmstb_memory_memc_size);
> 
> Why is this exported?

Because the PCIE RC driver can be built as a module.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux