[PATCH v2 3/8] sysinfo: Add sysinfo driver and data structure for smbios

[Simon Glass]

Hi Raymond,

On Mon, 28 Oct 2024 at 20:44, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 13:04, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Raymond,
>>
>> On Tue, 22 Oct 2024 at 22:06, Raymond Mao <raymond.mao at linaro.org> wrote:
>> >
>> > Add sysinfo driver to retrieve smbios information (Type 4 and 7).
>> > So that the smbios library can use it for getting values from the
>> > hardware platform instead of device tree.
>> >
>> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
>> > ---
>> > Changes in v2
>> > - Move the changes to smbios.c instead of creating new file.
>> > - Move the headfile to include dir.
>> > - Combine with #6(v1) patch.
>> > - Clean-up the private data structures.
>> > - Clean-up the operations of the strings and common values.
>> >
>> >  drivers/sysinfo/smbios.c | 228 +++++++++++++++++++++++++++++++++++++++
>> >  include/smbios.h         |  60 +++++++++++
>> >  include/smbios_plat.h    |  79 ++++++++++++++
>> >  include/sysinfo.h        |  95 +++++++++++++++-
>> >  4 files changed, 461 insertions(+), 1 deletion(-)
>> >  create mode 100644 include/smbios_plat.h
>> >
>> > diff --git a/drivers/sysinfo/smbios.c b/drivers/sysinfo/smbios.c
>> > index a7ac8e3f072..3980845b3ba 100644
>> > --- a/drivers/sysinfo/smbios.c
>> > +++ b/drivers/sysinfo/smbios.c
>> > @@ -5,14 +5,240 @@
>> >   */
>> >
>> >  #include <dm.h>
>> > +#include <smbios_plat.h>
>> >  #include <sysinfo.h>
>> >
>> > +/* platform information storage */
>> > +struct processor_info processor_info;
>> > +struct cache_info cache_info[SYSINFO_CACHE_LVL_MAX];
>> > +struct sysinfo_plat sysinfo_smbios_p = {
>> > +       /* Processor Information */
>> > +       .processor = &processor_info,
>> > +       /* Cache Information */
>> > +       .cache = &cache_info[0],
>> > +};
>> > +
>> > +/* structure for smbios private data storage */
>> > +struct sysinfo_plat_priv {
>> > +       struct processor_info *t4;
>> > +       struct smbios_type7 t7[SYSINFO_CACHE_LVL_MAX];
>> > +       u16 cache_handles[SYSINFO_CACHE_LVL_MAX];
>> > +       u8 cache_level;
>> > +};
>> > +
>> > +static void smbios_cache_info_dump(struct smbios_type7 *cache_info)
>> > +{
>> > +       log_debug("SMBIOS Type 7 (Cache Information):\n");
>> > +       log_debug("Cache Configuration: 0x%04x\n", cache_info->config.data);
>> > +       log_debug("Maximum Cache Size: %u KB\n", cache_info->max_size.data);
>> > +       log_debug("Installed Size: %u KB\n", cache_info->inst_size.data);
>> > +       log_debug("Supported SRAM Type: 0x%04x\n",
>>
>> %#04x
>>
>> > +                 cache_info->supp_sram_type.data);
>> > +       log_debug("Current SRAM Type: 0x%04x\n",
>> > +                 cache_info->curr_sram_type.data);
>> > +       log_debug("Cache Speed: %u\n", cache_info->speed);
>> > +       log_debug("Error Correction Type: %u\n", cache_info->err_corr_type);
>> > +       log_debug("System Cache Type: %u\n", cache_info->sys_cache_type);
>> > +       log_debug("Associativity: %u\n", cache_info->associativity);
>> > +       log_debug("Maximum Cache Size 2: %u KB\n", cache_info->max_size2.data);
>> > +       log_debug("Installed Cache Size 2: %u KB\n",
>> > +                 cache_info->inst_size2.data);
>> > +}
>> > +
>> > +/* weak function for the platforms not yet supported */
>> > +__weak int sysinfo_get_cache_info(u8 level, struct cache_info *cache_info)
>> > +{
>> > +       return -ENOSYS;
>> > +}
>> > +
>> > +__weak int sysinfo_get_processor_info(struct processor_info *pinfo)
>> > +{
>> > +       return -ENOSYS;
>> > +}
>> > +
>> > +void sysinfo_cache_info_default(struct cache_info *ci)
>> > +{
>> > +       memset(ci, 0, sizeof(*ci));
>> > +       ci->config.data = SMBIOS_CACHE_LOCATE_UNKNOWN | SMBIOS_CACHE_OP_UND;
>> > +       ci->supp_sram_type.fields.unknown = 1;
>> > +       ci->curr_sram_type.fields.unknown = 1;
>> > +       ci->speed = SMBIOS_CACHE_SPEED_UNKNOWN;
>> > +       ci->err_corr_type = SMBIOS_CACHE_ERRCORR_UNKNOWN;
>> > +       ci->cache_type = SMBIOS_CACHE_SYSCACHE_TYPE_UNKNOWN;
>> > +}
>> > +
>> > +static int sysinfo_plat_detect(struct udevice *dev)
>> > +{
>> > +       return 0;
>> > +}
>> > +
>> > +static int sysinfo_plat_get_str(struct udevice *dev, int id,
>> > +                               size_t size, char *val)
>> > +{
>> > +       struct sysinfo_plat_priv *priv = dev_get_priv(dev);
>> > +       const char *str = NULL;
>> > +
>> > +       switch (id) {
>> > +       case SYSINFO_ID_SMBIOS_PROCESSOR_MANUFACT:
>>
>> These are getting too long.
>>
>> How about SYSINFOSM_PROC_MANUF ?
>>
>> We can use SYSINFOSM as short for SYSINFO_ID_SMBIOS
>>
> Other existing macros are starting from SYSINFO_ID_, so I guess something
> reasonable can be SYSINFO_ID_SMB_PROC_MANU.

That is certainly better. Perhaps SYSINFO_ID_ should become SYSID_...?

[..]

Regards,
Simon