2020-03-20 07:34:01

by Weiyi Lu

[permalink] [raw]
Subject: [PATCH v13 00/11] Mediatek MT8183 scpsys support

This series is based on v5.6-rc1

change since v12:
- separate the fix of comma at the end into a new patch [PATCH 09/11]

changes since v11:
- re-order patches "Remove infracfg misc driver support" and "Add multiple step bus protection"
- add cap MTK_SCPD_SRAM_ISO for extra sram control
- minor coding sytle fixes and reword commit messages

changes since v10:
- squash PATCH 04 and PATCH 06 in v9 into its previous patch
- add "ignore_clr_ack" for multiple step bus protection control to have a clean definition of power domain data
- keep the mask register bit definitions and do the same for MT8183

changes since v9:
- add new PATCH 04 and PATCH 06 to replace by new method for all compatibles
- add new PATCH 07 to remove infracfg misc driver
- minor coding sytle fix

changes since v7:
- reword in binding document [PATCH 02/14]
- fix error return checking bug in subsys clock control [PATCH 10/14]
- add power domains properity to mfgcfg patch [PATCH 14/14] from
https://patchwork.kernel.org/patch/11126199/

changes since v6:
- remove the patch of SPDX license identifier because it's already fixed

changes since v5:
- fix documentation in [PATCH 04/14]
- remove useless variable checking and reuse API of clock control in [PATCH 06/14]
- coding style fix of bus protection control in [PATCH 08/14]
- fix naming of new added data in [PATCH 09/14]
- small refactor of multiple step bus protection control in [PATCH 10/14]

changes since v4:
- add property to mt8183 smi-common
- seperate refactor patches and new add function
- add power controller device node


Weiyi Lu (11):
dt-bindings: mediatek: Add property to mt8183 smi-common
dt-bindings: soc: Add MT8183 power dt-bindings
soc: mediatek: Add basic_clk_name to scp_power_data
soc: mediatek: Remove infracfg misc driver support
soc: mediatek: Add multiple step bus protection control
soc: mediatek: Add subsys clock control for bus protection
soc: mediatek: Add extra sram control
soc: mediatek: Add MT8183 scpsys support
soc: mediatek: Add a comma at the end
arm64: dts: Add power controller device node of MT8183
arm64: dts: Add power-domains property to mfgcfg

.../mediatek,smi-common.txt | 2 +-
.../bindings/soc/mediatek/scpsys.txt | 20 +-
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 63 ++
drivers/soc/mediatek/Kconfig | 10 -
drivers/soc/mediatek/Makefile | 1 -
drivers/soc/mediatek/mtk-infracfg.c | 79 ---
drivers/soc/mediatek/mtk-scpsys.c | 654 ++++++++++++++----
drivers/soc/mediatek/scpsys.h | 90 +++
include/dt-bindings/power/mt8183-power.h | 26 +
include/linux/soc/mediatek/infracfg.h | 39 --
10 files changed, 707 insertions(+), 277 deletions(-)
delete mode 100644 drivers/soc/mediatek/mtk-infracfg.c
create mode 100644 drivers/soc/mediatek/scpsys.h
create mode 100644 include/dt-bindings/power/mt8183-power.h
delete mode 100644 include/linux/soc/mediatek/infracfg.h


2020-03-20 07:34:13

by Weiyi Lu

[permalink] [raw]
Subject: [PATCH v13 01/11] dt-bindings: mediatek: Add property to mt8183 smi-common

For scpsys driver using regmap based syscon driver API.

Signed-off-by: Weiyi Lu <[email protected]>
---
.../devicetree/bindings/memory-controllers/mediatek,smi-common.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
index b478ade..01744ec 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
@@ -20,7 +20,7 @@ Required properties:
"mediatek,mt2712-smi-common"
"mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common"
"mediatek,mt8173-smi-common"
- "mediatek,mt8183-smi-common"
+ "mediatek,mt8183-smi-common", "syscon"
- reg : the register and size of the SMI block.
- power-domains : a phandle to the power domain of this local arbiter.
- clocks : Must contain an entry for each entry in clock-names.
--
1.8.1.1.dirty

2020-03-20 07:34:22

by Weiyi Lu

[permalink] [raw]
Subject: [PATCH v13 09/11] soc: mediatek: Add a comma at the end

A minor coding style fix

Signed-off-by: Weiyi Lu <[email protected]>
---
drivers/soc/mediatek/mtk-scpsys.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 94823e2..8538408 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -1374,7 +1374,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_domains = ARRAY_SIZE(scp_domain_data_mt2701),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
},
};

