| Message ID | 1438272681-29338-1-git-send-email-shenwei.wang@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ping. Shenwei > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On > Behalf Of Shenwei Wang > Sent: 2015?7?30? 11:11 > To: gregkh@linuxfoundation.org; arnd@arndb.de > Cc: Huang Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: [PATCH v3 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. > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > Change log: > > PATCH v3 > Removed the unnecessary clk_enable/clk_disable. > > PATCH v2 > Use vmalloc to allocate the SRAM backup memory. > Code clean up. > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > drivers/misc/sram.c | 42 > +++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > b/Documentation/devicetree/bindings/misc/sram.txt > index 36cbe5a..1170086 100644 > --- a/Documentation/devicetree/bindings/misc/sram.txt > +++ b/Documentation/devicetree/bindings/misc/sram.txt > @@ -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: > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 15c33cc..db9f1fa > 100644 > --- a/drivers/misc/sram.c > +++ b/drivers/misc/sram.c > @@ -30,7 +30,9 @@ > > struct sram_dev { > struct device *dev; > + void *power_off_save; > void __iomem *virt_base; > + u32 size; > > struct gen_pool *pool; > struct clk *clk; > @@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev > *sram, struct resource *res) > return ret; > } > > +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 */ > + memcpy(sram->power_off_save, sram->virt_base, sram->size); > + > + return 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; > + > + memcpy(sram->virt_base, sram->power_off_save, sram->size); > + > + return 0; > +} > + > static int sram_probe(struct platform_device *pdev) { > struct sram_dev *sram; > @@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev) > } > > size = resource_size(res); > + sram->size = size; > > if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) { > dev_err(sram->dev, "could not request region for resource\n"); @@ > -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, sram); > > + if (of_property_read_bool(pdev->dev.of_node, "can-power-gate")) > + sram->power_off_save = vmalloc(size); > + > dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n", > gen_pool_size(sram->pool) / 1024, sram->virt_base); > > @@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev) > if (sram->clk) > clk_disable_unprepare(sram->clk); > > + if (sram->power_off_save) > + vfree(sram->power_off_save); > + > return 0; > } > > @@ -229,10 +265,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, > +}; > + > static struct platform_driver sram_driver = { > .driver = { > .name = "sram", > .of_match_table = of_match_ptr(sram_dt_ids), > + .pm = &sram_pm_ops, > }, > .probe = sram_probe, > .remove = sram_remove, > -- > 2.5.0.rc2 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
2nd Ping. Regards, Shenwei > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On > Behalf Of Shenwei Wang > Sent: 2015?7?30? 11:11 > To: gregkh@linuxfoundation.org; arnd@arndb.de > Cc: Huang Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: [PATCH v3 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. > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > Change log: > > PATCH v3 > Removed the unnecessary clk_enable/clk_disable. > > PATCH v2 > Use vmalloc to allocate the SRAM backup memory. > Code clean up. > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > drivers/misc/sram.c | 42 > +++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > b/Documentation/devicetree/bindings/misc/sram.txt > index 36cbe5a..1170086 100644 > --- a/Documentation/devicetree/bindings/misc/sram.txt > +++ b/Documentation/devicetree/bindings/misc/sram.txt > @@ -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: > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 15c33cc..db9f1fa > 100644 > --- a/drivers/misc/sram.c > +++ b/drivers/misc/sram.c > @@ -30,7 +30,9 @@ > > struct sram_dev { > struct device *dev; > + void *power_off_save; > void __iomem *virt_base; > + u32 size; > > struct gen_pool *pool; > struct clk *clk; > @@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev > *sram, struct resource *res) > return ret; > } > > +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 */ > + memcpy(sram->power_off_save, sram->virt_base, sram->size); > + > + return 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; > + > + memcpy(sram->virt_base, sram->power_off_save, sram->size); > + > + return 0; > +} > + > static int sram_probe(struct platform_device *pdev) { > struct sram_dev *sram; > @@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev) > } > > size = resource_size(res); > + sram->size = size; > > if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) { > dev_err(sram->dev, "could not request region for resource\n"); @@ > -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, sram); > > + if (of_property_read_bool(pdev->dev.of_node, "can-power-gate")) > + sram->power_off_save = vmalloc(size); > + > dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n", > gen_pool_size(sram->pool) / 1024, sram->virt_base); > > @@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev) > if (sram->clk) > clk_disable_unprepare(sram->clk); > > + if (sram->power_off_save) > + vfree(sram->power_off_save); > + > return 0; > } > > @@ -229,10 +265,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, > +}; > + > static struct platform_driver sram_driver = { > .driver = { > .name = "sram", > .of_match_table = of_match_ptr(sram_dt_ids), > + .pm = &sram_pm_ops, > }, > .probe = sram_probe, > .remove = sram_remove, > -- > 2.5.0.rc2 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
3rd Ping. Shenwei > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On > Behalf Of Shenwei Wang > Sent: 2015?8?12? 10:47 > To: gregkh@linuxfoundation.org; arnd@arndb.de > Cc: Huang Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: RE: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module > power gate > > 2nd Ping. > > Regards, > Shenwei > > > -----Original Message----- > > From: linux-arm-kernel > > [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of > > Shenwei Wang > > Sent: 2015?7?30? 11:11 > > To: gregkh@linuxfoundation.org; arnd@arndb.de > > Cc: Huang Yongcai-B20788; linux-arm-kernel@lists.infradead.org > > Subject: [PATCH v3 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. > > > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > > Signed-off-by: Anson Huang <b20788@freescale.com> > > --- > > Change log: > > > > PATCH v3 > > Removed the unnecessary clk_enable/clk_disable. > > > > PATCH v2 > > Use vmalloc to allocate the SRAM backup memory. > > Code clean up. > > > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > > drivers/misc/sram.c | 42 > > +++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > > b/Documentation/devicetree/bindings/misc/sram.txt > > index 36cbe5a..1170086 100644 > > --- a/Documentation/devicetree/bindings/misc/sram.txt > > +++ b/Documentation/devicetree/bindings/misc/sram.txt > > @@ -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: > > > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index > > 15c33cc..db9f1fa > > 100644 > > --- a/drivers/misc/sram.c > > +++ b/drivers/misc/sram.c > > @@ -30,7 +30,9 @@ > > > > struct sram_dev { > > struct device *dev; > > + void *power_off_save; > > void __iomem *virt_base; > > + u32 size; > > > > struct gen_pool *pool; > > struct clk *clk; > > @@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev > > *sram, struct resource *res) > > return ret; > > } > > > > +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 */ > > + memcpy(sram->power_off_save, sram->virt_base, sram->size); > > + > > + return 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; > > + > > + memcpy(sram->virt_base, sram->power_off_save, sram->size); > > + > > + return 0; > > +} > > + > > static int sram_probe(struct platform_device *pdev) { > > struct sram_dev *sram; > > @@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev) > > } > > > > size = resource_size(res); > > + sram->size = size; > > > > if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) { > > dev_err(sram->dev, "could not request region for resource\n"); @@ > > -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, sram); > > > > + if (of_property_read_bool(pdev->dev.of_node, "can-power-gate")) > > + sram->power_off_save = vmalloc(size); > > + > > dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n", > > gen_pool_size(sram->pool) / 1024, sram->virt_base); > > > > @@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev) > > if (sram->clk) > > clk_disable_unprepare(sram->clk); > > > > + if (sram->power_off_save) > > + vfree(sram->power_off_save); > > + > > return 0; > > } > > > > @@ -229,10 +265,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, > > +}; > > + > > static struct platform_driver sram_driver = { > > .driver = { > > .name = "sram", > > .of_match_table = of_match_ptr(sram_dt_ids), > > + .pm = &sram_pm_ops, > > }, > > .probe = sram_probe, > > .remove = sram_remove, > > -- > > 2.5.0.rc2 > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi GregKH, Can you please take a look at this patch? Thanks, Shenwei > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On > Behalf Of Shenwei Wang > Sent: 2015?7?30? 11:11 > To: gregkh@linuxfoundation.org; arnd@arndb.de > Cc: Huang Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: [PATCH v3 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. > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > Change log: > > PATCH v3 > Removed the unnecessary clk_enable/clk_disable. > > PATCH v2 > Use vmalloc to allocate the SRAM backup memory. > Code clean up. > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > drivers/misc/sram.c | 42 > +++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > b/Documentation/devicetree/bindings/misc/sram.txt > index 36cbe5a..1170086 100644 > --- a/Documentation/devicetree/bindings/misc/sram.txt > +++ b/Documentation/devicetree/bindings/misc/sram.txt > @@ -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: > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 15c33cc..db9f1fa > 100644 > --- a/drivers/misc/sram.c > +++ b/drivers/misc/sram.c > @@ -30,7 +30,9 @@ > > struct sram_dev { > struct device *dev; > + void *power_off_save; > void __iomem *virt_base; > + u32 size; > > struct gen_pool *pool; > struct clk *clk; > @@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev > *sram, struct resource *res) > return ret; > } > > +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 */ > + memcpy(sram->power_off_save, sram->virt_base, sram->size); > + > + return 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; > + > + memcpy(sram->virt_base, sram->power_off_save, sram->size); > + > + return 0; > +} > + > static int sram_probe(struct platform_device *pdev) { > struct sram_dev *sram; > @@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev) > } > > size = resource_size(res); > + sram->size = size; > > if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) { > dev_err(sram->dev, "could not request region for resource\n"); @@ > -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, sram); > > + if (of_property_read_bool(pdev->dev.of_node, "can-power-gate")) > + sram->power_off_save = vmalloc(size); > + > dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n", > gen_pool_size(sram->pool) / 1024, sram->virt_base); > > @@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev) > if (sram->clk) > clk_disable_unprepare(sram->clk); > > + if (sram->power_off_save) > + vfree(sram->power_off_save); > + > return 0; > } > > @@ -229,10 +265,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, > +}; > + > static struct platform_driver sram_driver = { > .driver = { > .name = "sram", > .of_match_table = of_match_ptr(sram_dt_ids), > + .pm = &sram_pm_ops, > }, > .probe = sram_probe, > .remove = sram_remove, > -- > 2.5.0.rc2 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 30/07/15 17:11, Shenwei Wang wrote: > 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. > Who uses this SRAM ? Can't they be released before suspending ? This might not be feasible and time consuming as the size of SRAM increases. Also I just check the entire tree for the users of gen_pool, I found it's used for video/audio and crypto which needs to on-demand basis. Apart from that it's mainly used for low power modes in which case, SRAM needs to be powered on. So can you provide more details on your use-case. > A new property string named "can-power-gate" is added to > the devicetree bindings too. > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > Change log: > > PATCH v3 > Removed the unnecessary clk_enable/clk_disable. > > PATCH v2 > Use vmalloc to allocate the SRAM backup memory. > Code clean up. > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > drivers/misc/sram.c | 42 +++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt > index 36cbe5a..1170086 100644 > --- a/Documentation/devicetree/bindings/misc/sram.txt > +++ b/Documentation/devicetree/bindings/misc/sram.txt > @@ -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 > Please cc device-tree list for such generic bindings. Regards, Sudeep
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > Sent: 2015?8?25? 8:03 > To: Wang Shenwei-B38339 > Cc: gregkh@linuxfoundation.org; arnd@arndb.de; Sudeep Holla; Huang > Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module > power gate > > > > On 30/07/15 17:11, Shenwei Wang wrote: > > 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. > > > > Who uses this SRAM ? Can't they be released before suspending ? > This might not be feasible and time consuming as the size of SRAM increases. On Freescale i.MX7D, we are using this kind of SRAM. As I implemented this as an optional feature, a user can balance between the suspend/resume time and the power consumption. > Also I just check the entire tree for the users of gen_pool, I found it's used for > video/audio and crypto which needs to on-demand basis. > Apart from that it's mainly used for low power modes in which case, SRAM needs > to be powered on. So can you provide more details on your use-case. On latest Freescale i.MX7D, it has two SRAMs in side. One is for low power mode which will be powered on in suspend state. The other one is for other peripherals which can be powered off on demand. Thanks, Shenwei > > A new property string named "can-power-gate" is added to the > > devicetree bindings too. > > > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > > Signed-off-by: Anson Huang <b20788@freescale.com> > > --- > > Change log: > > > > PATCH v3 > > Removed the unnecessary clk_enable/clk_disable. > > > > PATCH v2 > > Use vmalloc to allocate the SRAM backup memory. > > Code clean up. > > > > Documentation/devicetree/bindings/misc/sram.txt | 2 ++ > > drivers/misc/sram.c | 42 > +++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > > b/Documentation/devicetree/bindings/misc/sram.txt > > index 36cbe5a..1170086 100644 > > --- a/Documentation/devicetree/bindings/misc/sram.txt > > +++ b/Documentation/devicetree/bindings/misc/sram.txt > > @@ -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 > > > > Please cc device-tree list for such generic bindings. > > Regards, > Sudeep
On 25/08/15 14:47, Shenwei Wang wrote: > > >> -----Original Message----- >> From: Sudeep Holla [mailto:sudeep.holla@arm.com] >> Sent: 2015?8?25? 8:03 >> To: Wang Shenwei-B38339 >> Cc: gregkh@linuxfoundation.org; arnd@arndb.de; Sudeep Holla; Huang >> Yongcai-B20788; linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module >> power gate >> >> >> >> On 30/07/15 17:11, Shenwei Wang wrote: >>> 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. >>> >> >> Who uses this SRAM ? Can't they be released before suspending ? >> This might not be feasible and time consuming as the size of SRAM increases. > > On Freescale i.MX7D, we are using this kind of SRAM. As I implemented this as an > optional feature, a user can balance between the suspend/resume time and the power > consumption. > >> Also I just check the entire tree for the users of gen_pool, I found it's used for >> video/audio and crypto which needs to on-demand basis. >> Apart from that it's mainly used for low power modes in which case, SRAM needs >> to be powered on. So can you provide more details on your use-case. > > On latest Freescale i.MX7D, it has two SRAMs in side. One is for low > power mode which will be powered on in suspend state. The other one > is for other peripherals which can be powered off on demand. > What I meant is who will be using the SRAM when entering S2R that you need to save the content. Either the user needs to take care of the content or just release the pool when not in use. When the sram pool has no user, clock can be gated. Again can you give details of this use-case. Regards, Sudeep
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > Sent: 2015?8?25? 9:00 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang > Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module > power gate > > On latest Freescale i.MX7D, it has two SRAMs in side. One is for low > > power mode which will be powered on in suspend state. The other one is > > for other peripherals which can be powered off on demand. > > > > What I meant is who will be using the SRAM when entering S2R that you need to > save the content. Either the user needs to take care of the content or just release > the pool when not in use. When the sram pool has no user, clock can be gated. > > Again can you give details of this use-case. We are not talking about clock gate state. Here we are handling the power gate for this SRAM IP block. Of course, you can ask each driver to save its contents during low power state, but I think the easier way is to save the contents in this sram driver and just leave the drivers who use this sram behave as is today. Thanks, Shenwei > Regards, > Sudeep
On 25/08/15 15:20, Shenwei Wang wrote: > > >> -----Original Message----- >> From: Sudeep Holla [mailto:sudeep.holla@arm.com] >> Sent: 2015?8?25? 9:00 >> To: Wang Shenwei-B38339 >> Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang >> Yongcai-B20788; linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module >> power gate >>> On latest Freescale i.MX7D, it has two SRAMs in side. One is for low >>> power mode which will be powered on in suspend state. The other one is >>> for other peripherals which can be powered off on demand. >>> >> >> What I meant is who will be using the SRAM when entering S2R that you need to >> save the content. Either the user needs to take care of the content or just release >> the pool when not in use. When the sram pool has no user, clock can be gated. >> >> Again can you give details of this use-case. > > We are not talking about clock gate state. Here we are handling the power gate for > this SRAM IP block. Of course, you can ask each driver to save its contents during > low power state, but I think the easier way is to save the contents in this sram driver > and just leave the drivers who use this sram behave as is today. > I don't like that solution as it's not scalable with SRAM size and saving all the content of SRAM just because of few active regions makes no sense to me at-least. I prefer leaving it to the users to handle that rather than SRAM driver. Regards, Sudeep
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > Sent: 2015?8?25? 9:41 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang > Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module > power gate > >> What I meant is who will be using the SRAM when entering S2R that you > >> need to save the content. Either the user needs to take care of the > >> content or just release the pool when not in use. When the sram pool has no > user, clock can be gated. > >> > >> Again can you give details of this use-case. > > > > We are not talking about clock gate state. Here we are handling the > > power gate for this SRAM IP block. Of course, you can ask each driver > > to save its contents during low power state, but I think the easier > > way is to save the contents in this sram driver and just leave the drivers who > use this sram behave as is today. > > > > I don't like that solution as it's not scalable with SRAM size and saving all the > content of SRAM just because of few active regions makes no sense to me > at-least. I prefer leaving it to the users to handle that rather than SRAM driver. I don't think it is not scalable. The choice should be made by the application. From the driver side, it should be able to provide the way to retain its contents during power gate. Regards, Shenwei > Regards, > Sudeep
On 25/08/15 16:05, Shenwei Wang wrote: > > >> -----Original Message----- >> From: Sudeep Holla [mailto:sudeep.holla@arm.com] >> Sent: 2015?8?25? 9:41 >> To: Wang Shenwei-B38339 >> Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang >> Yongcai-B20788; linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module >> power gate >>>> What I meant is who will be using the SRAM when entering S2R that you >>>> need to save the content. Either the user needs to take care of the >>>> content or just release the pool when not in use. When the sram pool has no >> user, clock can be gated. >>>> >>>> Again can you give details of this use-case. >>> >>> We are not talking about clock gate state. Here we are handling >>> the power gate for this SRAM IP block. Of course, you can ask >>> each driver to save its contents during low power state, but I >>> think the easier way is to save the contents in this sram driver >>> and just leave the drivers who use this sram behave as is today. >> >> I don't like that solution as it's not scalable with SRAM size and >> saving all the content of SRAM just because of few active regions >> makes no sense to me at-least. I prefer leaving it to the users to >> handle that rather than SRAM driver. > > I don't think it is not scalable. The choice should be made by the > application. From the driver side, it should be able to provide the > way to retain its contents during power gate. > OK that was just my opinion. I just wanted to avoid unnecessary save/restore, e.g. if say just few kilobytes are active in say 32MB SRAM, you will save/restore entire SRAM ? Anyways, the driver is saving/restoring the memory unconditionally whenever DT sets that boolean right ? at-least as the patch stands. Regards, Sudeep
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > Sent: 2015?8?25? 11:03 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang > Yongcai-B20788; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module > power gate > >> I don't like that solution as it's not scalable with SRAM size and > >> saving all the content of SRAM just because of few active regions > >> makes no sense to me at-least. I prefer leaving it to the users to > >> handle that rather than SRAM driver. > > > > I don't think it is not scalable. The choice should be made by the > > application. From the driver side, it should be able to provide the > > way to retain its contents during power gate. > > > > OK that was just my opinion. I just wanted to avoid unnecessary save/restore, e.g. > if say just few kilobytes are active in say 32MB SRAM, you will save/restore entire > SRAM ? Although in theory a SoC can integrate such a big SRAM like 32MB inside, but I don't think anyone will practice it because the cost is too high. So far, the most popular size of a SRAM in a SoC is hundreds of KB. > Anyways, the driver is saving/restoring the memory unconditionally whenever DT > sets that boolean right ? at-least as the patch stands. The saving/restoring feature is not enabled by default in this patch. To enable it you need to add the can-power-gate string in the relating DT node. Regards, Shenwei > Regards, > Sudeep
On 25/08/15 20:35, Shenwei Wang wrote: > > >> -----Original Message----- >> From: Sudeep Holla [mailto:sudeep.holla@arm.com] >> Sent: 2015?8?25? 11:03 >> To: Wang Shenwei-B38339 >> Cc: Sudeep Holla; gregkh@linuxfoundation.org; arnd@arndb.de; Huang >> Yongcai-B20788; linux-arm-kernel@lists.infradead.org >> Subject: Re: [PATCH v3 1/1] misc: sram: add dev_pm_ops to support module >> power gate >>>> I don't like that solution as it's not scalable with SRAM size and >>>> saving all the content of SRAM just because of few active regions >>>> makes no sense to me at-least. I prefer leaving it to the users to >>>> handle that rather than SRAM driver. >>> >>> I don't think it is not scalable. The choice should be made by the >>> application. From the driver side, it should be able to provide the >>> way to retain its contents during power gate. >>> >> >> OK that was just my opinion. I just wanted to avoid unnecessary save/restore, e.g. >> if say just few kilobytes are active in say 32MB SRAM, you will save/restore entire >> SRAM ? > > Although in theory a SoC can integrate such a big SRAM like 32MB inside, but I don't > think anyone will practice it because the cost is too high. So far, the most popular size > of a SRAM in a SoC is hundreds of KB. > I have seen 32MB SRAM and people are thinking of doubling it. So I disagree, especially if that's your main argument. On Android targets, I have seen suspend-to-ram used quite aggressively and this save/restore might add significant delay and might be unnecessary most of the time. >> Anyways, the driver is saving/restoring the memory unconditionally whenever DT >> sets that boolean right ? at-least as the patch stands. > > The saving/restoring feature is not enabled by default in this patch. To enable it you > need to add the can-power-gate string in the relating DT node. Yes I understood and that's what I mentioned above, but once it's present in DT, it's unconditional for the entire SRAM which might not be used actively. So I still don't like this approach. Regards, Sudeep
diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt index 36cbe5a..1170086 100644 --- a/Documentation/devicetree/bindings/misc/sram.txt +++ b/Documentation/devicetree/bindings/misc/sram.txt @@ -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: diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 15c33cc..db9f1fa 100644 --- a/drivers/misc/sram.c +++ b/drivers/misc/sram.c @@ -30,7 +30,9 @@ struct sram_dev { struct device *dev; + void *power_off_save; void __iomem *virt_base; + u32 size; struct gen_pool *pool; struct clk *clk; @@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev *sram, struct resource *res) return ret; } +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 */ + memcpy(sram->power_off_save, sram->virt_base, sram->size); + + return 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; + + memcpy(sram->virt_base, sram->power_off_save, sram->size); + + return 0; +} + static int sram_probe(struct platform_device *pdev) { struct sram_dev *sram; @@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev) } size = resource_size(res); + sram->size = size; if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) { dev_err(sram->dev, "could not request region for resource\n"); @@ -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev) platform_set_drvdata(pdev, sram); + if (of_property_read_bool(pdev->dev.of_node, "can-power-gate")) + sram->power_off_save = vmalloc(size); + dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n", gen_pool_size(sram->pool) / 1024, sram->virt_base); @@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev) if (sram->clk) clk_disable_unprepare(sram->clk); + if (sram->power_off_save) + vfree(sram->power_off_save); + return 0; } @@ -229,10 +265,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, +}; + static struct platform_driver sram_driver = { .driver = { .name = "sram", .of_match_table = of_match_ptr(sram_dt_ids), + .pm = &sram_pm_ops, }, .probe = sram_probe, .remove = sram_remove,