| Message ID | 1584689540-5227-7-git-send-email-weiyi.lu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Mediatek MT8183 scpsys support | expand |
Hi Weiyi Lu, Thank you for the patch. Missatge de Weiyi Lu <weiyi.lu@mediatek.com> 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 <weiyi.lu@mediatek.com> > --- > 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 > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
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 <weiyi.lu@mediatek.com> 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 <weiyi.lu@mediatek.com> > > --- > > 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 > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
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;
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 <weiyi.lu@mediatek.com> --- drivers/soc/mediatek/mtk-scpsys.c | 71 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-)