[PATCH v3] firmware: smccc: Support optional Arm SMC SOC_ID name

Paul Benoit posted 1 patch 12 months ago
There is a newer version of this series
drivers/firmware/smccc/soc_id.c | 79 +++++++++++++++++++++++++++++++++
include/linux/arm-smccc.h       | 37 +++++++++++++++
2 files changed, 116 insertions(+)
[PATCH v3] firmware: smccc: Support optional Arm SMC SOC_ID name
Posted by Paul Benoit 12 months ago
Issue Number 1.6 of the Arm SMC Calling Convention introduces an
optional SOC_ID name string.  If available, point the 'machine' field of
the SoC Device Attributes at this string so that it will appear under
/sys/bus/soc/devices/soc0/machine.  On Arm SMC compliant SoCs, this will
allow things like 'lscpu' to eventually get a SoC provider model name
from there rather than each tool/utility needing to get a possibly
inconsistent, obsolete, or incorrect model/machine name from its own
hardcoded model/machine name table.

Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
---

v2->v3: Add conditionalization to exclude SOC_ID Name from 32-bit builds.
v1->v2: Address code review identified issues.

 drivers/firmware/smccc/soc_id.c | 79 +++++++++++++++++++++++++++++++++
 include/linux/arm-smccc.h       | 37 +++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
index 1990263fbba0..3b50ff5d2cbd 100644
--- a/drivers/firmware/smccc/soc_id.c
+++ b/drivers/firmware/smccc/soc_id.c
@@ -32,6 +32,12 @@
 static struct soc_device *soc_dev;
 static struct soc_device_attribute *soc_dev_attr;
 
+static char __init *smccc_soc_name_init(void);
+
+#ifdef CONFIG_ARM64
+static char __ro_after_init smccc_soc_id_name[136] = "";
+#endif
+
 static int __init smccc_soc_init(void)
 {
 	int soc_id_rev, soc_id_version;
@@ -72,6 +78,7 @@ static int __init smccc_soc_init(void)
 	soc_dev_attr->soc_id = soc_id_str;
 	soc_dev_attr->revision = soc_id_rev_str;
 	soc_dev_attr->family = soc_id_jep106_id_str;
+	soc_dev_attr->machine = smccc_soc_name_init();
 
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev)) {
@@ -93,3 +100,75 @@ static void __exit smccc_soc_exit(void)
 	kfree(soc_dev_attr);
 }
 module_exit(smccc_soc_exit);
+
+
+#ifdef CONFIG_ARM64
+static inline void str_fragment_from_reg(char *dst, unsigned long reg)
+{
+	dst[0] = (reg >> 0)  & 0xff;
+	dst[1] = (reg >> 8)  & 0xff;
+	dst[2] = (reg >> 16) & 0xff;
+	dst[3] = (reg >> 24) & 0xff;
+	dst[4] = (reg >> 32) & 0xff;
+	dst[5] = (reg >> 40) & 0xff;
+	dst[6] = (reg >> 48) & 0xff;
+	dst[7] = (reg >> 56) & 0xff;
+}
+#endif
+
+static char __init *smccc_soc_name_init(void)
+{
+#ifdef CONFIG_ARM64
+	struct arm_smccc_1_2_regs args;
+	struct arm_smccc_1_2_regs res;
+	size_t len;
+
+	/*
+	 * Issue Number 1.6 of the Arm SMC Calling Convention
+	 * specification introduces an optional "name" string
+	 * to the ARM_SMCCC_ARCH_SOC_ID function.  Fetch it if
+	 * available.
+	 */
+	args.a0 = ARM_SMCCC_ARCH_SOC_ID;
+	args.a1 = 2;    /* SOC_ID name */
+	arm_smccc_1_2_invoke(&args, &res);
+	if ((u32)res.a0 == 0) {
+		const unsigned int regsize = sizeof(res.a1);
+
+		/*
+		 * Copy res.a1..res.a17 to the smccc_soc_id_name string
+		 * 8 bytes at a time.  As per Issue 1.6 of the Arm SMC
+		 * Calling Convention, the string will be NUL terminated
+		 * and padded, from the end of the string to the end of the
+		 * 136 byte buffer, with NULs.
+		 */
+		str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
+		str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
+		str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
+		str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
+		str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
+		str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
+		str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
+		str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
+		str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
+		str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
+		str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
+		str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
+		str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
+		str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
+		str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
+		str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
+		str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
+
+		len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
+		if (len) {
+			if (len == sizeof(smccc_soc_id_name))
+				pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
+			else
+				return smccc_soc_id_name;
+		}
+	}
+#endif
+
+	return NULL;
+}
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 67f6fdf2e7cd..9d444e5862fe 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -607,6 +607,12 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
 			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
 	} while (0)
 
