[PATCH v2] soc: fsl: qe: Add check for ioremap

[PATCH v2] soc: fsl: qe: Add check for ioremap

[Jiasheng Jiang]

As ioremap can return NULL pointer, it should
be better to check the return value return error
if fails.
Moreover, the return value of qe_reset should be
checked by cascade.

Fixes: 68f047e3d62e ("fsl/qe: add rx_sync and tx_sync for TDM mode")
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
Changelog:

v1 -> v2:

1. Change the position of the check for ioremap.
2. Simplify the check for qe_reset.
3. Remove the 'extern' keyword.
---
 drivers/soc/fsl/qe/qe.c | 12 ++++++++----
 include/soc/fsl/qe/qe.h |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index b3c226eb5292..33f76cc5872e 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -83,11 +83,14 @@ static phys_addr_t get_qe_base(void)
 	return qebase;
 }
 
-void qe_reset(void)
+int qe_reset(void)
 {
 	if (qe_immr == NULL)
 		qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
 
+	if (qe_immr == NULL)
+		return -ENOMEM;
+
 	qe_snums_init();
 
 	qe_issue_cmd(QE_RESET, QE_CR_SUBBLOCK_INVALID,
@@ -98,6 +101,8 @@ void qe_reset(void)
 
 	if (qe_sdma_init())
 		panic("sdma init failed!");
+
+	return 0;
 }
 
 int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
@@ -644,9 +649,8 @@ static int __init qe_init(void)
 	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
 	if (!np)
 		return -ENODEV;
-	qe_reset();
 	of_node_put(np);
-	return 0;
+	return qe_reset();
 }
 subsys_initcall(qe_init);
 
@@ -654,7 +658,7 @@ subsys_initcall(qe_init);
 static int qe_resume(struct platform_device *ofdev)
 {
 	if (!qe_alive_during_sleep())
-		qe_reset();
+		return qe_reset();
 	return 0;
 }
 
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index b02e9fe69146..f6653f4b41df 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -84,9 +84,9 @@ extern spinlock_t cmxgcr_lock;
 
 /* Export QE common operations */
 #ifdef CONFIG_QUICC_ENGINE
-extern void qe_reset(void);
+int qe_reset(void);
 #else
-static inline void qe_reset(void) {}
+static inline int qe_reset(void) {}
 #endif
 
 int cpm_muram_init(void);
-- 
2.25.1

Re: [PATCH v2] soc: fsl: qe: Add check for ioremap

[Christophe Leroy]

Le 11/10/2022 à 08:32, Jiasheng Jiang a écrit :
> As ioremap can return NULL pointer, it should
> be better to check the return value return error
> if fails.
> Moreover, the return value of qe_reset should be
> checked by cascade.
> 
> Fixes: 68f047e3d62e ("fsl/qe: add rx_sync and tx_sync for TDM mode")
> Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Did you try a build without CONFIG_QUICC_ENGINE selected ?

Doesn't it fail loudly ?

Christophe

> ---
> Changelog:
> 
> v1 -> v2:
> 
> 1. Change the position of the check for ioremap.
> 2. Simplify the check for qe_reset.
> 3. Remove the 'extern' keyword.
> ---
>   drivers/soc/fsl/qe/qe.c | 12 ++++++++----
>   include/soc/fsl/qe/qe.h |  4 ++--
>   2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index b3c226eb5292..33f76cc5872e 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -83,11 +83,14 @@ static phys_addr_t get_qe_base(void)
>   	return qebase;
>   }
>   
> -void qe_reset(void)
> +int qe_reset(void)
>   {
>   	if (qe_immr == NULL)
>   		qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
>   
> +	if (qe_immr == NULL)
> +		return -ENOMEM;
> +
>   	qe_snums_init();
>   
>   	qe_issue_cmd(QE_RESET, QE_CR_SUBBLOCK_INVALID,
> @@ -98,6 +101,8 @@ void qe_reset(void)
>   
>   	if (qe_sdma_init())
>   		panic("sdma init failed!");
> +
> +	return 0;
>   }
>   
>   int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
> @@ -644,9 +649,8 @@ static int __init qe_init(void)
>   	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
>   	if (!np)
>   		return -ENODEV;
> -	qe_reset();
>   	of_node_put(np);
> -	return 0;
> +	return qe_reset();
>   }
>   subsys_initcall(qe_init);
>   
> @@ -654,7 +658,7 @@ subsys_initcall(qe_init);
>   static int qe_resume(struct platform_device *ofdev)
>   {
>   	if (!qe_alive_during_sleep())
> -		qe_reset();
> +		return qe_reset();
>   	return 0;
>   }
>   
> diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
> index b02e9fe69146..f6653f4b41df 100644
> --- a/include/soc/fsl/qe/qe.h
> +++ b/include/soc/fsl/qe/qe.h
> @@ -84,9 +84,9 @@ extern spinlock_t cmxgcr_lock;
>   
>   /* Export QE common operations */
>   #ifdef CONFIG_QUICC_ENGINE
> -extern void qe_reset(void);
> +int qe_reset(void);
>   #else
> -static inline void qe_reset(void) {}
> +static inline int qe_reset(void) {}
>   #endif
>   
>   int cpm_muram_init(void);