Re: [PATCH 2/3] i3c: mipi-i3c-hci: Add optional System Suspend support

[Adrian Hunter <adrian.hunter@intel.com>]

On 19/01/2026 09:39, David Nyström wrote:
> 
> 
> On Mon, 19 Jan 2026, Adrian Hunter wrote:
> 
>> On 19/01/2026 08:31, David Nyström wrote:
>>>
>>>
>>> On Fri, 16 Jan 2026, Adrian Hunter wrote:
>>>
>>>> Add system suspend callbacks.  Implement them by forcing runtime PM.
>>>> Consequently bail out if Runtime PM is not allowed.
>>>>
>>>> On resume from System Suspend (suspend to RAM), rerun Dynamic Address
>>>> Assignment to restore addresses for devices that may have lost power.
>>>>
>>>> On resume from System Hibernation (suspend to disk), use the new
>>>> i3c_master_restore_daa() helper which additionally handles the case where
>>>> devices are assigned different dynamic addresses after a hibernation boot.
>>>
>>> Why do you make this distinction ?
>>> RSTDAA+DAA during boot is motivated by that uboot might have assigned addresses.
>>> Why would we need to behave differently in suspend and suspend-to-disk?
>>
>> Because, in the case of suspend-to-disk, there is a complete
>> boot process in between.
>>
>>> Its fair to assume that I3C devices might have been powered off in suspend too, right ?
>>
>> Hibernation is quite different.  Briefly:
>>
>> Entry
>>     ops->freeze
>>     create image
>>     ops->thaw
>>     write image
>>     ops->poweroff
>>
>> Exit
>>     boot            <- address assignment not guaranteed to be the same as previous boot
>>     ops->freeze
>>     restore image
>>     ops->restore        <- deal with I3C devices that may have different addresses
> 
> OK, so bootloader might have messed things up for us again.

No, it is the kernel that cannot guarantee that an I3C device
will have the same dynamic address from one boot to the next.
Generally, the same address will be assigned.  However the
case I have experienced is the first I3C device does not come
back after hibernation, and all the subsequent devices are given
different addresses, and then none of them work.

> Thanks for elaborating.
> 
>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>> drivers/i3c/master/mipi-i3c-hci/core.c | 53 ++++++++++++++++++++++++++
>>>> 1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> index 02c5a133e329..b972d8bc81e6 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> @@ -769,6 +769,53 @@ static int i3c_hci_runtime_resume(struct device *dev)
>>>>     return 0;
>>>> }
>>>>
>>>> +static int i3c_hci_suspend(struct device *dev)
>>>> +{
>>>> +    struct i3c_hci *hci = dev_get_drvdata(dev);
>>>> +
>>>> +    if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
>>>> +        return 0;
>>>> +
>>>> +    return pm_runtime_force_suspend(dev);
>>>> +}
>>>> +
>>>> +static int i3c_hci_resume_common(struct device *dev, bool restore)
>>>> +{
>>>> +    struct i3c_hci *hci = dev_get_drvdata(dev);
>>>> +    int ret;
>>>> +
>>>> +    if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
>>>> +        return 0;
>>>> +
>>>> +    ret = pm_runtime_force_resume(dev);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (restore)
>>>> +        ret = i3c_master_restore_daa(&hci->master);
>>>> +    else
>>>> +        ret = i3c_master_do_daa(&hci->master);
>>>> +
>>>> +    if (ret)
>>>> +        dev_err(dev, "Dynamic Address Assignment failed on resume, error %d\n", ret);
>>>> +
>>>> +    /*
>>>> +     * I3C devices may have retained their dynamic address anyway. Do not
>>>> +     * fail the resume because of DAA error.
>>>> +     */
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int i3c_hci_resume(struct device *dev)
>>>> +{
>>>> +    return i3c_hci_resume_common(dev, false);
>>>> +}
>>>> +
>>>> +static int i3c_hci_restore(struct device *dev)
>>>> +{
>>>> +    return i3c_hci_resume_common(dev, true);
>>>> +}
>>>> +
>>>> #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
>>>>
>>>> static void i3c_hci_rpm_enable(struct device *dev)
>>>> @@ -945,6 +992,12 @@ static const struct platform_device_id i3c_hci_driver_ids[] = {
>>>> MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
>>>>
>>>> static const struct dev_pm_ops i3c_hci_pm_ops = {
>>>> +    .suspend  = pm_sleep_ptr(i3c_hci_suspend),
>>>> +    .resume   = pm_sleep_ptr(i3c_hci_resume),
>>>> +    .freeze   = pm_sleep_ptr(i3c_hci_suspend),
>>>> +    .thaw     = pm_sleep_ptr(i3c_hci_resume),
>>>> +    .poweroff = pm_sleep_ptr(i3c_hci_suspend),
>>>> +    .restore  = pm_sleep_ptr(i3c_hci_restore),
>>>>     RUNTIME_PM_OPS(i3c_hci_runtime_suspend, i3c_hci_runtime_resume, NULL)
>>>> };
>>>>
>>>> -- 
>>>> 2.51.0
>>>>
>>>>
>>>> -- 
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>>>
>>
>>
>> -- 
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c