[PATCH u-boot-mvebu v3 11/18] arm: mvebu: system-controller: Add support for SYSRESET

[Stefan Roese]

On 3/28/24 12:21, Marek Behún wrote:
> On Thu, 28 Mar 2024 11:04:45 +0100
> Stefan Roese <sr at denx.de> wrote:
> 
>>> +static int mvebu_sysreset_request(struct udevice *dev, enum sysreset_t type)
>>> +{
>>> +	struct regmap *regmap = syscon_get_regmap(dev->parent);
>>> +	uint bit;
>>> +
>>> +	if (type != SYSRESET_COLD)
>>> +		return -EPROTONOSUPPORT;
>>> +
>>> +	bit = MVEBU_GLOBAL_SOFT_RST_BIT;
>>> +
>>> +	regmap_update_bits(regmap, MVEBU_RSTOUTN_MASK_REG, bit, bit);
>>> +	regmap_update_bits(regmap, MVEBU_SYS_SOFT_RST_REG, bit, bit);
>>> +
>>> +	while (1)
>>> +		;
>>
>> A comment before this endless loop might be helpful here.
> 
> The code does the same as reset_cpu() in cpu.c, and the while() cycle
> is not commented there.

Sure, other code might suffer this undocumented endless loop as well.
And again, this is more a nitpicking comment than a real requirement.

> But we can add something like
>    /* something has gone wrong if we reach here, so we may as well stay
>     * here */
> 
> What do you think? Could you amend the patch?

More something like this:

	/* Loop while waiting for the reset */
	while (1)
		;

And yes, I can fold this into your patchset, if I don't forget about
this. Still, if there is need for a v4, then please add it yourself.

Thanks,
Stefan