bt: use the maximum retry count returned by the BMC

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(-)

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
>