Re: [PATCH v3] soc: fsl: qe: Add check for ioremap
From: Christophe Leroy
Date: Sun Nov 06 2022 - 05:48:28 EST
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 <christophe.leroy@xxxxxxxxxx>
> Signed-off-by: Jiasheng Jiang <jiasheng@xxxxxxxxxxx>
Reviewed-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> ---
> 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);