[U-Boot] [PATCH v4 5/8] smbios: Generate type 4 on non-x86 systems
Simon Glass
sjg at chromium.org
Thu Aug 18 05:45:31 CEST 2016
Hi Alex,
On 17 August 2016 at 04:29, 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
>
> v4 -> v5:
>
> - s/get_info/get_vendor/ typo
> - s/smbios_write_type4_arch/smbios_write_type4_dm/
> ---
> 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(-)
This looks good to me, thanks.
Can you please split out the CPU uclass changes into a separate patch?
Also please see a few nits below.
>
> 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);
Instead of returning a pointer, can you use a buffer and size as with
get_desc()? Or is there a reason why this is worse? It does make
things inconsistent.
> +
> #endif /* _ASM_CPU_X86_H */
> diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c
> index 7660f99..b25c3b4 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->vendor)
> + 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];
Please add comments on these. Actually I see that device_id is missing
a comment, oops.
> };
>
> /* 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..6e8f00a 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_dm(struct smbios_type4 *t)
> +{
> + u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
> + const char *vendor = "Unknown";
> + const char *name = "Unknown";
> +
> +#ifdef CONFIG_CPU
> + char processor_name[49];
> + struct udevice *dev = NULL;
> +
> + uclass_find_first_device(UCLASS_CPU, &dev);
> + if (dev) {
> + struct cpu_platdata *plat = dev_get_parent_platdata(dev);
> +
> + if (plat->family)
> + processor_family = plat->family;
> + t->processor_id[0] = plat->id[0];
> + t->processor_id[1] = plat->id[1];
> +
> + cpu_get_vendor(dev, &vendor);
> + if (!cpu_get_desc(dev, processor_name, sizeof(processor_name)))
> + name = processor_name;
> + }
> +#endif
> +
> + t->processor_family = processor_family;
> + t->processor_manufacturer = smbios_add_string(t->eos, vendor);
> + t->processor_version = smbios_add_string(t->eos, name);
> +}
> +
> static int smbios_write_type4(uintptr_t *current, int handle)
> {
> struct smbios_type4 *t = (struct smbios_type4 *)*current;
> int len = sizeof(struct smbios_type4);
> - const char *vendor;
> - char *name;
> - char processor_name[CPU_MAX_NAME_LEN];
> - struct cpuid_result res;
>
> memset(t, 0, sizeof(struct smbios_type4));
> fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
> t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
> - t->processor_family = gd->arch.x86;
> - vendor = cpu_vendor_name(gd->arch.x86_vendor);
> - t->processor_manufacturer = smbios_add_string(t->eos, vendor);
> - res = cpuid(1);
> - t->processor_id[0] = res.eax;
> - t->processor_id[1] = res.edx;
> - name = cpu_get_name(processor_name);
> - t->processor_version = smbios_add_string(t->eos, name);
> + smbios_write_type4_dm(t);
> t->status = SMBIOS_PROCESSOR_STATUS_ENABLED;
> t->processor_upgrade = SMBIOS_PROCESSOR_UPGRADE_NONE;
> t->l1_cache_handle = 0xffff;
> --
> 1.8.5.6
>
Regards,
Simon
More information about the U-Boot
mailing list