+#define __fail_smccc_1_2(___res)					\
+	do {								\
+		if (___res)						\
+			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
+	} while (0)
+
 /*
  * arm_smccc_1_1_invoke() - make an SMCCC v1.1 compliant call
  *
@@ -639,5 +645,36 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
 		method;							\
 	})
 
+/*
+ * arm_smccc_1_2_invoke() - make an SMCCC v1.2 compliant call
+ *
+ * @args: SMC args are in the a0..a17 fields of the arm_smcc_1_2_regs structure
+ * @res: result values from registers 0 to 17
+ *
+ * This macro will make either an HVC call or an SMC call depending on the
+ * current SMCCC conduit. If no valid conduit is available then -1
+ * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
+ *
+ * The return value also provides the conduit that was used.
+ */
+#define arm_smccc_1_2_invoke(args, res) ({				\
+		struct arm_smccc_1_2_regs *__args = args;		\
+		struct arm_smccc_1_2_regs *__res = res;			\
+		int method = arm_smccc_1_1_get_conduit();		\
+		switch (method) {					\
+		case SMCCC_CONDUIT_HVC:					\
+			arm_smccc_1_2_hvc(__args, __res);		\
+			break;						\
+		case SMCCC_CONDUIT_SMC:					\
+			arm_smccc_1_2_smc(__args, __res);		\
+			break;						\
+		default:						\
+			__fail_smccc_1_2(__res);			\
+			method = SMCCC_CONDUIT_NONE;			\
+			break;						\
+		}							\
+		method;							\
+	})
+
 #endif /*__ASSEMBLY__*/
 #endif /*__LINUX_ARM_SMCCC_H*/
-- 
2.47.0
Re: [PATCH v3] firmware: smccc: Support optional Arm SMC SOC_ID name
Posted by Sudeep Holla 10 months, 1 week ago
Mostly minor coding style comments from me, otherwise LGTM.

On Tue, Dec 17, 2024 at 04:13:38PM -0800, Paul Benoit wrote:

Split the commit into multiple paragraphs, it looks too crowded 😄.

> Issue Number 1.6 of the Arm SMC Calling Convention introduces an
> optional SOC_ID name string.

> If available, point the 'machine' field of

      ^^^^ I prefer implemented instead of available.

> the SoC Device Attributes at this string so that it will appear under
> /sys/bus/soc/devices/soc0/machine.

Break into new paragraph here.

> On Arm SMC compliant SoCs, this will
> allow things like 'lscpu' to eventually get a SoC provider model name
> from there rather than each tool/utility needing to get a possibly
> inconsistent, obsolete, or incorrect model/machine name from its own
> hardcoded model/machine name table.
>
> Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> 
> v2->v3: Add conditionalization to exclude SOC_ID Name from 32-bit builds.
> v1->v2: Address code review identified issues.
> 
>  drivers/firmware/smccc/soc_id.c | 79 +++++++++++++++++++++++++++++++++
>  include/linux/arm-smccc.h       | 37 +++++++++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
> index 1990263fbba0..3b50ff5d2cbd 100644
> --- a/drivers/firmware/smccc/soc_id.c
> +++ b/drivers/firmware/smccc/soc_id.c
> @@ -32,6 +32,12 @@
>  static struct soc_device *soc_dev;
>  static struct soc_device_attribute *soc_dev_attr;
>  
> +static char __init *smccc_soc_name_init(void);
> +

Not really needed if you move you code before smccc_soc_init()

> +#ifdef CONFIG_ARM64
> +static char __ro_after_init smccc_soc_id_name[136] = "";

Move all in one block under #ifdef, details below.

