Use retry machanism to give device more opportunitys to correctly response
kernel when we received specific completion codes.
This is similar to what we done in __get_device_id().
Signed-off-by: Xianting Tian <[email protected]>
---
drivers/char/ipmi/ipmi_msghandler.c | 2 --
drivers/char/ipmi/ipmi_si_intf.c | 17 +++++++++++++++++
include/uapi/linux/ipmi.h | 2 ++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index b9685093e..75cb7e062 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -62,8 +62,6 @@ enum ipmi_panic_event_op {
#define IPMI_PANIC_DEFAULT IPMI_SEND_PANIC_EVENT_NONE
#endif
-#define GET_DEVICE_ID_MAX_RETRY 5
-
static enum ipmi_panic_event_op ipmi_send_panic_event = IPMI_PANIC_DEFAULT;
static int panic_op_write_handler(const char *val,
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 77b8d551a..beeb705f1 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1316,6 +1316,7 @@ static int try_get_dev_id(struct smi_info *smi_info)
unsigned char *resp;
unsigned long resp_len;
int rv = 0;
+ unsigned int retry_count = 0;
resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
if (!resp)
@@ -1327,6 +1328,8 @@ static int try_get_dev_id(struct smi_info *smi_info)
*/
msg[0] = IPMI_NETFN_APP_REQUEST << 2;
msg[1] = IPMI_GET_DEVICE_ID_CMD;
+
+retry:
smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2);
rv = wait_for_msg_done(smi_info);
@@ -1339,6 +1342,20 @@ static int try_get_dev_id(struct smi_info *smi_info)
/* Check and record info from the get device id, in case we need it. */
rv = ipmi_demangle_device_id(resp[0] >> 2, resp[1],
resp + 2, resp_len - 2, &smi_info->device_id);
+ if (rv) {
+ /* record completion code */
+ char cc = *(resp + 2);
+
+ if ((cc == IPMI_DEVICE_IN_FW_UPDATE_ERR
+ || cc == IPMI_DEVICE_IN_INIT_ERR
+ || cc == IPMI_NOT_IN_MY_STATE_ERR)
+ && ++retry_count <= GET_DEVICE_ID_MAX_RETRY) {
+ dev_warn(smi_info->io.dev,
+ "retry to get device id as completion code 0x%x\n",
+ cc);
+ goto retry;
+ }
+ }
out:
kfree(resp);
diff --git a/include/uapi/linux/ipmi.h b/include/uapi/linux/ipmi.h
index 32d148309..bc57f07e3 100644
--- a/include/uapi/linux/ipmi.h
+++ b/include/uapi/linux/ipmi.h
@@ -426,4 +426,6 @@ struct ipmi_timing_parms {
#define IPMICTL_GET_MAINTENANCE_MODE_CMD _IOR(IPMI_IOC_MAGIC, 30, int)
#define IPMICTL_SET_MAINTENANCE_MODE_CMD _IOW(IPMI_IOC_MAGIC, 31, int)
+#define GET_DEVICE_ID_MAX_RETRY 5
+
#endif /* _UAPI__LINUX_IPMI_H */
--
2.17.1
Thanks you Corey for your kindly guides to me for these three patches :)
-----Original Message-----
From: Corey Minyard [mailto:[email protected]] On Behalf Of Corey Minyard
Sent: Wednesday, September 16, 2020 10:01 PM
To: tianxianting (RD) <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ipmi: add retry in try_get_dev_id()
On Wed, Sep 16, 2020 at 02:21:29PM +0800, Xianting Tian wrote:
> Use retry machanism to give device more opportunitys to correctly
> response kernel when we received specific completion codes.
>
> This is similar to what we done in __get_device_id().
Thanks. I moved GET_DEVICE_ID_MAX_RETRY to include/linux/ipmi.h since uapi is for things exported to userspace. But this is good, it's in my next tree.
-corey
>
> Signed-off-by: Xianting Tian <[email protected]>
> ---
> drivers/char/ipmi/ipmi_msghandler.c | 2 --
> drivers/char/ipmi/ipmi_si_intf.c | 17 +++++++++++++++++
> include/uapi/linux/ipmi.h | 2 ++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c
> b/drivers/char/ipmi/ipmi_msghandler.c
> index b9685093e..75cb7e062 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -62,8 +62,6 @@ enum ipmi_panic_event_op { #define
> IPMI_PANIC_DEFAULT IPMI_SEND_PANIC_EVENT_NONE #endif
>
> -#define GET_DEVICE_ID_MAX_RETRY 5
> -
> static enum ipmi_panic_event_op ipmi_send_panic_event =
> IPMI_PANIC_DEFAULT;
>
> static int panic_op_write_handler(const char *val, diff --git
> a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 77b8d551a..beeb705f1 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1316,6 +1316,7 @@ static int try_get_dev_id(struct smi_info *smi_info)
> unsigned char *resp;
> unsigned long resp_len;
> int rv = 0;
> + unsigned int retry_count = 0;
>
> resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
> if (!resp)
> @@ -1327,6 +1328,8 @@ static int try_get_dev_id(struct smi_info *smi_info)
> */
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_GET_DEVICE_ID_CMD;
> +
> +retry:
> smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2);
>
> rv = wait_for_msg_done(smi_info);
> @@ -1339,6 +1342,20 @@ static int try_get_dev_id(struct smi_info *smi_info)
> /* Check and record info from the get device id, in case we need it. */
> rv = ipmi_demangle_device_id(resp[0] >> 2, resp[1],
> resp + 2, resp_len - 2, &smi_info->device_id);
> + if (rv) {
> + /* record completion code */
> + char cc = *(resp + 2);
> +
> + if ((cc == IPMI_DEVICE_IN_FW_UPDATE_ERR
> + || cc == IPMI_DEVICE_IN_INIT_ERR
> + || cc == IPMI_NOT_IN_MY_STATE_ERR)
> + && ++retry_count <= GET_DEVICE_ID_MAX_RETRY) {
> + dev_warn(smi_info->io.dev,
> + "retry to get device id as completion code 0x%x\n",
> + cc);
> + goto retry;
> + }
> + }
>
> out:
> kfree(resp);
> diff --git a/include/uapi/linux/ipmi.h b/include/uapi/linux/ipmi.h
> index 32d148309..bc57f07e3 100644
> --- a/include/uapi/linux/ipmi.h
> +++ b/include/uapi/linux/ipmi.h
> @@ -426,4 +426,6 @@ struct ipmi_timing_parms {
> #define IPMICTL_GET_MAINTENANCE_MODE_CMD _IOR(IPMI_IOC_MAGIC, 30, int)
> #define IPMICTL_SET_MAINTENANCE_MODE_CMD _IOW(IPMI_IOC_MAGIC, 31, int)
>
> +#define GET_DEVICE_ID_MAX_RETRY 5
> +
> #endif /* _UAPI__LINUX_IPMI_H */
> --
> 2.17.1
>
On Wed, Sep 16, 2020 at 02:21:29PM +0800, Xianting Tian wrote:
> Use retry machanism to give device more opportunitys to correctly response
> kernel when we received specific completion codes.
>
> This is similar to what we done in __get_device_id().
Thanks. I moved GET_DEVICE_ID_MAX_RETRY to include/linux/ipmi.h since
uapi is for things exported to userspace. But this is good, it's in my
next tree.
-corey
>
> Signed-off-by: Xianting Tian <[email protected]>
> ---
> drivers/char/ipmi/ipmi_msghandler.c | 2 --
> drivers/char/ipmi/ipmi_si_intf.c | 17 +++++++++++++++++
> include/uapi/linux/ipmi.h | 2 ++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index b9685093e..75cb7e062 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -62,8 +62,6 @@ enum ipmi_panic_event_op {
> #define IPMI_PANIC_DEFAULT IPMI_SEND_PANIC_EVENT_NONE
> #endif
>
> -#define GET_DEVICE_ID_MAX_RETRY 5
> -
> static enum ipmi_panic_event_op ipmi_send_panic_event = IPMI_PANIC_DEFAULT;
>
> static int panic_op_write_handler(const char *val,
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 77b8d551a..beeb705f1 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1316,6 +1316,7 @@ static int try_get_dev_id(struct smi_info *smi_info)
> unsigned char *resp;
> unsigned long resp_len;
> int rv = 0;
> + unsigned int retry_count = 0;
>
> resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
> if (!resp)
> @@ -1327,6 +1328,8 @@ static int try_get_dev_id(struct smi_info *smi_info)
> */
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_GET_DEVICE_ID_CMD;
> +
> +retry:
> smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2);
>
> rv = wait_for_msg_done(smi_info);
> @@ -1339,6 +1342,20 @@ static int try_get_dev_id(struct smi_info *smi_info)
> /* Check and record info from the get device id, in case we need it. */
> rv = ipmi_demangle_device_id(resp[0] >> 2, resp[1],
> resp + 2, resp_len - 2, &smi_info->device_id);
> + if (rv) {
> + /* record completion code */
> + char cc = *(resp + 2);
> +
> + if ((cc == IPMI_DEVICE_IN_FW_UPDATE_ERR
> + || cc == IPMI_DEVICE_IN_INIT_ERR
> + || cc == IPMI_NOT_IN_MY_STATE_ERR)
> + && ++retry_count <= GET_DEVICE_ID_MAX_RETRY) {
> + dev_warn(smi_info->io.dev,
> + "retry to get device id as completion code 0x%x\n",
> + cc);
> + goto retry;
> + }
> + }
>
> out:
> kfree(resp);
> diff --git a/include/uapi/linux/ipmi.h b/include/uapi/linux/ipmi.h
> index 32d148309..bc57f07e3 100644
> --- a/include/uapi/linux/ipmi.h
> +++ b/include/uapi/linux/ipmi.h
> @@ -426,4 +426,6 @@ struct ipmi_timing_parms {
> #define IPMICTL_GET_MAINTENANCE_MODE_CMD _IOR(IPMI_IOC_MAGIC, 30, int)
> #define IPMICTL_SET_MAINTENANCE_MODE_CMD _IOW(IPMI_IOC_MAGIC, 31, int)
>
> +#define GET_DEVICE_ID_MAX_RETRY 5
> +
> #endif /* _UAPI__LINUX_IPMI_H */
> --
> 2.17.1
>