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

["Marek Behún" <kabel@kernel.org>]

On Thu, 28 Mar 2024 14:01:22 +0100
Stefan Roese <sr@denx.de> wrote:

> On 3/28/24 12:21, Marek Behún wrote:
> > On Thu, 28 Mar 2024 11:04:45 +0100
> > Stefan Roese <sr@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)
> 		;

As of now I don't see a need for v4.

I may sent another patches regarding DDR training, but it will be
made on top of this series.

Marek