Message ID | 20221012075434.30009-1-tinghan.shen@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v1] soc: mediatek: mtk-pm-domains: Allow mt8186 ADSP default power on | expand |
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>
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, > }, > }; >
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, > }, > }; >
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, > > }, > > }; > >
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, > > > }, > > > }; > > > > >
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
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, }, };
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(-)