Re: [PATCH 2/3] i3c: master: Add missing runtime PM get in dev_nack_retry_count_store()
Adrian Hunter <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,dev.linux.lists.sashiko-reviews |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 16/06/2026 14:51, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > Pre-existing issues: > - [High] Hardware configuration, including the NACK retry count, is lost across runtime suspend/resume cycles. > -- > > commit c41eb9cead24507098d7a57db0101da0e5c68c34 > Author: Adrian Hunter <[email protected]> > > i3c: master: Add missing runtime PM get in dev_nack_retry_count_store() > > Ensure the device is runtime resumed while updating the retry > configuration to avoid accessing the controller while suspended. > >> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c >> index 05ea9e3c4d46f..a24944047e30c 100644 >> --- a/drivers/i3c/master.c >> +++ b/drivers/i3c/master.c > > [ ... ] > >> @@ -767,12 +767,18 @@ static ssize_t dev_nack_retry_count_store(struct device *dev, >> i3c_bus_maintenance_lock(i3cbus); >> ret = master->ops->set_dev_nack_retry(master, val); >> if (!ret) >> master->dev_nack_retry_count = val; >> i3c_bus_maintenance_unlock(i3cbus); >> >> + i3c_master_rpm_put(master); > > [Severity: High] > This isn't a bug introduced by this patch, but does putting the device to > sleep here immediately cause the newly written retry count to be lost? > > When the dw-i3c-master device enters runtime suspend, the hardware is reset > via reset_control_assert() in dw_i3c_master_runtime_suspend(). > > Upon resuming, dw_i3c_master_restore_addrs() reads the newly reset hardware > register (which returns 0), updates only the address bits, and writes it > back. This permanently discards other configuration fields, including the > DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT configured here, as well as other fields > like SIR_REJECT and IBI_MDB. > > Because dev_nack_retry_count_store() now correctly drops the runtime PM > reference, the controller can suspend immediately after the configuration is > written. Will this silently revert the retry count back to its reset state? + Adrian Ng Ho Yin <[email protected]> Looks like dw_i3c_master_restore_addrs() needs to do more to restore dev_nack_retry_count values. -- linux-i3c mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-i3c