[PATCH] Fix fastboot handling of partitions when no slots are supported

[PATCH] Fix fastboot handling of partitions when no slots are supported

[Federico Fuga via B4 Relay]

From: Federico Fuga <fuga@studiofuga.com>

The fastboot module has a bug that prevents some command to work
properly on devices that haven't an Android-like partition scheme, that
is, just one spl and one kernel partition, instead of the redundant
scheme with _a and _b slots.

This is the schema of our NAND storage (board is based on an AllWinner
A33 sunxi chip):

=> mtdparts

device nand0 <1c03000.nand>, # parts = 4
 #: name         size            net size        offset          mask_flags
 0: spl          0x00020000      0x00020000      0x00000000      0
 1: uboot        0x00100000      0x00100000      0x00020000      0
 2: kernel_a     0x00400000      0x00400000      0x00120000      0
 3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0

active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000

This happens when we try to erase the spl partition using fastboot:

$ fastboot erase spl
Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
fastboot: error: Command failed

The error occurs because getvars fails to handle the error returned by
nand layer when a partition cannot be found.

Indeed, getvar_get_part_info returns what is returned by
fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
return -ENODEV or -EINVAL instead. Since the cause of failure is not
returned by the nand function, I decided to return -EINVAL to make it
simple.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
 drivers/fastboot/fb_getvar.c | 5 ++++-
 drivers/fastboot/fb_nand.c   | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..816ed8a6213b5c1f0948a813c6f6a865a4b47ba8 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -121,8 +121,11 @@ static int getvar_get_part_info(const char *part_name, char *response,
 			*size = disk_part.size * disk_part.blksz;
 	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
 		r = fastboot_nand_get_part_info(part_name, &part_info, response);
-		if (r >= 0 && size)
+		if (r == 0 && size) {
 			*size = part_info->size;
+		} else {
+			r = -EINVAL;
+		}
 	} else {
 		fastboot_fail("this storage is not supported in bootloader", response);
 		r = -ENODEV;
diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
index afc64fd5280717ae4041ed70268ccc01cfbb0496..9e2f7c01895785a4409eb67ea48abd02a6a6da26 100644
--- a/drivers/fastboot/fb_nand.c
+++ b/drivers/fastboot/fb_nand.c
@@ -151,7 +151,7 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
  *
  * @part_name: Named device to lookup
  * @part_info: Pointer to returned part_info pointer
- * @response: Pointer to fastboot response buffer
+ * @response: 0 on success, 1 otherwise
  */
 int fastboot_nand_get_part_info(const char *part_name,
 				struct part_info **part_info, char *response)

---
base-commit: a517796cfa5d8f4ca2f0c11c78c24a08a102c047
change-id: 20250128-fastboot_slot_fix-69251576d9bb

Best regards,
-- 
Federico Fuga <fuga@studiofuga.com>

Re: [PATCH] Fix fastboot handling of partitions when no slots are supported

[Mattijs Korpershoek]

Hi Federico,

Thank you for the patch.

On mar., janv. 28, 2025 at 12:18, Federico Fuga via B4 Relay <devnull+fuga.studiofuga.com@kernel.org> wrote:

> From: Federico Fuga <fuga@studiofuga.com>
>
> The fastboot module has a bug that prevents some command to work
> properly on devices that haven't an Android-like partition scheme, that
> is, just one spl and one kernel partition, instead of the redundant
> scheme with _a and _b slots.
>
> This is the schema of our NAND storage (board is based on an AllWinner
> A33 sunxi chip):
>
> => mtdparts
>
> device nand0 <1c03000.nand>, # parts = 4
>  #: name         size            net size        offset          mask_flags
>  0: spl          0x00020000      0x00020000      0x00000000      0
>  1: uboot        0x00100000      0x00100000      0x00020000      0
>  2: kernel_a     0x00400000      0x00400000      0x00120000      0
>  3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0
>
> active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000
>
> This happens when we try to erase the spl partition using fastboot:
>
> $ fastboot erase spl
> Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
> fastboot: error: Command failed
>
> The error occurs because getvars fails to handle the error returned by
> nand layer when a partition cannot be found.
>
> Indeed, getvar_get_part_info returns what is returned by
> fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
> return -ENODEV or -EINVAL instead. Since the cause of failure is not
> returned by the nand function, I decided to return -EINVAL to make it
> simple.
>
> Signed-off-by: Federico Fuga <fuga@studiofuga.com>

I could apply this version, finally :)

b4 got a bit confused since it has been send as a v1 but using the
following command worked for me:

$ b4 shazam -s -l --check 20250128-fastboot_slot_fix-v1-1-1a4688936662@studiofuga.com -v1

Now for the patch itself:

Please use 'fastboot:' prefix as a commit title.
Use '$ git log --oneline -- drivers/fastboot/' to see what other commits
where using as a title.