@@ -1385,7 +1385,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt2712),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
},
};

@@ -1396,7 +1396,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt6797),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS_MT6797,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797,
},
};

@@ -1405,7 +1405,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_domains = ARRAY_SIZE(scp_domain_data_mt7622),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
},
};

@@ -1414,7 +1414,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_domains = ARRAY_SIZE(scp_domain_data_mt7623a),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
},
};

@@ -1425,7 +1425,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt8173),
.regs = {
.pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND,
},
};

@@ -1436,7 +1436,7 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt8183),
.regs = {
.pwr_sta_offs = 0x0180,
- .pwr_sta2nd_offs = 0x0184
+ .pwr_sta2nd_offs = 0x0184,
}
};

--
1.8.1.1.dirty

2020-03-20 07:34:52

by Weiyi Lu

[permalink] [raw]
Subject: [PATCH v13 06/11] soc: mediatek: Add subsys clock control for bus protection

For the bus protection operations, some subsys clocks need to be enabled
before releasing the protection, and vise versa.
But those subsys clocks could only be controlled once its corresponding
power domain is turned on first.
In this patch, we add the subsys clock control into its relavent steps.

Signed-off-by: Weiyi Lu <[email protected]>
---
drivers/soc/mediatek/mtk-scpsys.c | 71 +++++++++++++++++++++++++++++++++++++--
1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index a4fb0b23..2a9478f 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -80,6 +80,7 @@
#define PWR_STATUS_WB BIT(27) /* MT7622 */

#define MAX_CLKS 3
+#define MAX_SUBSYS_CLKS 10

/**
* struct scp_domain_data - scp domain data for power on/off flow
@@ -89,6 +90,8 @@
* @sram_pdn_bits: The mask for sram power control bits.
* @sram_pdn_ack_bits: The mask for sram power control acked bits.
* @basic_clk_name: The basic clocks required by this power domain.
+ * @subsys_clk_prefix: The prefix name of the clocks need to be enabled
+ * before releasing bus protection.
* @caps: The flag for active wake-up action.
* @bp_table: The mask table for multiple step bus protection.
*/
@@ -99,6 +102,7 @@ struct scp_domain_data {
u32 sram_pdn_bits;
u32 sram_pdn_ack_bits;
const char *basic_clk_name[MAX_CLKS];
+ const char *subsys_clk_prefix;
u8 caps;
struct bus_prot bp_table[MAX_STEPS];
};
@@ -109,6 +113,7 @@ struct scp_domain {
struct generic_pm_domain genpd;
struct scp *scp;
struct clk *clk[MAX_CLKS];
+ struct clk *subsys_clk[MAX_SUBSYS_CLKS];
const struct scp_domain_data *data;
struct regulator *supply;
};
@@ -384,16 +389,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
val |= PWR_RST_B_BIT;
writel(val, ctl_addr);

- ret = scpsys_sram_enable(scpd, ctl_addr);
+ ret = scpsys_clk_enable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
if (ret < 0)
goto err_pwr_ack;

+ ret = scpsys_sram_enable(scpd, ctl_addr);
+ if (ret < 0)
+ goto err_sram;
+
ret = scpsys_bus_protect_disable(scpd);
if (ret < 0)
- goto err_pwr_ack;
+ goto err_sram;

return 0;

+err_sram:
+ scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
err_pwr_ack:
scpsys_clk_disable(scpd->clk, MAX_CLKS);
err_clk:
@@ -420,6 +431,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
if (ret < 0)
goto out;

+ scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
+
/* subsys power off */
val = readl(ctl_addr);
val |= PWR_ISO_BIT;
@@ -457,6 +470,48 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
return ret;
}

