On 09.12.2019 05:36, Matt B wrote:
> Hi,
Hi Matt,
>
> Thanks for the pointer.
>
> I need a bit more context here, being completely unfamiliar with how
> coreboot works.
> I've never done any non-userspace programming, and this is the first
> time I've gotten a freshly-compiled coreboot to actually boot.
> I do have most of a degree in computer engineering, but that's
> unfortunately 99% theory with little-to-no hands-on experience.
>
> A) If I understand correctly, [1] is the complete change (not the last
> commit in a series for this board) to remove romcc usage from
> asrock/imb-a180.
> The same changes need to be applied to asus/am1i-a.
>
> B) If I understand correctly, the necessary family16h changes have
> been made elsewhere, so only changes to the mainboard-specific code
> need to be made.
> This would explain why there is a net removal of code, not addition.
>
> C) I can't say that I really understand the contents of this file. Can
> someone explain how
>
>     /* Set LPC decode enables. */
>     pci_devfn_t dev = PCI_DEV(0, 0x14, 3);
>     pci_write_config32(dev, 0x44, 0xff03ffd5);
>
> and 
>
>     /* Enable the AcpiMmio space */
>     outb(0x24, 0xcd6);
>     outb(0x1, 0xcd7);
>
> and 
>
>     /* Disable PCI-PCI bridge and release GPIO32/33 for other uses. */
>     outb(0xea, 0xcd6);
>     outb(0x1, 0xcd7);
>
> becomes
>
>     /* Disable PCI-PCI bridge and release GPIO32/33 for other uses. */
>     pm_write8(0xea, 0x1);
>
> ?
We did a lot of cleanup with Kyösti and unified implementations of LPC
decode enables and ACPIMMIO. Those instructions you point to were moved
to southbridge bootblock initialization, because it is not mainboard
specific. Also pm_write8(0xea, 0x1); does the same thing as outb(0xea,
0xcd6); and outb(0x1, 0xcd7); but in more readable form. See:
https://review.coreboot.org/c/coreboot/+/37177
https://review.coreboot.org/c/coreboot/+/37329/
https://review.coreboot.org/c/coreboot/+/37168/
>
> D) I'm a bit confused about what [2] is. Is this an earlier, unrefined
> version of the changes?
>
> I don't think that a naive transform would be wise, as there is a lot
> more stuff going on in [3] :P
> For instance:
> 1) would
>
>     /* In Hudson RRG, PMIOxD2[5:4] is "Drive strength control for
>     * LpcClk[1:0]".  To be consistent with Parmer, setting to 4mA
>     * even though the register is not documented in the Kabini BKDG.
>     * Otherwise the serial output is bad code.
>     */
>     outb(0xD2, 0xcd6);
>     outb(0x00, 0xcd7);
>
> and
This was also moved to southbridge bootblock initialization - not
mainboard specific.
>
>     /* Disable PCI-PCI bridge and release GPIO32/33 for other uses. */
>     outb(0xEA, 0xcd6);
>     outb(0x1, 0xcd7);
>
> be transformed similarly to how
>
>      /* Disable PCI-PCI bridge and release GPIO32/33 for other uses. */
>     outb(0xea, 0xcd6);
>     outb(0x1, 0xcd7);
>
coreboot uses lowercase for hex values. This should be transformed to
pm_write8(0xea, 1).
> was? 
> 2) would
>
>     /* Set LPC decode enables. */
>     pci_devfn_t dev2 = PCI_DEV(0, 0x14, 3);
>     pci_write_config32(dev2, 0x44, 0xff03ffd5);
>
> and
>
>     /* Enable the AcpiMmio space */
>     outb(0x24, 0xcd6);
>     outb(0x1, 0xcd7);
>
> completely disappear?
> what about
>
>     hudson_lpc_port80();
>
> in the middle?
> 4) Would
>
>     /* Configure ClkDrvStr1 settings */
>     addr32 = (u32 *)0xfed80e24;
>     *addr32 = 0x030800aa;
>
>     /* Configure MiscClkCntl1 settings */
>     addr32 = (u32 *)0xfed80e40;
>     *addr32 = 0x000c4050;
>     /* enable SIO LPC decode */
>     dev = PCI_DEV(0, 0x14, 3);
>     byte = pci_read_config8(dev, 0x48);
>     byte |= 3;/* 2e, 2f & 4e, 4f */
>     pci_write_config8(dev, 0x48, byte);
>     ite_gpio_conf(GPIO_DEV);
>     ite_evc_conf(ENVC_DEV);
>     ite_conf_clkin(CLKIN_DEV, ITE_UART_CLK_PREDIVIDE_48);
>     ite_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
>     ite_kill_watchdog(GPIO_DEV);
>     /*
>     * On Larne, after LpcClkDrvSth is set, it needs some time to be
>     stable,
>     * because of the buffer ICS551M
>     */
>     for (i = 0; i < 200000; i++)
>     val = inb(0xcd6);
>
> remain unchanged?
It should be something like:

/* Configure ClkDrvStr1 settings */
misc_write32(0x24, 0x030800aa);
/* Configure MiscClkCntl1 settings */
misc_write32(0x40, 0x000c4050);
ite_gpio_conf(GPIO_DEV);
ite_evc_conf(ENVC_DEV);
ite_conf_clkin(CLKIN_DEV, ITE_UART_CLK_PREDIVIDE_48);
ite_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
ite_kill_watchdog(GPIO_DEV);
/*
* On Larne, after LpcClkDrvSth is set, it needs some time to be stable,
* because of the buffer ICS551M
*/
for (i = 0; i < 200000; i++)
val = inb(0xcd6);

The rest is already done in southbridge bootblock init. The file with
modifications should include
#include <amdblocks/acpimmio.h>

Regards,

-- 
Michał Żygowski
Firmware Engineer
http://3mdeb.com | @3mdeb_com

>
> Sincerely,
>     -Matt
>
> [1] https://review.coreboot.org/c/coreboot/+/37453/9
> [2] https://review.coreboot.org/c/coreboot/+/37440/10
> [3] src/mainboard/asus/am1i-a/romstage.c (for
> reference: https://pastebin.com/kSka2YL7)
>
>
> On Sun, Dec 8, 2019 at 6:57 PM Kyösti Mälkki <[email protected]
> <mailto:[email protected]>> wrote:
>
>     On Mon, Dec 9, 2019 at 12:32 AM Matt B <[email protected]
>     <mailto:[email protected]>> wrote:
>     >
>     > Question specifically for Kyösti Mälkki or those similarly
>     knowledgeable:
>     >
>     > My current .config seems to say I'm still using romcc.
>
>     Please read the recent thread "AMD AGESA board removals".
>
>     > I assume I've pulled the latest ("git clone
>     https://review.coreboot.org/coreboot"; as of a couple days ago) so
>     would those changes you mentioned be present?
>     > If so, is there an option to test the C bootblock in the menu
>     somewhere?
>
>     There will be no option as romcc will be gone for good. You should do
>     a checkout on commit f66da11 [1], make similar changes to asus/am1i-a,
>     and push your work for review. I have done quick boottest on
>     asrock/imb-a180 with commit ca150dc [2] and it got past the made
>     bootblock changes.
>
>     [1] https://review.coreboot.org/c/coreboot/+/37453/9
>     [2] https://review.coreboot.org/c/coreboot/+/37440/10
>
>     Kyösti
>
>
> _______________________________________________
> coreboot mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to