Hi Raymond,
On Tue, 3 Sept 2024 at 10:07, Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Sat, 17 Aug 2024 at 11:58, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, 16 Aug 2024 at 09:47, Raymond Mao raymond.mao@linaro.org wrote:
Motivations for changes: Current SMBIOS library and command-line tool is not fully matching with the requirements:
- Missing support for other mandatory types (#7, #9, #16, #17, #19).
- Only a few platforms support SMBIOS node from the device tree.
- Values of some fields are hardcoded in the library other than fetching from the device hardware.
- Embedded data with dynamic length is not supported (E.g. Contained Object Handles in Type #2 and Contained Elements in Type #3)
Changes:
- Refactor the SMBIOS library and command-line tool to better align with the SMBIOS spec.
- Create an arch-specific driver for all aarch64-based platforms to fetch SMBIOS private data from the device hardware.
- Create a sysinfo driver to poppulate platform SMBIOS private data.
- Put device tree SMBIOS node as a fallback solution when SMBIOS data is missing from sysinfo driver.
- Add support for Type #7 (Cache Information) and link its handles to Type #4.
Once this patch is acceptted, subsequent patch sets will add other missing types (#9, #16, #17, #19).
Raymond Mao (10): sysinfo: Add sysinfo API for accessing data area sysinfo: Add sysinfo driver and data structure for SMBIOS smbios: Refactor SMBIOS library smbios: ignore the non-existence of platform sysinfo detect armv8: Add arch-specific sysinfo driver sysinfo: Add sysinfo driver for SMBIOS type 7 smbios: Add support to SMBIOS type 7 armv8: Add sysinfo driver for cache information configs: Enable sysinfo for QEMU Arm64 tests: update smbios pytest
arch/arm/cpu/armv8/Makefile | 5 + arch/arm/cpu/armv8/sysinfo.c | 391 ++++++++++++++++++++++++++ cmd/smbios.c | 350 ++++++++++++++++++++++- configs/qemu_arm64_defconfig | 2 + drivers/misc/Kconfig | 2 +- drivers/sysinfo/Makefile | 1 + drivers/sysinfo/smbios_plat.c | 442 +++++++++++++++++++++++++++++ drivers/sysinfo/smbios_plat.h | 131 +++++++++ drivers/sysinfo/sysinfo-uclass.c | 20 ++ include/smbios.h | 240 ++++++++++++++-- include/sysinfo.h | 124 ++++++++- lib/Makefile | 2 + lib/smbios.c | 461 ++++++++++++++++++++++++++----- test/py/tests/test_smbios.py | 2 +- 14 files changed, 2058 insertions(+), 115 deletions(-) create mode 100644 arch/arm/cpu/armv8/sysinfo.c create mode 100644 drivers/sysinfo/smbios_plat.c create mode 100644 drivers/sysinfo/smbios_plat.h
-- 2.25.1
As a general comment, this is adding a load of code which is used by a lot of platforms. As more and more aarch64 platforms are created, this data grows. Why not use the devicetree for this hardware information? That is what it is for.
Some of the information detected makes sense, such as cache setup, but some of it seems like an approximation, or is missing, but suggests it is authoritative.
The idea is that precise information can still come from dt (if the node exists, but as a fact, not many platforms have it up to now). When it does not exist, system drivers provides the information as much as it can (some comes from registers, some comes from generic strings/enums). So it is not assumed that each vendor to add their code but just uses the arch-specific code in this series - if vendors want precise information they can still add into the dt.
I fully understand what you are doing. I just don't think it is a great idea. We should have a clear boundary between:
1. things which are part of the hardware (although not explicitly in the devicetree) and can be probed 2. things which we can only guess at 3. grey area
I am very happy with 1). It is just 2) that I want to avoid.
The grey area is where your series adds lots of strings for different hardware...that just seems like code-size bloat to me.
How about working on a devicetree binding for this stuff? Or perhaps add the info to the boards you care about?
Regards, Simon