Re: [PATCH 5/5] arm/arm64: smccc: Add ARCH_SOC_ID support

From: John Garry
Date: Fri May 01 2020 - 12:40:55 EST


On 01/05/2020 17:05, Sudeep Holla wrote:
On Fri, May 01, 2020 at 04:25:27PM +0100, John Garry wrote:
On 30/04/2020 12:48, Sudeep Holla wrote:
+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;
+
+ ret = smccc_soc_id_support_check();
+ if (ret)
+ return ret;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res);
+
+ ret = smccc_map_error_codes(res.a0);
+ if (ret)
+ return ret;
+
+ soc_id_version = res.a0;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res);
+
+ ret = smccc_map_error_codes(res.a0);
+ if (ret)
+ return ret;
+
+ 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);
+

Just wondering, what about if the platform already had a SoC driver - now it
could have another one, such that we may have multiple sysfs soc devices,
right?


Yes I had a quick look at that.

1. Such platform has option not to implement this SOC_ID if it doesn't
really require it.

True


2. If the firmware starts implementing it on some variants, then we can
distinguish them with compatibles and blacklist them from the other
SoC driver if having both is an issue

3. SoC bus layer supports adding multiple SoC ID driver and it may show
up as /sys/devices/soc<n> which may or may not be fine.

Yeah, it's this scenario which I'm concerned about, where some userspace expects, for example, soc0 to have a soc id from a known, expected list, and now may get something else. However it could be argued then that userspace is just too fragile then and there is no read problem here.

But this
happens only if neither [1] nor [2] is done. I am happy to see if there's
any solution for this. Any suggestions ?

Not sure, but taking a slight deviation, maybe a way could be found to supplement this dev attribute info to other ARM soc drivers.

Cheers,
John