[U-Boot] [PATCH 2/5] common: add blkcache init

[Eric Nelson]

Hi Angelo,

On 11/25/19 2:59 AM, Angelo Dureghello wrote:
> Hi Eric,
> 
> On Sun, Nov 24, 2019 at 5:00 PM Eric Nelson <ericnelsonaz at gmail.com> wrote:
>>
>> Hi Angelo,
>>
>> On 11/23/19 3:47 PM, Angelo Dureghello wrote:
>>> From: Angelo Durgehello <angelo.dureghello at timesys.com>
>>>
>>> On m68k, block_cache list is relocated, but next and prev list
>>> pointers are not adjusted to the relocated struct list_head address,
>>> so the first iteration over the block_cache list hangs.
>>>
>>> This patch initializes the block_cache list after relocation.
>>>
>>> Signed-off-by: Angelo Durgehello <angelo.dureghello at timesys.com>
>>> ---
>>>    common/board_r.c         | 12 ++++++++++++
>>>    drivers/block/blkcache.c |  7 ++++++-
>>>    include/blk.h            |  6 ++++++
>>>    3 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/board_r.c b/common/board_r.c
>>> index 65720849cd..13e70a5ffb 100644
>>> --- a/common/board_r.c
>>> +++ b/common/board_r.c
>>> @@ -628,6 +628,15 @@ static int initr_bedbug(void)
>>>    }
>>>    #endif
>>>
>>> +#ifdef CONFIG_BLOCK_CACHE
>>> +static int initr_blkcache(void)
>>> +{
>>> +     blkcache_init();
>>> +
>>> +     return 0;
>>> +}
>>> +#endif
>>> +
>>
>> Why the extra level of indirection?
>>
>>>    static int run_main_loop(void)
>>>    {
>>>    #ifdef CONFIG_SANDBOX
>>> @@ -832,6 +841,9 @@ static init_fnc_t init_sequence_r[] = {
>>>    #endif
>>>    #if defined(CONFIG_PRAM)
>>>        initr_mem,
>>> +#endif
>>> +#ifdef CONFIG_BLOCK_CACHE
>>
>> It seems you could call blkcache_init from here directly:
>>
> 
> reason for this is to maintain "initr_" naming convention, used from
> quite all the initr_ calls,
> as i.e. static int initr_mmc(void) that's doing the same.
> 

Okay. I think this isn't a hard rule though (see log_init,
stdio_init_tables, etc).

I'm not sure that it would be a bad thing to rename blkcache_init
to initr_blkcache to indicate the usage.

>>
>>> +     initr_blkcache,
>>>    #endif
>>>        run_main_loop,
>>>    };
>>> diff --git a/drivers/block/blkcache.c b/drivers/block/blkcache.c
>>> index 1fa64989d3..bf0fa1ea6f 100644
>>> --- a/drivers/block/blkcache.c
>>> +++ b/drivers/block/blkcache.c
>>> @@ -21,13 +21,18 @@ struct block_cache_node {
>>>        char *cache;
>>>    };
>>>
>>> -static LIST_HEAD(block_cache);
>>> +static struct list_head block_cache;
>>>
>>>    static struct block_cache_stats _stats = {
>>>        .max_blocks_per_entry = 8,
>>>        .max_entries = 32
>>>    };
>>>
>>> +void blkcache_init(void)
>>> +{
>>> +     INIT_LIST_HEAD(&block_cache);
>>> +}
>>> +
>>>    static struct block_cache_node *cache_find(int iftype, int devnum,
>>>                                           lbaint_t start, lbaint_t blkcnt,
>>>                                           unsigned long blksz)
>>> diff --git a/include/blk.h b/include/blk.h
>>> index d0c033aece..7070fd6af3 100644
>>> --- a/include/blk.h
>>> +++ b/include/blk.h
>>> @@ -113,6 +113,12 @@ struct blk_desc {
>>>        (PAD_SIZE(size, blk_desc->blksz))
>>>
>>>    #if CONFIG_IS_ENABLED(BLOCK_CACHE)
>>> +
>>> +/**
>>> + * blkcache_init() - initialize the block cache list pointers
>>> + */
>>> +void blkcache_init(void);
>>> +
>>>    /**
>>>     * blkcache_read() - attempt to read a set of blocks from cache
>>>     *
>>>
>>
> Regards,
> --
> Angelo Dureghello
>