[PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Sudeep Holla]

Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
the controller sets txdone_by_poll. However some clients can have a
mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
case. In such scenario, we may end up with below warnings as the tx
ticker is run both by mailbox framework and the client.

WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
Hardware name: ARM LTD ARM Juno Development Platform
task: ffff8009768ca700 task.stack: ffff8009768f8000
PC is at hrtimer_forward+0x88/0xd8
LR is at txdone_hrtimer+0xd4/0xf8
Call trace:
 hrtimer_forward+0x88/0xd8
 __hrtimer_run_queues+0xe4/0x158
 hrtimer_interrupt+0xa4/0x220
 arch_timer_handler_phys+0x30/0x40
 handle_percpu_devid_irq+0x78/0x130
 generic_handle_irq+0x24/0x38
 __handle_domain_irq+0x5c/0xb8
 gic_handle_irq+0x54/0xa8

This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
knows_txdone.

Cc: Alexey Klimov <alexey.klimov@arm.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 537f4f6d009b..8da30f833262 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
 	init_completion(&chan->tx_complete);
 
 	if (chan->txdone_method	== TXDONE_BY_POLL && cl->knows_txdone)
-		chan->txdone_method |= TXDONE_BY_ACK;
+		chan->txdone_method = TXDONE_BY_ACK;
 
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-- 
2.7.4

[PATCH 2/2] mailbox: mailbox-test: don't rely on rx_buffer content to signal data ready

[Sudeep Holla]

Currently we rely on the first byte of the Rx buffer to check if there's
any data available to be read. If the first byte of the received buffer
is zero (i.e. null character), then we fail to signal that data is
available even when it's available.

Instead introduce a boolean variable to track the data availability and
update it in the channel receive callback as ready and clear it when the
data is read.

Cc: Jassi Brar <jaswinder.singh@linaro.org>
Fixes: baef9a35d246 ("mailbox: mailbox-test: add support for fasync/poll")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox-test.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 97fb956bb6e0..93f3d4d61fa7 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -30,6 +30,7 @@
 #define MBOX_HEXDUMP_MAX_LEN	(MBOX_HEXDUMP_LINE_LEN *		\
 				 (MBOX_MAX_MSG_LEN / MBOX_BYTES_PER_LINE))
 
+static bool mbox_data_ready;
 static struct dentry *root_debugfs_dir;
 
 struct mbox_test_device {
@@ -152,16 +153,14 @@ static ssize_t mbox_test_message_write(struct file *filp,
 
 static bool mbox_test_message_data_ready(struct mbox_test_device *tdev)
 {
-	unsigned char data;
+	bool data_ready;
 	unsigned long flags;
 
 	spin_lock_irqsave(&tdev->lock, flags);
-	data = tdev->rx_buffer[0];
+	data_ready = mbox_data_ready;
 	spin_unlock_irqrestore(&tdev->lock, flags);
 
-	if (data != '\0')
-		return true;
-	return false;
+	return data_ready;
 }
 
 static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf,
@@ -223,6 +222,7 @@ static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf,
 	*(touser + l) = '\0';
 
 	memset(tdev->rx_buffer, 0, MBOX_MAX_MSG_LEN);
+	mbox_data_ready = false;
 
 	spin_unlock_irqrestore(&tdev->lock, flags);
 
@@ -292,6 +292,7 @@ static void mbox_test_receive_message(struct mbox_client *client, void *message)
 				     message, MBOX_MAX_MSG_LEN);
 		memcpy(tdev->rx_buffer, message, MBOX_MAX_MSG_LEN);
 	}
+	mbox_data_ready = true;
 	spin_unlock_irqrestore(&tdev->lock, flags);
 
 	wake_up_interruptible(&tdev->waitq);
-- 
2.7.4

Re: [PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Jassi Brar]

