Message ID | 1457685035-20213-1-git-send-email-clg@fr.ibm.com |
---|---|
State | Superseded |
Headers | show |
On 03/11/2016 02:00 PM, Cédric Le Goater wrote: > OpenPower systems using a AMI firmware on the BMC have a BT device > configured with a capability of '1' maximum retry. The following code > is equivalent to what skiboot currently supports but it will now also > handle setups of other BT devices, like in qemu or OpenBMC. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> Looks good. Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > hw/bt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > Index: skiboot.git/hw/bt.c > =================================================================== > --- skiboot.git.orig/hw/bt.c > +++ skiboot.git/hw/bt.c > @@ -74,7 +74,7 @@ > /* > * Maximum number of times to attempt sending a message before giving up. > */ > -#define BT_MAX_SEND_COUNT 2 > +#define BT_MAX_SEND_COUNT 1 > > #define BT_QUEUE_DEBUG 0 > > @@ -392,7 +392,7 @@ static void bt_expire_old_msg(uint64_t t > > if (bt_msg && bt_msg->tb > 0 && > (tb_compare(tb, bt_msg->tb + secs_to_tb(bt.caps.msg_timeout)) == TB_AAFTERB)) { > - if (bt_msg->send_count < BT_MAX_SEND_COUNT) { > + if (bt_msg->send_count < bt.caps.num_retries + 1) { You may want comment why we are adding +1 here. -Vasant
Hi Cedric, Some suggestions below: On Friday 11 March 2016 02:00 PM, Cédric Le Goater wrote: > OpenPower systems using a AMI firmware on the BMC have a BT device > configured with a capability of '1' maximum retry. The following code > is equivalent to what skiboot currently supports but it will now also > handle setups of other BT devices, like in qemu or OpenBMC. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > hw/bt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > Index: skiboot.git/hw/bt.c > =================================================================== > --- skiboot.git.orig/hw/bt.c > +++ skiboot.git/hw/bt.c > @@ -74,7 +74,7 @@ > /* > * Maximum number of times to attempt sending a message before giving up. > */ > -#define BT_MAX_SEND_COUNT 2 > +#define BT_MAX_SEND_COUNT 1 We can better use #define BT_MAX_RETRY 1 > > #define BT_QUEUE_DEBUG 0 > > @@ -392,7 +392,7 @@ static void bt_expire_old_msg(uint64_t t > > if (bt_msg && bt_msg->tb > 0 && > (tb_compare(tb, bt_msg->tb + secs_to_tb(bt.caps.msg_timeout)) == TB_AAFTERB)) { > - if (bt_msg->send_count < BT_MAX_SEND_COUNT) { > + if (bt_msg->send_count < bt.caps.num_retries + 1) { if (bt_msg->send_count <= bt.caps.num_retries) { > /* A message timeout is usually due to the BMC > clearing the H2B_ATN flag without actually > doing anything. The data will still be in the > @@ -629,7 +629,7 @@ void bt_init(void) > bt.caps.input_buf_len = BT_FIFO_LEN; > bt.caps.output_buf_len = BT_FIFO_LEN; > bt.caps.msg_timeout = BT_MSG_TIMEOUT; > - bt.caps.num_retries = 1; > + bt.caps.num_retries = BT_MAX_SEND_COUNT; bt.caps.num_retries = BT_MAX_RETRY; > > /* We support only one */ > n = dt_find_compatible_node(dt_root, NULL, "ipmi-bt"); > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot Regards, Vipin
On 03/12/2016 06:33 AM, Vipin K Parashar wrote: > Hi Cedric, > Some suggestions below: Yes. your suggestion clarifies the code and should address Vasant request to add a comment. I will send a v2. Thanks, C. > On Friday 11 March 2016 02:00 PM, Cédric Le Goater wrote: >> OpenPower systems using a AMI firmware on the BMC have a BT device >> configured with a capability of '1' maximum retry. The following code >> is equivalent to what skiboot currently supports but it will now also >> handle setups of other BT devices, like in qemu or OpenBMC. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >> --- >> hw/bt.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> Index: skiboot.git/hw/bt.c >> =================================================================== >> --- skiboot.git.orig/hw/bt.c >> +++ skiboot.git/hw/bt.c >> @@ -74,7 +74,7 @@ >> /* >> * Maximum number of times to attempt sending a message before giving up. >> */ >> -#define BT_MAX_SEND_COUNT 2 >> +#define BT_MAX_SEND_COUNT 1 > > We can better use > #define BT_MAX_RETRY 1 > >> #define BT_QUEUE_DEBUG 0 >> @@ -392,7 +392,7 @@ static void bt_expire_old_msg(uint64_t t >> if (bt_msg && bt_msg->tb > 0 && >> (tb_compare(tb, bt_msg->tb + secs_to_tb(bt.caps.msg_timeout)) == TB_AAFTERB)) { >> - if (bt_msg->send_count < BT_MAX_SEND_COUNT) { >> + if (bt_msg->send_count < bt.caps.num_retries + 1) { > > if (bt_msg->send_count <= bt.caps.num_retries) { > > >> /* A message timeout is usually due to the BMC >> clearing the H2B_ATN flag without actually >> doing anything. The data will still be in the >> @@ -629,7 +629,7 @@ void bt_init(void) >> bt.caps.input_buf_len = BT_FIFO_LEN; >> bt.caps.output_buf_len = BT_FIFO_LEN; >> bt.caps.msg_timeout = BT_MSG_TIMEOUT; >> - bt.caps.num_retries = 1; >> + bt.caps.num_retries = BT_MAX_SEND_COUNT; > > bt.caps.num_retries = BT_MAX_RETRY; > >> /* We support only one */ >> n = dt_find_compatible_node(dt_root, NULL, "ipmi-bt"); >> >> _______________________________________________ >> Skiboot mailing list >> Skiboot@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot > > Regards, > Vipin >
Index: skiboot.git/hw/bt.c =================================================================== --- skiboot.git.orig/hw/bt.c +++ skiboot.git/hw/bt.c @@ -74,7 +74,7 @@ /* * Maximum number of times to attempt sending a message before giving up. */ -#define BT_MAX_SEND_COUNT 2 +#define BT_MAX_SEND_COUNT 1 #define BT_QUEUE_DEBUG 0 @@ -392,7 +392,7 @@ static void bt_expire_old_msg(uint64_t t if (bt_msg && bt_msg->tb > 0 && (tb_compare(tb, bt_msg->tb + secs_to_tb(bt.caps.msg_timeout)) == TB_AAFTERB)) { - if (bt_msg->send_count < BT_MAX_SEND_COUNT) { + if (bt_msg->send_count < bt.caps.num_retries + 1) { /* A message timeout is usually due to the BMC clearing the H2B_ATN flag without actually doing anything. The data will still be in the @@ -629,7 +629,7 @@ void bt_init(void) bt.caps.input_buf_len = BT_FIFO_LEN; bt.caps.output_buf_len = BT_FIFO_LEN; bt.caps.msg_timeout = BT_MSG_TIMEOUT; - bt.caps.num_retries = 1; + bt.caps.num_retries = BT_MAX_SEND_COUNT; /* We support only one */ n = dt_find_compatible_node(dt_root, NULL, "ipmi-bt");
OpenPower systems using a AMI firmware on the BMC have a BT device configured with a capability of '1' maximum retry. The following code is equivalent to what skiboot currently supports but it will now also handle setups of other BT devices, like in qemu or OpenBMC. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- hw/bt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)