> +#endif
> +
>  static int __init smccc_soc_init(void)
>  {
>  	int soc_id_rev, soc_id_version;
> @@ -72,6 +78,7 @@ static int __init smccc_soc_init(void)
>  	soc_dev_attr->soc_id = soc_id_str;
>  	soc_dev_attr->revision = soc_id_rev_str;
>  	soc_dev_attr->family = soc_id_jep106_id_str;
> +	soc_dev_attr->machine = smccc_soc_name_init();
>  
>  	soc_dev = soc_device_register(soc_dev_attr);
>  	if (IS_ERR(soc_dev)) {
> @@ -93,3 +100,75 @@ static void __exit smccc_soc_exit(void)
>  	kfree(soc_dev_attr);
>  }
>  module_exit(smccc_soc_exit);

Generally it good to have module_{init,exit} at the end of the file.
Move you additions above these.

> +
> +
> +#ifdef CONFIG_ARM64
> +static inline void str_fragment_from_reg(char *dst, unsigned long reg)
> +{
> +	dst[0] = (reg >> 0)  & 0xff;
> +	dst[1] = (reg >> 8)  & 0xff;
> +	dst[2] = (reg >> 16) & 0xff;
> +	dst[3] = (reg >> 24) & 0xff;
> +	dst[4] = (reg >> 32) & 0xff;
> +	dst[5] = (reg >> 40) & 0xff;
> +	dst[6] = (reg >> 48) & 0xff;
> +	dst[7] = (reg >> 56) & 0xff;
> +}
> +#endif
> +
> +static char __init *smccc_soc_name_init(void)
> +{
> +#ifdef CONFIG_ARM64
> +	struct arm_smccc_1_2_regs args;
> +	struct arm_smccc_1_2_regs res;
> +	size_t len;
> +
> +	/*
> +	 * Issue Number 1.6 of the Arm SMC Calling Convention
> +	 * specification introduces an optional "name" string
> +	 * to the ARM_SMCCC_ARCH_SOC_ID function.  Fetch it if
> +	 * available.
> +	 */
> +	args.a0 = ARM_SMCCC_ARCH_SOC_ID;
> +	args.a1 = 2;    /* SOC_ID name */
> +	arm_smccc_1_2_invoke(&args, &res);
> +	if ((u32)res.a0 == 0) {
> +		const unsigned int regsize = sizeof(res.a1);
> +
> +		/*
> +		 * Copy res.a1..res.a17 to the smccc_soc_id_name string
> +		 * 8 bytes at a time.  As per Issue 1.6 of the Arm SMC
> +		 * Calling Convention, the string will be NUL terminated
> +		 * and padded, from the end of the string to the end of the
> +		 * 136 byte buffer, with NULs.
> +		 */
> +		str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
> +		str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
> +		str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
> +		str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
> +		str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
> +		str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
> +		str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
> +		str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
> +		str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
> +		str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
> +		str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
> +		str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
> +		str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
> +		str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
> +		str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
> +		str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
> +		str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
> +
> +		len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
> +		if (len) {
> +			if (len == sizeof(smccc_soc_id_name))
> +				pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
> +			else
> +				return smccc_soc_id_name;
> +		}
> +	}
> +#endif
> +
> +	return NULL;
> +}

Can we improve readability with

#ifdef CONFIG_ARM64

static char __ro_after_init smccc_soc_id_name[136] = "";

<both str_fragment_from_reg and smccc_soc_name_init here>

#else
static char __init *smccc_soc_name_init(void)
{
	return NULL;
}

#endif

> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 67f6fdf2e7cd..9d444e5862fe 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -607,6 +607,12 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>  			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
>  	} while (0)
>  
> +#define __fail_smccc_1_2(___res)					\
> +	do {								\
> +		if (___res)						\
> +			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
> +	} while (0)
> +
>  /*
>   * arm_smccc_1_1_invoke() - make an SMCCC v1.1 compliant call
>   *
> @@ -639,5 +645,36 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>  		method;							\
>  	})
>  
> +/*
> + * arm_smccc_1_2_invoke() - make an SMCCC v1.2 compliant call
> + *
> + * @args: SMC args are in the a0..a17 fields of the arm_smcc_1_2_regs structure
> + * @res: result values from registers 0 to 17
> + *
> + * This macro will make either an HVC call or an SMC call depending on the
> + * current SMCCC conduit. If no valid conduit is available then -1
> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
> + *
> + * The return value also provides the conduit that was used.
> + */
> +#define arm_smccc_1_2_invoke(args, res) ({				\
> +		struct arm_smccc_1_2_regs *__args = args;		\

