Re: [PATCH 2/3] soc: mediatek: pm-domains: Add domain regulator supply
Hi Hsin-Yi,
Thank you for the patch.
On 10/1/21 2:49, Nicolas Boichat wrote:
> On Thu, Jan 7, 2021 at 6:49 PM Hsin-Yi Wang wrote:
>>
>> Some power domains (eg. mfg) needs to turn on power supply before power
>> on.
>>
>> Signed-off-by: Hsin-Yi Wang
>
> Reviewed-by: Nicolas Boichat
>
Reviewed-by: Enric Balletbo i Serra
>> ---
>> drivers/soc/mediatek/mt8183-pm-domains.h | 1 +
>> drivers/soc/mediatek/mtk-pm-domains.c| 36 +++-
>> drivers/soc/mediatek/mtk-pm-domains.h| 1 +
>> 3 files changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/mt8183-pm-domains.h
>> b/drivers/soc/mediatek/mt8183-pm-domains.h
>> index 8d996c5d2682d..aa5230e6c12f8 100644
>> --- a/drivers/soc/mediatek/mt8183-pm-domains.h
>> +++ b/drivers/soc/mediatek/mt8183-pm-domains.h
>> @@ -38,6 +38,7 @@ static const struct scpsys_domain_data
>> scpsys_domain_data_mt8183[] = {
>> .ctl_offs = 0x0338,
>> .sram_pdn_bits = GENMASK(8, 8),
>> .sram_pdn_ack_bits = GENMASK(12, 12),
>> + .caps = MTK_SCPD_DOMAIN_SUPPLY,
>> },
>> [MT8183_POWER_DOMAIN_MFG_CORE0] = {
>> .sta_mask = BIT(7),
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
>> b/drivers/soc/mediatek/mtk-pm-domains.c
>> index fb70cb3b07b36..ae255aa7b1a97 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.c
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> [snip]
>> @@ -275,6 +295,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
>> *scpsys, struct device_no
>> {
>> const struct scpsys_domain_data *domain_data;
>> struct scpsys_domain *pd;
>> + struct device_node *np = scpsys->dev->of_node;
>> struct property *prop;
>> const char *clk_name;
>> int i, ret, num_clks;
>> @@ -307,6 +328,19 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
>> *scpsys, struct device_no
>> pd->data = domain_data;
>> pd->scpsys = scpsys;
>>
>> + if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
>> + /* Find regulator in current power domain node */
>> + scpsys->dev->of_node = node;
>> + pd->supply = devm_regulator_get(scpsys->dev, "domain");
>> + scpsys->dev->of_node = np;
>
> This pattern is a bit strange to me. But Hsin-Yi pointed out that
> there are precedents:
> https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/iio/adc/rcar-gyroadc.c#L397
> .
nit: Strange to me too. Maybe it needs a better comment/explanation and/or use
child/parent as a temporal of_node names to make a bit more readable. Looks like
[devm_]regulator_get only accepts a device as argument and will look into child
nodes.
>
>> + if (IS_ERR(pd->supply)) {
>> + dev_err_probe(scpsys->dev, PTR_ERR(pd->supply),
>> + "%pOF: failed to get power supply.\n",
>> + node);
>> + return ERR_CAST(pd->supply);
>> + }
>> + }
>> +
>> pd->infracfg = syscon_regmap_lookup_by_phandle_optional(node,
>> "mediatek,infracfg");
>> if (IS_ERR(pd->infracfg))
>> return ERR_CAST(pd->infracfg);
>> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
>> b/drivers/soc/mediatek/mtk-pm-domains.h
>> index a2f4d8f97e058..b2770b5266dba 100644
>> --- a/drivers/soc/mediatek/mtk-pm-domains.h
>> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
>> @@ -7,6 +7,7 @@
>> #define MTK_SCPD_FWAIT_SRAMBIT(1)
>> #define MTK_SCPD_SRAM_ISO BIT(2)
>> #define MTK_SCPD_KEEP_DEFAULT_OFF BIT(3)
>> +#define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
>> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>>
>> #define SPM_VDE_PWR_CON0x0210
>> --
>> 2.29.2.729.g45daf8777d-goog
>>
>>
>> ___
>> Linux-mediatek mailing list
>> linux-media...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH 2/3] soc: mediatek: pm-domains: Add domain regulator supply
On Thu, Jan 7, 2021 at 6:49 PM Hsin-Yi Wang wrote:
>
> Some power domains (eg. mfg) needs to turn on power supply before power
> on.
>
> Signed-off-by: Hsin-Yi Wang
Reviewed-by: Nicolas Boichat
> ---
> drivers/soc/mediatek/mt8183-pm-domains.h | 1 +
> drivers/soc/mediatek/mtk-pm-domains.c| 36 +++-
> drivers/soc/mediatek/mtk-pm-domains.h| 1 +
> 3 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mt8183-pm-domains.h
> b/drivers/soc/mediatek/mt8183-pm-domains.h
> index 8d996c5d2682d..aa5230e6c12f8 100644
> --- a/drivers/soc/mediatek/mt8183-pm-domains.h
> +++ b/drivers/soc/mediatek/mt8183-pm-domains.h
> @@ -38,6 +38,7 @@ static const struct scpsys_domain_data
> scpsys_domain_data_mt8183[] = {
> .ctl_offs = 0x0338,
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> + .caps = MTK_SCPD_DOMAIN_SUPPLY,
> },
> [MT8183_POWER_DOMAIN_MFG_CORE0] = {
> .sta_mask = BIT(7),
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
> b/drivers/soc/mediatek/mtk-pm-domains.c
> index fb70cb3b07b36..ae255aa7b1a97 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
[snip]
> @@ -275,6 +295,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
> *scpsys, struct device_no
> {
> const struct scpsys_domain_data *domain_data;
> struct scpsys_domain *pd;
> + struct device_node *np = scpsys->dev->of_node;
> struct property *prop;
> const char *clk_name;
> int i, ret, num_clks;
> @@ -307,6 +328,19 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
> *scpsys, struct device_no
> pd->data = domain_data;
> pd->scpsys = scpsys;
>
> + if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
> + /* Find regulator in current power domain node */
> + scpsys->dev->of_node = node;
> + pd->supply = devm_regulator_get(scpsys->dev, "domain");
> + scpsys->dev->of_node = np;
This pattern is a bit strange to me. But Hsin-Yi pointed out that
there are precedents:
https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/iio/adc/rcar-gyroadc.c#L397
.
> + if (IS_ERR(pd->supply)) {
> + dev_err_probe(scpsys->dev, PTR_ERR(pd->supply),
> + "%pOF: failed to get power supply.\n",
> + node);
> + return ERR_CAST(pd->supply);
> + }
> + }
> +
> pd->infracfg = syscon_regmap_lookup_by_phandle_optional(node,
> "mediatek,infracfg");
> if (IS_ERR(pd->infracfg))
> return ERR_CAST(pd->infracfg);
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
> b/drivers/soc/mediatek/mtk-pm-domains.h
> index a2f4d8f97e058..b2770b5266dba 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -7,6 +7,7 @@
> #define MTK_SCPD_FWAIT_SRAMBIT(1)
> #define MTK_SCPD_SRAM_ISO BIT(2)
> #define MTK_SCPD_KEEP_DEFAULT_OFF BIT(3)
> +#define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>
> #define SPM_VDE_PWR_CON0x0210
> --
> 2.29.2.729.g45daf8777d-goog
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
[PATCH 2/3] soc: mediatek: pm-domains: Add domain regulator supply
Some power domains (eg. mfg) needs to turn on power supply before power
on.
Signed-off-by: Hsin-Yi Wang
---
drivers/soc/mediatek/mt8183-pm-domains.h | 1 +
drivers/soc/mediatek/mtk-pm-domains.c| 36 +++-
drivers/soc/mediatek/mtk-pm-domains.h| 1 +
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mt8183-pm-domains.h
b/drivers/soc/mediatek/mt8183-pm-domains.h
index 8d996c5d2682d..aa5230e6c12f8 100644
--- a/drivers/soc/mediatek/mt8183-pm-domains.h
+++ b/drivers/soc/mediatek/mt8183-pm-domains.h
@@ -38,6 +38,7 @@ static const struct scpsys_domain_data
scpsys_domain_data_mt8183[] = {
.ctl_offs = 0x0338,
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
+ .caps = MTK_SCPD_DOMAIN_SUPPLY,
},
[MT8183_POWER_DOMAIN_MFG_CORE0] = {
.sta_mask = BIT(7),
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
b/drivers/soc/mediatek/mtk-pm-domains.c
index fb70cb3b07b36..ae255aa7b1a97 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include "mt8173-pm-domains.h"
@@ -40,6 +41,7 @@ struct scpsys_domain {
struct clk_bulk_data *subsys_clks;
struct regmap *infracfg;
struct regmap *smi;
+ struct regulator *supply;
};
struct scpsys {
@@ -187,6 +189,16 @@ static int scpsys_bus_protect_disable(struct scpsys_domain
*pd)
return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
}
+static int scpsys_regulator_enable(struct regulator *supply)
+{
+ return supply ? regulator_enable(supply) : 0;
+}
+
+static int scpsys_regulator_disable(struct regulator *supply)
+{
+ return supply ? regulator_disable(supply) : 0;
+}
+
static int scpsys_power_on(struct generic_pm_domain *genpd)
{
struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain,
genpd);
@@ -194,10 +206,14 @@ static int scpsys_power_on(struct generic_pm_domain
*genpd)
bool tmp;
int ret;
- ret = clk_bulk_enable(pd->num_clks, pd->clks);
+ ret = scpsys_regulator_enable(pd->supply);
if (ret)
return ret;
+ ret = clk_bulk_enable(pd->num_clks, pd->clks);
+ if (ret)
+ goto err_reg;
+
/* subsys power on */
regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_BIT);
regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_2ND_BIT);
@@ -232,6 +248,8 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
err_pwr_ack:
clk_bulk_disable(pd->num_clks, pd->clks);
+err_reg:
+ scpsys_regulator_disable(pd->supply);
return ret;
}
@@ -267,6 +285,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
clk_bulk_disable(pd->num_clks, pd->clks);
+ scpsys_regulator_disable(pd->supply);
+
return 0;
}
@@ -275,6 +295,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
*scpsys, struct device_no
{
const struct scpsys_domain_data *domain_data;
struct scpsys_domain *pd;
+ struct device_node *np = scpsys->dev->of_node;
struct property *prop;
const char *clk_name;
int i, ret, num_clks;
@@ -307,6 +328,19 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys
*scpsys, struct device_no
pd->data = domain_data;
pd->scpsys = scpsys;
+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
+ /* Find regulator in current power domain node */
+ scpsys->dev->of_node = node;
+ pd->supply = devm_regulator_get(scpsys->dev, "domain");
+ scpsys->dev->of_node = np;
+ if (IS_ERR(pd->supply)) {
+ dev_err_probe(scpsys->dev, PTR_ERR(pd->supply),
+ "%pOF: failed to get power supply.\n",
+ node);
+ return ERR_CAST(pd->supply);
+ }
+ }
+
pd->infracfg = syscon_regmap_lookup_by_phandle_optional(node,
"mediatek,infracfg");
if (IS_ERR(pd->infracfg))
return ERR_CAST(pd->infracfg);
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h
b/drivers/soc/mediatek/mtk-pm-domains.h
index a2f4d8f97e058..b2770b5266dba 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -7,6 +7,7 @@
#define MTK_SCPD_FWAIT_SRAMBIT(1)
#define MTK_SCPD_SRAM_ISO BIT(2)
#define MTK_SCPD_KEEP_DEFAULT_OFF BIT(3)
+#define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
#define SPM_VDE_PWR_CON0x0210
--
2.29.2.729.g45daf8777d-goog
