diff mbox series

[v2,1/7] common: avb_verify: don't call mmc_switch_part for SD

Message ID 20240209192045.3961832-2-igor.opaniuk@foundries.io
State Accepted
Commit c2f72d00ab88402038d3c9625124575715acdf4f
Delegated to: Mattijs Korpershoek
Headers show
Series AVB: cosmetic adjustments/improvements | expand

Commit Message

Igor Opaniuk Feb. 9, 2024, 7:20 p.m. UTC 
From: Igor Opaniuk <igor.opaniuk@gmail.com>

mmc_switch_part() is used for switching between hw partitions
on eMMC (boot0, boot1, user, rpmb).
There is no need to do that for SD card.

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
---

Changes in v2:
- Mattijs Korpershoek R-b tag applied

 common/avb_verify.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Dragan Simic Feb. 9, 2024, 7:37 p.m. UTC | #1 
On 2024-02-09 20:20, Igor Opaniuk wrote:
> From: Igor Opaniuk <igor.opaniuk@gmail.com>
> 
> mmc_switch_part() is used for switching between hw partitions
> on eMMC (boot0, boot1, user, rpmb).
> There is no need to do that for SD card.
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>

Looking good to me.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
> 
> Changes in v2:
> - Mattijs Korpershoek R-b tag applied
> 
>  common/avb_verify.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index 48ba8db51e5..59f2c25e0de 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -358,9 +358,11 @@ static struct mmc_part *get_partition(AvbOps
> *ops, const char *partition)
>  		goto err;
>  	}
> 
> -	ret = mmc_switch_part(part->mmc, part_num);
> -	if (ret)
> -		goto err;
> +	if (IS_MMC(part->mmc)) {
> +		ret = mmc_switch_part(part->mmc, part_num);
> +		if (ret)
> +			goto err;
> +	}
> 
>  	mmc_blk = mmc_get_blk_desc(part->mmc);
>  	if (!mmc_blk) {
Dan Carpenter Feb. 12, 2024, 7:05 a.m. UTC | #2 
On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
> From: Igor Opaniuk <igor.opaniuk@gmail.com>
> 
> mmc_switch_part() is used for switching between hw partitions
> on eMMC (boot0, boot1, user, rpmb).
> There is no need to do that for SD card.
> 

Is this a clean up or a bugfix?  What are the visible effects for the
user?

regards,
dan carpenter
Igor Opaniuk Feb. 12, 2024, 8:05 a.m. UTC | #3 
Hi Dan,

On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
> > From: Igor Opaniuk <igor.opaniuk@gmail.com>
> >
> > mmc_switch_part() is used for switching between hw partitions
> > on eMMC (boot0, boot1, user, rpmb).
> > There is no need to do that for SD card.
> >
>
> Is this a clean up or a bugfix?  What are the visible effects for the
> user?
avb cmd fails for SD cards, as mmc_switch_part() fails after trying to access
EXT_CSD register, which obviously is not available.
>
> regards,
> dan carpenter
>
Mattijs Korpershoek Feb. 13, 2024, 8:13 a.m. UTC | #4 
Hi Igor,

On lun., févr. 12, 2024 at 09:05, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:

> Hi Dan,
>
> On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>>
>> On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
>> > From: Igor Opaniuk <igor.opaniuk@gmail.com>
>> >
>> > mmc_switch_part() is used for switching between hw partitions
>> > on eMMC (boot0, boot1, user, rpmb).
>> > There is no need to do that for SD card.
>> >
>>
>> Is this a clean up or a bugfix?  What are the visible effects for the
>> user?
> avb cmd fails for SD cards, as mmc_switch_part() fails after trying to access
> EXT_CSD register, which obviously is not available.

Does this means that we only need this patch to fix AVB commands when
booting from SD cards?

If yes, I propose adding the following note to the commit message:

"This fixes the avb command usage on on SD cards."

If you agree, I can do this while applying.

If not, we can keep the message as is.

>>
>> regards,
>> dan carpenter
>>
>
>
> -- 
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk@gmail.com
> skype: igor.opanyuk
> http://ua.linkedin.com/in/iopaniuk
Igor Opaniuk Feb. 13, 2024, 11:19 a.m. UTC | #5 
Hi Mattijs,

On Tue, Feb 13, 2024 at 9:13 AM Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> Hi Igor,
>
> On lun., févr. 12, 2024 at 09:05, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> > Hi Dan,
> >
> > On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >>
> >> On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
> >> > From: Igor Opaniuk <igor.opaniuk@gmail.com>
> >> >
> >> > mmc_switch_part() is used for switching between hw partitions
> >> > on eMMC (boot0, boot1, user, rpmb).
> >> > There is no need to do that for SD card.
> >> >
> >>
> >> Is this a clean up or a bugfix?  What are the visible effects for the
> >> user?
> > avb cmd fails for SD cards, as mmc_switch_part() fails after trying to access
> > EXT_CSD register, which obviously is not available.
>
> Does this means that we only need this patch to fix AVB commands when
> booting from SD cards?
>
> If yes, I propose adding the following note to the commit message:
>
> "This fixes the avb command usage on on SD cards."
>
> If you agree, I can do this while applying.
Yes, could you please add to the commit message so I don't
send v3 for that (if there are no any additional objections/comments)

Thanks
>
> If not, we can keep the message as is.
>
> >>
> >> regards,
> >> dan carpenter
> >>
> >
> >
> > --
> > Best regards - Atentamente - Meilleures salutations
> >
> > Igor Opaniuk
> >
> > mailto: igor.opaniuk@gmail.com
> > skype: igor.opanyuk
> > http://ua.linkedin.com/in/iopaniuk
Mattijs Korpershoek Feb. 13, 2024, 1:31 p.m. UTC | #6 
Hi Igor,

On mar., févr. 13, 2024 at 12:19, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:

> Hi Mattijs,
>
> On Tue, Feb 13, 2024 at 9:13 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
>>
>> Hi Igor,
>>
>> On lun., févr. 12, 2024 at 09:05, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>>
>> > Hi Dan,
>> >
>> > On Mon, Feb 12, 2024 at 8:05 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>> >>
>> >> On Fri, Feb 09, 2024 at 08:20:39PM +0100, Igor Opaniuk wrote:
>> >> > From: Igor Opaniuk <igor.opaniuk@gmail.com>
>> >> >
>> >> > mmc_switch_part() is used for switching between hw partitions
>> >> > on eMMC (boot0, boot1, user, rpmb).
>> >> > There is no need to do that for SD card.
>> >> >
>> >>
>> >> Is this a clean up or a bugfix?  What are the visible effects for the
>> >> user?
>> > avb cmd fails for SD cards, as mmc_switch_part() fails after trying to access
>> > EXT_CSD register, which obviously is not available.
>>
>> Does this means that we only need this patch to fix AVB commands when
>> booting from SD cards?
>>
>> If yes, I propose adding the following note to the commit message:
>>
>> "This fixes the avb command usage on on SD cards."
>>
>> If you agree, I can do this while applying.
> Yes, could you please add to the commit message so I don't
> send v3 for that (if there are no any additional objections/comments)

There are no additional objections, comments on my end.

I will add this to the commit message when applying.

>
> Thanks
>>
>> If not, we can keep the message as is.
>>
>> >>
>> >> regards,
>> >> dan carpenter
>> >>
>> >
>> >
>> > --
>> > Best regards - Atentamente - Meilleures salutations
>> >
>> > Igor Opaniuk
>> >
>> > mailto: igor.opaniuk@gmail.com
>> > skype: igor.opanyuk
>> > http://ua.linkedin.com/in/iopaniuk
>
>
>
> -- 
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk@gmail.com
> skype: igor.opanyuk
> http://ua.linkedin.com/in/iopaniuk
diff mbox series

Patch

diff --git a/common/avb_verify.c b/common/avb_verify.c
index 48ba8db51e5..59f2c25e0de 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -358,9 +358,11 @@  static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
 		goto err;
 	}
 
-	ret = mmc_switch_part(part->mmc, part_num);
-	if (ret)
-		goto err;
+	if (IS_MMC(part->mmc)) {
+		ret = mmc_switch_part(part->mmc, part_num);
+		if (ret)
+			goto err;
+	}
 
 	mmc_blk = mmc_get_blk_desc(part->mmc);
 	if (!mmc_blk) {