Re: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
On 5/18/26 16:05, Maarten Brock wrote:
Hello Michal,
From: Michal Simek
On 5/18/26 15:40, Maarten Brock wrote:
How about something like this?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int i, ret, soft = 0;
for (i = 0; i < bulk->count; i++) {
struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
if (ops->rst_reset) {
ret = ops->rst_reset(&bulk->resets[i], delay_us);
} else {
ret = reset_assert(&bulk->resets[i]);
soft++;
But this will have weird behavior when you have multiple reset controllers where
one has rst_reset implemented and another not. Then on that one with rst_reset
another deassert is going to be called.
for (i = 0; i < bulk->count; i++) {
struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
if (!ops->rst_reset) {
Did you miss the ( ! ops->rst_reset ) here? It will not call deassert for the
rst_reset cases.
ah.
ret = reset_deassert(&bulk->resets[i]);
if (ret < 0)
return ret;
}
}
I do agree that timing might become sketchy. But I don't see a
one-size-fits-all solution.
TBH I think it is too complicated and we don't have any evidence that Linux
implementation is wrong. I think that make sense to use what Linux has and if
there is any problem we can change this sequence.
Thanks,
Michal
RE: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
Hello Michal,
> From: Michal Simek
> On 5/18/26 15:40, Maarten Brock wrote:
> >
> > How about something like this?
> >
> > int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
> > {
> > int i, ret, soft = 0;
> >
> > for (i = 0; i < bulk->count; i++) {
> > struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
> >
> > if (ops->rst_reset) {
> > ret = ops->rst_reset(&bulk->resets[i], delay_us);
> > } else {
> > ret = reset_assert(&bulk->resets[i]);
> > soft++;
>
> But this will have weird behavior when you have multiple reset controllers
> where
> one has rst_reset implemented and another not. Then on that one with rst_reset
> another deassert is going to be called.
> for (i = 0; i < bulk->count; i++) {
> struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
>
> if (!ops->rst_reset) {
Did you miss the ( ! ops->rst_reset ) here? It will not call deassert for the
rst_reset cases.
> ret = reset_deassert(&bulk->resets[i]);
> if (ret < 0)
> return ret;
> }
> }
I do agree that timing might become sketchy. But I don't see a
one-size-fits-all solution.
Kind regards,
Maarten
Re: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
On 5/18/26 15:40, Maarten Brock wrote:
Hello Michal,
From: Michal Simek
On 5/18/26 14:07, Maarten Brock wrote:
From: Michal Simek
On 5/18/26 12:21, Maarten Brock wrote:
I expect you would like to do it like this right?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int ret;
ret = reset_assert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to assert reset: %d\n", ret);
return ret;
}
udelay(delay_us);
/* Deassert all OSPI reset lines */
ret = reset_deassert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to deassert reset: %d\n", ret);
return ret;
}
return 0;
}
That is correct.
And a single delay of delay_us takes less time than N*delay_us (for N>1), so
the total startup time is smaller (a.k.a. faster startup).
It really depends on other factors and delay time. In our case it is not just
MMIO access. But it is SMC to TF-A and then seconding request over mailbox to
another firmware to ack it first and then do it. And with above you would have
to do twice compare to once.
I can do another optimization which is that if bulk->count is 1 then call
reset_reset() directly and for others do what you see above.
What do you think?
I very much doubt that all sorts of actions and compares will ever take more
time than 1 us, unless some serial protocol is involved (uart/spi/i2c).
So, I expect that delay_us is always the dominant elapsed time. And N*delay_us
even more so.
I do understand that if the delay is handled by rst_reset() and underlying
hardware, things might be different.
So special handling of bulk->count=1 or *all* bulk resets are handled by
rst_reset() could be a good idea.
How about something like this?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int i, ret, soft = 0;
for (i = 0; i < bulk->count; i++) {
struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
if (ops->rst_reset) {
ret = ops->rst_reset(&bulk->resets[i], delay_us);
} else {
ret = reset_assert(&bulk->resets[i]);
soft++;
But this will have weird behavior when you have multiple reset controllers where
one has rst_reset implemented and another not. Then on that one with rst_reset
another deassert is going to be called.
Thanks,
Michal
RE: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
Hello Michal,
> From: Michal Simek
> On 5/18/26 14:07, Maarten Brock wrote:
> >> From: Michal Simek
> >> On 5/18/26 12:21, Maarten Brock wrote:
> >>
> >> I expect you would like to do it like this right?
> >>
> >> int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
> >> {
> >>int ret;
> >>
> >>ret = reset_assert_bulk(bulk);
> >>if (ret) {
> >>dev_err(bus, "Failed to assert reset: %d\n", ret);
> >>return ret;
> >>}
> >>
> >>udelay(delay_us);
> >>
> >>/* Deassert all OSPI reset lines */
> >>ret = reset_deassert_bulk(bulk);
> >>if (ret) {
> >>dev_err(bus, "Failed to deassert reset: %d\n", ret);
> >>return ret;
> >>}
> >>
> >>return 0;
> >> }
> >
> > That is correct.
> > And a single delay of delay_us takes less time than N*delay_us (for N>1),
> > so the total startup time is smaller (a.k.a. faster startup).
>
> It really depends on other factors and delay time. In our case it is not just
> MMIO access. But it is SMC to TF-A and then seconding request over mailbox to
> another firmware to ack it first and then do it. And with above you would have
> to do twice compare to once.
>
> I can do another optimization which is that if bulk->count is 1 then call
> reset_reset() directly and for others do what you see above.
>
> What do you think?
I very much doubt that all sorts of actions and compares will ever take more
time than 1 us, unless some serial protocol is involved (uart/spi/i2c).
So, I expect that delay_us is always the dominant elapsed time. And N*delay_us
even more so.
I do understand that if the delay is handled by rst_reset() and underlying
hardware, things might be different.
So special handling of bulk->count=1 or *all* bulk resets are handled by
rst_reset() could be a good idea.
How about something like this?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int i, ret, soft = 0;
for (i = 0; i < bulk->count; i++) {
struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
if (ops->rst_reset) {
ret = ops->rst_reset(&bulk->resets[i], delay_us);
} else {
ret = reset_assert(&bulk->resets[i]);
soft++;
}
if (ret < 0)
return ret;
}
if (soft == 0)
return 0;
udelay(delay_us);
for (i = 0; i < bulk->count; i++) {
struct reset_ops *ops = reset_dev_ops(bulk->resets[i].dev);
if (!ops->rst_reset) {
ret = reset_deassert(&bulk->resets[i]);
if (ret < 0)
return ret;
}
}
return 0;
}
Kind regards,
Maarten Brock
Re: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
On 5/18/26 14:07, Maarten Brock wrote:
Hello Michal,
-Original Message-
From: Michal Simek
On 5/18/26 12:21, Maarten Brock wrote:
Although properly commented, I find the reset_reset_bulk() implementation
counterintuitive. Are we so certain that the Linux implementation is the right
and obvious one? I personally would prefer to have it assert all resets at the
same time. It is faster and prevents races if the resets are interdependent.
Why do you think that it is faster?
I expect you would like to do it like this right?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int ret;
ret = reset_assert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to assert reset: %d\n", ret);
return ret;
}
udelay(delay_us);
/* Deassert all OSPI reset lines */
ret = reset_deassert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to deassert reset: %d\n", ret);
return ret;
}
return 0;
}
That is correct.
And a single delay of delay_us takes less time than N*delay_us (for N>1), so
the total startup time is smaller (a.k.a. faster startup).
It really depends on other factors and delay time. In our case it is not just
MMIO access. But it is SMC to TF-A and then seconding request over mailbox to
another firmware to ack it first and then do it. And with above you would have
to do twice compare to once.
I can do another optimization which is that if bulk->count is 1 then call
reset_reset() directly and for others do what you see above.
What do you think?
Thanks,
Michal
RE: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
Hello Michal,
> -Original Message-
> From: Michal Simek
> On 5/18/26 12:21, Maarten Brock wrote:
> >
> > Although properly commented, I find the reset_reset_bulk() implementation
> > counterintuitive. Are we so certain that the Linux implementation is the
> > right
> > and obvious one? I personally would prefer to have it assert all resets at
> > the
> > same time. It is faster and prevents races if the resets are interdependent.
>
> Why do you think that it is faster?
>
> I expect you would like to do it like this right?
>
> int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
> {
> int ret;
>
> ret = reset_assert_bulk(bulk);
> if (ret) {
> dev_err(bus, "Failed to assert reset: %d\n", ret);
> return ret;
> }
>
> udelay(delay_us);
>
> /* Deassert all OSPI reset lines */
> ret = reset_deassert_bulk(bulk);
> if (ret) {
> dev_err(bus, "Failed to deassert reset: %d\n", ret);
> return ret;
> }
>
> return 0;
> }
That is correct.
And a single delay of delay_us takes less time than N*delay_us (for N>1), so
the total startup time is smaller (a.k.a. faster startup).
Kind regards,
Maarten Brock
Re: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
Hi Maarten,
On 5/18/26 12:21, Maarten Brock wrote:
Hello Michal,
Although properly commented, I find the reset_reset_bulk() implementation
counterintuitive. Are we so certain that the Linux implementation is the right
and obvious one? I personally would prefer to have it assert all resets at the
same time. It is faster and prevents races if the resets are interdependent.
Why do you think that it is faster?
I expect you would like to do it like this right?
int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us)
{
int ret;
ret = reset_assert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to assert reset: %d\n", ret);
return ret;
}
udelay(delay_us);
/* Deassert all OSPI reset lines */
ret = reset_deassert_bulk(bulk);
if (ret) {
dev_err(bus, "Failed to deassert reset: %d\n", ret);
return ret;
}
return 0;
}
Thanks,
Michal
RE: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API
Hello Michal, Although properly commented, I find the reset_reset_bulk() implementation counterintuitive. Are we so certain that the Linux implementation is the right and obvious one? I personally would prefer to have it assert all resets at the same time. It is faster and prevents races if the resets are interdependent. Kind regards, Maarten Brock > -Original Message- > From: U-Boot On Behalf Of Michal Simek > Sent: Wednesday 13 May 2026 11:42 > To: [email protected]; [email protected]; Simon Glass > Cc: Tom Rini > Subject: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API > > Add reset_reset() and reset_reset_bulk() functions to the reset > controller API. These functions assert and then deassert reset signals > in a single call, providing a convenient way to pulse/toggle a reset > line. > > This mimics the Linux kernel's reset_control_reset() and > reset_control_bulk_reset() APIs. The new functions are useful for > drivers that need to cycle a reset line during initialization or > error recovery but with also passing delay parameter. > > If a driver implements the rst_reset op, it will be called directly > with the delay parameter. Otherwise, the reset core performs > reset_assert(), optional udelay(), and reset_deassert() as fallback. > > Signed-off-by: Michal Simek > Reviewed-by: Simon Glass > --- > > Changes in v3: > - extend function documentation > > Changes in v2: > - Add delay_us parameter to specify delay between assert and deassert > - Pass delay_us to rst_reset op so drivers can use it if needed > - Return -ENOSYS in stubs when !CONFIG_DM_RESET (like clk.h) > - Fix line length to stay within 80 characters > > drivers/reset/reset-uclass.c | 34 + > include/reset-uclass.h | 19 ++ > include/reset.h | 49 > 3 files changed, 102 insertions(+) > > diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c > index fe4cebf54f15..c199e3e5da71 100644 > --- a/drivers/reset/reset-uclass.c > +++ b/drivers/reset/reset-uclass.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > static inline struct reset_ops *reset_dev_ops(struct udevice *dev) > { > @@ -225,6 +226,39 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk) > return 0; > } > > +int reset_reset(struct reset_ctl *reset_ctl, ulong delay_us) > +{ > + struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + int ret; > + > + debug("%s(reset_ctl=%p, delay_us=%lu)\n", __func__, reset_ctl, > + delay_us); > + > + if (ops->rst_reset) > + return ops->rst_reset(reset_ctl, delay_us); > + > + ret = reset_assert(reset_ctl); > + if (ret < 0) > + return ret; > + > + udelay(delay_us); > + > + return reset_deassert(reset_ctl); > +} > + > +int reset_reset_bulk(struct reset_ctl_bulk *bulk, ulong delay_us) > +{ > + int i, ret; > + > + for (i = 0; i < bulk->count; i++) { > + ret = reset_reset(&bulk->resets[i], delay_us); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > int reset_status(struct reset_ctl *reset_ctl) > { > struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > diff --git a/include/reset-uclass.h b/include/reset-uclass.h > index 9a0696dd1e3b..7af090b60b57 100644 > --- a/include/reset-uclass.h > +++ b/include/reset-uclass.h > @@ -76,6 +76,25 @@ struct reset_ops { >* @return 0 if OK, or a negative error code. >*/ > int (*rst_deassert)(struct reset_ctl *reset_ctl); > + /** > + * rst_reset - Reset a HW module. > + * > + * This optional function triggers a reset pulse on the reset line. > + * If not implemented, reset_reset() falls back to rst_assert(), > + * udelay(@delay_us), then rst_deassert(); that delay is therefore > + * observed only on the fallback path. > + * > + * When rst_reset is provided, @delay_us is controller-specific: the > + * implementation should honour it if the hardware needs a minimum > + * assertion time before release. It may ignore @delay_us when the > + * pulse shape is fixed elsewhere (for example a firmware pulse). > + * > + * @reset_ctl: The reset signal to pulse. > + * @delay_us: Minimum delay in microseconds between assert and > + * deassert where applicable; see above. > + * @return 0 if OK, or a negative error code. > + */ > + int (*rst_reset)(struct reset_ctl *reset_ctl, ulong delay_us); > /** >* rst_status - Check reset signal status. >* > diff --git a/include/reset.h b/include/reset.h > index 036a786d2ace..58574b983f66 100644 > --- a/include/reset.h > +++ b/include/reset.h > @@ -320,6 +320,45 @@ int reset_deassert(struct reset_ctl *reset_ctl); > */ > int reset_deassert_bulk(struct reset_ctl_bulk *bulk); > > +/** > + * reset_reset - Re
