On 11/27/2025 3:18 PM, Peng Fan wrote:
Subject: Re: [PATCH 1/8] mmc: sdhci-cadence: Add reset control
support

Thanks for the suggestion, Peng.

On 11/26/2025 7:47 AM, Peng Fan wrote:
On Thu, Nov 20, 2025 at 08:29:11PM +0530, Tanmay Kathpalia
wrote:
Thanks for your comment, Peng.

On 11/18/2025 10:39 AM, Peng Fan wrote:
On Mon, Nov 10, 2025 at 09:37:30AM -0800, Tanmay Kathpalia
wrote:
Add reset control functionality to the SDHCI Cadence driver to
properly handle hardware reset sequences during probe. This
ensures
the controller is in a known state before initialization.

Signed-off-by: Tanmay Kathpalia <[email protected]>
Reviewed-by: Balsundar Ponnusamy
<[email protected]>
---
drivers/mmc/sdhci-cadence.c | 8 ++++++++
1 file changed, 8 insertions(+)

....

@@ -225,6 +227,12 @@ static int sdhci_cdns_probe(struct
udevice *dev)
        if (!plat->hrs_addr)
                return -ENOMEM;

+       ret = reset_get_bulk(dev, &reset_bulk);

Should this be optional? Some in tree platforms may not have the
reset supported.


Yes, you're right-some in-tree platforms may not have reset support.
In those cases, the code will print a warning message ("Can't get
reset") and continue the probe process.
If you prefer, I can remove the warning and let the function fail
silently instead, or is there any other way you would suggest to
make this optional?

devm_reset_bulk_get_optional() may help.

Regards
Peng


I looked into devm_reset_bulk_get_optional(), and I see that it
dynamically allocates the struct reset_ctl_bulk and adds it to the
device resources list if CONFIG_DEVRES is enabled. However, if
CONFIG_DEVRES is not enabled, we need to manually free the memory
using
reset_release_bulk() in the driver's remove function. This means we
would need to store a pointer to struct reset_ctl_bulk in the driver's
private data, which would require additional changes to the sdhci-
cadence driver (since it currently uses the generic struct sdhci_host
with .priv_auto = sizeof(struct sdhci_host)).

Let me know if you’re okay with this approach, as it would require
other changes in the driver, or if you have any further
recommendations.
Alternatively, I can simplify the implementation as shown below:

ret = reset_get_bulk(dev, &reset_bulk);
if (!ret)
        reset_deassert_bulk(&reset_bulk);

For better, an optional API is preferred, since non-devres API
is not there, I am fine with your above changes.

Regards
Peng.


I will make the changes and push the V2 series.

Regards,
Tanmay


Please let me know your preference.

Regards,
Tanmay


Regards
Peng




Reply via email to