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

[Mattijs Korpershoek]

Hi Igor,

Thank you for the review.

On lun., juin 10, 2024 at 11:31, Igor Opaniuk  wrote:

> Hi Mattijs,
>
> On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
>  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 
>> ---
>>  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 
>
> 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.opan...@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk

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

[Igor Opaniuk]

Hi Mattijs,

On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
 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 
> ---
>  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 

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.

-- 
Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk

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

[Mattijs Korpershoek]

Some bootflows might be able to only boot from MMC devices.

Add a helper function these bootflows can use.

Signed-off-by: Mattijs Korpershoek 
---
 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