diff mbox series

[v4,2/4] firmware: meson_sm: Add secure power domain support

Message ID 1572868028-73076-3-git-send-email-jianxin.pan@amlogic.com (mailing list archive)
State New, archived
Headers show
Series arm64: meson: add support for A1 Power Domains | expand

Commit Message

Jianxin Pan Nov. 4, 2019, 11:47 a.m. UTC
The Amlogic Meson A1/C1 Secure Monitor implements calls to control power
domain.

Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 drivers/firmware/meson/meson_sm.c       | 2 ++
 include/linux/firmware/meson/meson_sm.h | 2 ++
 2 files changed, 4 insertions(+)

Comments

Kevin Hilman Nov. 9, 2019, 8:11 p.m. UTC | #1
Jianxin Pan <jianxin.pan@amlogic.com> writes:

> The Amlogic Meson A1/C1 Secure Monitor implements calls to control power
> domain.
>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
>  drivers/firmware/meson/meson_sm.c       | 2 ++
>  include/linux/firmware/meson/meson_sm.h | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index 1d5b4d7..7ec09f5 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -44,6 +44,8 @@ static const struct meson_sm_chip gxbb_chip = {
>  		CMD(SM_EFUSE_WRITE,	0x82000031),
>  		CMD(SM_EFUSE_USER_MAX,	0x82000033),
>  		CMD(SM_GET_CHIP_ID,	0x82000044),
> +		CMD(SM_PWRC_SET,	0x82000093),
> +		CMD(SM_PWRC_GET,	0x82000095),
>  		{ /* sentinel */ },
>  	},
>  };
> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
> index 6669e2a..4ed3989 100644
> --- a/include/linux/firmware/meson/meson_sm.h
> +++ b/include/linux/firmware/meson/meson_sm.h
> @@ -12,6 +12,8 @@ enum {
>  	SM_EFUSE_WRITE,
>  	SM_EFUSE_USER_MAX,
>  	SM_GET_CHIP_ID,
> +	SM_PWRC_SET,
> +	SM_PWRC_GET,

These new IDs are unique to the A1/C1 family.  Maybe we should add a
prefix to better indicate that.  Maybe:

       SM_A1_PWRC_SET,
       SM_A1_PWRC_GET,

Thoughts?

Kevin
Jianxin Pan Nov. 11, 2019, 10:46 a.m. UTC | #2
Hi Kevin,

Please see my comments below:

On 2019/11/10 4:11, Kevin Hilman wrote:
> Jianxin Pan <jianxin.pan@amlogic.com> writes:
> 
>> The Amlogic Meson A1/C1 Secure Monitor implements calls to control power
>> domain.
>>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> ---
>>  drivers/firmware/meson/meson_sm.c       | 2 ++
>>  include/linux/firmware/meson/meson_sm.h | 2 ++
>>  2 files changed, 4 insertions(+)
>>
[...]
>> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
>> index 6669e2a..4ed3989 100644
>> --- a/include/linux/firmware/meson/meson_sm.h
>> +++ b/include/linux/firmware/meson/meson_sm.h
>> @@ -12,6 +12,8 @@ enum {
>>  	SM_EFUSE_WRITE,
>>  	SM_EFUSE_USER_MAX,
>>  	SM_GET_CHIP_ID,
>> +	SM_PWRC_SET,
>> +	SM_PWRC_GET,
> 
> These new IDs are unique to the A1/C1 family.  Maybe we should add a
> prefix to better indicate that.  Maybe:
> 
>        SM_A1_PWRC_SET,
>        SM_A1_PWRC_GET,
> 
> Thoughts?
> 
I consulted with the internal VLSI team, and it's likely that the latter new SOC will follow A1/C1.
And then it may become common function in the future.
> Kevin
> 
> .
>
Kevin Hilman Nov. 11, 2019, 2:40 p.m. UTC | #3
Jianxin Pan <jianxin.pan@amlogic.com> writes:

> Hi Kevin,
>
> Please see my comments below:
>
> On 2019/11/10 4:11, Kevin Hilman wrote:
>> Jianxin Pan <jianxin.pan@amlogic.com> writes:
>> 
>>> The Amlogic Meson A1/C1 Secure Monitor implements calls to control power
>>> domain.
>>>
>>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>>> ---
>>>  drivers/firmware/meson/meson_sm.c       | 2 ++
>>>  include/linux/firmware/meson/meson_sm.h | 2 ++
>>>  2 files changed, 4 insertions(+)
>>>
> [...]
>>> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
>>> index 6669e2a..4ed3989 100644
>>> --- a/include/linux/firmware/meson/meson_sm.h
>>> +++ b/include/linux/firmware/meson/meson_sm.h
>>> @@ -12,6 +12,8 @@ enum {
>>>  	SM_EFUSE_WRITE,
>>>  	SM_EFUSE_USER_MAX,
>>>  	SM_GET_CHIP_ID,
>>> +	SM_PWRC_SET,
>>> +	SM_PWRC_GET,
>> 
>> These new IDs are unique to the A1/C1 family.  Maybe we should add a
>> prefix to better indicate that.  Maybe:
>> 
>>        SM_A1_PWRC_SET,
>>        SM_A1_PWRC_GET,
>> 
>> Thoughts?
>
> I consulted with the internal VLSI team, and it's likely that the latter new SOC will follow A1/C1.
> And then it may become common function in the future.

OK, but it's not a common function for the past, so it's useful to mark
that distinction.

Just like in device-tree, we often have compatibles named for previous
SoC families (e.g. "gxbb") used on newer SoCs, but we use that to mean
"GXBB or newer".

Similarily here, we can use SM_A1_ prefix to mean "A1 or newer.

Kevin
Jianxin Pan Nov. 11, 2019, 2:59 p.m. UTC | #4
Hi Kevin,

On 2019/11/11 22:40, Kevin Hilman wrote:
> Jianxin Pan <jianxin.pan@amlogic.com> writes:
> 
>> Hi Kevin,
>>
>> Please see my comments below:
>>
>> On 2019/11/10 4:11, Kevin Hilman wrote:
>>> Jianxin Pan <jianxin.pan@amlogic.com> writes:
>>>
>>>> The Amlogic Meson A1/C1 Secure Monitor implements calls to control power
>>>> domain.
>>>>
>>>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>>>> ---
>>>>  drivers/firmware/meson/meson_sm.c       | 2 ++
>>>>  include/linux/firmware/meson/meson_sm.h | 2 ++
>>>>  2 files changed, 4 insertions(+)
>>>>
>> [...]
>>>> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
>>>> index 6669e2a..4ed3989 100644
>>>> --- a/include/linux/firmware/meson/meson_sm.h
>>>> +++ b/include/linux/firmware/meson/meson_sm.h
>>>> @@ -12,6 +12,8 @@ enum {
>>>>  	SM_EFUSE_WRITE,
>>>>  	SM_EFUSE_USER_MAX,
>>>>  	SM_GET_CHIP_ID,
>>>> +	SM_PWRC_SET,
>>>> +	SM_PWRC_GET,
>>>
>>> These new IDs are unique to the A1/C1 family.  Maybe we should add a
>>> prefix to better indicate that.  Maybe:
>>>
>>>        SM_A1_PWRC_SET,
>>>        SM_A1_PWRC_GET,
>>>
>>> Thoughts?
>>
>> I consulted with the internal VLSI team, and it's likely that the latter new SOC will follow A1/C1.
>> And then it may become common function in the future.
> 
> OK, but it's not a common function for the past, so it's useful to mark
> that distinction.
> 
> Just like in device-tree, we often have compatibles named for previous
> SoC families (e.g. "gxbb") used on newer SoCs, but we use that to mean
> "GXBB or newer".
> 
> Similarily here, we can use SM_A1_ prefix to mean "A1 or newer.
> 
Thanks for your explaination, I will fix it in the next version.
> Kevin
> 
> .
>
diff mbox series

Patch

diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 1d5b4d7..7ec09f5 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -44,6 +44,8 @@  static const struct meson_sm_chip gxbb_chip = {
 		CMD(SM_EFUSE_WRITE,	0x82000031),
 		CMD(SM_EFUSE_USER_MAX,	0x82000033),
 		CMD(SM_GET_CHIP_ID,	0x82000044),
+		CMD(SM_PWRC_SET,	0x82000093),
+		CMD(SM_PWRC_GET,	0x82000095),
 		{ /* sentinel */ },
 	},
 };
diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
index 6669e2a..4ed3989 100644
--- a/include/linux/firmware/meson/meson_sm.h
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -12,6 +12,8 @@  enum {
 	SM_EFUSE_WRITE,
 	SM_EFUSE_USER_MAX,
 	SM_GET_CHIP_ID,
+	SM_PWRC_SET,
+	SM_PWRC_GET,
 };
 
 struct meson_sm_firmware;