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?
> +
> return ret ?: count;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.