* fix IPMI I/O error on NEC FC storage array
@ 2012-05-30 14:21 Naresh Bhat
2012-06-27 0:19 ` Andrew Morton
0 siblings, 1 reply; 2+ messages in thread
From: Naresh Bhat @ 2012-05-30 14:21 UTC (permalink / raw)
To: openipmi-developer, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]
Hi,
The code changes will fix the IPMI I/O error on NEC FC storage array.
===========================================================================================
Signed-off-by: Naresh Bhat <nbhat@mvista.com>
--- linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c.orig 2012-05-31
06:11:54.753480863 -0700
+++ linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c 2012-05-31
06:21:26.780977397 -0700
@@ -344,6 +344,24 @@ static int get_kcs_result(struct si_sm_d
*/
static enum si_sm_result kcs_event(struct si_sm_data *kcs, long time)
{
+#define READ_STATUS_MAX_LOOP 4
+#define SI_SM_WAIT_STATE(_state_) \
+ do { \
+ int i; \
+ for (i = 0; (i < READ_STATUS_MAX_LOOP) \
+ && (state != _state_); i++) { \
+ state = GET_STATUS_STATE(read_status(kcs)); \
+ } \
+ } while (0);
+
+#define SI_SM_WAIT_STATE2(_state_, _state2_) \
+ do { \
+ int i; \
+ for (i = 0; (i < READ_STATUS_MAX_LOOP) && \
+ (state != _state_) && (state != _state2_); i++) { \
+ state = GET_STATUS_STATE(read_status(kcs)); \
+ } \
+ } while (0);
unsigned char status;
unsigned char state;
@@ -370,6 +388,7 @@ static enum si_sm_result kcs_event(struc
return SI_SM_IDLE;
case KCS_START_OP:
+ SI_SM_WAIT_STATE(KCS_IDLE_STATE);
if (state != KCS_IDLE) {
start_error_recovery(kcs,
"State machine not idle at start");
@@ -382,6 +401,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_START:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(
kcs,
@@ -399,6 +419,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state for write");
@@ -414,6 +435,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_END:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state"
@@ -426,6 +448,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_READ:
+ SI_SM_WAIT_STATE2(KCS_READ_STATE, KCS_IDLE_STATE);
if ((state != KCS_READ_STATE) && (state != KCS_IDLE_STATE)) {
start_error_recovery(
kcs,
@@ -472,6 +495,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR2:
+ SI_SM_WAIT_STATE(KCS_READ_STATE);
if (state != KCS_READ_STATE) {
start_error_recovery(kcs,
"Not in read state for error2");
@@ -486,6 +510,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR3:
+ SI_SM_WAIT_STATE(KCS_IDLE_STATE);
if (state != KCS_IDLE_STATE) {
start_error_recovery(kcs,
"Not in idle state for error3");
===========================================================================================
Thanks
-Naresh Bhat
[-- Attachment #2: Fix-IPMI-I-O-error-on-NEC-FC-storage-array.patch --]
[-- Type: application/octet-stream, Size: 2563 bytes --]
Signed-off-by: Naresh Bhat <nbhat@mvista.com>
--- linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c.orig 2012-05-31 06:11:54.753480863 -0700
+++ linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c 2012-05-31 06:21:26.780977397 -0700
@@ -344,6 +344,24 @@ static int get_kcs_result(struct si_sm_d
*/
static enum si_sm_result kcs_event(struct si_sm_data *kcs, long time)
{
+#define READ_STATUS_MAX_LOOP 4
+#define SI_SM_WAIT_STATE(_state_) \
+ do { \
+ int i; \
+ for (i = 0; (i < READ_STATUS_MAX_LOOP) \
+ && (state != _state_); i++) { \
+ state = GET_STATUS_STATE(read_status(kcs)); \
+ } \
+ } while (0);
+
+#define SI_SM_WAIT_STATE2(_state_, _state2_) \
+ do { \
+ int i; \
+ for (i = 0; (i < READ_STATUS_MAX_LOOP) && \
+ (state != _state_) && (state != _state2_); i++) { \
+ state = GET_STATUS_STATE(read_status(kcs)); \
+ } \
+ } while (0);
unsigned char status;
unsigned char state;
@@ -370,6 +388,7 @@ static enum si_sm_result kcs_event(struc
return SI_SM_IDLE;
case KCS_START_OP:
+ SI_SM_WAIT_STATE(KCS_IDLE_STATE);
if (state != KCS_IDLE) {
start_error_recovery(kcs,
"State machine not idle at start");
@@ -382,6 +401,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_START:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(
kcs,
@@ -399,6 +419,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state for write");
@@ -414,6 +435,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_END:
+ SI_SM_WAIT_STATE(KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state"
@@ -426,6 +448,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_READ:
+ SI_SM_WAIT_STATE2(KCS_READ_STATE, KCS_IDLE_STATE);
if ((state != KCS_READ_STATE) && (state != KCS_IDLE_STATE)) {
start_error_recovery(
kcs,
@@ -472,6 +495,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR2:
+ SI_SM_WAIT_STATE(KCS_READ_STATE);
if (state != KCS_READ_STATE) {
start_error_recovery(kcs,
"Not in read state for error2");
@@ -486,6 +510,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR3:
+ SI_SM_WAIT_STATE(KCS_IDLE_STATE);
if (state != KCS_IDLE_STATE) {
start_error_recovery(kcs,
"Not in idle state for error3");
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: fix IPMI I/O error on NEC FC storage array
2012-05-30 14:21 fix IPMI I/O error on NEC FC storage array Naresh Bhat
@ 2012-06-27 0:19 ` Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2012-06-27 0:19 UTC (permalink / raw)
To: Naresh Bhat; +Cc: openipmi-developer, linux-kernel
On Wed, 30 May 2012 07:21:17 -0700
Naresh Bhat <nbhat@mvista.com> wrote:
> Hi,
>
> The code changes will fix the IPMI I/O error on NEC FC storage array.
When fixing a bug, please fully describe that bug. This helps people
to work out which kernel version(s) should be patched and helps others
work out whether your patch will fix a problem which they or their
customers are seeing.
> Signed-off-by: Naresh Bhat <nbhat@mvista.com>
> --- linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c.orig 2012-05-31
> 06:11:54.753480863 -0700
> +++ linux-2.6.32.59/drivers/char/ipmi/ipmi_kcs_sm.c 2012-05-31
> 06:21:26.780977397 -0700
Your email client is wordwrapping the patches.
> @@ -344,6 +344,24 @@ static int get_kcs_result(struct si_sm_d
> */
> static enum si_sm_result kcs_event(struct si_sm_data *kcs, long time)
> {
> +#define READ_STATUS_MAX_LOOP 4
> +#define SI_SM_WAIT_STATE(_state_) \
> + do { \
> + int i; \
> + for (i = 0; (i < READ_STATUS_MAX_LOOP) \
> + && (state != _state_); i++) { \
> + state = GET_STATUS_STATE(read_status(kcs)); \
> + } \
> + } while (0);
> +
> +#define SI_SM_WAIT_STATE2(_state_, _state2_) \
> + do { \
> + int i; \
> + for (i = 0; (i < READ_STATUS_MAX_LOOP) && \
> + (state != _state_) && (state != _state2_); i++) { \
> + state = GET_STATUS_STATE(read_status(kcs)); \
> + } \
> + } while (0);
> unsigned char status;
> unsigned char state;
>
> @@ -370,6 +388,7 @@ static enum si_sm_result kcs_event(struc
> return SI_SM_IDLE;
>
> case KCS_START_OP:
> + SI_SM_WAIT_STATE(KCS_IDLE_STATE);
This is really ugly :(
Please convert the above two macros into regular C functions. Please
ensure that those functions are suitably commented. Something like
state = si_sm_wait_state(kcs, KCS_IDLE_STATE);
note that we don't pass `state' into si_sm_wait_state(). let's remove
the "Just about everything looks at the KCS stat" assignment and do the
read within those switch cases which actually need it.
Here, like this:
--- a/drivers/char/ipmi/ipmi_kcs_sm.c~fix-ipmi-i-o-error-on-nec-fc-storage-array
+++ a/drivers/char/ipmi/ipmi_kcs_sm.c
@@ -337,6 +337,40 @@ static int get_kcs_result(struct si_sm_d
return kcs->read_pos;
}
+#define READ_STATUS_MAX_LOOP 4
+
+/*
+ * comment goes here
+ */
+static int si_sm_wait_state(struct si_sm_data *kcs, int state)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < READ_STATUS_MAX_LOOP; i++) {
+ ret = GET_STATUS_STATE(read_status(kcs));
+ if (ret == state)
+ break;
+ }
+ return ret;
+}
+
+/*
+ * comment goes here
+ */
+static int si_sm_wait_state2(struct si_sm_data *kcs, int state1, int state2)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < READ_STATUS_MAX_LOOP; i++) {
+ ret = GET_STATUS_STATE(read_status(kcs));
+ if (ret == state1 || ret == state2)
+ break;
+ }
+ return ret;
+}
+
/*
* This implements the state machine defined in the IPMI manual, see
* that for details on how this works. Divide that flowchart into
@@ -356,9 +390,6 @@ static enum si_sm_result kcs_event(struc
if (!check_ibf(kcs, status, time))
return SI_SM_CALL_WITH_DELAY;
- /* Just about everything looks at the KCS state, so grab that, too. */
- state = GET_STATUS_STATE(status);
-
switch (kcs->state) {
case KCS_IDLE:
/* If there's and interrupt source, turn it off. */
@@ -370,6 +401,7 @@ static enum si_sm_result kcs_event(struc
return SI_SM_IDLE;
case KCS_START_OP:
+ state = si_sm_wait_state(kcs, KCS_IDLE_STATE);
if (state != KCS_IDLE_STATE) {
start_error_recovery(kcs,
"State machine not idle at start");
@@ -382,6 +414,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_START:
+ state = si_sm_wait_state(kcs, KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(
kcs,
@@ -399,6 +432,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE:
+ state = si_sm_wait_state(kcs, KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state for write");
@@ -414,6 +448,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_WRITE_END:
+ state = si_sm_wait_state(kcs, KCS_WRITE_STATE);
if (state != KCS_WRITE_STATE) {
start_error_recovery(kcs,
"Not in write state"
@@ -426,6 +461,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_WAIT_READ:
+ state = si_sm_wait_state2(kcs, KCS_READ_STATE, KCS_IDLE_STATE);
if ((state != KCS_READ_STATE) && (state != KCS_IDLE_STATE)) {
start_error_recovery(
kcs,
@@ -472,6 +508,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR2:
+ state = si_sm_wait_state(kcs, KCS_READ_STATE);
if (state != KCS_READ_STATE) {
start_error_recovery(kcs,
"Not in read state for error2");
@@ -486,6 +523,7 @@ static enum si_sm_result kcs_event(struc
break;
case KCS_ERROR3:
+ state = si_sm_wait_state(kcs, KCS_IDLE_STATE);
if (state != KCS_IDLE_STATE) {
start_error_recovery(kcs,
"Not in idle state for error3");
_
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-06-27 0:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-30 14:21 fix IPMI I/O error on NEC FC storage array Naresh Bhat
2012-06-27 0:19 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome