Re: [PATCH 3/6] bootstd: Add bootflow_iter_check_mmc() helper

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Igor,

Thank you for the review.

On lun., juin 10, 2024 at 11:31, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:

> Hi Mattijs,
>
> On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
>>
>> Some bootflows might be able to only boot from MMC devices.
>>
>> Add a helper function these bootflows can use.
>>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> ---
>>  boot/bootflow.c    | 12 ++++++++++++
>>  include/bootflow.h |  9 +++++++++
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/boot/bootflow.c b/boot/bootflow.c
>> index 9aa3179c3881..59d77d2385f4 100644
>> --- a/boot/bootflow.c
>> +++ b/boot/bootflow.c
>> @@ -575,6 +575,18 @@ int bootflow_iter_check_blk(const struct bootflow_iter *iter)
>>         return -ENOTSUPP;
>>  }
>>
>> +int bootflow_iter_check_mmc(const struct bootflow_iter *iter)
>> +{
>> +       const struct udevice *media = dev_get_parent(iter->dev);
>> +       enum uclass_id id = device_get_uclass_id(media);
>> +
>> +       log_debug("uclass %d: %s\n", id, uclass_get_name(id));
>> +       if (id == UCLASS_MMC)
>> +               return 0;
>> +
>> +       return -ENOTSUPP;
>> +}
>> +
>>  int bootflow_iter_check_sf(const struct bootflow_iter *iter)
>>  {
>>         const struct udevice *media = dev_get_parent(iter->dev);
>> diff --git a/include/bootflow.h b/include/bootflow.h
>> index 080ee8501225..6058ddd89b16 100644
>> --- a/include/bootflow.h
>> +++ b/include/bootflow.h
>> @@ -407,6 +407,15 @@ void bootflow_remove(struct bootflow *bflow);
>>   */
>>  int bootflow_iter_check_blk(const struct bootflow_iter *iter);
>>
>> +/**
>> + * bootflow_iter_check_mmc() - Check that a bootflow uses a MMC device
>> + *
>> + * This checks the bootdev in the bootflow to make sure it uses a mmc device
>> + *
>> + * Return: 0 if OK, -ENOTSUPP if some other device is used (e.g. ethernet)
>> + */
>> +int bootflow_iter_check_mmc(const struct bootflow_iter *iter);
>> +
>>  /**
>>   * bootflow_iter_check_sf() - Check that a bootflow uses SPI FLASH
>>   *
>>
>> --
>> 2.45.0
>>
>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
>
> A bit offtopic (just an idea for future refactoring), but I think all these
> bootflow_iter_check_* helpers should be replaced by just one
> int bootflow_iter_check_id(const struct bootflow_iter *iter, enum uclass_id id)
> to avoid code duplication or at least keep all these
> bootmedia-specific functions as
> wrappers with one-line call to bootflow_iter_check_id(iter,
> UCLASS_SPI_*) inside.

I like this idea as well, I'll consider to implement this as a future refactoring.

>
> -- 
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk