[PATCH 1/8] mmc: sdhci-cadence: Add reset control support

[Peng Fan]

> 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 <tanmay.kathpalia at altera.com>
> >>>> Reviewed-by: Balsundar Ponnusamy
> <balsundar.ponnusamy at altera.com>
> >>>> ---
> >>>> 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.

> 
> Please let me know your preference.
> 
> Regards,
> Tanmay
> 
> >>
> >>> Regards
> >>> Peng
> >>
> >>