[PATCH v2 2/4] reset: Add sandbox tests for reset_reset() and reset_reset_bulk()

[Michal Simek]

On 5/13/26 16:47, Simon Glass wrote:
> Hi Michal,
> 
> On Wed, 13 May 2026 at 02:53, Michal Simek <michal.simek at amd.com> wrote:
>>
>>
>>
>> On 5/7/26 23:11, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On 2026-05-05T12:30:28, Michal Simek <michal.simek at amd.com> wrote:
>>>> reset: Add sandbox tests for reset_reset() and reset_reset_bulk()
>>>>
>>>> Add DM test coverage for the new reset_reset() and reset_reset_bulk()
>>>> API functions. The tests exercise the assert + deassert fallback path
>>>> since the sandbox reset driver does not implement the rst_reset op.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek at amd.com>
>>>>
>>>> arch/sandbox/include/asm/reset.h   |  3 ++
>>>>    drivers/reset/sandbox-reset-test.c | 14 ++++++++
>>>>    drivers/reset/sandbox-reset.c      | 31 ++++++++++++++++++
>>>>    test/dm/reset.c                    | 67 ++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 115 insertions(+)
>>>
>>>> diff --git a/drivers/reset/sandbox-reset.c b/drivers/reset/sandbox-reset.c
>>>> @@ -66,6 +69,21 @@ static int sandbox_reset_deassert(struct reset_ctl *reset_ctl)
>>>>         return 0;
>>>>    }
>>>>
>>>> +static int sandbox_reset_reset(struct reset_ctl *reset_ctl, ulong delay_us)
>>>> +{
>>>> +     struct sandbox_reset *sbr = dev_get_priv(reset_ctl->dev);
>>>> +
>>>> +     debug("%s(reset_ctl=%p, delay_us=%lu)\n", __func__, reset_ctl,
>>>> +           delay_us);
>>>> +
>>>> +     sbr->signals[reset_ctl->id].asserted = true;
>>>> +     udelay(delay_us);
>>>> +     sbr->signals[reset_ctl->id].asserted = false;
>>>> +     sbr->signals[reset_ctl->id].reset_count++;
>>>> +
>>>> +     return 0;
>>>> +}
>>>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>
>>> Some optional nits / thoughts below.
>>>
>>> The commit message says the tests exercise the fallback path since the
>>> sandbox reset driver doesn't have rst_reset op, this patch adds that
>>> method below - so the commit message could use an update.
>>
>> Will update.
>>
>>>
>>> If you want coverage for the fallback you could add a second
>>> sandbox-reset driver without rst_reset, or hack the rest to clear and
>>> restart ops->rst_reset,
>>>
>>>> diff --git a/drivers/reset/sandbox-reset-test.c b/drivers/reset/sandbox-reset-test.c
>>>> @@ -96,6 +96,20 @@ int sandbox_reset_test_deassert_bulk(struct udevice *dev)
>>>> +int sandbox_reset_test_reset(struct udevice *dev)
>>>> +{
>>>> +     struct sandbox_reset_test *sbrt = dev_get_priv(dev);
>>>> +
>>>> +     return reset_reset(sbrt->ctlp, 0);
>>>> +}
>>>> +
>>>> +int sandbox_reset_test_reset_bulk(struct udevice *dev)
>>>> +{
>>>> +     struct sandbox_reset_test *sbrt = dev_get_priv(dev);
>>>> +
>>>> +     return reset_reset_bulk(sbrt->bulkp, 0);
>>>> +}
>>>
>>> You could pass a non-zero value here if you want to test the delay.
>>
>> udelay is called and because there is missing check for 0 value it does timer
>> reading once. Don't think that make sense to delay tests.
> 
> We try to avoid delays in tests, although 10us wouldn't matter. We do
> have timer_test_add_offset() but I don't think it helps with udelay()

I have created new version and also test fallback which has 1us delay there.
Please take a look at that new patch and this part should be covered.

M