Re: [PATCH v3 14/18] misc: sram: Enable runtime PM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] misc: sram: Missing pm_runtime_get_sync() in sysfs read/write paths
- [Medium] misc: sram: Hardware initialization is not restored after runtime resume
--

commit b2dc9fd44cd639efd34dbecb064c4f1f3097e1eb
Author: Linus Walleij <linusw@kernel.org>

    misc: sram: Enable runtime PM
    
    Enable runtime PM for generic SRAM devices after probe has initialized
    the SRAM mappings and partitions.
    
    This lets SRAM devices with attached power domains be managed through
    runtime PM references from their consumers.

> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
> index c8990e83a65a2..7f19dca7b818e 100644
> --- a/drivers/misc/sram.c
> +++ b/drivers/misc/sram.c

[ ... ]

> @@ -425,6 +426,10 @@ static int sram_probe(struct platform_device *pdev)
>  			goto err_free_partitions;
>  	}
>  
> +	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);

[Severity: High]
Does this code allow sysfs read/write paths to access unpowered MMIO hardware?

Since runtime PM is now enabled, the SRAM device can be suspended by its
attached power domain. If userspace accesses the exported sysfs bin
attributes, sram_read() and sram_write() perform MMIO accesses without
waking the device:

drivers/misc/sram.c:sram_read() {
	...
	mutex_lock(&part->lock);
	memcpy_fromio(buf, part->base + pos, count);
	mutex_unlock(&part->lock);
	...
}

Will this cause an asynchronous external abort and crash the system if the
hardware is accessed while suspended?

[Severity: Medium]
Will hardware state be lost and not restored on resume?

Earlier in sram_probe(), hardware initialization is performed:

drivers/misc/sram.c:sram_probe() {
	...
	if (config && config->init) {
		ret = config->init();
	...
}

If the device is attached to a PM domain that powers it down, waking it up
will turn on the power but will not re-apply the config->init() callback
(such as atmel_securam_wait) because the driver lacks a runtime_resume
callback.

Can this lead to hardware faults or data corruption if the RAM is accessed
before it is ready?

> +	if (ret)
> +		goto err_free_partitions;
> +
>  	if (sram->pool)
>  		dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n",
>  			gen_pool_size(sram->pool) / 1024, sram->virt_base);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-ux500-power-domains-v7-1-v3-0-9352be6b86b1@kernel.org?part=14