Messages in this thread | | | Date | Tue, 4 Jul 2023 10:43:01 +0800 | Subject | Re: [PATCH 2/3] soc: c3: Add support for power domains controller | From | Xianwei Zhao <> |
| |
Hi Dmitry, Thanks for your advise.
On 2023/7/4 03:37, Dmitry Rokosov wrote: > [ EXTERNAL EMAIL ] > > On Mon, Jul 03, 2023 at 03:29:31PM +0200, Neil Armstrong wrote: >> Hi, >> >> On 03/07/2023 11:31, =Xianwei Zhao wrote: >>> From: Xianwei Zhao <xianwei.zhao@amlogic.com> >>> >>> Add support for C3 Power controller. C3 power control >>> registers are in secure domain, and should be accessed by SMC. >>> >>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com> >>> --- >>> drivers/soc/amlogic/meson-secure-pwrc.c | 28 ++++++++++++++++++++++++- >>> 1 file changed, 27 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c >>> index 25b4b71df9b8..39ccc8f2e630 100644 >>> --- a/drivers/soc/amlogic/meson-secure-pwrc.c >>> +++ b/drivers/soc/amlogic/meson-secure-pwrc.c >>> @@ -12,6 +12,7 @@ >>> #include <linux/pm_domain.h> >>> #include <dt-bindings/power/meson-a1-power.h> >>> #include <dt-bindings/power/meson-s4-power.h> >>> +#include <dt-bindings/power/amlogic-c3-power.h> >>> #include <linux/arm-smccc.h> >>> #include <linux/firmware/meson/meson_sm.h> >>> #include <linux/module.h> >>> @@ -132,6 +133,22 @@ static struct meson_secure_pwrc_domain_desc s4_pwrc_domains[] = { >>> SEC_PD(S4_AUDIO, 0), >>> }; >>> +static struct meson_secure_pwrc_domain_desc c3_pwrc_domains[] = { >>> + SEC_PD(C3_NNA, 0), >>> + SEC_PD(C3_AUDIO, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_SDIOA, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_EMMC, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_USB_COMB, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_SDCARD, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_ETH, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_GE2D, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_CVE, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_GDC_WRAP, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_ISP_TOP, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_MIPI_ISP_WRAP, GENPD_FLAG_ALWAYS_ON), >>> + SEC_PD(C3_VCODEC, 0), >>> +}; >> >> Please move this struct before _s4_ >> >>> + >>> static int meson_secure_pwrc_probe(struct platform_device *pdev) >>> { >>> int i; >>> @@ -179,7 +196,7 @@ static int meson_secure_pwrc_probe(struct platform_device *pdev) >>> for (i = 0 ; i < match->count ; ++i) { >>> struct meson_secure_pwrc_domain *dom = &pwrc->domains[i]; >>> - if (!match->domains[i].index) >>> + if (!match->domains[i].name) >> >> Is this change necessary ? If yes please move it to another patch >> and explain it's purpose. If it fixes something, add a Fixes tag so >> it can be backported. >> >> Thanks, >> Neil >> > > I suppose, this change fixes the situation with SEC_PD(C3_NNA, 0) > domain, because it has index == 0. That's true. > May be it's better to introduce the separate struct member for that? For > example, 'present' (true or false). > I think code would be more readable and clean. > > [...] > > -- > Thank you, > Dmitry
| |