Attention is currently required from: Christian Walter, Jonathan Zhang, Krystian Hebel, Martin L Roth, Martin Roth, Michał Żygowski, Paul Menzel.
Sergii Dmytruk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/67057?usp=email )
Change subject: drivers/ipmi: add Block Transfer (BT) interface ......................................................................
Patch Set 34:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/67057/comment/483cca6c_62778661?usp... : PS33, Line 14:
How did you test this? Can it be tested with QEMU? Maybe mention the datasheet name?
Updated the comment. QEMU does support some BMCs and looks like it should be possible to enable one along with the main CPU ([pdf](https://www.linux-kvm.org/images/7/76/03x08-Juniper-Corey_Minyard-UsingIPMIi...)) but I tested the implementation on Talos II system where a watchdog needed to be set and KCS isn't supported.
File Documentation/drivers/index.md:
https://review.coreboot.org/c/coreboot/+/67057/comment/5f750540_37da21e0?usp... : PS33, Line 15: BT
Include Block Transfer?
Added in parenthesis.
File Documentation/drivers/ipmi_bt.md:
https://review.coreboot.org/c/coreboot/+/67057/comment/acc4c60b_fead1184?usp... : PS33, Line 14: ``` : chip drivers/ipmi : device pnp e4.0 on end # IPMI BT : end : ```
I’d prefer intending by four spaces. But feel free to mark as resolved.
Edited. It was formatting that I preserved from KCS documentation.
https://review.coreboot.org/c/coreboot/+/67057/comment/a1a58e52_d839891e?usp... : PS33, Line 22: The following registers can be set:
Where can they be set?
In a device tree. Updated.
File src/drivers/ipmi/Kconfig:
https://review.coreboot.org/c/coreboot/+/67057/comment/6e0eb4e7_ccd0a608?usp... : PS33, Line 85: for implementation
*an* implementation? […]
Done
File src/drivers/ipmi/ipmi_bt_ops.c:
https://review.coreboot.org/c/coreboot/+/67057/comment/130d93a0_efcac008?usp... : PS33, Line 103: printk(BIOS_ERR, "%s: Unsupported device type\n",
Print both values?
Done
https://review.coreboot.org/c/coreboot/+/67057/comment/9d358a99_82443e7a?usp... : PS33, Line 106: printk(BIOS_ERR, "%s: Base address needs to be aligned to 4\n",
Print the address?
Done