On Tue, Dec 11, 2018 at 2:15 AM Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > > On Tue, Dec 11, 2018 at 02:00:48AM +0530, Jassi Brar wrote: > > On Mon, Dec 10, 2018 at 3:22 PM Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > > > > > > > > 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. > > > 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.
