On Thu, 28 Mar 2024 14:01:22 +0100 Stefan Roese <s...@denx.de> wrote:
> On 3/28/24 12:21, Marek Behún wrote: > > On Thu, 28 Mar 2024 11:04:45 +0100 > > Stefan Roese <s...@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