[PATCH v2 2/5] arm: mach-imx: Add command to expose QB functionality

[Marek Vasut]

On 3/17/26 4:28 PM, Simona Toaca wrote:

[...]

>>> +	if (!cp) {
>>> +		printf("%s cp is null\n", __func__);
>>
>> What does this error output even mean ? The user will be confused, please
>> reword it.
>>
> 
> Looking at this, these error messages might be redundant, as U-Boot displays
> the command usage anyway if CMD_RET_USAGE is returned. The repeatable error is
> also redundant because the command is declared repeatable.
> 
> I have a suggestion: I could use the U-Boot API for commands from
> include/command.h (U_BOOT_CMD_WITH_SUBCMDS), which declares and registers
> do_<cmdname> with the default method body (which is almost the same as this
> version). This would imply no actual error messages, just printing the usage,
> and a cleaner code.

Aren't the error messages helpful to the user to determine what went 
wrong during the command execution ?

[...]

>>> +++ b/arch/arm/mach-imx/imx9/Makefile
>>> @@ -15,5 +15,7 @@ obj-y += imx_bootaux.o
>>>    endif
>>>    ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
>>> -obj-y += qb.o
>>> +ifneq ($(CONFIG_XPL_BUILD),y)
>>> +obj-$(CONFIG_CMD_IMX_QB) += qb.o
>>> +endif
>>
>> This shoudld be part of 1/5 , should it not ?
> 
> And squash commits 1 and 2, as this is cmd_qb related?

No, only add the qb.o Makefile entry in 1/5 , since qb.c is added in 1/5 
too.