[PATCH] audio: sai: Add Power Management support

[PATCH] audio: sai: Add Power Management support

[Alison Wang]

This patch adds Power Management support for SAI.
Activate regmap cache with REGCACHE_RBTREE, and use
regmap cache code to save and restore registers in
suspend and resume. The Transmit Control Register
(TCSR) and Receive Control Register(RCSR) should
be volatile registers.

Signed-off-by: Alison Wang <alison.wang@freescale.com>
---
 sound/soc/fsl/fsl_sai.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 7eeb1dd..c7dd953 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -509,9 +509,11 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
+	case FSL_SAI_TCSR:
 	case FSL_SAI_TFR:
-	case FSL_SAI_RFR:
 	case FSL_SAI_TDR:
+	case FSL_SAI_RCSR:
+	case FSL_SAI_RFR:
 	case FSL_SAI_RDR:
 		return true;
 	default:
@@ -553,6 +555,7 @@ static const struct regmap_config fsl_sai_regmap_config = {
 	.readable_reg = fsl_sai_readable_reg,
 	.volatile_reg = fsl_sai_volatile_reg,
 	.writeable_reg = fsl_sai_writeable_reg,
+	.cache_type = REGCACHE_RBTREE,
 };
 
 static int fsl_sai_probe(struct platform_device *pdev)
@@ -668,6 +671,33 @@ static int fsl_sai_probe(struct platform_device *pdev)
 				SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int fsl_sai_suspend(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	regcache_cache_only(sai->regmap, true);
+	regcache_mark_dirty(sai->regmap);
+
+	return 0;
+}
+
+static int fsl_sai_resume(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	/* Restore all registers */
+	regcache_cache_only(sai->regmap, false);
+	regcache_sync(sai->regmap);
+
+	return 0;
+};
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops fsl_sai_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)
+};
+
 static const struct of_device_id fsl_sai_ids[] = {
 	{ .compatible = "fsl,vf610-sai", },
 	{ .compatible = "fsl,imx6sx-sai", },
@@ -680,6 +710,7 @@ static struct platform_driver fsl_sai_driver = {
 		.name = "fsl-sai",
 		.owner = THIS_MODULE,
 		.of_match_table = fsl_sai_ids,
+		.pm = &fsl_sai_pm,
 	},
 };
 module_platform_driver(fsl_sai_driver);
-- 
2.1.0.27.g96db324

[PATCH] audio: sai: Add Power Management support

[Alison Wang]

This patch adds Power Management support for SAI.
Activate regmap cache with REGCACHE_RBTREE, and use
regmap cache code to save and restore registers in
suspend and resume. The Transmit Control Register
(TCSR) and Receive Control Register(RCSR) should
be volatile registers.

Signed-off-by: Alison Wang <alison.wang@freescale.com>
---
 sound/soc/fsl/fsl_sai.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 7eeb1dd..c7dd953 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -509,9 +509,11 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
+	case FSL_SAI_TCSR:
 	case FSL_SAI_TFR:
-	case FSL_SAI_RFR:
 	case FSL_SAI_TDR:
+	case FSL_SAI_RCSR:
+	case FSL_SAI_RFR:
 	case FSL_SAI_RDR:
 		return true;
 	default:
@@ -553,6 +555,7 @@ static const struct regmap_config fsl_sai_regmap_config = {
 	.readable_reg = fsl_sai_readable_reg,
 	.volatile_reg = fsl_sai_volatile_reg,
 	.writeable_reg = fsl_sai_writeable_reg,
+	.cache_type = REGCACHE_RBTREE,
 };
 
 static int fsl_sai_probe(struct platform_device *pdev)
@@ -668,6 +671,33 @@ static int fsl_sai_probe(struct platform_device *pdev)
 				SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int fsl_sai_suspend(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	regcache_cache_only(sai->regmap, true);
+	regcache_mark_dirty(sai->regmap);
+
+	return 0;
+}
+
+static int fsl_sai_resume(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	/* Restore all registers */
+	regcache_cache_only(sai->regmap, false);
+	regcache_sync(sai->regmap);
+
+	return 0;
+};
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops fsl_sai_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)
+};
+
 static const struct of_device_id fsl_sai_ids[] = {
 	{ .compatible = "fsl,vf610-sai", },
 	{ .compatible = "fsl,imx6sx-sai", },
@@ -680,6 +710,7 @@ static struct platform_driver fsl_sai_driver = {
 		.name = "fsl-sai",
 		.owner = THIS_MODULE,
 		.of_match_table = fsl_sai_ids,
+		.pm = &fsl_sai_pm,
 	},
 };
 module_platform_driver(fsl_sai_driver);
-- 
2.1.0.27.g96db324

[PATCH] audio: sai: Add Power Management support

[Alison Wang]

This patch adds Power Management support for SAI.
Activate regmap cache with REGCACHE_RBTREE, and use
regmap cache code to save and restore registers in
suspend and resume. The Transmit Control Register
(TCSR) and Receive Control Register(RCSR) should
be volatile registers.

Signed-off-by: Alison Wang <alison.wang@freescale.com>
---
 sound/soc/fsl/fsl_sai.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 7eeb1dd..c7dd953 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -509,9 +509,11 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
+	case FSL_SAI_TCSR:
 	case FSL_SAI_TFR:
-	case FSL_SAI_RFR:
 	case FSL_SAI_TDR:
+	case FSL_SAI_RCSR:
+	case FSL_SAI_RFR:
 	case FSL_SAI_RDR:
 		return true;
 	default:
@@ -553,6 +555,7 @@ static const struct regmap_config fsl_sai_regmap_config = {
 	.readable_reg = fsl_sai_readable_reg,
 	.volatile_reg = fsl_sai_volatile_reg,
 	.writeable_reg = fsl_sai_writeable_reg,
+	.cache_type = REGCACHE_RBTREE,
 };
 
 static int fsl_sai_probe(struct platform_device *pdev)
@@ -668,6 +671,33 @@ static int fsl_sai_probe(struct platform_device *pdev)
 				SND_DMAENGINE_PCM_FLAG_NO_RESIDUE);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int fsl_sai_suspend(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	regcache_cache_only(sai->regmap, true);
+	regcache_mark_dirty(sai->regmap);
+
+	return 0;
+}
+
+static int fsl_sai_resume(struct device *dev)
+{
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+
+	/* Restore all registers */
+	regcache_cache_only(sai->regmap, false);
+	regcache_sync(sai->regmap);
+
+	return 0;
+};
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops fsl_sai_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)
+};
+
 static const struct of_device_id fsl_sai_ids[] = {
 	{ .compatible = "fsl,vf610-sai", },
 	{ .compatible = "fsl,imx6sx-sai", },
@@ -680,6 +710,7 @@ static struct platform_driver fsl_sai_driver = {
 		.name = "fsl-sai",
 		.owner = THIS_MODULE,
 		.of_match_table = fsl_sai_ids,
+		.pm = &fsl_sai_pm,
 	},
 };
 module_platform_driver(fsl_sai_driver);
-- 
2.1.0.27.g96db324

Re: [PATCH] audio: sai: Add Power Management support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> This patch adds Power Management support for SAI.
> Activate regmap cache with REGCACHE_RBTREE, and use

Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
devices the cost tradeoff for the rbtree is usually higher than people
like so flat caches are preferred.  But if it works for you that's fine,
this shouldn't be *that* performance critical.

I'm also a bit surprised that this works without register defaults being
provided since we need to make sure we allocate the rbtree nodes outside
of the spinlock we use to lock MMIO access - was this tested with
mainline?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH] audio: sai: Add Power Management support

[Mark Brown]

On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> This patch adds Power Management support for SAI.
> Activate regmap cache with REGCACHE_RBTREE, and use

Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
devices the cost tradeoff for the rbtree is usually higher than people
like so flat caches are preferred.  But if it works for you that's fine,
this shouldn't be *that* performance critical.

I'm also a bit surprised that this works without register defaults being
provided since we need to make sure we allocate the rbtree nodes outside
of the spinlock we use to lock MMIO access - was this tested with
mainline?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141029/7349df14/attachment.sig>

Re: [PATCH] audio: sai: Add Power Management support

[Fabio Estevam]

Hi Alison,

Please always add the driver maintainers (Xiubo Li and Nicolin Chen).

On Wed, Oct 29, 2014 at 1:21 AM, Alison Wang <b18965@freescale.com> wrote:

> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_sai_suspend(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(sai->regmap, true);
> +       regcache_mark_dirty(sai->regmap);
> +
> +       return 0;
> +}
> +
> +static int fsl_sai_resume(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       /* Restore all registers */
> +       regcache_cache_only(sai->regmap, false);
> +       regcache_sync(sai->regmap);
> +
> +       return 0;
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct dev_pm_ops fsl_sai_pm = {
> +       SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)

This could be simplified to:
static SIMPLE_DEV_PM_OPS(fsl_sai_pm, fsl_sai_suspend, fsl_sai_resume);

I am also curious as to how you tested it, as I noticed that
suspend/resume is broken on 3.18-rc for mx6sx.

Are you able to do suspend/resume on 3.18-rc on a mx6sx sdb board?

[PATCH] audio: sai: Add Power Management support

[Fabio Estevam]

Hi Alison,

Please always add the driver maintainers (Xiubo Li and Nicolin Chen).

On Wed, Oct 29, 2014 at 1:21 AM, Alison Wang <b18965@freescale.com> wrote:

> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_sai_suspend(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(sai->regmap, true);
> +       regcache_mark_dirty(sai->regmap);
> +
> +       return 0;
> +}
> +
> +static int fsl_sai_resume(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       /* Restore all registers */
> +       regcache_cache_only(sai->regmap, false);
> +       regcache_sync(sai->regmap);
> +
> +       return 0;
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct dev_pm_ops fsl_sai_pm = {
> +       SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)

This could be simplified to:
static SIMPLE_DEV_PM_OPS(fsl_sai_pm, fsl_sai_suspend, fsl_sai_resume);

I am also curious as to how you tested it, as I noticed that
suspend/resume is broken on 3.18-rc for mx6sx.

Are you able to do suspend/resume on 3.18-rc on a mx6sx sdb board?

RE: [PATCH] audio: sai: Add Power Management support

[Li.Xiubo]

Hi,


> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Mark Brown
> Sent: Wednesday, October 29, 2014 7:37 PM
> To: Wang Huan-B18965
> Cc: alsa-devel@alsa-project.org; tiwai@suse.de; linux-kernel@vger.kernel.org;
> lgirdwood@gmail.com; perex@perex.cz; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH] audio: sai: Add Power Management support
> 
> On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> > This patch adds Power Management support for SAI.
> > Activate regmap cache with REGCACHE_RBTREE, and use
> 
> Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
> devices the cost tradeoff for the rbtree is usually higher than people
> like so flat caches are preferred.  But if it works for you that's fine,
> this shouldn't be *that* performance critical.
> 

Yes, the flat caches will have higher performance and can also fix the
Register defaults and spinlock issue here.

One more thing, if the device is not performance critical, then shouldn't we
Take care of the cache memory consumption to determine using flat or rbtree
Type ?

Thanks,

BRs
Xiubo


> I'm also a bit surprised that this works without register defaults being
> provided since we need to make sure we allocate the rbtree nodes outside
> of the spinlock we use to lock MMIO access - was this tested with
> mainline?

[PATCH] audio: sai: Add Power Management support

[Li.Xiubo at freescale.com]

Hi,


> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org]
> On Behalf Of Mark Brown
> Sent: Wednesday, October 29, 2014 7:37 PM
> To: Wang Huan-B18965
> Cc: alsa-devel at alsa-project.org; tiwai at suse.de; linux-kernel at vger.kernel.org;
> lgirdwood at gmail.com; perex at perex.cz; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] audio: sai: Add Power Management support
> 
> On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> > This patch adds Power Management support for SAI.
> > Activate regmap cache with REGCACHE_RBTREE, and use
> 
> Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
> devices the cost tradeoff for the rbtree is usually higher than people
> like so flat caches are preferred.  But if it works for you that's fine,
> this shouldn't be *that* performance critical.
> 

Yes, the flat caches will have higher performance and can also fix the
Register defaults and spinlock issue here.

One more thing, if the device is not performance critical, then shouldn't we
Take care of the cache memory consumption to determine using flat or rbtree
Type ?

Thanks,

BRs
Xiubo


> I'm also a bit surprised that this works without register defaults being
> provided since we need to make sure we allocate the rbtree nodes outside
> of the spinlock we use to lock MMIO access - was this tested with
> mainline?

Re: [PATCH] audio: sai: Add Power Management support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 633 bytes --]

On Thu, Oct 30, 2014 at 03:30:40AM +0000, Li.Xiubo@freescale.com wrote:

> One more thing, if the device is not performance critical, then shouldn't we
> Take care of the cache memory consumption to determine using flat or rbtree
> Type ?

Yes, it's always fine to use a rbtree if it makes sense - it was just an
unusual choice for a device like this that didn't seem to be discussed.

Depending on the register map a flat cache can actually be more memory
efficient sometimes since there's some overhead for the rbtree data
structures, if you've just got one block of registers with no gaps a
flat cache is going to be a win there.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH] audio: sai: Add Power Management support