I think we can move this macro and the above under CONFIG_ARM64 as
arm_smccc_1_2_regs is defined only for ARM64 for now. Otherwise one
could use this macro and get undefined compiler errors for the structure.

-- 
Regards,
Sudeep
Re: [PATCH v3] firmware: smccc: Support optional Arm SMC SOC_ID name
Posted by Paul Benoit 10 months ago
Hi Sudeep,

Thank you for your feedback.

I hope to submit the suggested changes as v4 of the patch early next week.

On 2/10/2025 7:13 AM, Sudeep Holla wrote:
> Mostly minor coding style comments from me, otherwise LGTM.
> 
> On Tue, Dec 17, 2024 at 04:13:38PM -0800, Paul Benoit wrote:
> 
> Split the commit into multiple paragraphs, it looks too crowded 😄 >
>> Issue Number 1.6 of the Arm SMC Calling Convention introduces an
>> optional SOC_ID name string.
> 
>> If available, point the 'machine' field of
> 
>        ^^^^ I prefer implemented instead of available.

I changed the wording to use "implemented" rather than "available".

> 
>> the SoC Device Attributes at this string so that it will appear under
>> /sys/bus/soc/devices/soc0/machine.
> 
> Break into new paragraph here.

I made the split, and adjusted the line lengths/breaks so as to not 
exceed 75 columns/characters.

> 
>> On Arm SMC compliant SoCs, this will
>> allow things like 'lscpu' to eventually get a SoC provider model name
>> from there rather than each tool/utility needing to get a possibly
>> inconsistent, obsolete, or incorrect model/machine name from its own
>> hardcoded model/machine name table.
>>
>> Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> ---
>>
>> v2->v3: Add conditionalization to exclude SOC_ID Name from 32-bit builds.
>> v1->v2: Address code review identified issues.
>>
>>   drivers/firmware/smccc/soc_id.c | 79 +++++++++++++++++++++++++++++++++
>>   include/linux/arm-smccc.h       | 37 +++++++++++++++
>>   2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
>> index 1990263fbba0..3b50ff5d2cbd 100644
>> --- a/drivers/firmware/smccc/soc_id.c
>> +++ b/drivers/firmware/smccc/soc_id.c
>> @@ -32,6 +32,12 @@
>>   static struct soc_device *soc_dev;
>>   static struct soc_device_attribute *soc_dev_attr;
>>   
>> +static char __init *smccc_soc_name_init(void);
>> +
> 
> Not really needed if you move you code before smccc_soc_init()
> 

It has been eliminated.

>> +#ifdef CONFIG_ARM64
>> +static char __ro_after_init smccc_soc_id_name[136] = "";
> 
> Move all in one block under #ifdef, details below.
> 

I have made that change.

