On Tue, Dec 11, 2018 at 2:15 AM Thierry Reding [off-list ref] wrote:

On Tue, Dec 11, 2018 at 02:00:48AM +0530, Jassi Brar wrote:

quoted

On Mon, Dec 10, 2018 at 3:22 PM Thierry Reding [off-list ref] wrote:

quoted

quoted

quoted

The 'flush' api is going to have no use other than implement
busy-waits. I am afraid people are simply going to take it easy and
copy the busy-waits from the test/verification codes.
"discouraging" seldom works because the developer comes with the
failproof excuse "the h/w doesn't provide any other way". Frankly I
would like to push back on such designs.

I certainly approve of that stance.

However, I'd like to reiterate that the TCU design does in fact support
"another way". If you look at the mailbox driver implementation (in the
drivers/mailbox/tegra-hsp.c driver), you'll see that it does support an
interrupt driven mode. After you had reviewed Mikko's earliest proposal
that was indeed implementing busy looping exclusively we both set out
to properly implement interrupt support. This took quite a while to do
because of some gaps in the documentation, but we eventually got it to
work properly. And then it was discovered that it was all a waste of
effort because the console driver couldn't make use of it anyway. Well,
I should say "waste of effort" because I'm still happy that the proper
implementation exists and we can use it under the right circumstances.

So, at least in this particular case, it is not the hardware or firmware
design that's flawed or was taking any shortcuts. It's really just the
particular use-case of the console that doesn't allow us to make use of
the right implementation, which is why we have to resort to the inferior
method of busy-looping.

I am not complaining about the hardware, but the firmware.
It is essential we dump logs during early boot but the platform(Linux)
doesn't have access to serial port. All the firmware allows is 24bits
per transfer!! We could do better.

Hardware UARTs don't usually have much more FIFO space than that either.

quoted

A smarter option could have been Linux simply placing characters in
the shmem ring-buffer, while the remote consuming (and then poisoning)
the ring buffer upon 'hint' from Linux.

I don't think that would've been much smarter, especially not in this
case. As we discussed earlier, no matter how large you make the ring-
buffer you can always run into situations where you overflow it. The
ring-buffer implementation is also a lot more complicated and error-
prone.

Please think about it again.
The ring buffer becomes the effective h/w fifo. And you don't have to
wait at all for the mailbox register to clear .... you could simply
overwrite it when Linux puts some data in the ring-buffer (the data
written will just be a 'hint' command for remote to go look into the
ring-buffer for new data).

Plus there is the fact that in this particular case we actually
don't want buffering because the buffer may hide important information
in case of a crash.

Even if the Linux crashes, whatever data is placed in the ring-buffer
will eventually be printed by the still-alive remote.

Anyways once we have the flush api, I don't really care how broken your f/w is.