2021-01-07 10:52:03

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 0/3] Add domain supply for mtk power domains

Some power domains (eg. mfg on mt8183) needs to turn on its domain
power supply before power on.

Otherwise readx_poll_timeout() in scpsys_power_on() would timeout.

Hsin-Yi Wang (3):
dt-bindings: power: Add domain regulator supply
soc: mediatek: pm-domains: Add domain regulator supply
arm64: dts: mediatek: mt8183: Add domain supply for mfg

.../power/mediatek,power-controller.yaml | 9 +++++
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 4 +++
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
drivers/soc/mediatek/mt8183-pm-domains.h | 1 +
drivers/soc/mediatek/mtk-pm-domains.c | 36 ++++++++++++++++++-
drivers/soc/mediatek/mtk-pm-domains.h | 1 +
6 files changed, 51 insertions(+), 2 deletions(-)

--
2.29.2.729.g45daf8777d-goog


2021-01-07 10:52:09

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: power: Add domain regulator supply

Some power domains (eg. mfg) needs to turn on power supply before power
on.

Signed-off-by: Hsin-Yi Wang <[email protected]>
---
.../bindings/power/mediatek,power-controller.yaml | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index d14cb9bac8497..e529586af5a12 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -82,6 +82,9 @@ patternProperties:
be specified by order, adding first the BASIC clocks followed by the
SUSBSYS clocks.

+ domain-supply:
+ description: domain regulator supply.
+
mediatek,infracfg:
$ref: /schemas/types.yaml#/definitions/phandle
description: phandle to the device containing the INFRACFG register range.
@@ -130,6 +133,9 @@ patternProperties:
be specified by order, adding first the BASIC clocks followed by the
SUSBSYS clocks.

+ domain-supply:
+ description: domain regulator supply.
+
mediatek,infracfg:
$ref: /schemas/types.yaml#/definitions/phandle
description: phandle to the device containing the INFRACFG register range.
@@ -178,6 +184,9 @@ patternProperties:
be specified by order, adding first the BASIC clocks followed by the
SUSBSYS clocks.

+ domain-supply:
+ description: domain regulator supply.
+
mediatek,infracfg:
$ref: /schemas/types.yaml#/definitions/phandle
description: phandle to the device containing the INFRACFG register range.
--
2.29.2.729.g45daf8777d-goog

2021-01-07 10:52:58

by Hsin-Yi Wang

[permalink] [raw]
Subject: [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 <[email protected]>
---
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 <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/soc/mediatek/infracfg.h>

#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_SRAM BIT(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_CON 0x0210
--
2.29.2.729.g45daf8777d-goog

2021-01-07 10:54:21

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 3/3] arm64: dts: mediatek: mt8183: Add domain supply for mfg

Add domain supply for mfg node.

Signed-off-by: Hsin-Yi Wang <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 4 ++++
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd30..ebd53755d538a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -709,6 +709,10 @@ cros_ec {
};
};

+&mfg {
+ domain-supply = <&mt6358_vgpu_reg>;
+};
+
&soc_data {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7e..bda283fa92452 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -360,7 +360,7 @@ power-domain@MT8183_POWER_DOMAIN_MFG_ASYNC {
#size-cells = <0>;
#power-domain-cells = <1>;

- power-domain@MT8183_POWER_DOMAIN_MFG {
+ mfg: power-domain@MT8183_POWER_DOMAIN_MFG {
reg = <MT8183_POWER_DOMAIN_MFG>;
#address-cells = <1>;
#size-cells = <0>;
--
2.29.2.729.g45daf8777d-goog

2021-01-10 01:53:30

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: mediatek: pm-domains: Add domain regulator supply

On Thu, Jan 7, 2021 at 6:49 PM Hsin-Yi Wang <[email protected]> wrote:
>
> Some power domains (eg. mfg) needs to turn on power supply before power
> on.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: Nicolas Boichat <[email protected]>

> ---
> 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_SRAM BIT(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_CON 0x0210
> --
> 2.29.2.729.g45daf8777d-goog
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2021-01-13 03:45:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: power: Add domain regulator supply

On Thu, 07 Jan 2021 18:49:14 +0800, Hsin-Yi Wang wrote:
> Some power domains (eg. mfg) needs to turn on power supply before power
> on.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
> ---
> .../bindings/power/mediatek,power-controller.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2021-01-13 20:04:49

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: power: Add domain regulator supply

Hi Hsin-Yi,

Thank you for the patch.

On 7/1/21 11:49, Hsin-Yi Wang wrote:
> Some power domains (eg. mfg) needs to turn on power supply before power
> on.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: Enric Balletbo i Serra <[email protected]>

> ---
> .../bindings/power/mediatek,power-controller.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> index d14cb9bac8497..e529586af5a12 100644
> --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -82,6 +82,9 @@ patternProperties:
> be specified by order, adding first the BASIC clocks followed by the
> SUSBSYS clocks.
>
> + domain-supply:
> + description: domain regulator supply.
> +
> mediatek,infracfg:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: phandle to the device containing the INFRACFG register range.
> @@ -130,6 +133,9 @@ patternProperties:
> be specified by order, adding first the BASIC clocks followed by the
> SUSBSYS clocks.
>
> + domain-supply:
> + description: domain regulator supply.
> +
> mediatek,infracfg:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: phandle to the device containing the INFRACFG register range.
> @@ -178,6 +184,9 @@ patternProperties:
> be specified by order, adding first the BASIC clocks followed by the
> SUSBSYS clocks.
>
> + domain-supply:
> + description: domain regulator supply.
> +
> mediatek,infracfg:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: phandle to the device containing the INFRACFG register range.
>

2021-01-14 02:18:07

by Enric Balletbo i Serra

[permalink] [raw]
Subject: 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 <[email protected]> wrote:
>>
>> Some power domains (eg. mfg) needs to turn on power supply before power
>> on.
>>
>> Signed-off-by: Hsin-Yi Wang <[email protected]>
>
> Reviewed-by: Nicolas Boichat <[email protected]>
>

Reviewed-by: Enric Balletbo i Serra <[email protected]>


>> ---
>> 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_SRAM BIT(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_CON 0x0210
>> --
>> 2.29.2.729.g45daf8777d-goog
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek