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

Alistair Popple alistair at popple.id.au
Thu Oct 29 12:17:31 AEDT 2015


Looks good, this has been an improvement I've been meaning to make for a 
while. It should allow use to much more easily send multiple messages at a 
time if we have a BMC that supports that (I think the AMI ones support two?)

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

On Thu, 29 Oct 2015 11:24:07 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 7266b2f..a4da87e 100644
> --- a/hw/bt.c
> +++ b/hw/bt.c
> @@ -97,11 +97,6 @@
>  #define BT_INF(msg, fmt, args...) \
>  	_BT_LOG(PR_INFO, msg, fmt, ##args)
>  
> -enum bt_states {
> -	BT_STATE_IDLE = 0,
> -	BT_STATE_RESP_WAIT,
> -};
> -
>  struct bt_msg {
>  	struct list_node link;
>  	unsigned long tb;
> @@ -112,7 +107,6 @@ struct bt_msg {
>  
>  struct bt {
>  	uint32_t base_addr;
> -	enum bt_states state;
>  	struct lock lock;
>  	struct list_head msgq;
>  	struct timer poller;
> @@ -149,11 +143,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)
>  {
> @@ -173,8 +162,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)
> @@ -215,7 +202,6 @@ static void bt_send_msg(struct bt_msg *bt_msg)
>  	bt_msg->send_count++;
>  	BT_DBG(bt_msg, "Message sent to host");
>  	bt_outb(BT_CTRL_H2B_ATN, BT_CTRL);
> -	bt_set_state(BT_STATE_RESP_WAIT);
>  
>  	return;
>  }
> @@ -270,7 +256,6 @@ static void bt_get_resp(void)
>  		prlog(PR_INFO, "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;
>  	}
>  
> @@ -293,8 +278,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);
> -
>  #if BT_QUEUE_DEBUG
>  	BT_DBG(bt_msg, "IPMI MSG done");
>  #endif
> @@ -378,7 +361,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);
>  	}
>  
> @@ -404,8 +392,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);
> @@ -572,7 +559,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;
>  
> 



More information about the Skiboot mailing list