[PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[Tinghan Shen]

In the use case of configuring the access permissions of the ADSP core,
the mt8186 SoC ADSP power will be switched on in the bootloader because
the permission control registers are located in the ADSP subsys.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/soc/mediatek/mt8186-pm-domains.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/soc/mediatek/mt8186-pm-domains.h b/drivers/soc/mediatek/mt8186-pm-domains.h
index 108af61854a3..fce86f79c505 100644
--- a/drivers/soc/mediatek/mt8186-pm-domains.h
+++ b/drivers/soc/mediatek/mt8186-pm-domains.h
@@ -304,7 +304,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
 		.ctl_offs = 0x9FC,
 		.pwr_sta_offs = 0x16C,
 		.pwr_sta2nd_offs = 0x170,
-		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
 	},
 	[MT8186_POWER_DOMAIN_ADSP_INFRA] = {
 		.name = "adsp_infra",
@@ -312,7 +311,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
 		.ctl_offs = 0x9F8,
 		.pwr_sta_offs = 0x16C,
 		.pwr_sta2nd_offs = 0x170,
-		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
 	},
 	[MT8186_POWER_DOMAIN_ADSP_TOP] = {
 		.name = "adsp_top",
@@ -332,7 +330,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
 				MT8186_TOP_AXI_PROT_EN_3_CLR,
 				MT8186_TOP_AXI_PROT_EN_3_STA),
 		},
-		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_KEEP_DEFAULT_OFF | MTK_SCPD_ACTIVE_WAKEUP,
+		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_ACTIVE_WAKEUP,
 	},
 };
 
-- 
2.18.0

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[AngeloGioacchino Del Regno]

Il 12/10/22 09:54, Tinghan Shen ha scritto:
> In the use case of configuring the access permissions of the ADSP core,
> the mt8186 SoC ADSP power will be switched on in the bootloader because
> the permission control registers are located in the ADSP subsys.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[TingHan Shen (沈廷翰)]

Hi Matthias,

Gently ping for this patch.
May I know this patch current status? It seems is under your reviewing state?

Best regards,
TingHan

On Wed, 2022-10-12 at 15:54 +0800, Tinghan Shen wrote:
> In the use case of configuring the access permissions of the ADSP core,
> the mt8186 SoC ADSP power will be switched on in the bootloader because
> the permission control registers are located in the ADSP subsys.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  drivers/soc/mediatek/mt8186-pm-domains.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mt8186-pm-domains.h b/drivers/soc/mediatek/mt8186-pm-domains.h
> index 108af61854a3..fce86f79c505 100644
> --- a/drivers/soc/mediatek/mt8186-pm-domains.h
> +++ b/drivers/soc/mediatek/mt8186-pm-domains.h
> @@ -304,7 +304,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>  		.ctl_offs = 0x9FC,
>  		.pwr_sta_offs = 0x16C,
>  		.pwr_sta2nd_offs = 0x170,
> -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
>  	},
>  	[MT8186_POWER_DOMAIN_ADSP_INFRA] = {
>  		.name = "adsp_infra",
> @@ -312,7 +311,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>  		.ctl_offs = 0x9F8,
>  		.pwr_sta_offs = 0x16C,
>  		.pwr_sta2nd_offs = 0x170,
> -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
>  	},
>  	[MT8186_POWER_DOMAIN_ADSP_TOP] = {
>  		.name = "adsp_top",
> @@ -332,7 +330,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>  				MT8186_TOP_AXI_PROT_EN_3_CLR,
>  				MT8186_TOP_AXI_PROT_EN_3_STA),
>  		},
> -		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_KEEP_DEFAULT_OFF | MTK_SCPD_ACTIVE_WAKEUP,
> +		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_ACTIVE_WAKEUP,
>  	},
>  };
>  

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[Matthias Brugger]

On 12/10/2022 09:54, Tinghan Shen wrote:
> In the use case of configuring the access permissions of the ADSP core,
> the mt8186 SoC ADSP power will be switched on in the bootloader because
> the permission control registers are located in the ADSP subsys.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Do we need a fixes tag here?
My bet is:
Fixes: 88590cbc1703 ("soc: mediatek: pm-domains: Add support for mt8186")

Or is this not yet a problem but needed to be able to implement access 
permission configuration in the ADSP driver?

Regards,
Matthias

