[U-Boot] [PATCH v4 5/8] smbios: Generate type 4 on non-x86 systems

Alexander Graf agraf at suse.de
Wed Aug 17 12:25:38 CEST 2016


> On 17 Aug 2016, at 11:20, Bin Meng <bmeng.cn at gmail.com> wrote:
> 
> Hi Alex,
> 
> On Wed, Aug 17, 2016 at 4:31 PM, Alexander Graf <agraf at suse.de> wrote:
>> The type 4 table generation code is very x86 centric today. Refactor things
>> out into the device model cpu class to allow the tables to get generated for
>> other architectures as well.
>> 
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>> 
>> ---
>> 
>> v3 -> v4:
>> 
>>  - Use device model
>> ---
>> arch/x86/cpu/baytrail/cpu.c          |  1 +
>> arch/x86/cpu/broadwell/cpu.c         |  1 +
>> arch/x86/cpu/cpu_x86.c               | 13 ++++++++++
>> arch/x86/cpu/ivybridge/model_206ax.c |  1 +
>> arch/x86/include/asm/cpu_x86.h       | 12 +++++++++
>> drivers/cpu/cpu-uclass.c             | 10 ++++++++
>> include/cpu.h                        | 20 +++++++++++++++
>> include/smbios.h                     |  3 +++
>> lib/smbios.c                         | 49 ++++++++++++++++++++++++++----------
>> 9 files changed, 97 insertions(+), 13 deletions(-)
>> 
>> diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c
>> index b1faf8c..c454db0 100644
>> --- a/arch/x86/cpu/baytrail/cpu.c
>> +++ b/arch/x86/cpu/baytrail/cpu.c
>> @@ -141,6 +141,7 @@ static const struct cpu_ops cpu_x86_baytrail_ops = {
>>        .get_desc       = cpu_x86_get_desc,
>>        .get_info       = baytrail_get_info,
>>        .get_count      = baytrail_get_count,
>> +       .get_vendor     = cpu_x86_get_vendor,
>> };
>> 
>> static const struct udevice_id cpu_x86_baytrail_ids[] = {
>> diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c
>> index 3ba21aa..6977e86 100644
>> --- a/arch/x86/cpu/broadwell/cpu.c
>> +++ b/arch/x86/cpu/broadwell/cpu.c
>> @@ -743,6 +743,7 @@ static const struct cpu_ops cpu_x86_broadwell_ops = {
>>        .get_desc       = cpu_x86_get_desc,
>>        .get_info       = broadwell_get_info,
>>        .get_count      = broadwell_get_count,
>> +       .get_vendor     = cpu_x86_get_vendor,
>> };
>> 
>> static const struct udevice_id cpu_x86_broadwell_ids[] = {
>> diff --git a/arch/x86/cpu/cpu_x86.c b/arch/x86/cpu/cpu_x86.c
>> index 0941041..3b0458d 100644
>> --- a/arch/x86/cpu/cpu_x86.c
>> +++ b/arch/x86/cpu/cpu_x86.c
>> @@ -15,9 +15,21 @@ DECLARE_GLOBAL_DATA_PTR;
>> int cpu_x86_bind(struct udevice *dev)
>> {
>>        struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>> +       struct cpuid_result res;
>> 
>>        plat->cpu_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>                                      "intel,apic-id", -1);
>> +       plat->family = gd->arch.x86;
>> +       res = cpuid(1);
>> +       plat->id[0] = res.eax;
>> +       plat->id[1] = res.edx;
>> +
>> +       return 0;
>> +}
>> +
>> +int cpu_x86_get_vendor(struct udevice *dev, const char **vendor)
>> +{
>> +       *vendor = cpu_vendor_name(gd->arch.x86_vendor);
>> 
>>        return 0;
>> }
>> @@ -60,6 +72,7 @@ static int cpu_x86_get_count(struct udevice *dev)
>> static const struct cpu_ops cpu_x86_ops = {
>>        .get_desc       = cpu_x86_get_desc,
>>        .get_count      = cpu_x86_get_count,
>> +       .get_vendor     = cpu_x86_get_vendor,
>> };
>> 
>> static const struct udevice_id cpu_x86_ids[] = {
>> diff --git a/arch/x86/cpu/ivybridge/model_206ax.c b/arch/x86/cpu/ivybridge/model_206ax.c
>> index 38e244b..d8eb2e7 100644
>> --- a/arch/x86/cpu/ivybridge/model_206ax.c
>> +++ b/arch/x86/cpu/ivybridge/model_206ax.c
>> @@ -478,6 +478,7 @@ static const struct cpu_ops cpu_x86_model_206ax_ops = {
>>        .get_desc       = cpu_x86_get_desc,
>>        .get_info       = model_206ax_get_info,
>>        .get_count      = model_206ax_get_count,
>> +       .get_vendor     = cpu_x86_get_vendor,
>> };
>> 
>> static const struct udevice_id cpu_x86_model_206ax_ids[] = {
>> diff --git a/arch/x86/include/asm/cpu_x86.h b/arch/x86/include/asm/cpu_x86.h
>> index 1940480..79aaf2d 100644
>> --- a/arch/x86/include/asm/cpu_x86.h
>> +++ b/arch/x86/include/asm/cpu_x86.h
>> @@ -31,4 +31,16 @@ int cpu_x86_bind(struct udevice *dev);
>>  */
>> int cpu_x86_get_desc(struct udevice *dev, char *buf, int size);
>> 
>> +/**
>> + * cpu_x86_get_vendor() - Get a vendor string for an x86 CPU
>> + *
>> + * This uses cpu_vendor_name() and is suitable to use as the get_vendor()
>> + * method for the CPU uclass.
>> + *
>> + * @dev:       Device to check (UCLASS_CPU)
>> + * @vendor:    Pointer to vendor string pointer
>> + * @return:    0 if OK, -ve on error
>> + */
>> +int cpu_x86_get_vendor(struct udevice *dev, const char **vendor);
>> +
>> #endif /* _ASM_CPU_X86_H */
>> diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c
>> index 7660f99..82a059f 100644
>> --- a/drivers/cpu/cpu-uclass.c
>> +++ b/drivers/cpu/cpu-uclass.c
>> @@ -44,6 +44,16 @@ int cpu_get_count(struct udevice *dev)
>>        return ops->get_count(dev);
>> }
>> 
>> +int cpu_get_vendor(struct udevice *dev, const char **vendor)
>> +{
>> +       struct cpu_ops *ops = cpu_get_ops(dev);
>> +
>> +       if (!ops->get_info)
> 
> This should be: if (!ops->get_vendor)

Oops, thanks :).

> 
>> +               return -ENOSYS;
>> +
>> +       return ops->get_vendor(dev, vendor);
>> +}
>> +
>> U_BOOT_DRIVER(cpu_bus) = {
>>        .name   = "cpu_bus",
>>        .id     = UCLASS_SIMPLE_BUS,
>> diff --git a/include/cpu.h b/include/cpu.h
>> index bda5315..2285aee 100644
>> --- a/include/cpu.h
>> +++ b/include/cpu.h
>> @@ -21,6 +21,8 @@ struct cpu_platdata {
>>        int cpu_id;
>>        int ucode_version;
>>        ulong device_id;
>> +       u16 family;
>> +       u32 id[2];
>> };
>> 
>> /* CPU features - mostly just a placeholder for now */
>> @@ -71,6 +73,15 @@ struct cpu_ops {
>>         * @return CPU count if OK, -ve on error
>>         */
>>        int (*get_count)(struct udevice *dev);
>> +
>> +       /**
>> +        * get_vendor() - Get vendor name of a CPU
>> +        *
>> +        * @dev:        Device to check (UCLASS_CPU)
>> +        * @vendor:     Pointer to put the pointer to the vendor string
>> +        * @return 0 if OK, -ve on error
>> +        */
>> +       int (*get_vendor)(struct udevice *dev, const char **vendor);
>> };
>> 
>> #define cpu_get_ops(dev)        ((struct cpu_ops *)(dev)->driver->ops)
>> @@ -102,4 +113,13 @@ int cpu_get_info(struct udevice *dev, struct cpu_info *info);
>>  */
>> int cpu_get_count(struct udevice *dev);
>> 
>> +/**
>> + * cpu_get_vendor() - Get vendor name of a CPU
>> + *
>> + * @dev:       Device to check (UCLASS_CPU)
>> + * @vendor:    Pointer to put the pointer to the vendor string
>> + * @return 0 if OK, -ve on error
>> + */
>> +int cpu_get_vendor(struct udevice *dev, const char **vendor);
>> +
>> #endif
>> diff --git a/include/smbios.h b/include/smbios.h
>> index 5962d4c..3cbc687 100644
>> --- a/include/smbios.h
>> +++ b/include/smbios.h
>> @@ -139,6 +139,9 @@ struct __packed smbios_type3 {
>> #define SMBIOS_PROCESSOR_STATUS_ENABLED        1
>> #define SMBIOS_PROCESSOR_UPGRADE_NONE  6
>> 
>> +#define SMBIOS_PROCESSOR_FAMILY_OTHER  1
>> +#define SMBIOS_PROCESSOR_FAMILY_UNKNOWN        2
>> +
>> struct __packed smbios_type4 {
>>        u8 type;
>>        u8 length;
>> diff --git a/lib/smbios.c b/lib/smbios.c
>> index 8dfd486..087d314 100644
>> --- a/lib/smbios.c
>> +++ b/lib/smbios.c
>> @@ -10,7 +10,11 @@
>> #include <smbios.h>
>> #include <tables_csum.h>
>> #include <version.h>
>> -#include <asm/cpu.h>
>> +#ifdef CONFIG_CPU
>> +#include <cpu.h>
>> +#include <dm.h>
>> +#include <dm/uclass-internal.h>
>> +#endif
>> 
>> DECLARE_GLOBAL_DATA_PTR;
>> 
>> @@ -152,26 +156,45 @@ static int smbios_write_type3(uintptr_t *current, int handle)
>>        return len;
>> }
>> 
>> +static void smbios_write_type4_arch(struct smbios_type4 *t)
> 
> Since now we use cpu uclass to implement this, I don't think
> smbios_write_type4_arch() is a proper name, as _arch may indicate this
> is architecture dependent, however it is not. The whole codes in this
> function can be put back in the original smbios_write_type4().

It certainly exceeds the number of lines that I would put into a single function if I put it all back. Plus variable declaration becomes much more intransparent.

But I can rename it to smbios_write_type4_dm if you like?


Alex



More information about the U-Boot mailing list