[U-Boot] [PATCH 6/8] cpu: Add cpu_print_info function

[Mario Six]

Hi Simon,

On Thu, Apr 26, 2018 at 4:40 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 26 April 2018 at 00:07, Mario Six <mario.six at gdsys.cc> wrote:
>> Hi Simon,
>>
>> On Tue, Apr 24, 2018 at 11:53 PM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Mario,
>>>
>>> On 19 April 2018 at 01:50, Mario Six <mario.six at gdsys.cc> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On Wed, Apr 18, 2018 at 5:45 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> > Hi Mario,
>>>> >
>>>> > On 18 April 2018 at 02:35, Mario Six <mario.six at gdsys.cc> wrote:
>>>> >> Hi Simon,
>>>> >>
>>>> >> On Thu, Apr 12, 2018 at 6:37 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> >>> Hi Mario,
>>>> >>>
>>>> >>> On 11 April 2018 at 00:39, Mario Six <mario.six at gdsys.cc> wrote:
>>>> >>>> Hi Simon,
>>>> >>>>
>>>> >>>> On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg at chromium.org> wrote:
>>>> >>>>> Hi,
>>>> >>>>>
>>>> >>>>> On 28 March 2018 at 20:38, Mario Six <mario.six at gdsys.cc> wrote:
>>>> >>>>>> Add a cpu_print_info function to the CPU uclass to emulate the behavior
>>>> >>>>>> of some current non-DM drivers (e.g. MPC83xx) to print CPU information
>>>> >>>>>> during startup.
>>>> >>>>>>
>>>> >>>>>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>>>> >>>>>> ---
>>>> >>>>>>  drivers/cpu/cpu-uclass.c | 10 ++++++++++
>>>> >>>>>>  include/cpu.h            | 15 +++++++++++++++
>>>> >>>>>>  2 files changed, 25 insertions(+)
>>>> >>>>>>
>>>> >>>>>
>>>> >>>>> I really don't want drivers printing stuff. Can you use the existing
>>>> >>>>> get_info() method?
>>>> >>>>>
>>>> >>>>
>>>> >>>> I could, but I'm just emulating the behavior of the legacy code here, which
>>>> >>>> prints some information when the CPU is initialized. I think that's pretty
>>>> >>>> useful, and I can do that in our board file, but that would mean implementing
>>>> >>>> the same routine in every MPC83xx board to get the legacy behavior back.
>>>> >>>
>>>> >>> Yes, but I don't want the legacy code creeping into the eclass. Can
>>>> >>> you convert the board to use the CPU eclass instead?
>>>> >>>
>>>> >>
>>>> >> That's what I did, and I just discovered DISPLAY_CPUINFO, which does exactly
>>>> >> what is needed. I'll implement the print_cpuinfo function in the CPU driver, so
>>>> >> I can get rid of the print function in the uclass (and still retain the
>>>> >> information printing at bootup).
>>>> >
>>>> > OK I see. Ideally we would have a function (perhaps in board_f) which
>>>> > prints out the CPU info after obtaining it from the uclass. So could
>>>> > you move your print_cpuinfo() function into board_f? Would it be
>>>> > possible to use that if CONFIG_CPU is defined?
>>>> >
>>>> > At some point print_cpuinfo() could be removed from various board files.
>>>> >
>>>>
>>>> The function prints the following (example for our device):
>>>>
>>>> --- >8 ---
>>>>
>>>> Reset Status: External/Internal Soft, External/Internal Hard
>>>>
>>>> CPU:   e300c3, MPC8308, Rev: 1.1 at 400 MHz, CSB: 133.333 MHz
>>>>
>>>> --- >8 ---
>>>>
>>>> So there are some values that are very specific to the platform, such as the
>>>> CSB (Coherent System Bus) frequency, or the reset status. While it's probably
>>>> possible to put some of that into a generic info printing routine, lots of it
>>>> is so MPC83xx-specific that it doesn't make much sense for other CPUs.
>>>
>>> Well you get to provide a string from get_desc() so you can add
>>> whatever you like!
>>>
>>
>> OK, I thought I was supposed to decode the cpu_info and print that in the
>> function. If i can use get_desc(), then it's fine. :-)
>>
>>>>
>>>> Any idea how to tackle this? I don't really want to get rid of the Reset Status
>>>> printing especially, since it's pretty useful information for debugging devices
>>>> in the field.
>>>
>>> Re the reset status, shouldn't that come from the sysreset driver? I
>>> wonder if we should add an API call for that? Or is it something
>>> printed by the board?
>>>
>>
>> No, that's indeed printed on every MPC83xx board (see prt_83xx_rsr in
>> arch/powerpc/cpu/mpc83xx/cpu_init.c). A sysreset driver would probably be a
>> good canonical candidate for printing such a message, true. But since there is
>> no real system reset on MPC83xx to speak of, that would (for now at least) be
>> the only thing a MPC83xx sysreset driver did. Also, we'd need another function
>> in board_f.c, and I don't really know if reset cause printing is very
>> wide-spread on other platforms to justify that.
>
> Well I've certainly printed this info myself so I think it is useful,
> particularly things like whether it was a watchdog reset or user
> reset.
>

OK, I'll write a sysreset driver for MPC83xx; shouldn't be too complicated.

> Regards,
> Simon
>

Best regards,
Mario