On 03/16/2016 03:40 PM, Eric Nelson wrote:
Signed-off-by: Eric Nelson <e...@nelint.com>
A patch description would be useful here; the cover letter wouldn't be
checked in.
diff --git a/drivers/block/cache_block.c b/drivers/block/cache_block.c
+static int cache_iftype = -1;
+static int cache_devnum = -1;
+static lbaint_t cache_start = -1;
+static lbaint_t cache_blkcnt = -1;
> +static unsigned long cache_blksz;
> +static void *cache;
Since variable "cache" is in BSS and hence is 0, which is included in
all checks below, none of the other values need to be initialized.
+int cache_block_read(int iftype, int dev,
+ lbaint_t start, lbaint_t blkcnt,
+ unsigned long blksz, void *buffer)
+{
+ if ((iftype == cache_iftype) &&
+ (dev == cache_devnum) &&
+ (start == cache_start) &&
+ (blkcnt <= cache_blkcnt) &&
+ (blksz == cache_blksz) &&
+ (cache != 0)) {
I'd suggest putting (cache != 0) first, since that check indicates
whether any of the others are relevant.
+ memcpy(buffer, cache, blksz*blkcnt);
Nit: space around the * operator. Same comment everywhere.
It's probably hard to fix this given the simple API and lack of MMU
page-tables to remap on a per-page basis, but one deficiency in this
(deliberately) simple implementation is that if someone caches blocks
0..7 then later tries to read 2..10 (or even 2..3) they won't get a
cache hit. While partial hits (the 2..10 case) are hard to deal with,
perhaps it's useful to make subset cases (2..3 with 0..7 cached) work?
+void cache_block_fill(int iftype, int dev,
+ lbaint_t start, lbaint_t blkcnt,
+ unsigned long blksz, void const *buffer)
+ bytes = blksz*blkcnt;
+ if (cache != 0) {
+ if (bytes != (cache_blksz*cache_blkcnt)) {
+ free(cache);
+ cache = malloc(blksz*blkcnt);
+ if (!cache)
+ return;
+ } /* change in size */
+ } else {
+ cache = malloc(blksz*blkcnt);
+ if (!cache)
+ return;
+ }
Is it better to put the if (!cache) test after the whole if/else block
so it's unified?
+void cache_block_invalidate(int iftype, int dev)
+{
+ cache_iftype = -1;
That isn't actually necessary, since assigning 0 to cache below will
prevent anything from using the cache.
+ if (cache) {
+ free(cache);
+ cache = 0;
+ }
+}
diff --git a/include/part.h b/include/part.h
+#ifdef CONFIG_BLOCK_CACHE
+/**
+ * cache_block_read() - attempt to read a set of blocks from cache
+ *
+ * @param iftype - IF_TYPE_x for type of device
+ * @param dev - device index of particular type
+ * @param start - starting block number
+ * @param blkcnt - number of blocks to read
+ * @param blksz - size in bytes of each block
+ * @param buf - buffer to contain cached data
s/contain/receive/?
+ *
+ * @return - '1' if block returned from cache, '0' otherwise.
+ */
+int cache_block_read
+ (int iftype, int dev,
+ lbaint_t start, lbaint_t blkcnt,
+ unsigned long blksz, void *buffer);
Nit: ( and probably the first n parameters should be on the same line as
the function name.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
