Message ID | 20200518091222.27467-8-sudeep.holla@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | firmware: smccc: Add basic SMCCC v1.2 + ARCH_SOC_ID support | expand |
On Mon, May 18, 2020 at 11:12 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > +static ssize_t > +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version)); > +} > + > +static DEVICE_ATTR_RO(jep106_cont_bank_code); > + > +static ssize_t > +jep106_identification_code_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version)); > +} I think we should try hard to avoid nonstandard attributes for the soc device. Did you run into a problem with finding one of the existing attributes that can be used to hold the fields? Arnd
On Mon, May 18, 2020 at 11:30:21AM +0200, Arnd Bergmann wrote: > On Mon, May 18, 2020 at 11:12 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > +static ssize_t > > +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version)); > > +} > > + > > +static DEVICE_ATTR_RO(jep106_cont_bank_code); > > + > > +static ssize_t > > +jep106_identification_code_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version)); > > +} > > I think we should try hard to avoid nonstandard attributes for the soc device. > I agree with that in general but this is bit different for below mentioned reason. > Did you run into a problem with finding one of the existing attributes > that can be used to hold the fields? > Not really! The 2 JEP106 codes can be used to derive the manufacturer which could match one of the existing attributes. However doing so might require importing the huge JEP106 list as it needs to be maintained and updated in the kernel. Also that approach will have the compatibility issue and that is the reason for introducing these attributes representing raw values for userspace. -- Regards, Sudeep
On Mon, May 18, 2020 at 1:55 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Mon, May 18, 2020 at 11:30:21AM +0200, Arnd Bergmann wrote: > > On Mon, May 18, 2020 at 11:12 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > +static ssize_t > > > +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version)); > > > +} > > > + > > > +static DEVICE_ATTR_RO(jep106_cont_bank_code); > > > + > > > +static ssize_t > > > +jep106_identification_code_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version)); > > > +} > > > > I think we should try hard to avoid nonstandard attributes for the soc device. > > > > I agree with that in general but this is bit different for below mentioned > reason. > > > Did you run into a problem with finding one of the existing attributes > > that can be used to hold the fields? > > > > Not really! The 2 JEP106 codes can be used to derive the manufacturer which > could match one of the existing attributes. However doing so might require > importing the huge JEP106 list as it needs to be maintained and updated > in the kernel. Also that approach will have the compatibility issue and > that is the reason for introducing these attributes representing raw > values for userspace. I was thinking they codes could just be part of the normal strings rather than get translated. Can you give an example what they would look like with your current code? If you think they should be standard attributes, how about adding them to the default list, and hardcoding them in the other soc device drivers based on the information we have available there? Arnd
On Wed, May 20, 2020 at 11:51:47PM +0200, Arnd Bergmann wrote: > On Mon, May 18, 2020 at 1:55 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Mon, May 18, 2020 at 11:30:21AM +0200, Arnd Bergmann wrote: > > > On Mon, May 18, 2020 at 11:12 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > +static ssize_t > > > > +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr, > > > > + char *buf) > > > > +{ > > > > + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version)); > > > > +} > > > > + > > > > +static DEVICE_ATTR_RO(jep106_cont_bank_code); > > > > + > > > > +static ssize_t > > > > +jep106_identification_code_show(struct device *dev, > > > > + struct device_attribute *attr, char *buf) > > > > +{ > > > > + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version)); > > > > +} > > > > > > I think we should try hard to avoid nonstandard attributes for the soc device. > > > > > > > I agree with that in general but this is bit different for below mentioned > > reason. > > > > > Did you run into a problem with finding one of the existing attributes > > > that can be used to hold the fields? > > > > > > > Not really! The 2 JEP106 codes can be used to derive the manufacturer which > > could match one of the existing attributes. However doing so might require > > importing the huge JEP106 list as it needs to be maintained and updated > > in the kernel. Also that approach will have the compatibility issue and > > that is the reason for introducing these attributes representing raw > > values for userspace. > > I was thinking they codes could just be part of the normal strings rather > than get translated. Can you give an example what they would look like > with your current code? > Sure. Couple of example: Cont Code Identifier Manufacturer 0 0x1 AMD 0 0x0e Freescale (Motorola) 4 0x3b ARM I initially thought of value like "jep106-0-1" for AMD "jep-4-3b" for ARM,..etc for the standard attribute family or machine. But I was not convinced fully on that approach as it will be deviation from normal values in those attributes. Further this represents the vendor name rather than the family or machine. > If you think they should be standard attributes, how about adding them > to the default list, and hardcoding them in the other soc device drivers > based on the information we have available there? > That may be possible, I can take a look at the existing drivers and check if that is feasible(which I think should be). Thanks for that suggestion. -- Regards, Sudeep [1] https://github.com/skottler/memtest86/blob/master/jedec_id.h
diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig index 27b675d76235..15e7466179a6 100644 --- a/drivers/firmware/smccc/Kconfig +++ b/drivers/firmware/smccc/Kconfig @@ -14,3 +14,12 @@ config HAVE_ARM_SMCCC_DISCOVERY to add SMCCC discovery mechanism though the PSCI firmware implementation of PSCI_FEATURES(SMCCC_VERSION) which returns success on firmware compliant to SMCCC v1.1 and above. + +config ARM_SMCCC_SOC_ID + bool "SoC bus device for the ARM SMCCC SOC_ID" + depends on HAVE_ARM_SMCCC_DISCOVERY + default y + select SOC_BUS + help + Include support for the SoC bus on the ARM SMCCC firmware based + platforms providing some sysfs information about the SoC variant. diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile index 6f369fe3f0b9..72ab84042832 100644 --- a/drivers/firmware/smccc/Makefile +++ b/drivers/firmware/smccc/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 # obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o +obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c new file mode 100644 index 000000000000..7e59e95e1fd3 --- /dev/null +++ b/drivers/firmware/smccc/soc_id.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2020 Arm Limited + */ + +#define pr_fmt(fmt) "SMCCC: SOC_ID: " fmt + +#include <linux/arm-smccc.h> +#include <linux/bitfield.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/sys_soc.h> + +#define SMCCC_SOC_ID_JEP106_BANK_IDX_MASK GENMASK(30, 24) +/* + * As per the SMC Calling Convention specification v1.2 (ARM DEN 0028C) + * Section 7.4 SMCCC_ARCH_SOC_ID bits[23:16] are JEP-106 identification + * code with parity bit for the SiP. We can drop the parity bit. + */ +#define SMCCC_SOC_ID_JEP106_ID_CODE_MASK GENMASK(22, 16) +#define SMCCC_SOC_ID_IMP_DEF_SOC_ID_MASK GENMASK(15, 0) + +/* The bank index is equal to the for continuation code bank number - 1 */ +#define JEP106_BANK_CONT_CODE(x) \ + (u8)(FIELD_GET(SMCCC_SOC_ID_JEP106_BANK_IDX_MASK, (x)) + 1) +#define JEP106_ID_CODE(x) \ + (u8)(FIELD_GET(SMCCC_SOC_ID_JEP106_ID_CODE_MASK, (x))) +#define IMP_DEF_SOC_ID(x) \ + (u16)(FIELD_GET(SMCCC_SOC_ID_IMP_DEF_SOC_ID_MASK, (x))) + +static int soc_id_version; +static struct soc_device *soc_dev; +static struct soc_device_attribute *soc_dev_attr; + +static ssize_t +jep106_cont_bank_code_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + return sprintf(buf, "0x%02x\n", JEP106_BANK_CONT_CODE(soc_id_version)); +} + +static DEVICE_ATTR_RO(jep106_cont_bank_code); + +static ssize_t +jep106_identification_code_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "0x%02x\n", JEP106_ID_CODE(soc_id_version)); +} + +static DEVICE_ATTR_RO(jep106_identification_code); + +static struct attribute *jep106_id_attrs[] = { + &dev_attr_jep106_cont_bank_code.attr, + &dev_attr_jep106_identification_code.attr, + NULL +}; + +ATTRIBUTE_GROUPS(jep106_id); + +static int __init smccc_soc_init(void) +{ + struct device *dev; + int ret, soc_id_rev; + struct arm_smccc_res res; + static char soc_id_str[8], soc_id_rev_str[12]; + + if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2) + return 0; + + if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) { + pr_err("%s: invalid SMCCC conduit\n", __func__); + return -EOPNOTSUPP; + } + + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, + ARM_SMCCC_ARCH_SOC_ID, &res); + + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) { + pr_info("ARCH_SOC_ID not implemented, skipping ....\n"); + return 0; + } + + if ((int)res.a0 < 0) { + pr_info("ARCH_FEATURES(ARCH_SOC_ID) returned error: %lx\n", + res.a0); + return -EINVAL; + } + + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res); + if ((int)res.a0 < 0) { + pr_err("ARCH_SOC_ID(0) returned error: %lx\n", res.a0); + return -EINVAL; + } + + soc_id_version = res.a0; + + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res); + if ((int)res.a0 < 0) { + pr_err("ARCH_SOC_ID(1) returned error: %lx\n", res.a0); + return -EINVAL; + } + + soc_id_rev = res.a0; + + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL); + if (!soc_dev_attr) + return -ENOMEM; + + sprintf(soc_id_str, "0x%04x", IMP_DEF_SOC_ID(soc_id_version)); + sprintf(soc_id_rev_str, "0x%08x", soc_id_rev); + + soc_dev_attr->soc_id = soc_id_str; + soc_dev_attr->revision = soc_id_rev_str; + + soc_dev = soc_device_register(soc_dev_attr); + if (IS_ERR(soc_dev)) { + ret = PTR_ERR(soc_dev); + goto free_soc; + } + + dev = soc_device_to_device(soc_dev); + + ret = devm_device_add_groups(dev, jep106_id_groups); + if (ret) { + dev_err(dev, "sysfs create failed: %d\n", ret); + goto unregister_soc; + } + + pr_info("ID = %s Revision = %s\n", soc_dev_attr->soc_id, + soc_dev_attr->revision); + + return 0; + +unregister_soc: + soc_device_unregister(soc_dev); +free_soc: + kfree(soc_dev_attr); + return ret; +} +module_init(smccc_soc_init); + +static void __exit smccc_soc_exit(void) +{ + if (soc_dev) + soc_device_unregister(soc_dev); + kfree(soc_dev_attr); +} +module_exit(smccc_soc_exit); diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index c491d210e3c3..6510f1bfcb05 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -70,6 +70,11 @@ ARM_SMCCC_SMC_32, \ 0, 1) +#define ARM_SMCCC_ARCH_SOC_ID \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_SMC_32, \ + 0, 2) + #define ARM_SMCCC_ARCH_WORKAROUND_1 \ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_SMC_32, \