On Tue, 2019-02-12 at 20:05 +0800, Hong Liu wrote:
> Currently we are using one additional static variable and a spinlock
> to
> prevent contention of writing IPC messages to ISH hardware, which is
> not necessary. Once ISH is ready to accept new data, we can push new
> data to hardware. This pushing of new data is already protected by
> wr_processing_spinlock for contention, which is enough. So use this
> spinlock to check both readiness for accepting new data and once
> ready
> allow writing of ipc message from queue to ISH hardware.
>
> While here, cleaned up some space after return.
>
> Signed-off-by: Hong Liu <hong.liu@xxxxxxxxx>
> Tested-by: Hongyan Song <hongyan.song@xxxxxxxxx>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> ---
> drivers/hid/intel-ish-hid/ipc/ipc.c | 19 +++----------------
> drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 2 --
> 2 files changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-
> ish-hid/ipc/ipc.c
> index 742191bb24c6..ff8eca11ff73 100644
> --- a/drivers/hid/intel-ish-hid/ipc/ipc.c
> +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
> @@ -256,33 +256,22 @@ static int write_ipc_from_queue(struct
> ishtp_device *dev)
> int i;
> void (*ipc_send_compl)(void *);
> void *ipc_send_compl_prm;
> - static int out_ipc_locked;
> - unsigned long out_ipc_flags;
>
> if (dev->dev_state == ISHTP_DEV_DISABLED)
> - return -EINVAL;
> + return -EINVAL;
>
> - spin_lock_irqsave(&dev->out_ipc_spinlock, out_ipc_flags);
> - if (out_ipc_locked) {
> - spin_unlock_irqrestore(&dev->out_ipc_spinlock,
> out_ipc_flags);
> - return -EBUSY;
> - }
> - out_ipc_locked = 1;
> + spin_lock_irqsave(&dev->wr_processing_spinlock, flags);
> if (!ish_is_input_ready(dev)) {
> - out_ipc_locked = 0;
> - spin_unlock_irqrestore(&dev->out_ipc_spinlock,
> out_ipc_flags);
> + spin_unlock_irqrestore(&dev->wr_processing_spinlock,
> flags);
> return -EBUSY;
> }
> - spin_unlock_irqrestore(&dev->out_ipc_spinlock, out_ipc_flags);
>
> - spin_lock_irqsave(&dev->wr_processing_spinlock, flags);
> /*
> * if tx send list is empty - return 0;
> * may happen, as RX_COMPLETE handler doesn't check list
> emptiness.
> */
> if (list_empty(&dev->wr_processing_list)) {
> spin_unlock_irqrestore(&dev->wr_processing_spinlock,
> flags);
> - out_ipc_locked = 0;
> return 0;
> }
>
> @@ -333,7 +322,6 @@ static int write_ipc_from_queue(struct
> ishtp_device *dev)
> dev->ipc_tx_bytes_cnt += IPC_HEADER_GET_LENGTH(doorbell_val);
>
> ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, doorbell_val);
> - out_ipc_locked = 0;
>
> ipc_send_compl = ipc_link->ipc_send_compl;
> ipc_send_compl_prm = ipc_link->ipc_send_compl_prm;
> @@ -914,7 +902,6 @@ struct ishtp_device *ish_dev_init(struct pci_dev
> *pdev)
> init_waitqueue_head(&dev->wait_hw_ready);
>
> spin_lock_init(&dev->wr_processing_spinlock);
> - spin_lock_init(&dev->out_ipc_spinlock);
>
> /* Init IPC processing and free lists */
> INIT_LIST_HEAD(&dev->wr_processing_list);
> diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
> b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
> index e7c6bfefaf9e..e54ce1ef27dd 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
> @@ -211,8 +211,6 @@ struct ishtp_device {
> /* For both processing list and free list */
> spinlock_t wr_processing_spinlock;
>
> - spinlock_t out_ipc_spinlock;
> -
> struct ishtp_fw_client *fw_clients; /*Note:memory has to be
> allocated*/
> DECLARE_BITMAP(fw_clients_map, ISHTP_CLIENTS_MAX);
> DECLARE_BITMAP(host_clients_map, ISHTP_CLIENTS_MAX);