> ---
>   drivers/soc/mediatek/mt8186-pm-domains.h | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mt8186-pm-domains.h b/drivers/soc/mediatek/mt8186-pm-domains.h
> index 108af61854a3..fce86f79c505 100644
> --- a/drivers/soc/mediatek/mt8186-pm-domains.h
> +++ b/drivers/soc/mediatek/mt8186-pm-domains.h
> @@ -304,7 +304,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>   		.ctl_offs = 0x9FC,
>   		.pwr_sta_offs = 0x16C,
>   		.pwr_sta2nd_offs = 0x170,
> -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
>   	},
>   	[MT8186_POWER_DOMAIN_ADSP_INFRA] = {
>   		.name = "adsp_infra",
> @@ -312,7 +311,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>   		.ctl_offs = 0x9F8,
>   		.pwr_sta_offs = 0x16C,
>   		.pwr_sta2nd_offs = 0x170,
> -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
>   	},
>   	[MT8186_POWER_DOMAIN_ADSP_TOP] = {
>   		.name = "adsp_top",
> @@ -332,7 +330,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
>   				MT8186_TOP_AXI_PROT_EN_3_CLR,
>   				MT8186_TOP_AXI_PROT_EN_3_STA),
>   		},
> -		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_KEEP_DEFAULT_OFF | MTK_SCPD_ACTIVE_WAKEUP,
> +		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_ACTIVE_WAKEUP,
>   	},
>   };
>   

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[TingHan Shen (沈廷翰)]

On Wed, 2022-11-23 at 11:20 +0100, Matthias Brugger wrote:
> 
> On 12/10/2022 09:54, Tinghan Shen wrote:
> > In the use case of configuring the access permissions of the ADSP core,
> > the mt8186 SoC ADSP power will be switched on in the bootloader because
> > the permission control registers are located in the ADSP subsys.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> 
> Do we need a fixes tag here?
> My bet is:
> Fixes: 88590cbc1703 ("soc: mediatek: pm-domains: Add support for mt8186")
> 
> Or is this not yet a problem but needed to be able to implement access 
> permission configuration in the ADSP driver?
> 
> Regards,
> Matthias

This patch is needed to fix a warning when implementing the ADSP access permission configuration in preloader.

	[    0.334154] mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@18/power-domain@19/power-domain@20: A default off power domain has been
ON
	[    0.334246] ------------[ cut here ]------------
	[    0.334252] top_adsp_bus already disabled
	[    0.334274] WARNING: CPU: 5 PID: 113 at drivers/clk/clk.c:969 clk_core_disable+0x90/0xb0

If not for this case, the pm-domain works fine.


Best regards,
TingHan

> 
> > ---
> >   drivers/soc/mediatek/mt8186-pm-domains.h | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mt8186-pm-domains.h b/drivers/soc/mediatek/mt8186-pm-domains.h
> > index 108af61854a3..fce86f79c505 100644
> > --- a/drivers/soc/mediatek/mt8186-pm-domains.h
> > +++ b/drivers/soc/mediatek/mt8186-pm-domains.h
> > @@ -304,7 +304,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> >   		.ctl_offs = 0x9FC,
> >   		.pwr_sta_offs = 0x16C,
> >   		.pwr_sta2nd_offs = 0x170,
> > -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> >   	},
> >   	[MT8186_POWER_DOMAIN_ADSP_INFRA] = {
> >   		.name = "adsp_infra",
> > @@ -312,7 +311,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> >   		.ctl_offs = 0x9F8,
> >   		.pwr_sta_offs = 0x16C,
> >   		.pwr_sta2nd_offs = 0x170,
> > -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> >   	},
> >   	[MT8186_POWER_DOMAIN_ADSP_TOP] = {
> >   		.name = "adsp_top",
> > @@ -332,7 +330,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> >   				MT8186_TOP_AXI_PROT_EN_3_CLR,
> >   				MT8186_TOP_AXI_PROT_EN_3_STA),
> >   		},
> > -		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_KEEP_DEFAULT_OFF | MTK_SCPD_ACTIVE_WAKEUP,
> > +		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_ACTIVE_WAKEUP,
> >   	},
> >   };
> >   

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[TingHan Shen (沈廷翰)]

Hi Matthias,

