Re: [PATCH v3 1/5] reset: Add reset_reset() and reset_reset_bulk() API

[Michal Simek]

On 5/18/26 15:40, Maarten Brock wrote:
Hello Michal,

From: Michal Simek <michal.si...@amd.com>
On 5/18/26 14:07, Maarten Brock wrote:
From: Michal Simek <michal.si...@amd.com>
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