diff mbox series

[U-Boot,06/13] arm64: zynqmp: Cleanup PM SMC macro composition

Message ID 4efe8e4ab6b3d4389746a2080cf93bc24ba7293d.1569591296.git.michal.simek@xilinx.com
State Deferred
Headers show
Series arm64: zynqmp: Clean communication with PMUFW | expand

Commit Message

Michal Simek Sept. 27, 2019, 1:34 p.m. UTC 
Cleanup PM ID handling by using enum values.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
---

 arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Luca Ceresoli Oct. 2, 2019, 9:34 a.m. UTC | #1 
Hi Michal,

On 27/09/19 15:34, Michal Simek wrote:
> Cleanup PM ID handling by using enum values.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> ---
> 
>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> index f25d414dcb1e..573c4ffceed9 100644
> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> @@ -10,7 +10,8 @@
>  #define PAYLOAD_ARG_CNT		5
>  
>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>  #define KEY_PTR_LEN	32
>  
>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
> @@ -21,7 +22,8 @@
>  
>  #define ZYNQMP_FPGA_AUTH_DDR	1
>  
> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>  
>  #define ZYNQMP_PM_VERSION_MAJOR		1
>  #define ZYNQMP_PM_VERSION_MINOR		0
> @@ -36,6 +38,13 @@
>  
>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>  
> +#define PM_SIP_SVC	0xc2000000
> +
> +enum pm_api_id {
> +	PM_GET_API_VERSION = 1,
> +	PM_SECURE_IMAGE = 45,
> +};
> +

This is a matter of personal taste, but I prefer to define values before
using them. So unless there is a good reason to define these values here
I'd rather move them before.
Michal Simek Oct. 2, 2019, 9:36 a.m. UTC | #2 
On 02. 10. 19 11:34, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 27/09/19 15:34, Michal Simek wrote:
>> Cleanup PM ID handling by using enum values.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>> ---
>>
>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> index f25d414dcb1e..573c4ffceed9 100644
>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> @@ -10,7 +10,8 @@
>>  #define PAYLOAD_ARG_CNT		5
>>  
>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>  #define KEY_PTR_LEN	32
>>  
>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>> @@ -21,7 +22,8 @@
>>  
>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>  
>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>  
>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>  #define ZYNQMP_PM_VERSION_MINOR		0
>> @@ -36,6 +38,13 @@
>>  
>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>  
>> +#define PM_SIP_SVC	0xc2000000
>> +
>> +enum pm_api_id {
>> +	PM_GET_API_VERSION = 1,
>> +	PM_SECURE_IMAGE = 45,
>> +};
>> +
> 
> This is a matter of personal taste, but I prefer to define values before
> using them. So unless there is a good reason to define these values here
> I'd rather move them before.

ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
they are defined because PM_SIP_SVC is really just SMC identification.

M
Luca Ceresoli Oct. 2, 2019, 9:44 a.m. UTC | #3 
Hi Michal,

On 02/10/19 11:36, Michal Simek wrote:
> On 02. 10. 19 11:34, Luca Ceresoli wrote:
>> Hi Michal,
>>
>> On 27/09/19 15:34, Michal Simek wrote:
>>> Cleanup PM ID handling by using enum values.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>> ---
>>>
>>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> index f25d414dcb1e..573c4ffceed9 100644
>>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> @@ -10,7 +10,8 @@
>>>  #define PAYLOAD_ARG_CNT		5
>>>  
>>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>>  #define KEY_PTR_LEN	32
>>>  
>>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>>> @@ -21,7 +22,8 @@
>>>  
>>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>>  
>>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>>  
>>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>>  #define ZYNQMP_PM_VERSION_MINOR		0
>>> @@ -36,6 +38,13 @@
>>>  
>>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>>  
>>> +#define PM_SIP_SVC	0xc2000000
>>> +
>>> +enum pm_api_id {
>>> +	PM_GET_API_VERSION = 1,
>>> +	PM_SECURE_IMAGE = 45,
>>> +};
>>> +
>>
>> This is a matter of personal taste, but I prefer to define values before
>> using them. So unless there is a good reason to define these values here
>> I'd rather move them before.
> 
> ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
> they are defined because PM_SIP_SVC is really just SMC identification.

My point is about order of lines in the file, not about patch order in
the series.

Let me clarify. The lines where PM_SECURE_IMAGE is used is above the
line there it is declared. Same for PM_GET_API_VERSION. I would (in this
same patch) declare enum pm_api_id above in the file.

I hope it is clearer now, sorry for the confusion.
Michal Simek Oct. 2, 2019, 10 a.m. UTC | #4 
On 02. 10. 19 11:44, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 02/10/19 11:36, Michal Simek wrote:
>> On 02. 10. 19 11:34, Luca Ceresoli wrote:
>>> Hi Michal,
>>>
>>> On 27/09/19 15:34, Michal Simek wrote:
>>>> Cleanup PM ID handling by using enum values.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>>> ---
>>>>
>>>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> index f25d414dcb1e..573c4ffceed9 100644
>>>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> @@ -10,7 +10,8 @@
>>>>  #define PAYLOAD_ARG_CNT		5
>>>>  
>>>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>>>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>>>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>>>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>>>  #define KEY_PTR_LEN	32
>>>>  
>>>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>>>> @@ -21,7 +22,8 @@
>>>>  
>>>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>>>  
>>>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>>>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>>>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>>>  
>>>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>>>  #define ZYNQMP_PM_VERSION_MINOR		0
>>>> @@ -36,6 +38,13 @@
>>>>  
>>>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>>>  
>>>> +#define PM_SIP_SVC	0xc2000000
>>>> +
>>>> +enum pm_api_id {
>>>> +	PM_GET_API_VERSION = 1,
>>>> +	PM_SECURE_IMAGE = 45,
>>>> +};
>>>> +
>>>
>>> This is a matter of personal taste, but I prefer to define values before
>>> using them. So unless there is a good reason to define these values here
>>> I'd rather move them before.
>>
>> ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
>> they are defined because PM_SIP_SVC is really just SMC identification.
> 
> My point is about order of lines in the file, not about patch order in
> the series.
> 
> Let me clarify. The lines where PM_SECURE_IMAGE is used is above the
> line there it is declared. Same for PM_GET_API_VERSION. I would (in this
> same patch) declare enum pm_api_id above in the file.
> 
> I hope it is clearer now, sorry for the confusion.
> 

Ok. Got you. Let me look at it. It shouldn't be a problem in this patch
because that values are going to move anyway. But we can resort these
stuff in zynqmp_firmware.h.

M
diff mbox series

Patch

diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index f25d414dcb1e..573c4ffceed9 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -10,7 +10,8 @@ 
 #define PAYLOAD_ARG_CNT		5
 
 #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
-#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
+#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
+	(PM_SIP_SVC + PM_SECURE_IMAGE)
 #define KEY_PTR_LEN	32
 
 #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
@@ -21,7 +22,8 @@ 
 
 #define ZYNQMP_FPGA_AUTH_DDR	1
 
-#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
+#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
+	(PM_SIP_SVC + PM_GET_API_VERSION)
 
 #define ZYNQMP_PM_VERSION_MAJOR		1
 #define ZYNQMP_PM_VERSION_MINOR		0
@@ -36,6 +38,13 @@ 
 
 #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
 
+#define PM_SIP_SVC	0xc2000000
+
+enum pm_api_id {
+	PM_GET_API_VERSION = 1,
+	PM_SECURE_IMAGE = 45,
+};
+
 enum {
 	IDCODE,
 	VERSION,