+static int init_subsys_clks(struct platform_device *pdev,
+ const char *prefix, struct clk **clk)
+{
+ struct device_node *node = pdev->dev.of_node;
+ u32 prefix_len, sub_clk_cnt = 0;
+ struct property *prop;
+ const char *clk_name;
+
+ if (!node) {
+ dev_err(&pdev->dev, "Cannot find scpsys node: %ld\n",
+ PTR_ERR(node));
+ return PTR_ERR(node);
+ }
+
+ prefix_len = strlen(prefix);
+
+ of_property_for_each_string(node, "clock-names", prop, clk_name) {
+ if (!strncmp(clk_name, prefix, prefix_len) &&
+ (clk_name[prefix_len] == '-')) {
+ if (sub_clk_cnt >= MAX_SUBSYS_CLKS) {
+ dev_err(&pdev->dev,
+ "subsys clk out of range %d\n",
+ sub_clk_cnt);
+ return -EINVAL;
+ }
+
+ clk[sub_clk_cnt] = devm_clk_get(&pdev->dev,
+ clk_name);
+
+ if (IS_ERR(clk[sub_clk_cnt])) {
+ dev_err(&pdev->dev,
+ "Subsys clk get fail %ld\n",
+ PTR_ERR(clk[sub_clk_cnt]));
+ return PTR_ERR(clk[sub_clk_cnt]);
+ }
+ sub_clk_cnt++;
+ }
+ }
+
+ return sub_clk_cnt;
+}
+
static int init_basic_clks(struct platform_device *pdev, struct clk **clk,
const char * const *name)
{
@@ -559,6 +614,18 @@ static struct scp *init_scp(struct platform_device *pdev,
if (ret)
return ERR_PTR(ret);

+ if (data->subsys_clk_prefix) {
+ ret = init_subsys_clks(pdev,
+ data->subsys_clk_prefix,
+ scpd->subsys_clk);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "%s: subsys clk unavailable\n",
+ data->name);
+ return ERR_PTR(ret);
+ }
+ }
+
genpd->name = data->name;
genpd->power_off = scpsys_power_off;
genpd->power_on = scpsys_power_on;
--
1.8.1.1.dirty

2020-04-23 19:37:13

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH v13 06/11] soc: mediatek: Add subsys clock control for bus protection

Hi Weiyi Lu,

Thank you for the patch.

Missatge de Weiyi Lu <[email protected]> del dia dv., 20 de març
2020 a les 8:33:
>
> For the bus protection operations, some subsys clocks need to be enabled
> before releasing the protection, and vise versa.

typo s/vise/vice/

> But those subsys clocks could only be controlled once its corresponding
> power domain is turned on first.
> In this patch, we add the subsys clock control into its relavent steps.

typo s/relavent/relevant/

