[Skiboot] [PATCH V2 3/5] hw/bt: Remove state machine and use only B2H_ATN and H2B_ATN.

[Alistair Popple]

Stewart,

These next few patches look fairly low risk and look good but they affect 
every OpenPower machine in a high impact area. So it would be best if these 
were merged to the less stable skiboot branch now that we have one.

Acked-by: Alistair Popple <alistair at popple.id.au>

On Thu, 12 Nov 2015 17:22:57 Cyril Bur wrote:
> Correct use of B2H_ATN and H2B_ATN means that we don't need to track
> states.
> 
> The only caveat is that currently we use states to know if we need
> to send a message to the BMC, change this to simply seeing if the top
> message has been sent or not.
> 
> Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> ---
>  hw/bt.c | 28 +++++++---------------------
>  1 file changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/bt.c b/hw/bt.c
> index 253ca2a..215a05b 100644
> --- a/hw/bt.c
> +++ b/hw/bt.c
> @@ -101,11 +101,6 @@
>  #define BT_Q_INF(msg, fmt, args...) BT_INF(fmt, ##args)
>  #endif
>  
> -enum bt_states {
> -	BT_STATE_IDLE = 0,
> -	BT_STATE_RESP_WAIT,
> -};
> -
>  struct bt_msg {
>  	struct list_node link;
>  	unsigned long tb;
> @@ -116,7 +111,6 @@ struct bt_msg {
>  
>  struct bt {
>  	uint32_t base_addr;
> -	enum bt_states state;
>  	struct lock lock;
>  	struct list_head msgq;
>  	struct timer poller;
> @@ -153,11 +147,6 @@ static inline bool bt_idle(void)
>  	return !(bt_ctrl & BT_CTRL_B_BUSY) && !(bt_ctrl & BT_CTRL_H2B_ATN);
>  }
>  
> -static inline void bt_set_state(enum bt_states next_state)
> -{
> -	bt.state = next_state;
> -}
> -
>  /* Must be called with bt.lock held */
>  static void bt_msg_del(struct bt_msg *bt_msg)
>  {
> @@ -177,8 +166,6 @@ static void bt_init_interface(void)
>  
>  	/* Take care of a stable H_BUSY if any */
>  	bt_set_h_busy(false);
> -
> -	bt_set_state(BT_STATE_IDLE);
>  }
>  
>  static void bt_reset_interface(void)
> @@ -220,7 +207,6 @@ static void bt_send_msg(struct bt_msg *bt_msg)
>  	bt_msg->send_count++;
>  
>  	bt_outb(BT_CTRL_H2B_ATN, BT_CTRL);
> -	bt_set_state(BT_STATE_RESP_WAIT);
>  
>  	return;
>  }
> @@ -275,7 +261,6 @@ static void bt_get_resp(void)
>  		BT_INF("BT: Nobody cared about a response to an BT/IPMI 
message"
>  			"(seq 0x%02x netfn 0x%02x cmd 0x%02x)", seq, netfn, 
cmd);
>  		bt_flush_msg();
> -		bt_set_state(BT_STATE_IDLE);
>  		return;
>  	}
>  
> @@ -298,8 +283,6 @@ static void bt_get_resp(void)
>  		ipmi_msg->data[i] = bt_inb(BT_HOST2BMC);
>  	bt_set_h_busy(false);
>  
> -	bt_set_state(BT_STATE_IDLE);
> -
>  	BT_Q_DBG(bt_msg, "IPMI MSG done");
>  
>  	list_del(&bt_msg->link);
> @@ -382,7 +365,12 @@ static void bt_send_and_unlock(void)
>  		if (bt_msg->tb == 0)
>  			bt_msg->tb = mftb();
>  
> -		if (bt_idle() && bt.state == BT_STATE_IDLE)
> +		/*
> +		 * Only send it if we haven't already.
> +		 * Timeouts and retries happen in bt_expire_old_msg()
> +		 * called from bt_poll()
> +		 */
> +		if (bt_idle() && bt_msg->send_count == 0)
>  			bt_send_msg(bt_msg);
>  	}
>  
> @@ -408,8 +396,7 @@ static void bt_poll(struct timer *t __unused, void *data 
__unused,
>  	bt_ctrl = bt_inb(BT_CTRL);
>  
>  	/* Is there a response waiting for us? */
> -	if (bt.state == BT_STATE_RESP_WAIT &&
> -	    (bt_ctrl & BT_CTRL_B2H_ATN))
> +	if (bt_ctrl & BT_CTRL_B2H_ATN)
>  		bt_get_resp();
>  
>  	bt_expire_old_msg(now);
> @@ -576,7 +563,6 @@ void bt_init(void)
>  	 * The iBT interface comes up in the busy state until the daemon has
>  	 * initialised it.
>  	 */
> -	bt_set_state(BT_STATE_IDLE);
>  	list_head_init(&bt.msgq);
>  	bt.queue_len = 0;
>  
>