misc: sram: add dev_pm_ops to support module power gate
Subsystems (2)
PATCH 1/1 misc: sram: add dev_pm_ops to support module power gate
When system goes into low power states like SUSPEND_MEM and HIBERNATION, the hardware IP block may be powered off to reduce the power consumption. This power down will lost all the data inside the ram. This patch added the dev_pm_ops and implemented two callbacks: suspend_noirq and resume_noirq, which will save the data in the on-chip-ram right before power down and restore it after system resumes.
A new property string named "can-power-gate" is added to the devicetree bindings too.
--- Documentation/devicetree/bindings/misc/sram.txt | 2 ++ drivers/misc/sram.c | 44 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+)
Documentation/devicetree/bindings/misc/sram.txt +2 -0
@@ -33,6 +33,8 @@ Optional properties in the area nodes: - compatible : standard definition, should contain a vendor specific string in the form <vendor>,[<device>-]<usage> +- can-power-gate: a property to tell the driver that the sram can support + power gate Example:
drivers/misc/sram.c +44 -0
72 lines not on this page · raw message
Signature · 1 line
2.5.0.rc2
PATCH 1/1 misc: sram: add dev_pm_ops to support module power gate
Quoted 14 lines · Shenwei Wang
When system goes into low power states like
SUSPEND_MEMand HIBERNATION, the hardware IP block may be powered off to reduce the power consumption. This power down will lost all the data inside the ram. This patch added thedev_pm_opsand implemented two callbacks:suspend_noirqandresume_noirq, which will save the data in the on-chip-ram right before power down and restore it after system resumes.A new property string named "can-power-gate" is added to the devicetree bindings too.
Signed-off-by Shenwei Wang <[email protected]>Signed-off-by Anson Huang <[email protected]>---
[snip]
drivers/misc/sram.c static int sram_reserve_regions(struct sram_dev *sram, struct resource *res) · line 171
Quoted 12 lines
drivers/misc/sram.c +12
-0+static int sram_suspend_noirq (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct sram_dev *sram =platform_get_drvdata (pdev); + +if (!sram->power_off_save) +return 0 ; + +/* Save necessary regs */ +clk_enable (sram->clk); +memcpy (sram->power_off_save, sram->virt_base,gen_pool_size (sram->pool)); +clk_disable (sram->clk);
memcpy_fromio()?
Quoted 4 lines
drivers/misc/sram.c +4
-0+ +return 0 ; +} +
drivers/misc/sram.c static int sram_reserve_regions(struct sram_dev *sram, struct resource *res) · line 185
Quoted 10 lines
drivers/misc/sram.c +10
-0+static int sram_resume_noirq (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct sram_dev *sram =platform_get_drvdata (pdev); + +if (!sram->power_off_save) +return 0 ; + +clk_enable (sram->clk); +memcpy (sram->virt_base, sram->power_off_save,gen_pool_size (sram->pool));
memcpy_toio()?
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 238
Quoted 13 lines
drivers/misc/sram.c +6
-0+clk_disable (sram->clk); + +return 0 ; +} +static int sram_probe (struct platform_device *pdev) {struct sram_dev *sram;@@ -203,6 +235,12 @@ static int sram_probe(struct platform_device *pdev)platform_set_drvdata (pdev, sram); +if (of_get_property (pdev->dev.of_node,"can-power-gate" ,NULL ))
Since this is a boolean you could use of_property_read_bool()
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 241
Quoted 3 lines
drivers/misc/sram.c +3
-0+ { + sram->power_off_save =devm_kzalloc (&pdev->dev, +gen_pool_size (sram->pool),GFP_KERNEL );
Parenthesis not needed.
This will only work with SRAM sizes which are small enough they can be serviced by kzalloc() allocations. The problem is that you might actually need direct-mapped memory but what if you have larger SRAMs that are several MB? Maybe there should be a check here to prevent that.
drivers/misc/sram.c static const struct of_device_id sram_dt_ids[] = { · line 273
Quoted 13 lines
drivers/misc/sram.c +6
-0+ } +dev_dbg (sram->dev,"SRAM pool: %zu KiB @ 0x%p\n" ,gen_pool_size (sram->pool) /1024 , sram->virt_base);@@ -229,10 +267,16 @@ static const struct of_device_id sram_dt_ids[] = { };#endif +static const struct dev_pm_ops sram_pm_ops = { + .suspend_noirq = sram_suspend_noirq, + .resume_noirq = sram_resume_noirq, +};
You may have to enclose this within an ifdef CONFIG_PM_SLEEP/endif statement not to cause warnings if CONFIG_PM_SLEEP is disabled.
Signature · 1 line
Florian
PATCH 1/1 misc: sram: add dev_pm_ops to support module power gate
Quoted 8 lines
-----Original Message-----
From: Florian Fainelli [mailto:f.fainelli at gmail.com]
Sent: 2015?7?24? 15:17
To: Wang Shenwei-B38339; gregkh at linuxfoundation.org; arnd at arndb.de
Cc: Huang Yongcai-B20788; linux-arm-kernel at lists.infradead.org; computersforpeace at gmail.com
Subject: Re: [PATCH 1/1] misc: sram: adddev_pm_opsto support module power
Quoted 8 lines
drivers/misc/sram.c static int sram_reserve_regions(struct sram_dev *sram, struct resource *res)
Quoted 3 lines
drivers/misc/sram.c +3
-0+/* Save necessary regs */ +clk_enable (sram->clk); + memcpy(sram->power_off_save,sram->virt_base,gen_pool_size(sram->pool));
Review
Quoted 1 line
(quoted diff) +1
-0+ clk_disable(sram->clk);
memcpy_fromio()?
memcpy is safe to call here, and it should be more efficient.
Quoted 7 lines
drivers/misc/sram.c static int sram_reserve_regions(struct sram_dev *sram, struct resource *res)
Quoted 4 lines
drivers/misc/sram.c +4
-0+ +clk_enable (sram->clk); +memcpy (sram->virt_base, sram->power_off_save, +gen_pool_size (sram->pool));
memcpy_toio()?
When system goes to suspend state, it will flush the cache in the end. Memcpy here is more efficient.
Quoted 10 lines
drivers/misc/sram.c static int sram_probe(struct platform_device · line 239
Quoted 7 lines
drivers/misc/sram.c +1
-0struct sram_dev *sram;@@ -203,6 +235,12 @@ static int sram_probe(struct platform_device *pdev)platform_set_drvdata (pdev, sram); +if (of_get_property (pdev->dev.of_node,"can-power-gate" ,NULL ))Since this is a boolean you could use
of_property_read_bool()
Yes, of_property_read_bool is better. Will adopt it.
Quoted 5 lines
drivers/misc/sram.c static int sram_probe(struct platform_device *pdev) · line 241
Quoted 3 lines
drivers/misc/sram.c +3
-0+ { + sram->power_off_save =devm_kzalloc (&pdev->dev, +gen_pool_size (sram->pool),GFP_KERNEL );Parenthesis not needed.
Will remove it.
Quoted 6 lines
This will only work with SRAM sizes which are small enough they can be serviced by
kzalloc()allocations. The problem is that you might actually need direct-mapped memory but what if you have larger SRAMs that are several MB? Maybe there should be a check here to prevent that.
Yes, it will be a problem. I think vmalloc should be used here.
Quoted 8 lines
drivers/misc/sram.c static const struct of_device_id sram_dt_ids[] = { · line 273
Quoted 4 lines
drivers/misc/sram.c +4
-0+static const struct dev_pm_ops sram_pm_ops = { + .suspend_noirq = sram_suspend_noirq, + .resume_noirq = sram_resume_noirq, +};You may have to enclose this within an ifdef CONFIG_PM_SLEEP/endif statement not to cause warnings if
CONFIG_PM_SLEEPis disabled.
--
The "struct dev_pm_ops *pm" is always a member of device_driver, and it has no relation with the CONFIG_PM_XXX.
Thanks,
Shenwei
Quoted 1 line
Florian