* [PATCH 0/2] base: soc: Add JEP106 manufacturer's identification code @ 2020-05-22 12:49 Sudeep Holla 2020-05-22 12:49 ` [PATCH 1/2] base: soc: Add JEDEC JEP106 manufacturer's identification code attribute Sudeep Holla 2020-05-22 12:49 ` [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support Sudeep Holla 0 siblings, 2 replies; 10+ messages in thread From: Sudeep Holla @ 2020-05-22 12:49 UTC (permalink / raw) To: linux-arm-kernel Cc: Mark Rutland, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon Hi, While attempting to add custom sysfs attributes for SMCCC ARCH_SOC_ID Arnd suggested to make it generic. Here is my first attempt. The original thread/discussion can be found here[1] Regards, Sudeep [1] https://lore.kernel.org/r/CAK8P3a3dV0B26XE3oFQGTFf8EWV0AHoLudNtpSSB_t+pCfkOkQ@mail.gmail.com/ Sudeep Holla (2): base: soc: Add JEDEC JEP106 manufacturer's identification code attribute firmware: smccc: Add ARCH_SOC_ID support Documentation/ABI/testing/sysfs-devices-soc | 31 ++++++ drivers/base/soc.c | 12 +++ drivers/firmware/smccc/Kconfig | 9 ++ drivers/firmware/smccc/Makefile | 1 + drivers/firmware/smccc/soc_id.c | 113 ++++++++++++++++++++ include/linux/arm-smccc.h | 5 + include/linux/sys_soc.h | 1 + 7 files changed, 172 insertions(+) create mode 100644 drivers/firmware/smccc/soc_id.c -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] base: soc: Add JEDEC JEP106 manufacturer's identification code attribute 2020-05-22 12:49 [PATCH 0/2] base: soc: Add JEP106 manufacturer's identification code Sudeep Holla @ 2020-05-22 12:49 ` Sudeep Holla 2020-05-22 12:49 ` [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support Sudeep Holla 1 sibling, 0 replies; 10+ messages in thread From: Sudeep Holla @ 2020-05-22 12:49 UTC (permalink / raw) To: linux-arm-kernel Cc: Mark Rutland, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon SMCCC v1.2 adds a new optional function SMCCC_ARCH_SOC_ID to obtain a SiP defined SoC identification value. Indeed of making it custom attribute, let us add the same as generic attribute to soc_device. There are various ways in which it can be represented in shortened form for efficiency and ease of parsing for userspace. The chosen form is described in the ABI document. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- Documentation/ABI/testing/sysfs-devices-soc | 31 +++++++++++++++++++++ drivers/base/soc.c | 12 ++++++++ include/linux/sys_soc.h | 1 + 3 files changed, 44 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-devices-soc b/Documentation/ABI/testing/sysfs-devices-soc index ba3a3fac0ee1..fd44c9b1e09a 100644 --- a/Documentation/ABI/testing/sysfs-devices-soc +++ b/Documentation/ABI/testing/sysfs-devices-soc @@ -54,6 +54,37 @@ contact: Lee Jones <lee.jones@linaro.org> Read-only attribute supported ST-Ericsson's silicon. Contains the the process by which the silicon chip was manufactured. +What: /sys/devices/socX/jep106_identification_code +Date: June 2020 +Contact: Sudeep Holla <sudeep.holla@arm.com> +Description: + Read-only attribute supported on many of ARM based silicon + with SMCCC v1.2+ compliant firmware. Contains the JEDEC + JEP106 manufacturer’s identification code. + + This manufacturer’s identification code is defined by one + or more eight (8) bit fields, each consisting of seven (7) + data bits plus one (1) odd parity bit. It is a single field, + limiting the possible number of vendors to 126. To expand + the maximum number of identification codes, a continuation + scheme has been defined. + + The specified mechanism is that an identity code of 0x7F + represents the "continuation code" and implies the presence + of an additional identity code field, and this mechanism + may be extended to multiple continuation codes followed + by the manufacturer's identity code. + + For example, ARM has identity code 0x7F 0x7F 0x7F 0x7F 0x3B, + which is code 0x3B on the fifth 'page'. This can be shortened + as JEP106 identity code of 0x3B and a continuation code of + 0x4 to represent the four continuation codes preceding the + identity code. + + This property represents it in the shortened form: + 8-bit continuation code followed by 8 bit identity code + without the parity bit. + What: /sys/bus/soc Date: January 2012 contact: Lee Jones <lee.jones@linaro.org> diff --git a/drivers/base/soc.c b/drivers/base/soc.c index 4af11a423475..44dc757aadf4 100644 --- a/drivers/base/soc.c +++ b/drivers/base/soc.c @@ -36,6 +36,7 @@ static DEVICE_ATTR(family, S_IRUGO, soc_info_get, NULL); static DEVICE_ATTR(serial_number, S_IRUGO, soc_info_get, NULL); static DEVICE_ATTR(soc_id, S_IRUGO, soc_info_get, NULL); static DEVICE_ATTR(revision, S_IRUGO, soc_info_get, NULL); +static DEVICE_ATTR(jep106_identification_code, S_IRUGO, soc_info_get, NULL); struct device *soc_device_to_device(struct soc_device *soc_dev) { @@ -64,6 +65,9 @@ static umode_t soc_attribute_mode(struct kobject *kobj, if ((attr == &dev_attr_soc_id.attr) && (soc_dev->attr->soc_id != NULL)) return attr->mode; + if ((attr == &dev_attr_jep106_identification_code.attr) + && (soc_dev->attr->jep106_id != NULL)) + return attr->mode; /* Unknown or unfilled attribute. */ return 0; @@ -85,6 +89,8 @@ static ssize_t soc_info_get(struct device *dev, return sprintf(buf, "%s\n", soc_dev->attr->serial_number); if (attr == &dev_attr_soc_id) return sprintf(buf, "%s\n", soc_dev->attr->soc_id); + if (attr == &dev_attr_jep106_identification_code) + return sprintf(buf, "%s\n", soc_dev->attr->jep106_id); return -EINVAL; @@ -96,6 +102,7 @@ static struct attribute *soc_attr[] = { &dev_attr_serial_number.attr, &dev_attr_soc_id.attr, &dev_attr_revision.attr, + &dev_attr_jep106_identification_code.attr, NULL, }; @@ -214,6 +221,11 @@ static int soc_device_match_attr(const struct soc_device_attribute *attr, (!attr->soc_id || !glob_match(match->soc_id, attr->soc_id))) return 0; + if (match->jep106_id && + (!attr->jep106_id || + !glob_match(match->jep106_id, attr->jep106_id))) + return 0; + return 1; } diff --git a/include/linux/sys_soc.h b/include/linux/sys_soc.h index d9b3cf0f410c..394fa70ae16f 100644 --- a/include/linux/sys_soc.h +++ b/include/linux/sys_soc.h @@ -14,6 +14,7 @@ struct soc_device_attribute { const char *revision; const char *serial_number; const char *soc_id; + const char *jep106_id; const void *data; const struct attribute_group *custom_attr_group; }; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 12:49 [PATCH 0/2] base: soc: Add JEP106 manufacturer's identification code Sudeep Holla 2020-05-22 12:49 ` [PATCH 1/2] base: soc: Add JEDEC JEP106 manufacturer's identification code attribute Sudeep Holla @ 2020-05-22 12:49 ` Sudeep Holla 2020-05-22 14:46 ` Arnd Bergmann 1 sibling, 1 reply; 10+ messages in thread From: Sudeep Holla @ 2020-05-22 12:49 UTC (permalink / raw) To: linux-arm-kernel Cc: Mark Rutland, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon SMCCC v1.2 adds a new optional function SMCCC_ARCH_SOC_ID to obtain a SiP defined SoC identification value. Add support for the same. Also using the SoC bus infrastructure, let us expose the platform specific SoC atrributes under sysfs. Reviewed-by: Steven Price <steven.price@arm.com> Acked-by: Etienne Carriere <etienne.carriere@st.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/smccc/Kconfig | 9 +++ drivers/firmware/smccc/Makefile | 1 + drivers/firmware/smccc/soc_id.c | 113 ++++++++++++++++++++++++++++++++ include/linux/arm-smccc.h | 5 ++ 4 files changed, 128 insertions(+) create mode 100644 drivers/firmware/smccc/soc_id.c 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..437175a589e2 --- /dev/null +++ b/drivers/firmware/smccc/soc_id.c @@ -0,0 +1,113 @@ +// 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) + +#define JEP106_BANK_CONT_CODE(x) \ + (u8)(FIELD_GET(SMCCC_SOC_ID_JEP106_BANK_IDX_MASK, (x))) +#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 struct soc_device *soc_dev; +static struct soc_device_attribute *soc_dev_attr; + +static int __init smccc_soc_init(void) +{ + struct arm_smccc_res res; + int soc_id_rev, soc_id_version; + static char soc_id_str[8], soc_id_rev_str[12]; + static char soc_id_jep106_id_str[8]; + + 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); + sprintf(soc_id_jep106_id_str, "0x%02x%02x", + JEP106_BANK_CONT_CODE(soc_id_version), + JEP106_ID_CODE(soc_id_version)); + + soc_dev_attr->soc_id = soc_id_str; + soc_dev_attr->revision = soc_id_rev_str; + soc_dev_attr->jep106_id = soc_id_jep106_id_str; + + soc_dev = soc_device_register(soc_dev_attr); + if (IS_ERR(soc_dev)) { + kfree(soc_dev_attr); + return PTR_ERR(soc_dev); + } + + pr_info("ID = %s Revision = %s\n", soc_dev_attr->soc_id, + soc_dev_attr->revision); + + return 0; +} +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 56d6a5c6e353..8254e11ea857 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -71,6 +71,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, \ -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 12:49 ` [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support Sudeep Holla @ 2020-05-22 14:46 ` Arnd Bergmann 2020-05-22 16:54 ` Sudeep Holla 0 siblings, 1 reply; 10+ messages in thread From: Arnd Bergmann @ 2020-05-22 14:46 UTC (permalink / raw) To: Sudeep Holla Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Greg Kroah-Hartman, linux-kernel, harb, Will Deacon, Linux ARM On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > + > + 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); > + sprintf(soc_id_jep106_id_str, "0x%02x%02x", > + JEP106_BANK_CONT_CODE(soc_id_version), > + JEP106_ID_CODE(soc_id_version)); > + > + soc_dev_attr->soc_id = soc_id_str; > + soc_dev_attr->revision = soc_id_rev_str; > + soc_dev_attr->jep106_id = soc_id_jep106_id_str; Ok, let me try to understand how this maps the 64-bit ID into the six strings in user space: For a chip that identifies as JEP106_BANK_CONT_CODE = 12 JEP106_ID_CODE = 34 IMP_DEF_SOC_ID = 5678 soc_id_rev = 9abcdef0 the normal sysfs attributes contain these strings: machine = "" family = "" revision = "0x9abcdef0 serial_number = "" soc_id = "0x5678" and the new attribute is jep106_identification_code = "0x1234" This still looks like a rather poorly designed interface to me, with a number of downsides: - Nothing in those strings identifies the numbers as using jep106 numbers rather than some something else that might use strings with hexadecimal numbers. - I think we should have something unique in "family" just because existing scripts can use that as the primary indentifier - It seems odd that there is no way to read the serial number through the same interface and publish it the usual way. Francois Ozog recently asked for a generic way to find out a serial number for inventory management, and this would be the obvious place to have it. It can of course be added later when the next revision of the spec is there, it just seems like a surprising omission. How about making the contents: machine = "" /* could be a future addition, but board specific */ family = "jep106:1234" revision = "0x9abcdef0 serial_number = "0xfedcba987654321" /* to be implemented later */ soc_id = "jep106:1234:5678" /* duplicates family but makes it unique*/ That would work without any new properties, dropping the other patch, and be easier to use for identification from user space. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 14:46 ` Arnd Bergmann @ 2020-05-22 16:54 ` Sudeep Holla 2020-05-22 17:13 ` Sudeep Holla 2020-05-22 18:41 ` Arnd Bergmann 0 siblings, 2 replies; 10+ messages in thread From: Sudeep Holla @ 2020-05-22 16:54 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Jose.Marinho, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon, Linux ARM (+ Jose (SMCCC Spec author)) On Fri, May 22, 2020 at 04:46:12PM +0200, Arnd Bergmann wrote: > On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > + > > + 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); > > + sprintf(soc_id_jep106_id_str, "0x%02x%02x", > > + JEP106_BANK_CONT_CODE(soc_id_version), > > + JEP106_ID_CODE(soc_id_version)); > > + > > + soc_dev_attr->soc_id = soc_id_str; > > + soc_dev_attr->revision = soc_id_rev_str; > > + soc_dev_attr->jep106_id = soc_id_jep106_id_str; > > Ok, let me try to understand how this maps the 64-bit ID into the > six strings in user space: > > For a chip that identifies as > > JEP106_BANK_CONT_CODE = 12 > JEP106_ID_CODE = 34 > IMP_DEF_SOC_ID = 5678 > soc_id_rev = 9abcdef0 > > the normal sysfs attributes contain these strings: > > machine = "" > family = "" > revision = "0x9abcdef0 > serial_number = "" > soc_id = "0x5678" > > and the new attribute is > > jep106_identification_code = "0x1234" > > This still looks like a rather poorly designed interface to me, with a > number of downsides: > > - Nothing in those strings identifies the numbers as using jep106 > numbers rather than some something else that might use strings > with hexadecimal numbers. > Not sure if I understand your concerns completely here. Anyways I wanted to clarify that the jep106 encoding is applicable only for manufacturer's id and not for SoC ID or revision. Not sure if that changes anything about your concerns. > - I think we should have something unique in "family" just because > existing scripts can use that as the primary indentifier > I agree with your idea of combining attributes, not sure exactly which ones yet. > - It seems odd that there is no way to read the serial number through > the same interface and publish it the usual way. Valid concern and I will pass this to interface authors. > Francois Ozog > recently asked for a generic way to find out a serial number for > inventory management, and this would be the obvious place to have it. Agreed, but not sure what author(s) have to say. I have cc-ed one of them. > It can of course be added later when the next revision of the spec > is there, it just seems like a surprising omission. > Yes, definitely. Good to get feedback. > How about making the contents: > > machine = "" /* could be a future addition, but board specific */ > family = "jep106:1234" But this just indicates manufacturer id and nothing related to SoC family. If it is jep106:043b, all it indicates is Arm Ltd and assigning it to family doesn't sound right to me. I had requests for both of the above during the design of interface but I was told vendors were happy with the interface. I will let the authors speak about that. > revision = "0x9abcdef0 > serial_number = "0xfedcba987654321" /* to be implemented later */ Sure. > soc_id = "jep106:1234:5678" /* duplicates family but makes it unique*/ Not sure again. > > That would work without any new properties, dropping the other patch, > and be easier to use for identification from user space. > OK, I agree on ease part. But for me, we don't have any property in the list to indicate the vendor/manufacturer's name. I don't see issue adding one, name can be fixed as jep106_identification_code is too specific. How about manufacturer with the value in the format "jep106:1234" if it is not normal string but jep106 encoding. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 16:54 ` Sudeep Holla @ 2020-05-22 17:13 ` Sudeep Holla 2020-05-22 18:41 ` Arnd Bergmann 1 sibling, 0 replies; 10+ messages in thread From: Sudeep Holla @ 2020-05-22 17:13 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Jose.Marinho, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon, Linux ARM On Fri, May 22, 2020 at 05:54:22PM +0100, Sudeep Holla wrote: > (+ Jose (SMCCC Spec author)) > > On Fri, May 22, 2020 at 04:46:12PM +0200, Arnd Bergmann wrote: > > On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > + > > > + 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); > > > + sprintf(soc_id_jep106_id_str, "0x%02x%02x", > > > + JEP106_BANK_CONT_CODE(soc_id_version), > > > + JEP106_ID_CODE(soc_id_version)); > > > + > > > + soc_dev_attr->soc_id = soc_id_str; > > > + soc_dev_attr->revision = soc_id_rev_str; > > > + soc_dev_attr->jep106_id = soc_id_jep106_id_str; > > > > Ok, let me try to understand how this maps the 64-bit ID into the > > six strings in user space: > > > > For a chip that identifies as > > > > JEP106_BANK_CONT_CODE = 12 > > JEP106_ID_CODE = 34 > > IMP_DEF_SOC_ID = 5678 > > soc_id_rev = 9abcdef0 > > > > the normal sysfs attributes contain these strings: > > > > machine = "" > > family = "" > > revision = "0x9abcdef0 > > serial_number = "" > > soc_id = "0x5678" > > > > and the new attribute is > > > > jep106_identification_code = "0x1234" > > > > This still looks like a rather poorly designed interface to me, with a > > number of downsides: > > > > - Nothing in those strings identifies the numbers as using jep106 > > numbers rather than some something else that might use strings > > with hexadecimal numbers. > > > > Not sure if I understand your concerns completely here. > > Anyways I wanted to clarify that the jep106 encoding is applicable only > for manufacturer's id and not for SoC ID or revision. Not sure if that > changes anything about your concerns. > > > - I think we should have something unique in "family" just because > > existing scripts can use that as the primary indentifier > > > > I agree with your idea of combining attributes, not sure exactly which > ones yet. > > > - It seems odd that there is no way to read the serial number through > > the same interface and publish it the usual way. > > Valid concern and I will pass this to interface authors. > > > Francois Ozog > > recently asked for a generic way to find out a serial number for > > inventory management, and this would be the obvious place to have it. > > Agreed, but not sure what author(s) have to say. I have cc-ed one of them. > > > It can of course be added later when the next revision of the spec > > is there, it just seems like a surprising omission. > > > > Yes, definitely. Good to get feedback. > > > How about making the contents: > > > > machine = "" /* could be a future addition, but board specific */ [...] > > serial_number = "0xfedcba987654321" /* to be implemented later */ OK more thoughts and few points I missed earlier. The SoC infrastructure is more wider than the SoC itself IMO. The machine and serial_number are beyond the SoC and this SMCCC SOC_ID confined itself to SoC for simple reason that it is part of SoC vendor firmware which could be used across various machines/boards/platforms. I wonder now that rather than adding this as a SoC sysfs driver on it's own, does it make sense to keep it as library which existing drivers can use. Or we need to standardise the machine level interface that may like outside SoC firmware and use both to have a generic SoC sysfs driver. Sorry, I am just writing as I am thinking and may be talking garbage. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 16:54 ` Sudeep Holla 2020-05-22 17:13 ` Sudeep Holla @ 2020-05-22 18:41 ` Arnd Bergmann 2020-05-23 17:27 ` Sudeep Holla 1 sibling, 1 reply; 10+ messages in thread From: Arnd Bergmann @ 2020-05-22 18:41 UTC (permalink / raw) To: Sudeep Holla Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Jose.Marinho, Greg Kroah-Hartman, linux-kernel, harb, Will Deacon, Linux ARM On Fri, May 22, 2020 at 6:54 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > (+ Jose (SMCCC Spec author)) > > On Fri, May 22, 2020 at 04:46:12PM +0200, Arnd Bergmann wrote: > > On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > + > > > + 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); > > > + sprintf(soc_id_jep106_id_str, "0x%02x%02x", > > > + JEP106_BANK_CONT_CODE(soc_id_version), > > > + JEP106_ID_CODE(soc_id_version)); > > > + > > > + soc_dev_attr->soc_id = soc_id_str; > > > + soc_dev_attr->revision = soc_id_rev_str; > > > + soc_dev_attr->jep106_id = soc_id_jep106_id_str; > > > > Ok, let me try to understand how this maps the 64-bit ID into the > > six strings in user space: > > > > For a chip that identifies as > > > > JEP106_BANK_CONT_CODE = 12 > > JEP106_ID_CODE = 34 > > IMP_DEF_SOC_ID = 5678 > > soc_id_rev = 9abcdef0 > > > > the normal sysfs attributes contain these strings: > > > > machine = "" > > family = "" > > revision = "0x9abcdef0 > > serial_number = "" > > soc_id = "0x5678" > > > > and the new attribute is > > > > jep106_identification_code = "0x1234" > > > > This still looks like a rather poorly designed interface to me, with a > > number of downsides: > > > > - Nothing in those strings identifies the numbers as using jep106 > > numbers rather than some something else that might use strings > > with hexadecimal numbers. > > > > Not sure if I understand your concerns completely here. > > Anyways I wanted to clarify that the jep106 encoding is applicable only > for manufacturer's id and not for SoC ID or revision. Not sure if that > changes anything about your concerns. The problem I see is that by looking at just the existing attributes, you have no way of telling what namespace the strings are in, and a script that tries pattern matching could confuse two hexadecimal numbers from a different namespace, such as pci vendor/device or usb vendor/device IDs that are similar in spirit to the jep106 codes. > > - I think we should have something unique in "family" just because > > existing scripts can use that as the primary indentifier This is part of the same issue: If we put just "jep106 identified SoC" as the "family", it would be something a driver could match against. > > How about making the contents: > > > > machine = "" /* could be a future addition, but board specific */ > > family = "jep106:1234" > > But this just indicates manufacturer id and nothing related to SoC family. > If it is jep106:043b, all it indicates is Arm Ltd and assigning it to > family doesn't sound right to me. > > I had requests for both of the above during the design of interface but > I was told vendors were happy with the interface. I will let the authors > speak about that. In most cases, the existing drivers put a hardcoded string into the family, such as "Samsung Exynos" "Freescale i.MX" "Amlogic Meson" or slightly more specific "R-Car Gen2" Having a numeric identifier for the SoC manufacturer here is a bit more coarse than that, but would be similar in spirit. > > soc_id = "jep106:1234:5678" /* duplicates family but makes it unique*/ > > Not sure again. > > That would work without any new properties, dropping the other patch, > > and be easier to use for identification from user space. > > > > OK, I agree on ease part. But for me, we don't have any property in the > list to indicate the vendor/manufacturer's name. I don't see issue adding > one, name can be fixed as jep106_identification_code is too specific. > > How about manufacturer with the value in the format "jep106:1234" if > it is not normal string but jep106 encoding. I don't think we need a real name like "Arm" or "Samsung" here, but just a number is not enough, it should at least be something that can be assumed to never conflict with the name of a chip by another scheme. jep106:5678 (the IMP_DEF_SOC_ID field in my example) would probably be sufficient to not conflict with a another soc_device driver, but is quite likely to clash with an ID used by another manufacturer. jep106:1234 (the manufacturer ID) in turn seems too broad from the soc_id field, as that would include every chip made by one company. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-22 18:41 ` Arnd Bergmann @ 2020-05-23 17:27 ` Sudeep Holla 2020-05-23 19:40 ` Arnd Bergmann 0 siblings, 1 reply; 10+ messages in thread From: Sudeep Holla @ 2020-05-23 17:27 UTC (permalink / raw) To: Arnd Bergmann Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Jose.Marinho, Greg Kroah-Hartman, linux-kernel, harb, Sudeep Holla, Will Deacon, Linux ARM On Fri, May 22, 2020 at 08:41:59PM +0200, Arnd Bergmann wrote: > On Fri, May 22, 2020 at 6:54 PM Sudeep Holla <sudeep.holla@arm.com> wrote: [...] > > > > Not sure if I understand your concerns completely here. > > > > Anyways I wanted to clarify that the jep106 encoding is applicable only > > for manufacturer's id and not for SoC ID or revision. Not sure if that > > changes anything about your concerns. > > The problem I see is that by looking at just the existing attributes, > you have no way of telling what namespace the strings are in, > and a script that tries pattern matching could confuse two > hexadecimal numbers from a different namespace, such as > pci vendor/device or usb vendor/device IDs that are similar > in spirit to the jep106 codes. > Ah OK, understood. > > > - I think we should have something unique in "family" just because > > > existing scripts can use that as the primary indentifier > > This is part of the same issue: If we put just "jep106 identified SoC" > as the "family", it would be something a driver could match against. > I understand that and that's the reason I was introducing new field as manufacture but I now see there is no point in adding that. [...] > > But this just indicates manufacturer id and nothing related to SoC family. > > If it is jep106:043b, all it indicates is Arm Ltd and assigning it to > > family doesn't sound right to me. > > > > I had requests for both of the above during the design of interface but > > I was told vendors were happy with the interface. I will let the authors > > speak about that. > > In most cases, the existing drivers put a hardcoded string into the > family, such as > > "Samsung Exynos" > "Freescale i.MX" > "Amlogic Meson" > > or slightly more specific > > "R-Car Gen2" > Hmmm.... > Having a numeric identifier for the SoC manufacturer here is a > bit more coarse than that, but would be similar in spirit. > Agreed. [..] > > > > OK, I agree on ease part. But for me, we don't have any property in the > > list to indicate the vendor/manufacturer's name. I don't see issue adding > > one, name can be fixed as jep106_identification_code is too specific. > > > > How about manufacturer with the value in the format "jep106:1234" if > > it is not normal string but jep106 encoding. > > I don't think we need a real name like "Arm" or "Samsung" here, > but just a number is not enough, it should at least be something > that can be assumed to never conflict with the name of a chip > by another scheme. > Fair enough. > jep106:5678 (the IMP_DEF_SOC_ID field in my example) would > probably be sufficient to not conflict with a another soc_device > driver, but is quite likely to clash with an ID used by another > manufacturer. > IIUC, you are fine with "jep106:1234:5678" where 1234 is jep106 manufacture id code and 5678 is soc_id as it may avoid all the conflicts across the manufacture namespaces. > jep106:1234 (the manufacturer ID) in turn seems too broad from > the soc_id field, as that would include every chip made by one > company. > I understand, and IIUC you prefer this to be embedded with soc_id especially to avoid namespace conflicts which makes sense. Thanks for all the discussion and valuable inputs. I am now bit nervous to add SoC info from SMCCC ARCH_SOC_ID to sysfs yet as we need more info especially "machine" and "serial_number" for elsewhere(OEM firmware mostly from DT or ACPI). TBH, the mix might be bit of a mess as there are soc drivers that rely on DT completely today. Anyways, this SOC_ID can be added as library that can be used by a *generic* soc driver once we define a standard way to fetch other information("machine" and "serial_number"). I am happy to get suggestions on that front especially from you and Francois as you have got some context already. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-23 17:27 ` Sudeep Holla @ 2020-05-23 19:40 ` Arnd Bergmann 2020-05-28 13:05 ` Jose Marinho 0 siblings, 1 reply; 10+ messages in thread From: Arnd Bergmann @ 2020-05-23 19:40 UTC (permalink / raw) To: Sudeep Holla Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Jose.Marinho, Greg Kroah-Hartman, linux-kernel, harb, Will Deacon, Linux ARM On Sat, May 23, 2020 at 7:27 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > On Fri, May 22, 2020 at 08:41:59PM +0200, Arnd Bergmann wrote: > > On Fri, May 22, 2020 at 6:54 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > jep106:5678 (the IMP_DEF_SOC_ID field in my example) would > > probably be sufficient to not conflict with a another soc_device > > driver, but is quite likely to clash with an ID used by another > > manufacturer. > > > > IIUC, you are fine with "jep106:1234:5678" where 1234 is jep106 manufacture > id code and 5678 is soc_id as it may avoid all the conflicts across > the manufacture namespaces. I think either jep106:5678 or jep106:1234:5678 (or some variation with different field separators if you prefer) would be fine here. > > jep106:1234 (the manufacturer ID) in turn seems too broad from > > the soc_id field, as that would include every chip made by one > > company. > > > > I understand, and IIUC you prefer this to be embedded with soc_id > especially to avoid namespace conflicts which makes sense. > > Thanks for all the discussion and valuable inputs. I am now bit nervous > to add SoC info from SMCCC ARCH_SOC_ID to sysfs yet as we need more info > especially "machine" and "serial_number" for elsewhere(OEM firmware mostly > from DT or ACPI). I probably wouldn't mix those in with the same driver. If machine and serial_number have no smccc interface, then they should be left out, but there could be a separate soc_device that gets that information by other means, usually using one of the existing hardware id register drivers. > TBH, the mix might be bit of a mess as there are soc drivers that rely > on DT completely today. Anyways, this SOC_ID can be added as library that > can be used by a *generic* soc driver once we define a standard way to > fetch other information("machine" and "serial_number"). I am happy to > get suggestions on that front especially from you and Francois as you > have got some context already. Well, I suppose we could have the entire data from the smccc interface in a central place somewhere, such as (to stay with my example) "1234:5678:9abcdef0" in an attribute of any soc_device driver that adds a call to a library function for the jep106 ID, or possibly in /sys/firmware or even a field added to /proc/cpuinfo. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support 2020-05-23 19:40 ` Arnd Bergmann @ 2020-05-28 13:05 ` Jose Marinho 0 siblings, 0 replies; 10+ messages in thread From: Jose Marinho @ 2020-05-28 13:05 UTC (permalink / raw) To: Arnd Bergmann, Sudeep Holla Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi, Greg Kroah-Hartman, linux-kernel, Harb Abdulhamid (harb@amperecomputing.com), nd, Will Deacon, Linux ARM > On Sat, May 23, 2020 at 7:27 PM Sudeep Holla <sudeep.holla@arm.com> > wrote: > > On Fri, May 22, 2020 at 08:41:59PM +0200, Arnd Bergmann wrote: > > > On Fri, May 22, 2020 at 6:54 PM Sudeep Holla <sudeep.holla@arm.com> > wrote: > > > > > jep106:5678 (the IMP_DEF_SOC_ID field in my example) would probably > > > be sufficient to not conflict with a another soc_device driver, but > > > is quite likely to clash with an ID used by another manufacturer. > > > > > > > IIUC, you are fine with "jep106:1234:5678" where 1234 is jep106 > manufacture > > id code and 5678 is soc_id as it may avoid all the conflicts across > > the manufacture namespaces. > > I think either jep106:5678 or jep106:1234:5678 (or some variation with > different field separators if you prefer) would be fine here. > > > > jep106:1234 (the manufacturer ID) in turn seems too broad from > > > the soc_id field, as that would include every chip made by one > > > company. > > > > > > > I understand, and IIUC you prefer this to be embedded with soc_id > > especially to avoid namespace conflicts which makes sense. > > > > Thanks for all the discussion and valuable inputs. I am now bit nervous > > to add SoC info from SMCCC ARCH_SOC_ID to sysfs yet as we need more > info > > especially "machine" and "serial_number" for elsewhere(OEM firmware > mostly > > from DT or ACPI). > > I probably wouldn't mix those in with the same driver. If machine and > serial_number have no smccc interface, then they should be left out, > but there could be a separate soc_device that gets that information > by other means, usually using one of the existing hardware id register > drivers. > > > TBH, the mix might be bit of a mess as there are soc drivers that rely > > on DT completely today. Anyways, this SOC_ID can be added as library that > > can be used by a *generic* soc driver once we define a standard way to > > fetch other information("machine" and "serial_number"). I am happy to > > get suggestions on that front especially from you and Francois as you > > have got some context already. > > Well, I suppose we could have the entire data from the smccc interface > in a central place somewhere, such as (to stay with my example) > "1234:5678:9abcdef0" in an attribute of any soc_device driver that > adds a call to a library function for the jep106 ID, or possibly in > /sys/firmware or even a field added to /proc/cpuinfo. I think this is a great way to expose the SoC ID info. It's important to have the SoC ID as a whole in a sysfs (or somewhere where it's easy to obtain and parse from user-space). The information provided by SoC ID should be listed in this form jep106:1234:5678, that is jep106:<manufacturer ID>:<SoC ID> . Regards, Jose _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-28 13:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-22 12:49 [PATCH 0/2] base: soc: Add JEP106 manufacturer's identification code Sudeep Holla 2020-05-22 12:49 ` [PATCH 1/2] base: soc: Add JEDEC JEP106 manufacturer's identification code attribute Sudeep Holla 2020-05-22 12:49 ` [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support Sudeep Holla 2020-05-22 14:46 ` Arnd Bergmann 2020-05-22 16:54 ` Sudeep Holla 2020-05-22 17:13 ` Sudeep Holla 2020-05-22 18:41 ` Arnd Bergmann 2020-05-23 17:27 ` Sudeep Holla 2020-05-23 19:40 ` Arnd Bergmann 2020-05-28 13:05 ` Jose Marinho
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).