[PATCH v2 01/10] spi: dw: Convert calls to debug to log_*

[Marek Vasut]

On 8/21/20 2:08 AM, Tom Rini wrote:
> On Thu, Aug 20, 2020 at 03:36:31PM +0200, Marek Vasut wrote:
>> On 8/20/20 3:26 PM, Simon Glass wrote:
>>> On Fri, 7 Aug 2020 at 08:43, Sean Anderson <seanga2 at gmail.com> wrote:
>>>>
>>>> This allows different log levels to be enabled or disabled depending on the
>>>> desired level of verbosity. In particular, it allows for general debug
>>>> information to be printed while excluding more verbose logging which may
>>>> interfere with timing.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - New
>>>>
>>>>  drivers/spi/designware_spi.c | 33 ++++++++++++++++-----------------
>>>>  1 file changed, 16 insertions(+), 17 deletions(-)
>>>>
>>>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> NAK, please use dev_dbg() instead.
> 
> For the record, Marek and I (and Sean) had talked a bit on IRC.  These
> messages being log_xxx() and not dev_xx() instead make the more
> functionally useful as dev_xxx() converts to printf() and messes with
> the timing.

The ones which were printf() before are not impacted, since nothing
changes there. The ones which were debug() were compiled out anyway or
turned into printf(), so nothing changes there either.

In this case , the timing argument simply does not apply.

> log_xxx() puts them in the buffer and will not break the
> timing.  Marek objects to the fundamentals that dev_xxx() does not
> function like it does in Linux (and please correct me if you feel I'm
> mis-representing your views).

I do NOT object to the way dev_err() works, see below.

> This could be solved by plumbing
> dev_xxx() to use log_xxx() if LOG is enabled automagically.  I don't see
> transitions like that as blocking for this patch, Marek does.

My objection is to yet another new logging facility, which is
incompatible with any of the Linux ones. Both pr_() and dev_() we
already have in U-Boot, and there is already ongoing conversion attempt
to move drivers to dev_(), but now another log_() one appears out of
nowhere and starts adding to the already bad mess. Moreover, there is
zero information or guideline on which logging primitive to use where.

It is likely others are used to those Linux logging facilities, because
embedded systems developers likely develop both U-Boot and Linux, likely
at the same time. So making the U-Boot more and more incompatible does
not help at all.

What I do propose is to put the new logging facility underneath the
existing logging primitives -- pr_() and dev_() -- if applicable and
make the dynamic logging configurable. That way the current mess will be
cleaned up. The use of log_() should then only be limited to where no
other logging primitives apply.