linux-kernel - Re: [PATCH v2] soc: qcom: Add SoC info driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Oct 2016 22:05:44 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Imran Khan <kimran@...eaurora.org>,
        Lee Jones <lee.jones@...aro.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:ARM/QUALCOMM SUPPORT" <linux-arm-msm@...r.kernel.org>,
        "open list:ARM/QUALCOMM SUPPORT" <linux-soc@...r.kernel.org>
Subject: Re: [PATCH v2] soc: qcom: Add SoC info driver

On Thu, Oct 27, 2016 at 11:22 AM, Imran Khan <kimran@...eaurora.org> wrote:

> The SoC info driver provides information such as Chip ID,
> Chip family, serial number and other such details about
> Qualcomm SoCs.
>
> Signed-off-by: Imran Khan <kimran@...eaurora.org>

I like this patch. It is good to know stuff about the hardware.

(...)
> +++ b/drivers/soc/qcom/socinfo.c
(...)
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/sys_soc.h>

Uses <linux/sys_soc.h> so you should definately get Lee Jones
to review this.

> +/*
> + * Qcom SoC IDs
> + */
> +enum qcom_cpu_id {
> +       MSM_UNKNOWN_ID,
> +       MSM_8960_ID = 87,
> +       APQ_8064_ID = 109,
> +       MSM_8660A_ID = 122,
> +       MSM_8260A_ID,
> +       APQ_8060A_ID,
> +       MSM_8974_ID = 126,
> +       MPQ_8064_ID = 130,
> +       MSM_8960AB_ID = 138,
> +       APQ_8060AB_ID,
> +       MSM_8260AB_ID,
> +       MSM_8660AB_ID,
> +       APQ_8084_ID = 178,
> +       APQ_8074_ID = 184,
> +       MSM_8274_ID,
> +       MSM_8674_ID,
> +       MSM_8974PRO_ID = 194,
> +       MSM_8916_ID = 206,
> +       APQ_8074_AA_ID = 208,
> +       APQ_8074_AB_ID,
> +       APQ_8074PRO_ID,
> +       MSM_8274_AA_ID,
> +       MSM_8274_AB_ID,
> +       MSM_8274PRO_ID,
> +       MSM_8674_AA_ID,
> +       MSM_8674_AB_ID,
> +       MSM_8674PRO_ID,
> +       MSM_8974_AA_ID,
> +       MSM_8974_AB_ID,
> +       MSM_8996_ID = 246,
> +       APQ_8016_ID,
> +       MSM_8216_ID,
> +       MSM_8116_ID,
> +       MSM_8616_ID,
> +       APQ8096_ID = 291,
> +       MSM_8996SG_ID = 305,
> +       MSM_8996AU_ID = 310,
> +       APQ_8096AU_ID,
> +       APQ_8096SG_ID
> +};

Seems to support everything and its dog. Good job!

Now I guess I first have to support smem on my pet peeves
(MSM8660 and APQ8060) for this to work.

> +enum hw_platform_type {
> +       HW_PLATFORM_UNKNOWN,
> +       HW_PLATFORM_SURF,
> +       HW_PLATFORM_FFA,
> +       HW_PLATFORM_FLUID,
> +       HW_PLATFORM_SVLTE_FFA,
> +       HW_PLATFORM_SVLTE_SURF,
> +       HW_PLATFORM_MTP_MDM = 7,
> +       HW_PLATFORM_MTP,
> +       HW_PLATFORM_LIQUID,
> +       /* Dragonboard platform id is assigned as 10 in CDT */
> +       HW_PLATFORM_DRAGON,

I guess that is not "my" dragonboard generation 1 with the APQ8060
but rather the APQ8074 Dragonboard?

Is mine just a variant of SURF then... I dunno. Will be interesting
to see what number it has in its hardware.

> +#ifdef CONFIG_SOC_BUS

Why not just select that in Kconfig and get rid of this #ifdef?

> +int qcom_socinfo_init(void *info)
> +{
> +       socinfo = info;
> +
> +       socinfo_select_format();
> +
> +       WARN(!socinfo_get_id(), "Unknown SOC ID!\n");
> +
> +       WARN(socinfo_get_id() >= ARRAY_SIZE(cpu_of_id),
> +               "New IDs added! ID => CPU mapping needs an update.\n");
> +
> +       socinfo_print();
> +
> +       socinfo_init_sysfs();
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(qcom_socinfo_init);

So something I've been adding to drivers that handle any kind of
device-unique numbers is:

#include <linux/random.h>

/* Toss the unique data into the entropy pool */
add_device_randomness(buf, sizeof(buf));

Can you do this here in init() with the register that are *device unique*
because I guess not all of them are?

Yours,
Linus Walleij