On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.ho...@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
