Thread (31 messages) 31 messages, 5 authors, 2018-08-09

[PATCH v3 3/8] mailbox: Add transmit done by blocking option

From: jassisinghbrar@gmail.com (Jassi Brar)
Date: 2018-08-09 09:27:01
Also in: linux-devicetree, linux-mediatek, linux-serial, linux-tegra, lkml

[dropping everyone else while we discuss the code]

Thanks. I assume that branch as all the code that there is. Let me
look and get back to you.

On Thu, Aug 9, 2018 at 2:19 PM, Mikko Perttunen [off-list ref] wrote:
Here's my current code:

https://github.com/cyndis/linux/commits/wip/t194-tcu-4

"fixup! mailbox: tegra-hsp: Add support for shared mailboxes" splits up the
controller into two. "tegra-hsp: use polling" changes it to use polling.

There are two lines in the top patch with comments:

- at the end of tegra_hsp_mailbox_send_data, I left a "while
(!tegra_hsp_mailbox_last_tx_done(chan));". Without it I wasn't able to see
even a few garbled characters in the output.

- as mentioned, if I enable tx_block on the client side, I get a BUG:
scheduling while atomic. I assume this gets printed through the earlycon as
it's printing out correctly.

Thanks,
Mikko


On 08.08.2018 17:46, Mikko Perttunen wrote:
quoted
On 08/08/2018 05:39 PM, Jassi Brar wrote:
quoted
On Wed, Aug 8, 2018 at 8:04 PM, Mikko Perttunen [off-list ref] wrote:
quoted


On 08/08/2018 05:10 PM, Jassi Brar wrote:
quoted

On Wed, Aug 8, 2018 at 5:08 PM, Mikko Perttunen [off-list ref]
wrote:
quoted



On 04.08.2018 13:45, Mikko Perttunen wrote:
quoted


On 08/03/2018 03:54 PM, Jassi Brar wrote:
quoted


On Mon, Jul 2, 2018 at 5:10 PM, Mikko Perttunen
[off-list ref]
wrote:
quoted


Add a new TXDONE option, TXDONE_BY_BLOCK. With this option, the
send_data function of the mailbox driver is expected to block until
the message has been sent. The new option is used with the Tegra
Combined UART driver to minimize unnecessary overhead when
transmitting
data.
1) TXDONE_BY_BLOCK flag :-
           Have you tried setting the flag
mbox_chan->mbox_client->tx_block
?



No - I suppose I should have done that. I'm a bit concerned about
overhead
as send_data may be called thousands of times per second, so I tried
to
make
it as close as possible to the downstream driver that just pokes the
mailbox
register directly.



I tried using polling in the mailbox framework. Some printing is done
from
atomic context so it seems tx_block cannot be used -
wait_for_completion_timeout understandably does not work in atomic
context.
I also tried without tx_block, in which case I got some horribly
garbled
output, but "Try increasing MBOX_TX_QUEUE_LEN" was readable there.

Any opinions?
The problems arise because your hardware (SM) supports TXDONE_BY_POLL,
but your client drives it by TXDONE_BY_ACK because the older DB
channels are so.

Please populate SM channels as a separate controller than DB.
The DB controller, as is, run by ACK method.
The SM controller should be run by polling, i.e, set txdone_poll =
true and the poll period small enough. The virtual tty client driver
should be able to safely set tx_block from appropriate context.
Sorry, I should have clarified that I already split up the controllers.
The
SM controller has txdone_poll = true. I didn't adjust txpoll_period so I
guess it's zero.
Can you please share your code (controller and client) ? Maybe offline
if you wish.
I'll upload a git branch tomorrow -- I'm not at the machine with the code
now.

Mikko
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help