[Mark Brown]

On Thu, Oct 30, 2014 at 03:30:40AM +0000, Li.Xiubo at freescale.com wrote:

> One more thing, if the device is not performance critical, then shouldn't we
> Take care of the cache memory consumption to determine using flat or rbtree
> Type ?

Yes, it's always fine to use a rbtree if it makes sense - it was just an
unusual choice for a device like this that didn't seem to be discussed.

Depending on the register map a flat cache can actually be more memory
efficient sometimes since there's some overhead for the rbtree data
structures, if you've just got one block of registers with no gaps a
flat cache is going to be a win there.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141030/56a2912c/attachment.sig>

Re: [PATCH] audio: sai: Add Power Management support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

On Thu, Oct 30, 2014 at 03:30:40AM +0000, Li.Xiubo@freescale.com wrote:

> One more thing, if the device is not performance critical, then shouldn't we
> Take care of the cache memory consumption to determine using flat or rbtree
> Type ?

Yes, it's always fine to use a rbtree if it makes sense - it was just an
unusual choice for a device like this that didn't seem to be discussed.

Depending on the register map a flat cache can actually be more memory
efficient sometimes since there's some overhead for the rbtree data
structures, if you've just got one block of registers with no gaps a
flat cache is going to be a win there.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] audio: sai: Add Power Management support

[Huan Wang]

Hi,

On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> This patch adds Power Management support for SAI.
> Activate regmap cache with REGCACHE_RBTREE, and use

Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
devices the cost tradeoff for the rbtree is usually higher than people
like so flat caches are preferred.  But if it works for you that's fine,
this shouldn't be *that* performance critical.

I'm also a bit surprised that this works without register defaults being
provided since we need to make sure we allocate the rbtree nodes outside
of the spinlock we use to lock MMIO access - was this tested with
mainline?

[Alison Wang] I tested rbtree and flat caches, they both work. But I didn't pay attention to the cost tradeoff and register defaults before, so I think flat caches are preferred now. 

Thanks.

Best Regards,
Alison Wang

[PATCH] audio: sai: Add Power Management support

[Huan Wang]

Hi,

On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> This patch adds Power Management support for SAI.
> Activate regmap cache with REGCACHE_RBTREE, and use

Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
devices the cost tradeoff for the rbtree is usually higher than people
like so flat caches are preferred.  But if it works for you that's fine,
this shouldn't be *that* performance critical.

I'm also a bit surprised that this works without register defaults being
provided since we need to make sure we allocate the rbtree nodes outside
of the spinlock we use to lock MMIO access - was this tested with
mainline?

[Alison Wang] I tested rbtree and flat caches, they both work. But I didn't pay attention to the cost tradeoff and register defaults before, so I think flat caches are preferred now. 

Thanks.

Best Regards,
Alison Wang

Re: [PATCH] audio: sai: Add Power Management support

[Huan Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1037 bytes --]

Hi,

On Wed, Oct 29, 2014 at 11:21:36AM +0800, Alison Wang wrote:
> This patch adds Power Management support for SAI.
> Activate regmap cache with REGCACHE_RBTREE, and use

Are you sure that REGCACHE_RBTREE is the best option here?  For MMIO
devices the cost tradeoff for the rbtree is usually higher than people
like so flat caches are preferred.  But if it works for you that's fine,
this shouldn't be *that* performance critical.

I'm also a bit surprised that this works without register defaults being
provided since we need to make sure we allocate the rbtree nodes outside
of the spinlock we use to lock MMIO access - was this tested with
mainline?

[Alison Wang] I tested rbtree and flat caches, they both work. But I didn't pay attention to the cost tradeoff and register defaults before, so I think flat caches are preferred now. 

Thanks.

Best Regards,
Alison Wang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] audio: sai: Add Power Management support

[Huan Wang]

Hi, Fabio,

Please always add the driver maintainers (Xiubo Li and Nicolin Chen).
[Alison Wang] ok, thanks for your reminder.

On Wed, Oct 29, 2014 at 1:21 AM, Alison Wang <b18965@freescale.com> wrote:

> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_sai_suspend(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(sai->regmap, true);
> +       regcache_mark_dirty(sai->regmap);
> +
> +       return 0;
> +}
> +
> +static int fsl_sai_resume(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       /* Restore all registers */
> +       regcache_cache_only(sai->regmap, false);
> +       regcache_sync(sai->regmap);
> +
> +       return 0;
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct dev_pm_ops fsl_sai_pm = {
> +       SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)

This could be simplified to:
static SIMPLE_DEV_PM_OPS(fsl_sai_pm, fsl_sai_suspend, fsl_sai_resume);
[Alison Wang] ok.

I am also curious as to how you tested it, as I noticed that
suspend/resume is broken on 3.18-rc for mx6sx.

Are you able to do suspend/resume on 3.18-rc on a mx6sx sdb board?
[Alison Wang] No, I don't have that board. I tested on LS1021A QDS board which supports deep sleep.

Best Regards,
Alison Wang

[PATCH] audio: sai: Add Power Management support

[Huan Wang]

Hi, Fabio,

Please always add the driver maintainers (Xiubo Li and Nicolin Chen).
[Alison Wang] ok, thanks for your reminder.

On Wed, Oct 29, 2014 at 1:21 AM, Alison Wang <b18965@freescale.com> wrote:

> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_sai_suspend(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(sai->regmap, true);
> +       regcache_mark_dirty(sai->regmap);
> +
> +       return 0;
> +}
> +
> +static int fsl_sai_resume(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       /* Restore all registers */
> +       regcache_cache_only(sai->regmap, false);
> +       regcache_sync(sai->regmap);
> +
> +       return 0;
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct dev_pm_ops fsl_sai_pm = {
> +       SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)

This could be simplified to:
static SIMPLE_DEV_PM_OPS(fsl_sai_pm, fsl_sai_suspend, fsl_sai_resume);
[Alison Wang] ok.

I am also curious as to how you tested it, as I noticed that
suspend/resume is broken on 3.18-rc for mx6sx.

Are you able to do suspend/resume on 3.18-rc on a mx6sx sdb board?
[Alison Wang] No, I don't have that board. I tested on LS1021A QDS board which supports deep sleep.

Best Regards,
Alison Wang

Re: [PATCH] audio: sai: Add Power Management support

[Huan Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1472 bytes --]

Hi, Fabio,

Please always add the driver maintainers (Xiubo Li and Nicolin Chen).
[Alison Wang] ok, thanks for your reminder.

On Wed, Oct 29, 2014 at 1:21 AM, Alison Wang <b18965@freescale.com> wrote:

> +#ifdef CONFIG_PM_SLEEP
> +static int fsl_sai_suspend(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(sai->regmap, true);
> +       regcache_mark_dirty(sai->regmap);
> +
> +       return 0;
> +}
> +
> +static int fsl_sai_resume(struct device *dev)
> +{
> +       struct fsl_sai *sai = dev_get_drvdata(dev);
> +
> +       /* Restore all registers */
> +       regcache_cache_only(sai->regmap, false);
> +       regcache_sync(sai->regmap);
> +
> +       return 0;
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct dev_pm_ops fsl_sai_pm = {
> +       SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)

This could be simplified to:
static SIMPLE_DEV_PM_OPS(fsl_sai_pm, fsl_sai_suspend, fsl_sai_resume);
[Alison Wang] ok.

I am also curious as to how you tested it, as I noticed that
suspend/resume is broken on 3.18-rc for mx6sx.

Are you able to do suspend/resume on 3.18-rc on a mx6sx sdb board?
[Alison Wang] No, I don't have that board. I tested on LS1021A QDS board which supports deep sleep.

Best Regards,
Alison Wangÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] audio: sai: Add Power Management support

[Li.Xiubo]

Hi Mark,



> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Thursday, October 30, 2014 7:23 PM
> To: Xiubo Li-B47053
> Cc: Wang Huan-B18965; alsa-devel@alsa-project.org; tiwai@suse.de; linux-
> kernel@vger.kernel.org; lgirdwood@gmail.com; perex@perex.cz; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH] audio: sai: Add Power Management support
> 
> On Thu, Oct 30, 2014 at 03:30:40AM +0000, Li.Xiubo@freescale.com wrote:
> 
> > One more thing, if the device is not performance critical, then shouldn't we
> > Take care of the cache memory consumption to determine using flat or rbtree
> > Type ?
> 
> Yes, it's always fine to use a rbtree if it makes sense - it was just an
> unusual choice for a device like this that didn't seem to be discussed.
> 
> Depending on the register map a flat cache can actually be more memory
> efficient sometimes since there's some overhead for the rbtree data
> structures, if you've just got one block of registers with no gaps a
> flat cache is going to be a win there.

Yes, so my comprehension of it was right.

Thanks very much,

BRs
Xiubo