>> +#endif
>> +
>>   static int __init smccc_soc_init(void)
>>   {
>>   	int soc_id_rev, soc_id_version;
>> @@ -72,6 +78,7 @@ static int __init smccc_soc_init(void)
>>   	soc_dev_attr->soc_id = soc_id_str;
>>   	soc_dev_attr->revision = soc_id_rev_str;
>>   	soc_dev_attr->family = soc_id_jep106_id_str;
>> +	soc_dev_attr->machine = smccc_soc_name_init();
>>   
>>   	soc_dev = soc_device_register(soc_dev_attr);
>>   	if (IS_ERR(soc_dev)) {
>> @@ -93,3 +100,75 @@ static void __exit smccc_soc_exit(void)
>>   	kfree(soc_dev_attr);
>>   }
>>   module_exit(smccc_soc_exit);
> 
> Generally it good to have module_{init,exit} at the end of the file.
> Move you additions above these.
> 

Agreed.  I have made that change as part of your suggestion to "Move all 
in one block under #ifdef".

>> +
>> +
>> +#ifdef CONFIG_ARM64
>> +static inline void str_fragment_from_reg(char *dst, unsigned long reg)
>> +{
>> +	dst[0] = (reg >> 0)  & 0xff;
>> +	dst[1] = (reg >> 8)  & 0xff;
>> +	dst[2] = (reg >> 16) & 0xff;
>> +	dst[3] = (reg >> 24) & 0xff;
>> +	dst[4] = (reg >> 32) & 0xff;
>> +	dst[5] = (reg >> 40) & 0xff;
>> +	dst[6] = (reg >> 48) & 0xff;
>> +	dst[7] = (reg >> 56) & 0xff;
>> +}
>> +#endif
>> +
>> +static char __init *smccc_soc_name_init(void)
>> +{
>> +#ifdef CONFIG_ARM64
>> +	struct arm_smccc_1_2_regs args;
>> +	struct arm_smccc_1_2_regs res;
>> +	size_t len;
>> +
>> +	/*
>> +	 * Issue Number 1.6 of the Arm SMC Calling Convention
>> +	 * specification introduces an optional "name" string
>> +	 * to the ARM_SMCCC_ARCH_SOC_ID function.  Fetch it if
>> +	 * available.
>> +	 */
>> +	args.a0 = ARM_SMCCC_ARCH_SOC_ID;
>> +	args.a1 = 2;    /* SOC_ID name */
>> +	arm_smccc_1_2_invoke(&args, &res);
>> +	if ((u32)res.a0 == 0) {
>> +		const unsigned int regsize = sizeof(res.a1);
>> +
>> +		/*
>> +		 * Copy res.a1..res.a17 to the smccc_soc_id_name string
>> +		 * 8 bytes at a time.  As per Issue 1.6 of the Arm SMC
>> +		 * Calling Convention, the string will be NUL terminated
>> +		 * and padded, from the end of the string to the end of the
>> +		 * 136 byte buffer, with NULs.
>> +		 */
>> +		str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
>> +		str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
>> +		str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
>> +		str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
>> +		str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
>> +		str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
>> +		str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
>> +		str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
>> +		str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
>> +		str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
>> +		str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
>> +		str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
>> +		str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
>> +		str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
>> +		str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
>> +		str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
>> +		str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
>> +
>> +		len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
>> +		if (len) {
>> +			if (len == sizeof(smccc_soc_id_name))
>> +				pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
>> +			else
>> +				return smccc_soc_id_name;
>> +		}
>> +	}
>> +#endif
>> +
>> +	return NULL;
>> +}
> 
> Can we improve readability with
> 
> #ifdef CONFIG_ARM64
> 
> static char __ro_after_init smccc_soc_id_name[136] = "";
> 
> <both str_fragment_from_reg and smccc_soc_name_init here>
> 
> #else
> static char __init *smccc_soc_name_init(void)
> {
> 	return NULL;
> }
> 
> #endif
> 

I have made the change suggested above.

>> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
>> index 67f6fdf2e7cd..9d444e5862fe 100644
>> --- a/include/linux/arm-smccc.h
>> +++ b/include/linux/arm-smccc.h
>> @@ -607,6 +607,12 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>>   			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
>>   	} while (0)
>>   
>> +#define __fail_smccc_1_2(___res)					\
>> +	do {								\
>> +		if (___res)						\
>> +			___res->a0 = SMCCC_RET_NOT_SUPPORTED;		\
>> +	} while (0)
>> +
>>   /*
>>    * arm_smccc_1_1_invoke() - make an SMCCC v1.1 compliant call
>>    *
>> @@ -639,5 +645,36 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
>>   		method;							\
>>   	})
>>   
>> +/*
>> + * arm_smccc_1_2_invoke() - make an SMCCC v1.2 compliant call
>> + *
>> + * @args: SMC args are in the a0..a17 fields of the arm_smcc_1_2_regs structure
>> + * @res: result values from registers 0 to 17
>> + *
>> + * This macro will make either an HVC call or an SMC call depending on the
>> + * current SMCCC conduit. If no valid conduit is available then -1
>> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
>> + *
>> + * The return value also provides the conduit that was used.
>> + */
>> +#define arm_smccc_1_2_invoke(args, res) ({				\
>> +		struct arm_smccc_1_2_regs *__args = args;		\
> 
> I think we can move this macro and the above under CONFIG_ARM64 as
> arm_smccc_1_2_regs is defined only for ARM64 for now. Otherwise one
> could use this macro and get undefined compiler errors for the structure.
>

I added another #ifdef CONFIG_ARM64 around arm_smccc_1_2_invoke.  It 
seemed cleaner to keep arm_smccc_1_2_invoke located right after 
arm_smccc_1_1_invoke rather than to move it amongst the other 
CONFIG_ARM64 conditionalization for struct definitions, prototypes, etc. 
  Though, I'm open to moving it if you think that is better than having 
it immediately follow arm_smccc_1_1_invoke.