>
> Signed-off-by: Weiyi Lu <[email protected]>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 71 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index a4fb0b23..2a9478f 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -80,6 +80,7 @@
> #define PWR_STATUS_WB BIT(27) /* MT7622 */
>
> #define MAX_CLKS 3
> +#define MAX_SUBSYS_CLKS 10
>
> /**
> * struct scp_domain_data - scp domain data for power on/off flow
> @@ -89,6 +90,8 @@
> * @sram_pdn_bits: The mask for sram power control bits.
> * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> * @basic_clk_name: The basic clocks required by this power domain.
> + * @subsys_clk_prefix: The prefix name of the clocks need to be enabled
> + * before releasing bus protection.
> * @caps: The flag for active wake-up action.
> * @bp_table: The mask table for multiple step bus protection.
> */
> @@ -99,6 +102,7 @@ struct scp_domain_data {
> u32 sram_pdn_bits;
> u32 sram_pdn_ack_bits;
> const char *basic_clk_name[MAX_CLKS];
> + const char *subsys_clk_prefix;
> u8 caps;
> struct bus_prot bp_table[MAX_STEPS];
> };
> @@ -109,6 +113,7 @@ struct scp_domain {
> struct generic_pm_domain genpd;
> struct scp *scp;
> struct clk *clk[MAX_CLKS];
> + struct clk *subsys_clk[MAX_SUBSYS_CLKS];
> const struct scp_domain_data *data;
> struct regulator *supply;
> };
> @@ -384,16 +389,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> val |= PWR_RST_B_BIT;
> writel(val, ctl_addr);
>
> - ret = scpsys_sram_enable(scpd, ctl_addr);
> + ret = scpsys_clk_enable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> if (ret < 0)
> goto err_pwr_ack;
>
> + ret = scpsys_sram_enable(scpd, ctl_addr);
> + if (ret < 0)
> + goto err_sram;
> +
> ret = scpsys_bus_protect_disable(scpd);
> if (ret < 0)
> - goto err_pwr_ack;
> + goto err_sram;
>
> return 0;
>
> +err_sram:
> + scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> err_pwr_ack:
> scpsys_clk_disable(scpd->clk, MAX_CLKS);
> err_clk:
> @@ -420,6 +431,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> if (ret < 0)
> goto out;
>
> + scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> +
> /* subsys power off */
> val = readl(ctl_addr);
> val |= PWR_ISO_BIT;
> @@ -457,6 +470,48 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> return ret;
> }
>
> +static int init_subsys_clks(struct platform_device *pdev,
> + const char *prefix, struct clk **clk)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + u32 prefix_len, sub_clk_cnt = 0;
> + struct property *prop;
> + const char *clk_name;
> +
> + if (!node) {

Is this possible? I suspect you can remove this check.

> + dev_err(&pdev->dev, "Cannot find scpsys node: %ld\n",
> + PTR_ERR(node));
> + return PTR_ERR(node);
> + }
> +
> + prefix_len = strlen(prefix);
> +
> + of_property_for_each_string(node, "clock-names", prop, clk_name) {
> + if (!strncmp(clk_name, prefix, prefix_len) &&
> + (clk_name[prefix_len] == '-')) {
> + if (sub_clk_cnt >= MAX_SUBSYS_CLKS) {
> + dev_err(&pdev->dev,
> + "subsys clk out of range %d\n",
> + sub_clk_cnt);
> + return -EINVAL;
> + }
> +
> + clk[sub_clk_cnt] = devm_clk_get(&pdev->dev,
> + clk_name);
> +
> + if (IS_ERR(clk[sub_clk_cnt])) {
> + dev_err(&pdev->dev,
> + "Subsys clk get fail %ld\n",
> + PTR_ERR(clk[sub_clk_cnt]));

This dev_err is redundant, devm_clk_get already prints it if the clock
is not found.

> + return PTR_ERR(clk[sub_clk_cnt]);
> + }
> + sub_clk_cnt++;
> + }
> + }
> +
> + return sub_clk_cnt;
> +}
> +
> static int init_basic_clks(struct platform_device *pdev, struct clk **clk,
> const char * const *name)
> {
> @@ -559,6 +614,18 @@ static struct scp *init_scp(struct platform_device *pdev,
> if (ret)
> return ERR_PTR(ret);
>
> + if (data->subsys_clk_prefix) {
> + ret = init_subsys_clks(pdev,
> + data->subsys_clk_prefix,
> + scpd->subsys_clk);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "%s: subsys clk unavailable\n",
> + data->name);

And this print is also redundant.


> + return ERR_PTR(ret);
> + }
> + }
> +
> genpd->name = data->name;
> genpd->power_off = scpsys_power_off;
> genpd->power_on = scpsys_power_on;
> --
> 1.8.1.1.dirty
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2020-05-06 07:46:53

by Weiyi Lu

[permalink] [raw]
Subject: Re: [PATCH v13 06/11] soc: mediatek: Add subsys clock control for bus protection


On Fri, 2020-04-24 at 02:19 +0800, Enric Balletbo Serra wrote:
> Hi Weiyi Lu,
>
> Thank you for the patch.
>
> Missatge de Weiyi Lu <[email protected]> del dia dv., 20 de març
> 2020 a les 8:33:
> >
> > For the bus protection operations, some subsys clocks need to be enabled
> > before releasing the protection, and vise versa.
>
> typo s/vise/vice/

Thanks, I'll fix it.

>
> > But those subsys clocks could only be controlled once its corresponding
> > power domain is turned on first.
> > In this patch, we add the subsys clock control into its relavent steps.
>
> typo s/relavent/relevant/
>

Thanks, I'll fix it.

