Re: [PATCH v8 2/4] misc: Generic on-chip SRAM allocation driver

[Paul Mundt]

On Mon, Feb 04, 2013 at 12:32:16PM +0100, Philipp Zabel wrote:
> This driver requests and remaps a memory region as configured in the
> device tree. It serves memory from this region via the genalloc API.
> It optionally enables the SRAM clock.
> 
> Other drivers can retrieve the genalloc pool from a phandle pointing
> to this drivers' device node in the device tree.
> 
> The allocation granularity is hard-coded to 32 bytes for now,
> to make the SRAM driver useful for the 6502 remoteproc driver.
> There is overhead for bigger SRAMs, where only a much coarser
> allocation granularity is needed: At 32 bytes minimum allocation
> size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.
> 
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Reviewed-by: Shawn Guo <shawn.guo@xxxxxxxxxx>

How exactly is this "generic" if you have randomly hard-coded an
allocation granularity that is larger than half of the in-tree SRAM pool
users today can even support? Did you even bother to look at in-tree SRAM
pool users other than the one you are working on?

There also doesn't seem to be any real reason for the hard-coding either,
this information could easily be fetched via platform data or the device
tree, and the driver in question would simply need to be able to
determine whether the size is suitable for it or not.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/