Hi Raymond,
On Mon, 28 Oct 2024 at 20:44, Raymond Mao <raymond....@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 13:04, Simon Glass <s...@chromium.org> wrote:
>>
>> Hi Raymond,
>>
>> On Tue, 22 Oct 2024 at 22:06, Raymond Mao <raymond....@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....@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