> >
> > Signed-off-by: Weiyi Lu <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 71 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 69 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index a4fb0b23..2a9478f 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -80,6 +80,7 @@
> > #define PWR_STATUS_WB BIT(27) /* MT7622 */
> >
> > #define MAX_CLKS 3
> > +#define MAX_SUBSYS_CLKS 10
> >
> > /**
> > * struct scp_domain_data - scp domain data for power on/off flow
> > @@ -89,6 +90,8 @@
> > * @sram_pdn_bits: The mask for sram power control bits.
> > * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> > * @basic_clk_name: The basic clocks required by this power domain.
> > + * @subsys_clk_prefix: The prefix name of the clocks need to be enabled
> > + * before releasing bus protection.
> > * @caps: The flag for active wake-up action.
> > * @bp_table: The mask table for multiple step bus protection.
> > */
> > @@ -99,6 +102,7 @@ struct scp_domain_data {
> > u32 sram_pdn_bits;
> > u32 sram_pdn_ack_bits;
> > const char *basic_clk_name[MAX_CLKS];
> > + const char *subsys_clk_prefix;
> > u8 caps;
> > struct bus_prot bp_table[MAX_STEPS];
> > };
> > @@ -109,6 +113,7 @@ struct scp_domain {
> > struct generic_pm_domain genpd;
> > struct scp *scp;
> > struct clk *clk[MAX_CLKS];
> > + struct clk *subsys_clk[MAX_SUBSYS_CLKS];
> > const struct scp_domain_data *data;
> > struct regulator *supply;
> > };
> > @@ -384,16 +389,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> > val |= PWR_RST_B_BIT;
> > writel(val, ctl_addr);
> >
> > - ret = scpsys_sram_enable(scpd, ctl_addr);
> > + ret = scpsys_clk_enable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> > if (ret < 0)
> > goto err_pwr_ack;
> >
> > + ret = scpsys_sram_enable(scpd, ctl_addr);
> > + if (ret < 0)
> > + goto err_sram;
> > +
> > ret = scpsys_bus_protect_disable(scpd);
> > if (ret < 0)
> > - goto err_pwr_ack;
> > + goto err_sram;
> >
> > return 0;
> >
> > +err_sram:
> > + scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> > err_pwr_ack:
> > scpsys_clk_disable(scpd->clk, MAX_CLKS);
> > err_clk:
> > @@ -420,6 +431,8 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> > if (ret < 0)
> > goto out;
> >
> > + scpsys_clk_disable(scpd->subsys_clk, MAX_SUBSYS_CLKS);
> > +
> > /* subsys power off */
> > val = readl(ctl_addr);
> > val |= PWR_ISO_BIT;
> > @@ -457,6 +470,48 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> > return ret;
> > }
> >
> > +static int init_subsys_clks(struct platform_device *pdev,
> > + const char *prefix, struct clk **clk)
> > +{
> > + struct device_node *node = pdev->dev.of_node;
> > + u32 prefix_len, sub_clk_cnt = 0;
> > + struct property *prop;
> > + const char *clk_name;
> > +
> > + if (!node) {
>
> Is this possible? I suspect you can remove this check.
>

You're right. It never happens, I'll remove it.

> > + dev_err(&pdev->dev, "Cannot find scpsys node: %ld\n",
> > + PTR_ERR(node));
> > + return PTR_ERR(node);
> > + }
> > +
> > + prefix_len = strlen(prefix);
> > +
> > + of_property_for_each_string(node, "clock-names", prop, clk_name) {
> > + if (!strncmp(clk_name, prefix, prefix_len) &&
> > + (clk_name[prefix_len] == '-')) {
> > + if (sub_clk_cnt >= MAX_SUBSYS_CLKS) {
> > + dev_err(&pdev->dev,
> > + "subsys clk out of range %d\n",
> > + sub_clk_cnt);
> > + return -EINVAL;
> > + }
> > +
> > + clk[sub_clk_cnt] = devm_clk_get(&pdev->dev,
> > + clk_name);
> > +
> > + if (IS_ERR(clk[sub_clk_cnt])) {
> > + dev_err(&pdev->dev,
> > + "Subsys clk get fail %ld\n",
> > + PTR_ERR(clk[sub_clk_cnt]));
>
> This dev_err is redundant, devm_clk_get already prints it if the clock
> is not found.
>

Got it.

> > + return PTR_ERR(clk[sub_clk_cnt]);
> > + }
> > + sub_clk_cnt++;
> > + }
> > + }
> > +
> > + return sub_clk_cnt;
> > +}
> > +
> > static int init_basic_clks(struct platform_device *pdev, struct clk **clk,
> > const char * const *name)
> > {
> > @@ -559,6 +614,18 @@ static struct scp *init_scp(struct platform_device *pdev,
> > if (ret)
> > return ERR_PTR(ret);
> >
> > + if (data->subsys_clk_prefix) {
> > + ret = init_subsys_clks(pdev,
> > + data->subsys_clk_prefix,
> > + scpd->subsys_clk);
> > + if (ret < 0) {
> > + dev_err(&pdev->dev,
> > + "%s: subsys clk unavailable\n",
> > + data->name);
>
> And this print is also redundant.
>

Got it.
>
> > + return ERR_PTR(ret);
> > + }
> > + }
> > +
> > genpd->name = data->name;
> > genpd->power_off = scpsys_power_off;
> > genpd->power_on = scpsys_power_on;
> > --
> > 1.8.1.1.dirty
> > _______________________________________________
> > Linux-mediatek mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek