Hi, On Fri, Jun 7, 2024 at 12:50 AM Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote: > > > + /* > > + * This function is used to poll bits, some of which (like CMD_DONE) > > + * might take as long as it takes for the FIFO plus the temp register > > + * on the geni side to drain. The Linux core calculates such a timeout > > + * for us and we can get it from uart_fifo_timeout(). > > + * > > + * It should be noted that during earlycon the variables that > > + * uart_fifo_timeout() makes use of in "uport" may not be setup yet. > > + * It's difficult to set things up for earlycon since it can't > > + * necessarily figure out the baud rate and reading the FIFO depth > > + * from the wrapper means some extra MMIO maps that we don't get by > > + * default. This isn't a big problem, though, since uart_fifo_timeout() > > + * gives back its "slop" of 20ms as a minimum and that should be > > + * plenty of time for earlycon unless we're running at an extremely > > + * low baud rate. > > + */ > > + timeout_us = jiffies_to_usecs(uart_fifo_timeout(uport)); > > Hi, > > While this is not exactly incorrect, the back and forth conversions nsecs > -> jiffies -> usecs feels somewhat odd, perhaps reworking > uart_fifo_timeout()'s return type from jiffies to e.g. usecs would be > preferrable. As is, the jiffies as its return type seems a small obstacle > for using uart_fifo_timeout() which has come up in other contexts too. Sure. I'll change it to "ms" instead of "us". We don't need the fidelity of "us" here given that the function is adding 20 ms of slop anyway so might as well return ms so that callers don't need to do so much math and don't need to work with u64. This means that I'll have to add a "* USEC_PER_MSEC" in my driver, but it still feels like the more correct thing to do. It also has the nice side effect of allowing the driver to remove the awkward "DIV_ROUND_UP(timeout_us, 10) * 10" because we know that the timeout will always be a proper multiple. I'll also add a new function with the _ms suffix instead of changing the old one. The suffix makes it clear to the caller what the unit of the returned value is and we might as well leave the old wrapper there--otherwise we just need to move the jiffies conversion into the existing callers.
