Skip to contentSkip to conversation

powerpc/qe: Make qe_reset() code path safe for repeated invocation

PATCH Applied Anton Vorontsov 2 participants 3 messages Aug 27, 2009 – Aug 28, 2009 PowerPC

lore
Share & tools
lore

Anton Vorontsov

author
#0 lore raw

PATCH 2/5 powerpc/qe: Make qe_reset() code path safe for repeated invocation

For MPC8569 CPUs we'll need to reset QE after each suspend, so make qe_reset() code path suitable for repeated invocation, that is:

- Don't initialize rheap structures if already initialized;
- Don't allocate muram for SDMA if already allocated, just reinitialize registers with previously allocated muram offset;

Signed-off-by Anton Vorontsov <[email protected]>
---
 arch/powerpc/sysdev/cpm_common.c |    3 +++
 arch/powerpc/sysdev/qe_lib/qe.c  |   10 ++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)
arch/powerpc/sysdev/cpm_common.c +3 -0
index e4b6d66..ddfca52 100644
@@ -81,6 +81,9 @@ int __init cpm_muram_init(void)
 	int i = 0;
 	int ret = 0;
 
+	if (muram_pbase)
+		return 0;
+
 	spin_lock_init(&cpm_muram_lock);
 	/* initialize the info header */
 	rh_init(&cpm_muram_info, 1,
arch/powerpc/sysdev/qe_lib/qe.c +6 -4
index b06564f..f485d5a 100644
@@ -317,16 +317,18 @@ EXPORT_SYMBOL(qe_put_snum);
 static int qe_sdma_init(void)
 {
 	struct sdma __iomem *sdma = &qe_immr->sdma;
-	unsigned long sdma_buf_offset;
+	static unsigned long sdma_buf_offset;
 
 	if (!sdma)
 		return -ENODEV;
 
 	/* allocate 2 internal temporary buffers (512 bytes size each) for
 	 * the SDMA */
- 	sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
-	if (IS_ERR_VALUE(sdma_buf_offset))
-		return -ENOMEM;
+	if (!sdma_buf_offset) {
+		sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
+		if (IS_ERR_VALUE(sdma_buf_offset))
+			return -ENOMEM;
+	}
 
 	out_be32(&sdma->sdebcr, (u32) sdma_buf_offset & QE_SDEBCR_BA_MASK);
  	out_be32(&sdma->sdmr, (QE_SDMR_GLB_1_MSK |
Signature · 1 line
1.6.3.3
#1 lore raw
Quoted 41 lines · Anton Vorontsov

For MPC8569 CPUs we'll need to reset QE after each suspend, so make qe_reset() code path suitable for repeated invocation, that is:

- Don't initialize rheap structures if already initialized;
- Don't allocate muram for SDMA if already allocated, just reinitialize
registers with previously allocated muram offset;

Signed-off-by Anton Vorontsov <[email protected]>

---
arch/powerpc/sysdev/cpm_common.c | 3 +++
arch/powerpc/sysdev/qe_lib/qe.c | 10 ++++++----

2 files changed, 9 insertions(+), 4 deletions(-)
arch/powerpc/sysdev/cpm_common.c +3 -0
index e4b6d66..ddfca52 100644
@@ -81,6 +81,9 @@ int __init cpm_muram_init(void)
 	int i = 0;
 	int ret = 0;
 
+	if (muram_pbase)
+		return 0;
+
 	spin_lock_init(&cpm_muram_lock);
 	/* initialize the info header */
 	rh_init(&cpm_muram_info, 1,
arch/powerpc/sysdev/qe_lib/qe.c +1 -1
index b06564f..f485d5a 100644
@@ -317,16 +317,18 @@ EXPORT_SYMBOL(qe_put_snum);
 static int qe_sdma_init(void)
 {
 	struct sdma __iomem *sdma = &qe_immr->sdma;
-	unsigned long sdma_buf_offset;
+	static unsigned long sdma_buf_offset;
 
 	if (!sdma)

arch/powerpc/sysdev/qe_lib/qe.c EXPORT_SYMBOL(qe_put_snum); · line 329

Quoted 10 lines
arch/powerpc/sysdev/qe_lib/qe.c +3 -3
 		return -ENODEV;
 
 	/* allocate 2 internal temporary buffers (512 bytes size each) for
 	 * the SDMA */
- 	sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
-	if (IS_ERR_VALUE(sdma_buf_offset))
-		return -ENOMEM;
+	if (!sdma_buf_offset) {
+		sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
+		if (IS_ERR_VALUE(sdma_buf_offset))

shouldn't we zero out sdma_buf_offset otherwise if we call this again we'll think its set.

Quoted 12 lines
arch/powerpc/sysdev/qe_lib/qe.c +2 -0
+			return -ENOMEM;
+	}
 
 	out_be32(&sdma->sdebcr, (u32) sdma_buf_offset & QE_SDEBCR_BA_MASK);
 	out_be32(&sdma->sdmr, (QE_SDMR_GLB_1_MSK |
Signature · 6 lines
1.6.3.3

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
author
#2 lore raw

On Fri, Aug 28, 2009 at 12:34:50AM -0500, Kumar Gala wrote:
[...]

Quoted 9 lines
Quoted 9 lines

static int qe_sdma_init(void)

{
	struct sdma __iomem *sdma = &qe_immr->sdma;
arch/powerpc/sysdev/qe_lib/qe.c +1 -1
-	unsigned long sdma_buf_offset;
+	static unsigned long sdma_buf_offset;

	if (!sdma)
		return -ENODEV;
Quoted 11 lines

arch/powerpc/sysdev/qe_lib/qe.c EXPORT_SYMBOL(qe_put_snum); · line 329

Quoted 8 lines
	/* allocate 2 internal temporary buffers (512 bytes size each) for
	 * the SDMA */
arch/powerpc/sysdev/qe_lib/qe.c +3 -3
- 	sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
-	if (IS_ERR_VALUE(sdma_buf_offset))
-		return -ENOMEM;
+	if (!sdma_buf_offset) {
+		sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
+		if (IS_ERR_VALUE(sdma_buf_offset))

shouldn't we zero out sdma_buf_offset otherwise if we call this again we'll think its set.

Technically, no. If qe_sdma_init() fails, kernel will panic:

void __init qe_reset(void)

{
        ...
        if (qe_sdma_init())
                panic("sdma init failed!");
}

But I see your point, it isn't obvious and may lead to a bug if we'll decide to not panic later on. Therefore I'd better make the change.

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2