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

[Tanmay Kathpalia <tanmay.kathpalia@altera.com>]

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@altera.com>
>>>> Reviewed-by: Balsundar Ponnusamy <balsundar.ponnusamy@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);

Please let me know your preference.

Regards,
Tanmay

>>
>>> Regards
>>> Peng
>>
>>