On Fri, Jun 30, 2017 at 8:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
> the controller sets txdone_by_poll. However some clients can have a
> mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
> However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
> case. In such scenario, we may end up with below warnings as the tx
> ticker is run both by mailbox framework and the client.
>
> WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
> Hardware name: ARM LTD ARM Juno Development Platform
> task: ffff8009768ca700 task.stack: ffff8009768f8000
> PC is at hrtimer_forward+0x88/0xd8
> LR is at txdone_hrtimer+0xd4/0xf8
> Call trace:
>  hrtimer_forward+0x88/0xd8
>  __hrtimer_run_queues+0xe4/0x158
>  hrtimer_interrupt+0xa4/0x220
>  arch_timer_handler_phys+0x30/0x40
>  handle_percpu_devid_irq+0x78/0x130
>  generic_handle_irq+0x24/0x38
>  __handle_domain_irq+0x5c/0xb8
>  gic_handle_irq+0x54/0xa8
>
> This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
> knows_txdone.
>
> Cc: Alexey Klimov <alexey.klimov@arm.com>
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/mailbox/mailbox.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 537f4f6d009b..8da30f833262 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>         init_completion(&chan->tx_complete);
>
>         if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
> -               chan->txdone_method |= TXDONE_BY_ACK;
> +               chan->txdone_method = TXDONE_BY_ACK;
>
It has to be restored back in mbox_free_channel()

thanks

Re: [PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Sudeep Holla]

On 01/07/17 12:25, Jassi Brar wrote:
> On Fri, Jun 30, 2017 at 8:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
>> the controller sets txdone_by_poll. However some clients can have a
>> mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
>> However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
>> case. In such scenario, we may end up with below warnings as the tx
>> ticker is run both by mailbox framework and the client.
>>
>> WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
>> Hardware name: ARM LTD ARM Juno Development Platform
>> task: ffff8009768ca700 task.stack: ffff8009768f8000
>> PC is at hrtimer_forward+0x88/0xd8
>> LR is at txdone_hrtimer+0xd4/0xf8
>> Call trace:
>>  hrtimer_forward+0x88/0xd8
>>  __hrtimer_run_queues+0xe4/0x158
>>  hrtimer_interrupt+0xa4/0x220
>>  arch_timer_handler_phys+0x30/0x40
>>  handle_percpu_devid_irq+0x78/0x130
>>  generic_handle_irq+0x24/0x38
>>  __handle_domain_irq+0x5c/0xb8
>>  gic_handle_irq+0x54/0xa8
>>
>> This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
>> knows_txdone.
>>
>> Cc: Alexey Klimov <alexey.klimov@arm.com>
>> Cc: Jassi Brar <jaswinder.singh@linaro.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  drivers/mailbox/mailbox.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>> index 537f4f6d009b..8da30f833262 100644
>> --- a/drivers/mailbox/mailbox.c
>> +++ b/drivers/mailbox/mailbox.c
>> @@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>>         init_completion(&chan->tx_complete);
>>
>>         if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>> -               chan->txdone_method |= TXDONE_BY_ACK;
>> +               chan->txdone_method = TXDONE_BY_ACK;
>>
> It has to be restored back in mbox_free_channel()

Ah right, will fix and repost.

-- 
Regards,
Sudeep

Re: [PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Sudeep Holla]

On 03/07/17 09:57, Sudeep Holla wrote:
> 
> 
> On 01/07/17 12:25, Jassi Brar wrote:
>> On Fri, Jun 30, 2017 at 8:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
>>> the controller sets txdone_by_poll. However some clients can have a
>>> mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
>>> However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
>>> case. In such scenario, we may end up with below warnings as the tx
>>> ticker is run both by mailbox framework and the client.
>>>
>>> WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
>>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
>>> Hardware name: ARM LTD ARM Juno Development Platform
>>> task: ffff8009768ca700 task.stack: ffff8009768f8000
>>> PC is at hrtimer_forward+0x88/0xd8
>>> LR is at txdone_hrtimer+0xd4/0xf8
>>> Call trace:
>>>  hrtimer_forward+0x88/0xd8
>>>  __hrtimer_run_queues+0xe4/0x158
>>>  hrtimer_interrupt+0xa4/0x220
>>>  arch_timer_handler_phys+0x30/0x40
>>>  handle_percpu_devid_irq+0x78/0x130
>>>  generic_handle_irq+0x24/0x38
>>>  __handle_domain_irq+0x5c/0xb8
>>>  gic_handle_irq+0x54/0xa8
>>>
>>> This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
>>> knows_txdone.
>>>
>>> Cc: Alexey Klimov <alexey.klimov@arm.com>
>>> Cc: Jassi Brar <jaswinder.singh@linaro.org>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>  drivers/mailbox/mailbox.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>>> index 537f4f6d009b..8da30f833262 100644
>>> --- a/drivers/mailbox/mailbox.c
>>> +++ b/drivers/mailbox/mailbox.c
>>> @@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>>>         init_completion(&chan->tx_complete);
>>>
>>>         if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>>> -               chan->txdone_method |= TXDONE_BY_ACK;
>>> +               chan->txdone_method = TXDONE_BY_ACK;
>>>
>> It has to be restored back in mbox_free_channel()
> 
> Ah right, will fix and repost.
> 

