> 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. > > Please let me know your preference. > > Regards, > Tanmay > > >> > >>> Regards > >>> Peng > >> > >>
