Hi Simon,
On Tue, 10 Sept 2024 at 14:44, Simon Glass sjg@chromium.org wrote:
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. 4. 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. 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:
- 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.
Regards, Raymond