On Wed, Jun 17, 2020 at 02:34:41PM +0100, Steven Price wrote: > On 17/06/2020 11:05, Sudeep Holla wrote: > > 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. > > > > 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. > > > > Cc: Steven Price <[email protected]> > > Cc: Etienne Carriere <[email protected]> > > Signed-off-by: Sudeep Holla <[email protected]> > > --- > > Documentation/ABI/testing/sysfs-devices-soc | 30 ++++++ > > drivers/firmware/smccc/Kconfig | 9 ++ > > drivers/firmware/smccc/Makefile | 1 + > > drivers/firmware/smccc/soc_id.c | 114 ++++++++++++++++++++ > > include/linux/arm-smccc.h | 5 + > > 5 files changed, 159 insertions(+) > > create mode 100644 drivers/firmware/smccc/soc_id.c > > > > Changes from v1[1] -> v2: > > - Dropped new jep106_id added to SoC infrastructure > > - Dropped all the tags(acks/reviews) as there is change in the format > > - Updated the format for SoC id to ensure there will be no > > conflict in the namespace > > > > [1] > > https://lore.kernel.org/lkml/[email protected]/ > > > > diff --git a/Documentation/ABI/testing/sysfs-devices-soc > > b/Documentation/ABI/testing/sysfs-devices-soc > > index ba3a3fac0ee1..50707f316ea9 100644 > > --- a/Documentation/ABI/testing/sysfs-devices-soc > > +++ b/Documentation/ABI/testing/sysfs-devices-soc > > @@ -26,6 +26,30 @@ contact: Lee Jones <[email protected]> > > Read-only attribute common to all SoCs. Contains SoC family name > > (e.g. DB8500). > > + On many of ARM based silicon with SMCCC v1.2+ compliant firmware > > + this will contain the JEDEC JEP106 manufacturer’s identification > > + code. The format is "jep106:XXYY" where XX is identity code and > > + YY is continuation 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 > > NIT: s/can be/is/ - since the format always uses the short form. >
Will fix it. [...] > > + > > + sprintf(soc_id_rev_str, "0x%08x", soc_id_rev); > > + sprintf(soc_id_jep106_id_str, "jep106:%02x%02x", > > + JEP106_BANK_CONT_CODE(soc_id_version), > > + JEP106_ID_CODE(soc_id_version)); > > + sprintf(soc_id_str, "%s:%04x", soc_id_jep106_id_str, > > + IMP_DEF_SOC_ID(soc_id_version)); > > My maths might be wrong, but I think this is one byte too long: > > soc_id_jep106_id_str can be 11 characters (without the NUL), then we have a > ':' byte followed by 4 hex digits and a trailing NUL: 11 + 1 + 4 + 1 = 17, > but the char array is 16 bytes long. > Nope, you are right. I was changing the format multiple times without bothering much about that size. Thanks for the catch. > With that fixed feel free to add my "Reviewed-by". > I will fix it up and thanks for the review. -- Regards, Sudeep
