| Message ID | 20221011063222.43572-1-jiasheng@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] soc: fsl: qe: Add check for ioremap | expand |
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);
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);
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(-)