Re: [PATCH] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume

[Artem Shimko]

Hi Andy,

Thank you for your review!

On Wed, Apr 15, 2026 at 5:52 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxx> wrote:
> No need to repeat the commit message in the comments. It does not add any value.

Got it =)

> This is serious behaviour change (might not be, one needs to understand what it
> does in comparison to no reset (and in accordance with datasheet).
>
> Do you have any HW to test?

I made these changes so that our MMC controller (dwc) can handle the
reset signal not only during the initialization function, but also
during suspend and resume.
Since the reset control was previously stored in vendor-specific
private structures (rk35xx_priv and eic7700_priv), it was inaccessible
from the common PM code.
To make suspend/resume work for our platform, I had to move the reset
pointer to the generic dwcmshc_priv structure, which required updating
the Rockchip and EIC7700 variants accordingly.



> The driver seems can be refactored a lot (with no functional changes) by:
> - replacing *sleep() with fsleep() API
> - dropping unneeded checks like above, clk_disable_unprepare() is error
> pointer-aware IIRC (or it can be made so it is either NULL or valid one)
> - using 'return dev_err_probe()'
>
> This is just a side note in case you are interested.

Yes, sure, thank you!

> No need to add an extra blank line.
okay =)


Instead of moving the reset control directly into the common path,
perhaps a cleaner solution would be to introduce a set of platform PM
ops (e.g., suspend/resume callbacks in the variant data).
These ops could be checked for NULL in the common
dwcmshc_suspend/resume functions. For platforms that don't implement
them, the behavior remains exactly as it was before (no reset
assertion),
ensuring zero risk of regression for Rockchip and EIC7700. Our
platform would then simply implement these ops to handle the custom
reset sequence.

What do you think about it?
--
Regards,
Artem