Hi Raymond, On Tue, 10 Sept 2024 at 13:20, Raymond Mao <raymond....@linaro.org> wrote: > > Hi Simon, > > On Tue, 10 Sept 2024 at 14:44, Simon Glass <s...@chromium.org> wrote: >> >> 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? >> > What I planned to do is to have a general one for arm64, then all the arm64 > boards missing the dt > node can leverage it. > Regarding to 2) you pointed out, maybe we can leave them to the dt later, my > patch can go first with > those architectural common stuffs, for example, cache information. > I will think about it before posting the next version.
OK. Let's try to avoid tables of strings. Regards, Simon