Some more comments below. Please make sure to answer this email to
acknowledge/reject review feedback.
Also, when sending a v2, please make sure to include a changelog.

If you need help setting things up properly, let me know. I'm also
reachable on IRC https://libera.chat/, channel #u-boot, my nickame is mkorpershoek.

> ---
>  drivers/fastboot/fb_getvar.c | 5 ++++-
>  drivers/fastboot/fb_nand.c   | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..816ed8a6213b5c1f0948a813c6f6a865a4b47ba8 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -121,8 +121,11 @@ static int getvar_get_part_info(const char *part_name, char *response,
>  			*size = disk_part.size * disk_part.blksz;
>  	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
>  		r = fastboot_nand_get_part_info(part_name, &part_info, response);
> -		if (r >= 0 && size)
> +		if (r == 0 && size) {
>  			*size = part_info->size;

Maybe the patch is simpler this way, but I think that it would be better
if fastboot_mmc_get_part_info() and fastboot_nand_get_part_info() would
behave the same. The naming is pretty close already, and having
different behaviours/return codes seems confusing to me.

Is there a strong reason for not modifying fb_nand_lookup() so that it
will return a negative error code?

This way, we can keep the same logic in getvar_get_part_info()

> +		} else {
> +			r = -EINVAL;
> +		}
>  	} else {
>  		fastboot_fail("this storage is not supported in bootloader", response);
>  		r = -ENODEV;
> diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
> index afc64fd5280717ae4041ed70268ccc01cfbb0496..9e2f7c01895785a4409eb67ea48abd02a6a6da26 100644
> --- a/drivers/fastboot/fb_nand.c
> +++ b/drivers/fastboot/fb_nand.c
> @@ -151,7 +151,7 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
>   *
>   * @part_name: Named device to lookup
>   * @part_info: Pointer to returned part_info pointer
> - * @response: Pointer to fastboot response buffer
> + * @response: 0 on success, 1 otherwise

Why has this been modified? @response is still a pointer to fastboot
response buffer.

If we wish to document the return code, use the Return: syntax:

For example, here it would be:

/**
 * fastboot_nand_get_part_info() - Lookup NAND partion by name
 *
 * @part_name: Named device to lookup
 * @part_info: Pointer to returned part_info pointer
 * @response: Pointer to fastboot response buffer
 *
 * Return: 0 on success, 1 otherwise
 */

>   */
>  int fastboot_nand_get_part_info(const char *part_name,
>  				struct part_info **part_info, char *response)
>
> ---
> base-commit: a517796cfa5d8f4ca2f0c11c78c24a08a102c047
> change-id: 20250128-fastboot_slot_fix-69251576d9bb
>
> Best regards,
> -- 
> Federico Fuga <fuga@studiofuga.com>

Re: [PATCH] Fix fastboot handling of partitions when no slots are supported

[Federico Fuga]

Hi Mattijs,

thanks for your kind response and suggestions.

We are actually suspending the development on this board, because it 
seems the support for our hardware (sunxi A33) requires too much work 
than expected. We were trying to update our 2018-flavoured bootloader 
but it seems we did a lot of steps back, for reason I can't fully grasp.

Anyway, I'll try to work on this patch in my spare time.


Specifically:



On 30/01/25 14:49, Mattijs Korpershoek wrote:
> Hi Federico,
>
> Thank you for the patch.
>
> On mar., janv. 28, 2025 at 12:18, Federico Fuga via B4 Relay <devnull+fuga.studiofuga.com@kernel.org> wrote:
>
>> From: Federico Fuga <fuga@studiofuga.com>
>>
>> The fastboot module has a bug that prevents some command to work
>> properly on devices that haven't an Android-like partition scheme, that
>> is, just one spl and one kernel partition, instead of the redundant
>> scheme with _a and _b slots.
>>
>> This is the schema of our NAND storage (board is based on an AllWinner
>> A33 sunxi chip):
>>
>> => mtdparts
>>
>> device nand0 <1c03000.nand>, # parts = 4
>>   #: name         size            net size        offset          mask_flags
>>   0: spl          0x00020000      0x00020000      0x00000000      0
>>   1: uboot        0x00100000      0x00100000      0x00020000      0
>>   2: kernel_a     0x00400000      0x00400000      0x00120000      0
>>   3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0
>>
>> active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000
>>
>> This happens when we try to erase the spl partition using fastboot:
>>
>> $ fastboot erase spl
>> Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
>> fastboot: error: Command failed
>>
>> The error occurs because getvars fails to handle the error returned by
>> nand layer when a partition cannot be found.
>>
>> Indeed, getvar_get_part_info returns what is returned by
>> fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
>> return -ENODEV or -EINVAL instead. Since the cause of failure is not
>> returned by the nand function, I decided to return -EINVAL to make it
>> simple.
>>
>> Signed-off-by: Federico Fuga <fuga@studiofuga.com>
> I could apply this version, finally :)
>
> b4 got a bit confused since it has been send as a v1 but using the
> following command worked for me:
>
> $ b4 shazam -s -l --check 20250128-fastboot_slot_fix-v1-1-1a4688936662@studiofuga.com -v1


