Hi Michal, On Wed, 13 May 2026 at 02:25, Michal Simek <[email protected]> wrote: > > > > On 5/7/26 23:11, Simon Glass wrote: > > Hi Michal, > > > > On 2026-05-05T12:30:28, Michal Simek <[email protected]> wrote: > >> 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 <[email protected]> > >> > >> drivers/reset/reset-uclass.c | 34 ++++++++++++++++++++++++++++++++++ > >> include/reset-uclass.h | 12 ++++++++++++ > >> include/reset.h | 41 > >> +++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 87 insertions(+) > > > > Reviewed-by: Simon Glass <[email protected]> > > > > optional nits / thoughts below > > > >> diff --git a/include/reset-uclass.h b/include/reset-uclass.h > >> @@ -76,6 +76,18 @@ struct reset_ops { > >> + /** > >> + * rst_reset - Reset a HW module. > >> + * > >> + * This optional function triggers a reset pulse on the reset line, > >> + * asserting and then deasserting the reset signal. If not > >> implemented, > >> + * the reset core will use rst_assert followed by rst_deassert. > >> + * > >> + * @reset_ctl: The reset signal to pulse. > >> + * @delay_us: Delay in microseconds between assert and deassert. > >> + * @return 0 if OK, or a negative error code. > >> + */ > >> + int (*rst_reset)(struct reset_ctl *reset_ctl, ulong delay_us); > > > > Please document that the core inserts a udelay(delay_us) between > > rst_assert and rst_deassert in the fallback path, and clarify whether > > an rst_reset implementation must honour delay_us or may treat it as a > > hint. Without that, authors don't really know what to do with the > > value. > > ok. Will fix in v3. > > > >> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c > >> @@ -225,6 +226,39 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk) > >> +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; > >> +} > > > > Just to check - pulsing each reset sequentially means the first > > finishes its full assert/delay/deassert before the next starts > > asserting. For the cadence_qspi case in 3/4, where multiple OSPI reset > > lines are expected to be asserted together, the previous code did > > assert_bulk() / udelay() / deassert_bulk() so all lines were held low > > simultaneously. The new behaviour is different. Is that intentional, > > and have you confirmed the cadence controller is happy with it? > > You are right that behavior has changed but it is aligned with what Linux > kernel > is doing. > We are using only one reset line that's why not an issue for us. But it saves > one firmware call (per reset line). > > int reset_control_bulk_reset(int num_rstcs, > struct reset_control_bulk_data *rstcs) > { > int ret, i; > > for (i = 0; i < num_rstcs; i++) { > ret = reset_control_reset(rstcs[i].rstc); > if (ret) > return ret; > } > > return 0; > } > EXPORT_SYMBOL_GPL(reset_control_bulk_reset); > > > If it is problematic we can go back to assert_bulk/delay/deassert_bulk in > cadence driver.
Well if it matches Linux we should be OK. Regards, Simon
