Re: [PATCH 16/26] spl: Don't cache devices when UNIT_TEST is enabled

[Sean Anderson <seanga2@gmail.com>]

On 10/11/23 23:41, Simon Glass wrote:
> Hi Sean,
> 
> On Wed, 11 Oct 2023 at 18:56, Sean Anderson <seanga2@gmail.com> wrote:
>>
>> Several SPL functions try to avoid performing initialization twice by
>> caching devices. This is fine for regular boot, but does not work with
>> UNIT_TEST, since all devices are torn down after each test. Disable caching
>> so we don't use stale devices.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>   common/spl/spl_fat.c | 2 +-
>>   common/spl/spl_mmc.c | 3 ++-
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>> index c6e2526ade1..8bec9cce5ca 100644
>> --- a/common/spl/spl_fat.c
>> +++ b/common/spl/spl_fat.c
>> @@ -24,7 +24,7 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>>   {
>>          int err = 0;
>>
>> -       if (fat_registered)
>> +       if (!CONFIG_IS_ENABLED(UNIT_TEST) && fat_registered)
> 
> These sort of things worry me, since we are bringing test code /
> conditions into the 'real' code. Is it possible to pass this as a
> parameter, or adjust the value of fat_registered?

At the moment these variables are static, and I would like to keep them that way
to avoid size growth. We really are doing an unusual thing with UNIT_TEST where
devices which were previously valid can be free'd and then recreated later. This
is why I decided on using UNIT_TEST as the determiner here, since nothing else
should cause this situation.

--Sean

>>                  return err;
>>
>>          err = fat_register_device(block_dev, partition);
>> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
>> index 67c7ae34a58..a8579e29dee 100644
>> --- a/common/spl/spl_mmc.c
>> +++ b/common/spl/spl_mmc.c
>> @@ -417,7 +417,8 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>>
>>          /* Perform peripheral init only once for an mmc device */
>>          mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
>> -       if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
>> +       if (CONFIG_IS_ENABLED(UNIT_TEST) || !mmc ||
>> +           spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
>>                  err = spl_mmc_find_device(&mmc, bootdev->boot_device);
>>                  if (err)
>>                          return err;
>> --
>> 2.37.1
>>
> 
> Regards,
> Simon