On Wed, 2022-11-23 at 18:40 +0800, TingHan Shen wrote:
> On Wed, 2022-11-23 at 11:20 +0100, Matthias Brugger wrote:
> > 
> > On 12/10/2022 09:54, Tinghan Shen wrote:
> > > In the use case of configuring the access permissions of the ADSP core,
> > > the mt8186 SoC ADSP power will be switched on in the bootloader because
> > > the permission control registers are located in the ADSP subsys.
> > > 
> > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > 
> > Do we need a fixes tag here?
> > My bet is:
> > Fixes: 88590cbc1703 ("soc: mediatek: pm-domains: Add support for mt8186")
> > 
> > Or is this not yet a problem but needed to be able to implement access 
> > permission configuration in the ADSP driver?
> > 
> > Regards,
> > Matthias
> 
> This patch is needed to fix a warning when implementing the ADSP access permission configuration in preloader.
> 
> 	[    0.334154] mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@18/power-domain@19/power-domain@20: A default off power domain has been
> ON
> 	[    0.334246] ------------[ cut here ]------------
> 	[    0.334252] top_adsp_bus already disabled
> 	[    0.334274] WARNING: CPU: 5 PID: 113 at drivers/clk/clk.c:969 clk_core_disable+0x90/0xb0
> 
> If not for this case, the pm-domain works fine.
> 
> 
> Best regards,
> TingHan

gentle ping for this.
from above descriptions, should I add the fixes tag to this patch?

Best regards,
TingHan

> 
> > 
> > > ---
> > >   drivers/soc/mediatek/mt8186-pm-domains.h | 4 +---
> > >   1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/soc/mediatek/mt8186-pm-domains.h b/drivers/soc/mediatek/mt8186-pm-domains.h
> > > index 108af61854a3..fce86f79c505 100644
> > > --- a/drivers/soc/mediatek/mt8186-pm-domains.h
> > > +++ b/drivers/soc/mediatek/mt8186-pm-domains.h
> > > @@ -304,7 +304,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> > >   		.ctl_offs = 0x9FC,
> > >   		.pwr_sta_offs = 0x16C,
> > >   		.pwr_sta2nd_offs = 0x170,
> > > -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > >   	},
> > >   	[MT8186_POWER_DOMAIN_ADSP_INFRA] = {
> > >   		.name = "adsp_infra",
> > > @@ -312,7 +311,6 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> > >   		.ctl_offs = 0x9F8,
> > >   		.pwr_sta_offs = 0x16C,
> > >   		.pwr_sta2nd_offs = 0x170,
> > > -		.caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > >   	},
> > >   	[MT8186_POWER_DOMAIN_ADSP_TOP] = {
> > >   		.name = "adsp_top",
> > > @@ -332,7 +330,7 @@ static const struct scpsys_domain_data scpsys_domain_data_mt8186[] = {
> > >   				MT8186_TOP_AXI_PROT_EN_3_CLR,
> > >   				MT8186_TOP_AXI_PROT_EN_3_STA),
> > >   		},
> > > -		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_KEEP_DEFAULT_OFF | MTK_SCPD_ACTIVE_WAKEUP,
> > > +		.caps = MTK_SCPD_SRAM_ISO | MTK_SCPD_ACTIVE_WAKEUP,
> > >   	},
> > >   };
> > >   
> 
> 

-- 
Best regards,
TingHan

Re: [PATCH v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on

[Matthias Brugger]

On 02/12/2022 12:13, TingHan Shen (沈廷翰) wrote:
> Hi Matthias,
> 
> On Wed, 2022-11-23 at 18:40 +0800, TingHan Shen wrote:
>> On Wed, 2022-11-23 at 11:20 +0100, Matthias Brugger wrote:
>> > 
>> > On 12/10/2022 09:54, Tinghan Shen wrote:
>> > > In the use case of configuring the access permissions of the ADSP core,
>> > > the mt8186 SoC ADSP power will be switched on in the bootloader because
>> > > the permission control registers are located in the ADSP subsys.
>> > > 
>> > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>> > 
>> > Do we need a fixes tag here?
>> > My bet is:
>> > Fixes: 88590cbc1703 ("soc: mediatek: pm-domains: Add support for mt8186")
>> > 
>> > Or is this not yet a problem but needed to be able to implement access 
>> > permission configuration in the ADSP driver?
>> > 
>> > Regards,
>> > Matthias
>> 
>> This patch is needed to fix a warning when implementing the ADSP access permission configuration in preloader.
>> 
>> [    0.334154] mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@18/power-domain@19/power-domain@20: A default off power domain has been
>> ON
>> [    0.334246] ------------[ cut here ]------------
>> [    0.334252] top_adsp_bus already disabled
>> [    0.334274] WARNING: CPU: 5 PID: 113 at drivers/clk/clk.c:969 clk_core_disable+0x90/0xb0
>> 
>> If not for this case, the pm-domain works fine.
>> 
>> 
>> Best regards,
>> TingHan
> 
> gentle ping for this.
> from above descriptions, should I add the fixes tag to this patch?
> 

No that's not needed in this case. Applied now.

Matthias