Hi Raymond, On Tue, 3 Sept 2024 at 10:07, Raymond Mao <raymond....@linaro.org> wrote: > > Hi Simon, > > On Sat, 17 Aug 2024 at 11:58, Simon Glass <s...@chromium.org> wrote: >> >> Hi Raymond, >> >> On Fri, 16 Aug 2024 at 09:47, Raymond Mao <raymond....@linaro.org> wrote: >> > >> > Motivations for changes: >> > Current SMBIOS library and command-line tool is not fully matching with >> > the requirements: >> > 1. Missing support for other mandatory types (#7, #9, #16, #17, #19). >> > 2. Only a few platforms support SMBIOS node from the device tree. >> > 3. Values of some fields are hardcoded in the library other than fetching >> > from the device hardware. >> > 4. Embedded data with dynamic length is not supported (E.g. Contained >> > Object Handles in Type #2 and Contained Elements in Type #3) >> > >> > Changes: >> > 1. Refactor the SMBIOS library and command-line tool to better align with >> > the SMBIOS spec. >> > 2. Create an arch-specific driver for all aarch64-based platforms to fetch >> > SMBIOS private data from the device hardware. >> > 3. Create a sysinfo driver to poppulate platform SMBIOS private data. >> > 4. Put device tree SMBIOS node as a fallback solution when SMBIOS data is >> > missing from sysinfo driver. >> > 5. 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
