Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31567 )
Change subject: soc/cavium/cn81xx: Enable RNG for DRAM init ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c File src/soc/cavium/cn81xx/sdram.c:
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c@47 PS1, Line 47: #define BDK_RNM_RANDOM 0x100000 I'd remove indentation and move them outside function body. Their scope is the file (even if not used elsewhere). Seem to be register definitions so they often go in separate files entirely.
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c@50 PS1, Line 50: #ifdef __SIMPLE_DEVICE__ You should not need this, unless you really need to build this file for ramstage.
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c@51 PS1, Line 51: pci_devfn_t dev = pci_locate_device_on_bus(0xa018177d, 2); See comments above pci_locate_device() implementation. We tend to use PCI_DEV() here usually. So this hardware has PCI bus 2 somehow hardwired and routed without programming any PCI bridges?
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c@72 PS1, Line 72: write64(&bar[BDK_RNM_CTL_STATUS], reg); I prefer syntax with 'bar + reg_offset' but your choice.
https://review.coreboot.org/#/c/31567/1/src/soc/cavium/cn81xx/sdram.c@82 PS1, Line 82: printk(BIOS_DEBUG, "RNG: RANDOM %llx\n", reg); BIOS_SPEW?