2022-10-11 07:33:43

by Jiasheng Jiang

Subject: [PATCH v3] soc: fsl: qe: Add check for ioremap

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 <[email protected]>
Signed-off-by: Jiasheng Jiang <[email protected]>
---
Changelog:

v2 -> v3:

1. Add return statement of qe_reset when CONFIG_QUICC_ENGINE is not selected.

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 | 7 +++++--
2 files changed, 13 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..0714b0ba7f49 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -84,9 +84,12 @@ 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)
+{
+ return 0;
+}
#endif

int cpm_muram_init(void);
--
2.25.1

2022-11-06 11:23:05

by Christophe Leroy

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

Le 11/10/2022 à 08:52, 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 <[email protected]>
> Signed-off-by: Jiasheng Jiang <[email protected]>

Reviewed-by: Christophe Leroy <[email protected]>

> ---
> Changelog:
>
> v2 -> v3:
>
> 1. Add return statement of qe_reset when CONFIG_QUICC_ENGINE is not selected.
>
> 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 | 7 +++++--
> 2 files changed, 13 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..0714b0ba7f49 100644
> --- a/include/soc/fsl/qe/qe.h
> +++ b/include/soc/fsl/qe/qe.h
> @@ -84,9 +84,12 @@ 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)
> +{
> + return 0;
> +}
> #endif
>
> int cpm_muram_init(void);