Is the name hibrid_romstage going to be final? I remember some discussions about using some different name. If so, shouldn't you change file name and the commit message?>

I'm flexible on the name but I've heard no better suggestions. Although it uses nearly all the romstage build infrastructure, I think it's important to try to draw a distinction between this implementation and a traditional romstage. If you're thinking of what I'd coined "pspstage", if it comes about, that's a completely separate entity and 0% romstage, with the makefile calling a new create_class_compiler, pspstage += <name> for all source, new environment definitions, etc. A pspstage approach is my current long-term preference.

View Change

5 comments:

• File src/soc/amd/picasso/early_stack.S:

  • Patch Set #13, Line 31:

    Extra space.

    It's not extra. Using a double space after a period, and between sentences, was always the norm for typewritten text.

  • Patch Set #13, Line 32: * may be enabled. This could be made more elegant for more

    Current limit is 96 columns, please use full new size. Same in next comment.

    Although the limit is 96, it's still not preferred. 96 should only be used when 80 affects readability.

• File src/soc/amd/picasso/hybrid_romstage.c:

  • Patch Set #13, Line 4: * Copyright (C) 2019 Advanced Micro Devices, Inc.

    In romstage.h you placed this line after previous copyright, here you placed it before. […]

    This line wasn't added. The two copyrights used to be AMD 2015-2016 and Intel 2015. I can't look at this file and determine what Intel might wish to claim ownership of, but OTOH I'm not wishing to remove their name from the list. However, I was comfortable making the claim that 100% of the AMD work is relevant to 2019. So the change was to replace "2015-2016" with "2019".

    I prefer keeping AMD's name first. If you really want a change, I'll put the copyright back to 2015 again.

• File src/soc/amd/picasso/reset_vector.S:

  • Patch Set #9, Line 111: .byte 0x00, 0x9b, 0xaf, 0x00

    So far I haven't needed it bigger than 64K but I specifically gave myself the option. […]

    Done

• File src/soc/amd/picasso/romstage.c:

  • Patch Set #9, Line 49: || (CONFIG_DCACHE_RAM_BASE >= CONFIG_ROMSTAGE_ADDR \

    Comparisons should place the constant on the right side of the test

    OBE

To view, visit change 33759. To unsubscribe, or for help writing mail filters, visit settings.