Yes I did notice this, I tried it after sending and I was puzzled why it 
spit out the same error as before. I'm still learning.


> Now for the patch itself:
>
> Please use 'fastboot:' prefix as a commit title.
> Use '$ git log --oneline -- drivers/fastboot/' to see what other commits
> where using as a title.
>
> Some more comments below. Please make sure to answer this email to
> acknowledge/reject review feedback.
> Also, when sending a v2, please make sure to include a changelog.
>
> If you need help setting things up properly, let me know. I'm also
> reachable on IRC https://libera.chat/, channel #u-boot, my nickame is mkorpershoek.


Thanks, I'll surely do.


>> ---
>>   drivers/fastboot/fb_getvar.c | 5 ++++-
>>   drivers/fastboot/fb_nand.c   | 2 +-
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
>> index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..816ed8a6213b5c1f0948a813c6f6a865a4b47ba8 100644
>> --- a/drivers/fastboot/fb_getvar.c
>> +++ b/drivers/fastboot/fb_getvar.c
>> @@ -121,8 +121,11 @@ static int getvar_get_part_info(const char *part_name, char *response,
>>   			*size = disk_part.size * disk_part.blksz;
>>   	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
>>   		r = fastboot_nand_get_part_info(part_name, &part_info, response);
>> -		if (r >= 0 && size)
>> +		if (r == 0 && size) {
>>   			*size = part_info->size;
> Maybe the patch is simpler this way, but I think that it would be better
> if fastboot_mmc_get_part_info() and fastboot_nand_get_part_info() would
> behave the same. The naming is pretty close already, and having
> different behaviours/return codes seems confusing to me.


Indeed, having the same code would be definitely proper.


> Is there a strong reason for not modifying fb_nand_lookup() so that it
> will return a negative error code?


The reason was I didn't think about it. I misread the code and thought 
the error code were different.


>
> This way, we can keep the same logic in getvar_get_part_info()
>> +		} else {
>> +			r = -EINVAL;
>> +		}
>>   	} else {
>>   		fastboot_fail("this storage is not supported in bootloader", response);
>>   		r = -ENODEV;
>> diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
>> index afc64fd5280717ae4041ed70268ccc01cfbb0496..9e2f7c01895785a4409eb67ea48abd02a6a6da26 100644
>> --- a/drivers/fastboot/fb_nand.c
>> +++ b/drivers/fastboot/fb_nand.c
>> @@ -151,7 +151,7 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
>>    *
>>    * @part_name: Named device to lookup
>>    * @part_info: Pointer to returned part_info pointer
>> - * @response: Pointer to fastboot response buffer
>> + * @response: 0 on success, 1 otherwise
> Why has this been modified? @response is still a pointer to fastboot
> response buffer.


Yes, reason is same as above. I simply misread the code.


Thanks


Federico


> If we wish to document the return code, use the Return: syntax:
>
> For example, here it would be:
>
> /**
>   * fastboot_nand_get_part_info() - Lookup NAND partion by name
>   *
>   * @part_name: Named device to lookup
>   * @part_info: Pointer to returned part_info pointer
>   * @response: Pointer to fastboot response buffer
>   *
>   * Return: 0 on success, 1 otherwise
>   */
>
>>    */
>>   int fastboot_nand_get_part_info(const char *part_name,
>>   				struct part_info **part_info, char *response)
>>
>> ---
>> base-commit: a517796cfa5d8f4ca2f0c11c78c24a08a102c047
>> change-id: 20250128-fastboot_slot_fix-69251576d9bb
>>
>> Best regards,
>> -- 
>> Federico Fuga <fuga@studiofuga.com>

Re: [PATCH] Fix fastboot handling of partitions when no slots are supported

[Mattijs Korpershoek]

Hi Federico,

On jeu., janv. 30, 2025 at 16:03, Federico Fuga <fuga@studiofuga.com> wrote:

> Hi Mattijs,
>
> thanks for your kind response and suggestions.

Happy to help!

>
> We are actually suspending the development on this board, because it 
> seems the support for our hardware (sunxi A33) requires too much work 
> than expected. We were trying to update our 2018-flavoured bootloader 
> but it seems we did a lot of steps back, for reason I can't fully grasp.
>
> Anyway, I'll try to work on this patch in my spare time.

Thank you for contributing, it's very appreciated.

>
>
> Specifically:
>
>
>
> On 30/01/25 14:49, Mattijs Korpershoek wrote:
>> Hi Federico,
>>
>> Thank you for the patch.
>>
>> On mar., janv. 28, 2025 at 12:18, Federico Fuga via B4 Relay <devnull+fuga.studiofuga.com@kernel.org> wrote:
>>
>>> From: Federico Fuga <fuga@studiofuga.com>
>>>
>>> The fastboot module has a bug that prevents some command to work
>>> properly on devices that haven't an Android-like partition scheme, that
>>> is, just one spl and one kernel partition, instead of the redundant
>>> scheme with _a and _b slots.
>>>
>>> This is the schema of our NAND storage (board is based on an AllWinner
>>> A33 sunxi chip):
>>>
>>> => mtdparts
>>>
>>> device nand0 <1c03000.nand>, # parts = 4
>>>   #: name         size            net size        offset          mask_flags
>>>   0: spl          0x00020000      0x00020000      0x00000000      0
>>>   1: uboot        0x00100000      0x00100000      0x00020000      0
>>>   2: kernel_a     0x00400000      0x00400000      0x00120000      0
>>>   3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0
>>>
>>> active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000
>>>
>>> This happens when we try to erase the spl partition using fastboot:
>>>
>>> $ fastboot erase spl
>>> Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
>>> fastboot: error: Command failed
>>>
>>> The error occurs because getvars fails to handle the error returned by
>>> nand layer when a partition cannot be found.
>>>
>>> Indeed, getvar_get_part_info returns what is returned by
>>> fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
>>> return -ENODEV or -EINVAL instead. Since the cause of failure is not
>>> returned by the nand function, I decided to return -EINVAL to make it
>>> simple.
>>>
>>> Signed-off-by: Federico Fuga <fuga@studiofuga.com>
>> I could apply this version, finally :)
>>
>> b4 got a bit confused since it has been send as a v1 but using the
>> following command worked for me:
>>
>> $ b4 shazam -s -l --check 20250128-fastboot_slot_fix-v1-1-1a4688936662@studiofuga.com -v1
>
>
> Yes I did notice this, I tried it after sending and I was puzzled why it 
> spit out the same error as before. I'm still learning.

b4 tries to do some intelligent lookup where it matches the subject name
to find "other versions" of the same patch. Since it found some other
patch with V2, it thought it was the latest one.

If you keep using b4 this should not happen anymore, as it automatically
increments the patch version.

>
>
>> Now for the patch itself:
>>
>> Please use 'fastboot:' prefix as a commit title.
>> Use '$ git log --oneline -- drivers/fastboot/' to see what other commits
>> where using as a title.
>>
>> Some more comments below. Please make sure to answer this email to
>> acknowledge/reject review feedback.
>> Also, when sending a v2, please make sure to include a changelog.
>>
>> If you need help setting things up properly, let me know. I'm also
>> reachable on IRC https://libera.chat/, channel #u-boot, my nickame is mkorpershoek.
>
>
> Thanks, I'll surely do.

Do not hesitate. I'm not always around but I try to answer when possible :)

>
>
>>> ---
>>>   drivers/fastboot/fb_getvar.c | 5 ++++-
>>>   drivers/fastboot/fb_nand.c   | 2 +-
>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
>>> index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..816ed8a6213b5c1f0948a813c6f6a865a4b47ba8 100644
>>> --- a/drivers/fastboot/fb_getvar.c
>>> +++ b/drivers/fastboot/fb_getvar.c
>>> @@ -121,8 +121,11 @@ static int getvar_get_part_info(const char *part_name, char *response,
>>>   			*size = disk_part.size * disk_part.blksz;
>>>   	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
>>>   		r = fastboot_nand_get_part_info(part_name, &part_info, response);
>>> -		if (r >= 0 && size)
>>> +		if (r == 0 && size) {
>>>   			*size = part_info->size;
>> Maybe the patch is simpler this way, but I think that it would be better
>> if fastboot_mmc_get_part_info() and fastboot_nand_get_part_info() would
>> behave the same. The naming is pretty close already, and having
>> different behaviours/return codes seems confusing to me.
>
>
> Indeed, having the same code would be definitely proper.

Yes. I thinks that if fastboot_mmc_get_part_info() and
fastboot_nand_get_part_info() don't behave the same, it will cause
confusing.

>
>
>> Is there a strong reason for not modifying fb_nand_lookup() so that it
>> will return a negative error code?
>
>
> The reason was I didn't think about it. I misread the code and thought 
> the error code were different.

Ok, thanks

>
>
>>
>> This way, we can keep the same logic in getvar_get_part_info()
>>> +		} else {
>>> +			r = -EINVAL;
>>> +		}
>>>   	} else {
>>>   		fastboot_fail("this storage is not supported in bootloader", response);
>>>   		r = -ENODEV;
>>> diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
>>> index afc64fd5280717ae4041ed70268ccc01cfbb0496..9e2f7c01895785a4409eb67ea48abd02a6a6da26 100644
>>> --- a/drivers/fastboot/fb_nand.c
>>> +++ b/drivers/fastboot/fb_nand.c
>>> @@ -151,7 +151,7 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
>>>    *
>>>    * @part_name: Named device to lookup
>>>    * @part_info: Pointer to returned part_info pointer
>>> - * @response: Pointer to fastboot response buffer
>>> + * @response: 0 on success, 1 otherwise
>> Why has this been modified? @response is still a pointer to fastboot
>> response buffer.
>
>
> Yes, reason is same as above. I simply misread the code.

Understood.

>
>
> Thanks
>
>
> Federico
>
>
>> If we wish to document the return code, use the Return: syntax:
>>
>> For example, here it would be:
>>
>> /**
>>   * fastboot_nand_get_part_info() - Lookup NAND partion by name
>>   *
>>   * @part_name: Named device to lookup
>>   * @part_info: Pointer to returned part_info pointer
>>   * @response: Pointer to fastboot response buffer
>>   *
>>   * Return: 0 on success, 1 otherwise
>>   */
>>
>>>    */
>>>   int fastboot_nand_get_part_info(const char *part_name,
>>>   				struct part_info **part_info, char *response)
>>>
>>> ---
>>> base-commit: a517796cfa5d8f4ca2f0c11c78c24a08a102c047
>>> change-id: 20250128-fastboot_slot_fix-69251576d9bb
>>>
>>> Best regards,
>>> -- 
>>> Federico Fuga <fuga@studiofuga.com>

[PATCH] Fix fastboot handling of partitions when no slots are supported

[Federico Fuga]

The fastboot module has a bug that prevents some command to work
properly on devices that haven't an Android-like partition scheme, that
is, just one spl and one kernel partition, instead of the redundant
scheme with _a and _b slots.

This is the schema of our NAND storage (board is based on an AllWinner
A33 sunxi chip):

=> mtdparts

device nand0 <1c03000.nand>, # parts = 4
 #: name         size            net size        offset          mask_flags
 0: spl          0x00020000      0x00020000      0x00000000      0
 1: uboot        0x00100000      0x00100000      0x00020000      0
 2: kernel_a     0x00400000      0x00400000      0x00120000      0
 3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0

active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000

This happens when we try to erase the spl partition using fastboot:

$ fastboot erase spl
Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
fastboot: error: Command failed

The error occurs because getvars fails to handle the error returned by
nand layer when a partition cannot be found.

Indeed, getvar_get_part_info returns what is returned by
fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
return -ENODEV or -EINVAL instead. Since the cause of failure is not
returned by the nand function, I decided to return -EINVAL to make it
simple.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
 drivers/fastboot/fb_getvar.c | 5 ++++-
 drivers/fastboot/fb_nand.c   | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 9c2ce65a4e..816ed8a621 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -121,8 +121,11 @@ static int getvar_get_part_info(const char *part_name, char *response,
 			*size = disk_part.size * disk_part.blksz;
 	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
 		r = fastboot_nand_get_part_info(part_name, &part_info, response);
-		if (r >= 0 && size)
+		if (r == 0 && size) {
 			*size = part_info->size;
+		} else {
+			r = -EINVAL;
+		}
 	} else {
 		fastboot_fail("this storage is not supported in bootloader", response);
 		r = -ENODEV;
diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
index afc64fd528..9e2f7c0189 100644
--- a/drivers/fastboot/fb_nand.c
+++ b/drivers/fastboot/fb_nand.c
@@ -151,7 +151,7 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
  *
  * @part_name: Named device to lookup
  * @part_info: Pointer to returned part_info pointer
- * @response: Pointer to fastboot response buffer
+ * @response: 0 on success, 1 otherwise
  */
 int fastboot_nand_get_part_info(const char *part_name,
 				struct part_info **part_info, char *response)
-- 
2.48.1

[PATCH] Fix fastboot handling of partitions when no slots are, supported

[Federico Fuga]

[-- Attachment #1.1.1: Type: text/plain, Size: 3207 bytes --]

The fastboot module has a bug that prevents some command to work
properly on devices that haven't an Android-like partition scheme, that
is, just one spl and one kernel partition, instead of the redundant
scheme with _a and _b slots.

This is the schema of our NAND storage (board is based on an AllWinner
A33 sunxi chip):

=> mtdparts

device nand0 <1c03000.nand>, # parts = 4
  #: name         size            net size        offset mask_flags
  0: spl          0x00020000      0x00020000      0x00000000      0
  1: uboot        0x00100000      0x00100000      0x00020000      0
  2: kernel       0x00400000      0x00400000      0x00120000      0
  3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0

active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000

This happens when we try to erase the spl partition using fastboot:

$ fastboot erase spl
Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
fastboot: error: Command failed

The error occurs because getvars fails to handle the error returned by
nand layer when a partition cannot be found.

Indeed, getvar_get_part_info returns what is returned by
fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
return -ENODEV or -EINVAL instead. Since the cause of failure is not
returned by the nand function, I decided to return -EINVAL to make it
simple.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
  drivers/fastboot/fb_getvar.c | 5 ++++-
  drivers/fastboot/fb_nand.c   | 2 +-
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 93cbd598e0..f5d8b03301 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -121,8 +121,11 @@ static int getvar_get_part_info(const char 
*part_name, char *response,
              *size = disk_part.size * disk_part.blksz;
      } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
          r = fastboot_nand_get_part_info(part_name, &part_info, response);
-        if (r >= 0 && size)
+        if (r == 0 && size) {
              *size = part_info->size;
+        } else {
+            r = -EINVAL;
+        }
      } else {
          fastboot_fail("this storage is not supported in bootloader", 
response);
          r = -ENODEV;
diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
index 5a55144479..3ee0f40ea8 100644
--- a/drivers/fastboot/fb_nand.c
+++ b/drivers/fastboot/fb_nand.c
@@ -141,7 +141,7 @@ static lbaint_t fb_nand_sparse_reserve(struct 
sparse_storage *info,
   *
   * @part_name: Named device to lookup
   * @part_info: Pointer to returned part_info pointer
- * @response: Pointer to fastboot response buffer
+ * @response: 0 on success, 1 otherwise
   */
  int fastboot_nand_get_part_info(const char *part_name,
                  struct part_info **part_info, char *response)
-- 
2.48.1



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

[PATCH] Fix fastboot handling of partitions when no slots are, supported

[Federico Fuga]

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]


The fastboot module has a bug that prevents some command to work
properly on devices that haven't an Android-like partition scheme, that
is, just one spl and one kernel partition, instead of the redundant
scheme with _a and _b slots.

This is the schema of our NAND storage (board is based on an AllWinner
A33 sunxi chip):

=> mtdparts

device nand0 <1c03000.nand>, # parts = 4
   #: name         size            net size        offset mask_flags
   0: spl          0x00020000      0x00020000      0x00000000      0
   1: uboot        0x00100000      0x00100000      0x00020000      0
   2: kernel       0x00400000      0x00400000      0x00120000      0
   3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0

active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000

This happens when we try to erase the spl partition using fastboot:

$ fastboot erase spl
Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
fastboot: error: Command failed

The error occurs because getvars fails to handle the error returned by
nand layer when a partition cannot be found.

Indeed, getvar_get_part_info returns what is returned by
fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
return -ENODEV or -EINVAL instead. Since the cause of failure is not
returned by the nand function, I decided to return -EINVAL to make it
simple.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
   drivers/fastboot/fb_getvar.c | 5 ++++-
   drivers/fastboot/fb_nand.c   | 2 +-
   2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 93cbd598e0..f5d8b03301 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -121,8 +121,11 @@ static int getvar_get_part_info(const char 
*part_name, char *response,
               *size = disk_part.size * disk_part.blksz;
       } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
           r = fastboot_nand_get_part_info(part_name, &part_info, response);
-        if (r >= 0 && size)
+        if (r == 0 && size) {
               *size = part_info->size;
+        } else {
+            r = -EINVAL;
+        }
       } else {
           fastboot_fail("this storage is not supported in bootloader", 
response);
           r = -ENODEV;
diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
index 5a55144479..3ee0f40ea8 100644
--- a/drivers/fastboot/fb_nand.c
+++ b/drivers/fastboot/fb_nand.c
@@ -141,7 +141,7 @@ static lbaint_t fb_nand_sparse_reserve(struct 
sparse_storage *info,
    *
    * @part_name: Named device to lookup
    * @part_info: Pointer to returned part_info pointer
- * @response: Pointer to fastboot response buffer
+ * @response: 0 on success, 1 otherwise
    */
   int fastboot_nand_get_part_info(const char *part_name,
                   struct part_info **part_info, char *response)
-- 
2.48.1


[-- Attachment #2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2449 bytes --]

Re: [PATCH] Fix fastboot handling of partitions when no slots are, supported

[Mattijs Korpershoek]

Hi Federico,

Thank you for the patch.

On ven., janv. 24, 2025 at 11:49, Federico Fuga <fuga@studiofuga.com> wrote:

> The fastboot module has a bug that prevents some command to work
> properly on devices that haven't an Android-like partition scheme, that
> is, just one spl and one kernel partition, instead of the redundant
> scheme with _a and _b slots.
>
> This is the schema of our NAND storage (board is based on an AllWinner
> A33 sunxi chip):
>
> => mtdparts
>
> device nand0 <1c03000.nand>, # parts = 4
>   #: name         size            net size        offset mask_flags
>   0: spl          0x00020000      0x00020000      0x00000000      0
>   1: uboot        0x00100000      0x00100000      0x00020000      0
>   2: kernel       0x00400000      0x00400000      0x00120000      0
>   3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0
>
> active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000
>
> This happens when we try to erase the spl partition using fastboot:
>
> $ fastboot erase spl
> Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
> fastboot: error: Command failed
>
> The error occurs because getvars fails to handle the error returned by
> nand layer when a partition cannot be found.
>
> Indeed, getvar_get_part_info returns what is returned by
> fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
> return -ENODEV or -EINVAL instead. Since the cause of failure is not
> returned by the nand function, I decided to return -EINVAL to make it
> simple.
>
> Signed-off-by: Federico Fuga <fuga@studiofuga.com>

Unfortunately, I'm unable to apply this. I've tried using b4 and via git am.

$ b4 shazam -s -l --check a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com

Grabbing thread from lore.kernel.org/all/a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 2 messages in the thread
Assuming new revision: v2 ([PATCH] Fix fastboot handling of partitions when no slots are, supported)
Analyzing 0 code-review messages
Will use the latest revision: v2
You can pick other revisions using the -vN flag
Checking attestation on all messages, may take a moment...
---
  [PATCH] Fix fastboot handling of partitions when no slots are, supported
    + Link: https://lore.kernel.org/r/add4f6ac-3dd5-4707-a40c-a41191e62912@studiofuga.com
    + Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Traceback (most recent call last):
  File "/mnt/work/upstream/b4/src/b4/command.py", line 435, in <module>
    cmd()
    ~~~^^
  File "/mnt/work/upstream/b4/src/b4/command.py", line 417, in cmd
    cmdargs.func(cmdargs)


Downloading the thread from this patchwork link (clicking on "mbox"):
https://patchwork.ozlabs.org/project/uboot/patch/a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com/

$ git am Fix-fastboot-handling-of-partitions-when-no-slots-are-supported.patch
Applying: Fix fastboot handling of partitions when no slots are, supported
error: corrupt patch at line 13
error: could not build fake ancestor
Patch failed at 0001 Fix fastboot handling of partitions when no slots are, supported
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Can you please confirm that you are able to apply it yourself?
Maybe the email client is misconfigured somehow?

I've also tried to apply the one that has been send yesterday
(https://patchwork.ozlabs.org/project/uboot/patch/add4f6ac-3dd5-4707-a40c-a41191e62912@studiofuga.com/)
but I'm encountering the same problem.

> ---
>   drivers/fastboot/fb_getvar.c | 5 ++++-
>   drivers/fastboot/fb_nand.c   | 2 +-
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 93cbd598e0..f5d8b03301 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -121,8 +121,11 @@ static int getvar_get_part_info(const char 
> *part_name, char *response,
>               *size = disk_part.size * disk_part.blksz;
>       } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
>           r = fastboot_nand_get_part_info(part_name, &part_info, response);
> -        if (r >= 0 && size)
> +        if (r == 0 && size) {
>               *size = part_info->size;
> +        } else {
> +            r = -EINVAL;
> +        }
>       } else {
>           fastboot_fail("this storage is not supported in bootloader", 
> response);
>           r = -ENODEV;
> diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
> index 5a55144479..3ee0f40ea8 100644
> --- a/drivers/fastboot/fb_nand.c
> +++ b/drivers/fastboot/fb_nand.c
> @@ -141,7 +141,7 @@ static lbaint_t fb_nand_sparse_reserve(struct 
> sparse_storage *info,
>    *
>    * @part_name: Named device to lookup
>    * @part_info: Pointer to returned part_info pointer
> - * @response: Pointer to fastboot response buffer
> + * @response: 0 on success, 1 otherwise
>    */
>   int fastboot_nand_get_part_info(const char *part_name,
>                   struct part_info **part_info, char *response)
> -- 
> 2.48.1

Re: [PATCH] Fix fastboot handling of partitions when no slots are, supported

[Federico Fuga]

Hi Mattijs,
sorry, my bad: I exported from the wrong branch, as I was working on the 
latest release, not from main.

Resending it.
Sorry for the mistake.


Federico




On 28/01/25 09:44, Mattijs Korpershoek wrote:
> Hi Federico,
>
> Thank you for the patch.
>
> On ven., janv. 24, 2025 at 11:49, Federico Fuga <fuga@studiofuga.com> wrote:
>
>> The fastboot module has a bug that prevents some command to work
>> properly on devices that haven't an Android-like partition scheme, that
>> is, just one spl and one kernel partition, instead of the redundant
>> scheme with _a and _b slots.
>>
>> This is the schema of our NAND storage (board is based on an AllWinner
>> A33 sunxi chip):
>>
>> => mtdparts
>>
>> device nand0 <1c03000.nand>, # parts = 4
>>    #: name         size            net size        offset mask_flags
>>    0: spl          0x00020000      0x00020000      0x00000000      0
>>    1: uboot        0x00100000      0x00100000      0x00020000      0
>>    2: kernel       0x00400000      0x00400000      0x00120000      0
>>    3: ubi          0x07ae0000      0x079e0000 (!)  0x00520000      0
>>
>> active partition: nand0,0 - (spl) 0x00020000 @ 0x00000000
>>
>> This happens when we try to erase the spl partition using fastboot:
>>
>> $ fastboot erase spl
>> Erasing 'spl_a'               FAILED (remote: 'invalid NAND device')
>> fastboot: error: Command failed
>>
>> The error occurs because getvars fails to handle the error returned by
>> nand layer when a partition cannot be found.
>>
>> Indeed, getvar_get_part_info returns what is returned by
>> fastboot_nand_get_part_info (0 on success, 1 on failure) but it should
>> return -ENODEV or -EINVAL instead. Since the cause of failure is not
>> returned by the nand function, I decided to return -EINVAL to make it
>> simple.
>>
>> Signed-off-by: Federico Fuga <fuga@studiofuga.com>
> Unfortunately, I'm unable to apply this. I've tried using b4 and via git am.
>
> $ b4 shazam -s -l --check a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com
>
> Grabbing thread from lore.kernel.org/all/a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com/t.mbox.gz
> Checking for newer revisions
> Grabbing search results from lore.kernel.org
> Analyzing 2 messages in the thread
> Assuming new revision: v2 ([PATCH] Fix fastboot handling of partitions when no slots are, supported)
> Analyzing 0 code-review messages
> Will use the latest revision: v2
> You can pick other revisions using the -vN flag
> Checking attestation on all messages, may take a moment...
> ---
>    [PATCH] Fix fastboot handling of partitions when no slots are, supported
>      + Link: https://lore.kernel.org/r/add4f6ac-3dd5-4707-a40c-a41191e62912@studiofuga.com
>      + Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Traceback (most recent call last):
>    File "/mnt/work/upstream/b4/src/b4/command.py", line 435, in <module>
>      cmd()
>      ~~~^^
>    File "/mnt/work/upstream/b4/src/b4/command.py", line 417, in cmd
>      cmdargs.func(cmdargs)
>
>
> Downloading the thread from this patchwork link (clicking on "mbox"):
> https://patchwork.ozlabs.org/project/uboot/patch/a70b19e6-4436-4fcf-bebd-b6877b66779e@studiofuga.com/
>
> $ git am Fix-fastboot-handling-of-partitions-when-no-slots-are-supported.patch
> Applying: Fix fastboot handling of partitions when no slots are, supported
> error: corrupt patch at line 13
> error: could not build fake ancestor
> Patch failed at 0001 Fix fastboot handling of partitions when no slots are, supported
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config advice.mergeConflict false"
>
> Can you please confirm that you are able to apply it yourself?
> Maybe the email client is misconfigured somehow?
>
> I've also tried to apply the one that has been send yesterday
> (https://patchwork.ozlabs.org/project/uboot/patch/add4f6ac-3dd5-4707-a40c-a41191e62912@studiofuga.com/)
> but I'm encountering the same problem.
>
>> ---
>>    drivers/fastboot/fb_getvar.c | 5 ++++-
>>    drivers/fastboot/fb_nand.c   | 2 +-
>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
>> index 93cbd598e0..f5d8b03301 100644
>> --- a/drivers/fastboot/fb_getvar.c
>> +++ b/drivers/fastboot/fb_getvar.c
>> @@ -121,8 +121,11 @@ static int getvar_get_part_info(const char
>> *part_name, char *response,
>>                *size = disk_part.size * disk_part.blksz;
>>        } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND)) {
>>            r = fastboot_nand_get_part_info(part_name, &part_info, response);
>> -        if (r >= 0 && size)
>> +        if (r == 0 && size) {
>>                *size = part_info->size;
>> +        } else {
>> +            r = -EINVAL;
>> +        }
>>        } else {
>>            fastboot_fail("this storage is not supported in bootloader",
>> response);
>>            r = -ENODEV;
>> diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c
>> index 5a55144479..3ee0f40ea8 100644
>> --- a/drivers/fastboot/fb_nand.c
>> +++ b/drivers/fastboot/fb_nand.c
>> @@ -141,7 +141,7 @@ static lbaint_t fb_nand_sparse_reserve(struct
>> sparse_storage *info,
>>     *
>>     * @part_name: Named device to lookup
>>     * @part_info: Pointer to returned part_info pointer
>> - * @response: Pointer to fastboot response buffer
>> + * @response: 0 on success, 1 otherwise
>>     */
>>    int fastboot_nand_get_part_info(const char *part_name,
>>                    struct part_info **part_info, char *response)
>> -- 
>> 2.48.1