All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipmi: add retry in try_get_dev_id()
@ 2020-09-16  6:21 Xianting Tian
  2020-09-16 14:01 ` Corey Minyard
  0 siblings, 1 reply; 3+ messages in thread
From: Xianting Tian @ 2020-09-16  6:21 UTC (permalink / raw)
  To: minyard, arnd, gregkh; +Cc: openipmi-developer, linux-kernel, Xianting Tian

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 <tian.xianting@h3c.com>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] ipmi: add retry in try_get_dev_id()
  2020-09-16  6:21 [PATCH] ipmi: add retry in try_get_dev_id() Xianting Tian
@ 2020-09-16 14:01 ` Corey Minyard
  2020-09-16 15:07   ` Tianxianting
  0 siblings, 1 reply; 3+ messages in thread
From: Corey Minyard @ 2020-09-16 14:01 UTC (permalink / raw)
  To: Xianting Tian; +Cc: arnd, gregkh, openipmi-developer, linux-kernel

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 <tian.xianting@h3c.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH] ipmi: add retry in try_get_dev_id()
  2020-09-16 14:01 ` Corey Minyard
@ 2020-09-16 15:07   ` Tianxianting
  0 siblings, 0 replies; 3+ messages in thread
From: Tianxianting @ 2020-09-16 15:07 UTC (permalink / raw)
  To: minyard; +Cc: arnd, gregkh, openipmi-developer, linux-kernel

Thanks you Corey for your kindly guides to me for these three patches :)

-----Original Message-----
From: Corey Minyard [mailto:tcminyard@gmail.com] On Behalf Of Corey Minyard
Sent: Wednesday, September 16, 2020 10:01 PM
To: tianxianting (RD) <tian.xianting@h3c.com>
Cc: arnd@arndb.de; gregkh@linuxfoundation.org; openipmi-developer@lists.sourceforge.net; linux-kernel@vger.kernel.org
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 <tian.xianting@h3c.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-16 20:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16  6:21 [PATCH] ipmi: add retry in try_get_dev_id() Xianting Tian
2020-09-16 14:01 ` Corey Minyard
2020-09-16 15:07   ` Tianxianting

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.