I was too fast to response, I see we already take care of that in free
channel. So I think it should be fine as is.

        if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
                chan->txdone_method = TXDONE_BY_POLL;
-- 
Regards,
Sudeep

Re: [PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Jassi Brar]

On 3 July 2017 at 15:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 03/07/17 09:57, Sudeep Holla wrote:
>>
>>
>> On 01/07/17 12:25, Jassi Brar wrote:
>>> On Fri, Jun 30, 2017 at 8:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>> Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
>>>> the controller sets txdone_by_poll. However some clients can have a
>>>> mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
>>>> However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
>>>> case. In such scenario, we may end up with below warnings as the tx
>>>> ticker is run both by mailbox framework and the client.
>>>>
>>>> WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
>>>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
>>>> Hardware name: ARM LTD ARM Juno Development Platform
>>>> task: ffff8009768ca700 task.stack: ffff8009768f8000
>>>> PC is at hrtimer_forward+0x88/0xd8
>>>> LR is at txdone_hrtimer+0xd4/0xf8
>>>> Call trace:
>>>>  hrtimer_forward+0x88/0xd8
>>>>  __hrtimer_run_queues+0xe4/0x158
>>>>  hrtimer_interrupt+0xa4/0x220
>>>>  arch_timer_handler_phys+0x30/0x40
>>>>  handle_percpu_devid_irq+0x78/0x130
>>>>  generic_handle_irq+0x24/0x38
>>>>  __handle_domain_irq+0x5c/0xb8
>>>>  gic_handle_irq+0x54/0xa8
>>>>
>>>> This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
>>>> knows_txdone.
>>>>
>>>> Cc: Alexey Klimov <alexey.klimov@arm.com>
>>>> Cc: Jassi Brar <jaswinder.singh@linaro.org>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>>  drivers/mailbox/mailbox.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>>>> index 537f4f6d009b..8da30f833262 100644
>>>> --- a/drivers/mailbox/mailbox.c
>>>> +++ b/drivers/mailbox/mailbox.c
>>>> @@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>>>>         init_completion(&chan->tx_complete);
>>>>
>>>>         if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>>>> -               chan->txdone_method |= TXDONE_BY_ACK;
>>>> +               chan->txdone_method = TXDONE_BY_ACK;
>>>>
>>> It has to be restored back in mbox_free_channel()
>>
>> Ah right, will fix and repost.
>>
>
> I was too fast to response, I see we already take care of that in free
> channel. So I think it should be fine as is.
>
>         if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
>                 chan->txdone_method = TXDONE_BY_POLL;
>
You were too fast this time :)

chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)

  is not same as

(chan->txdone_method == TXDONE_BY_POLL || chan->txdone_method == TXDONE_BY_ACK)

Re: [PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Sudeep Holla]

On 03/07/17 12:50, Jassi Brar wrote:
> On 3 July 2017 at 15:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 03/07/17 09:57, Sudeep Holla wrote:
>>>
>>>
>>> On 01/07/17 12:25, Jassi Brar wrote:
>>>> On Fri, Jun 30, 2017 at 8:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>> Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
>>>>> the controller sets txdone_by_poll. However some clients can have a
>>>>> mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
>>>>> However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
>>>>> case. In such scenario, we may end up with below warnings as the tx
>>>>> ticker is run both by mailbox framework and the client.
>>>>>
>>>>> WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
>>>>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
>>>>> Hardware name: ARM LTD ARM Juno Development Platform
>>>>> task: ffff8009768ca700 task.stack: ffff8009768f8000
>>>>> PC is at hrtimer_forward+0x88/0xd8
>>>>> LR is at txdone_hrtimer+0xd4/0xf8
>>>>> Call trace:
>>>>>  hrtimer_forward+0x88/0xd8
>>>>>  __hrtimer_run_queues+0xe4/0x158
>>>>>  hrtimer_interrupt+0xa4/0x220
>>>>>  arch_timer_handler_phys+0x30/0x40
>>>>>  handle_percpu_devid_irq+0x78/0x130
>>>>>  generic_handle_irq+0x24/0x38
>>>>>  __handle_domain_irq+0x5c/0xb8
>>>>>  gic_handle_irq+0x54/0xa8
>>>>>
>>>>> This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
>>>>> knows_txdone.
>>>>>
>>>>> Cc: Alexey Klimov <alexey.klimov@arm.com>
>>>>> Cc: Jassi Brar <jaswinder.singh@linaro.org>
>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>>> ---
>>>>>  drivers/mailbox/mailbox.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>>>>> index 537f4f6d009b..8da30f833262 100644
>>>>> --- a/drivers/mailbox/mailbox.c
>>>>> +++ b/drivers/mailbox/mailbox.c
>>>>> @@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>>>>>         init_completion(&chan->tx_complete);
>>>>>
>>>>>         if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>>>>> -               chan->txdone_method |= TXDONE_BY_ACK;
>>>>> +               chan->txdone_method = TXDONE_BY_ACK;
>>>>>
>>>> It has to be restored back in mbox_free_channel()
>>>
>>> Ah right, will fix and repost.
>>>
>>
>> I was too fast to response, I see we already take care of that in free
>> channel. So I think it should be fine as is.
>>
>>         if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
>>                 chan->txdone_method = TXDONE_BY_POLL;
>>
> You were too fast this time :)
> 

Indeed, my understanding was correct before.

> chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)
> 
>   is not same as
> 
> (chan->txdone_method == TXDONE_BY_POLL || chan->txdone_method == TXDONE_BY_ACK)
> 

Ah crap, yes and I have no idea why I thought so when I was about to
change it :(

-- 
Regards,
Sudeep

[UPDATE][PATCH 1/2] mailbox: reset txdone_method TXDONE_BY_POLL if client knows_txdone

[Sudeep Holla]

Currently the mailbox framework sets txdone_method to TXDONE_BY_POLL if
the controller sets txdone_by_poll. However some clients can have a
mechanism to do TXDONE_BY_ACK which they can specify by knows_txdone.
However, we endup setting both TXDONE_BY_POLL and TXDONE_BY_ACK in that
case. In such scenario, we may end up with below warnings as the tx
ticker is run both by mailbox framework and the client.

WARNING: CPU: 1 PID: 0 at kernel/time/hrtimer.c:805 hrtimer_forward+0x88/0xd8
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.12.0-rc5 #242
Hardware name: ARM LTD ARM Juno Development Platform
task: ffff8009768ca700 task.stack: ffff8009768f8000
PC is at hrtimer_forward+0x88/0xd8
LR is at txdone_hrtimer+0xd4/0xf8
Call trace:
 hrtimer_forward+0x88/0xd8
 __hrtimer_run_queues+0xe4/0x158
 hrtimer_interrupt+0xa4/0x220
 arch_timer_handler_phys+0x30/0x40
 handle_percpu_devid_irq+0x78/0x130
 generic_handle_irq+0x24/0x38
 __handle_domain_irq+0x5c/0xb8
 gic_handle_irq+0x54/0xa8

This patch fixes the issue by resetting TXDONE_BY_POLL if client has set
knows_txdone.

Cc: Alexey Klimov <alexey.klimov@arm.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/mailbox/mailbox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 537f4f6d009b..674b35f402f5 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -351,7 +351,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
 	init_completion(&chan->tx_complete);
 
 	if (chan->txdone_method	== TXDONE_BY_POLL && cl->knows_txdone)
-		chan->txdone_method |= TXDONE_BY_ACK;
+		chan->txdone_method = TXDONE_BY_ACK;
 
 	spin_unlock_irqrestore(&chan->lock, flags);
 
@@ -418,7 +418,7 @@ void mbox_free_channel(struct mbox_chan *chan)
 	spin_lock_irqsave(&chan->lock, flags);
 	chan->cl = NULL;
 	chan->active_req = NULL;
-	if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
+	if (chan->txdone_method == TXDONE_BY_ACK)
 		chan->txdone_method = TXDONE_BY_POLL;
 
 	module_put(chan->mbox->dev->driver->owner);
-- 
2.7.4