Tested-by: Andrew Banman Ran through 100+ boots with no error with your 1st patch applied. I don't see any endcases to worry about.
Thanks Corey! Andrew > -----Original Message----- > From: Corey Minyard [mailto:tcminy...@gmail.com] On Behalf Of > miny...@acm.org > Sent: Thursday, August 23, 2018 3:37 PM > To: Banman, Andrew <aban...@hpe.com> > Cc: openipmi-developer@lists.sourceforge.net; linux- > ker...@vger.kernel.org; Anderson, Russ <russ.ander...@hpe.com>; Ramsay, > Frank <frank.ram...@hpe.com>; Ernst, Justin <justin.er...@hpe.com>; > Corey Minyard <cminy...@mvista.com> > Subject: [PATCH 1/2] ipmi: Move BT capabilities detection to the detect > call > > From: Corey Minyard <cminy...@mvista.com> > > The capabilities detection was being done as part of the normal > state machine, but it was possible for it to be running while > the upper layers of the IPMI driver were initializing the > device, resulting in error and failure to initialize. > > Move the capabilities detection to the the detect function, > so it's done before anything else runs on the device. This also > simplifies the state machine and removes some code, as a bonus. > > Signed-off-by: Corey Minyard <cminy...@mvista.com> > Reported-by: Andrew Banman <aban...@hpe.com> > --- > drivers/char/ipmi/ipmi_bt_sm.c | 92 ++++++++++++++++++++++------------- > ------- > 1 file changed, 48 insertions(+), 44 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_bt_sm.c > b/drivers/char/ipmi/ipmi_bt_sm.c > index cbc6126..b4133832e 100644 > --- a/drivers/char/ipmi/ipmi_bt_sm.c > +++ b/drivers/char/ipmi/ipmi_bt_sm.c > @@ -59,8 +59,6 @@ enum bt_states { > BT_STATE_RESET3, > BT_STATE_RESTART, > BT_STATE_PRINTME, > - BT_STATE_CAPABILITIES_BEGIN, > - BT_STATE_CAPABILITIES_END, > BT_STATE_LONG_BUSY /* BT doesn't get hosed :-) */ > }; > > @@ -86,7 +84,6 @@ struct si_sm_data { > int error_retries; /* end of "common" fields */ > int nonzero_status; /* hung BMCs stay all 0 */ > enum bt_states complete; /* to divert the state machine */ > - int BT_CAP_outreqs; > long BT_CAP_req2rsp; > int BT_CAP_retries; /* Recommended retries */ > }; > @@ -137,8 +134,6 @@ static char *state2txt(unsigned char state) > case BT_STATE_RESET3: return("RESET3"); > case BT_STATE_RESTART: return("RESTART"); > case BT_STATE_LONG_BUSY: return("LONG_BUSY"); > - case BT_STATE_CAPABILITIES_BEGIN: return("CAP_BEGIN"); > - case BT_STATE_CAPABILITIES_END: return("CAP_END"); > } > return("BAD STATE"); > } > @@ -185,7 +180,6 @@ static unsigned int bt_init_data(struct si_sm_data > *bt, struct si_sm_io *io) > bt->complete = BT_STATE_IDLE; /* end here */ > bt->BT_CAP_req2rsp = BT_NORMAL_TIMEOUT * USEC_PER_SEC; > bt->BT_CAP_retries = BT_NORMAL_RETRY_LIMIT; > - /* BT_CAP_outreqs == zero is a flag to read BT Capabilities */ > return 3; /* We claim 3 bytes of space; ought to check SPMI table > */ > } > > @@ -447,7 +441,7 @@ static enum si_sm_result error_recovery(struct > si_sm_data *bt, > > static enum si_sm_result bt_event(struct si_sm_data *bt, long time) > { > - unsigned char status, BT_CAP[8]; > + unsigned char status; > static enum bt_states last_printed = BT_STATE_PRINTME; > int i; > > @@ -500,12 +494,6 @@ static enum si_sm_result bt_event(struct si_sm_data > *bt, long time) > if (status & BT_H_BUSY) /* clear a leftover H_BUSY */ > BT_CONTROL(BT_H_BUSY); > > - bt->timeout = bt->BT_CAP_req2rsp; > - > - /* Read BT capabilities if it hasn't been done yet */ > - if (!bt->BT_CAP_outreqs) > - BT_STATE_CHANGE(BT_STATE_CAPABILITIES_BEGIN, > - SI_SM_CALL_WITHOUT_DELAY); > BT_SI_SM_RETURN(SI_SM_IDLE); > > case BT_STATE_XACTION_START: > @@ -610,37 +598,6 @@ static enum si_sm_result bt_event(struct si_sm_data > *bt, long time) > BT_STATE_CHANGE(BT_STATE_XACTION_START, > SI_SM_CALL_WITH_DELAY); > > - /* > - * Get BT Capabilities, using timing of upper level state machine. > - * Set outreqs to prevent infinite loop on timeout. > - */ > - case BT_STATE_CAPABILITIES_BEGIN: > - bt->BT_CAP_outreqs = 1; > - { > - unsigned char GetBT_CAP[] = { 0x18, 0x36 }; > - bt->state = BT_STATE_IDLE; > - bt_start_transaction(bt, GetBT_CAP, sizeof(GetBT_CAP)); > - } > - bt->complete = BT_STATE_CAPABILITIES_END; > - BT_STATE_CHANGE(BT_STATE_XACTION_START, > - SI_SM_CALL_WITH_DELAY); > - > - case BT_STATE_CAPABILITIES_END: > - i = bt_get_result(bt, BT_CAP, sizeof(BT_CAP)); > - bt_init_data(bt, bt->io); > - if ((i == 8) && !BT_CAP[2]) { > - bt->BT_CAP_outreqs = BT_CAP[3]; > - bt->BT_CAP_req2rsp = BT_CAP[6] * USEC_PER_SEC; > - bt->BT_CAP_retries = BT_CAP[7]; > - } else > - pr_warn("IPMI BT: using default values\n"); > - if (!bt->BT_CAP_outreqs) > - bt->BT_CAP_outreqs = 1; > - pr_warn("IPMI BT: req2rsp=%ld secs retries=%d\n", > - bt->BT_CAP_req2rsp / USEC_PER_SEC, bt->BT_CAP_retries); > - bt->timeout = bt->BT_CAP_req2rsp; > - return SI_SM_CALL_WITHOUT_DELAY; > - > default: /* should never occur */ > return error_recovery(bt, > status, > @@ -651,6 +608,11 @@ static enum si_sm_result bt_event(struct si_sm_data > *bt, long time) > > static int bt_detect(struct si_sm_data *bt) > { > + unsigned char GetBT_CAP[] = { 0x18, 0x36 }; > + unsigned char BT_CAP[8]; > + enum si_sm_result smi_result; > + int rv; > + > /* > * It's impossible for the BT status and interrupt registers to be > * all 1's, (assuming a properly functioning, self-initialized BMC) > @@ -661,6 +623,48 @@ static int bt_detect(struct si_sm_data *bt) > if ((BT_STATUS == 0xFF) && (BT_INTMASK_R == 0xFF)) > return 1; > reset_flags(bt); > + > + /* > + * Try getting the BT capabilities here. > + */ > + rv = bt_start_transaction(bt, GetBT_CAP, sizeof(GetBT_CAP)); > + if (rv) { > + dev_warn(bt->io->dev, > + "Can't start capabilities transaction: %d\n", rv); > + goto out_no_bt_cap; > + } > + > + smi_result = SI_SM_CALL_WITHOUT_DELAY; > + for (;;) { > + if (smi_result == SI_SM_CALL_WITH_DELAY || > + smi_result == SI_SM_CALL_WITH_TICK_DELAY) { > + schedule_timeout_uninterruptible(1); > + smi_result = bt_event(bt, jiffies_to_usecs(1)); > + } else if (smi_result == SI_SM_CALL_WITHOUT_DELAY) { > + smi_result = bt_event(bt, 0); > + } else > + break; > + } > + > + rv = bt_get_result(bt, BT_CAP, sizeof(BT_CAP)); > + bt_init_data(bt, bt->io); > + if (rv < 8) { > + dev_warn(bt->io->dev, "bt cap response too short: %d\n", rv); > + goto out_no_bt_cap; > + } > + > + if (BT_CAP[2]) { > + dev_warn(bt->io->dev, "Error fetching bt cap: %x\n", > BT_CAP[2]); > +out_no_bt_cap: > + dev_warn(bt->io->dev, "using default values\n"); > + } else { > + bt->BT_CAP_req2rsp = BT_CAP[6] * USEC_PER_SEC; > + bt->BT_CAP_retries = BT_CAP[7]; > + } > + > + dev_info(bt->io->dev, "req2rsp=%ld secs retries=%d\n", > + bt->BT_CAP_req2rsp / USEC_PER_SEC, bt->BT_CAP_retries); > + > return 0; > } > > -- > 2.7.4 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
