Re: [PATCH v3 02/16] soc: mediatek: Add MediaTek SCPSYS power domains

From: Nicolas Boichat
Date: Mon Oct 26 2020 - 20:56:14 EST


On Tue, Oct 27, 2020 at 1:55 AM Enric Balletbo i Serra
<enric.balletbo@xxxxxxxxxxxxx> wrote:
>
> The System Control Processor System (SCPSYS) has several power management
> related tasks in the system. This driver implements support to handle
> the different power domains supported in order to meet high performance
> and low power requirements.
>
> Co-developed-by: Matthias Brugger <mbrugger@xxxxxxxx>
> Signed-off-by: Matthias Brugger <mbrugger@xxxxxxxx>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
> ---
>
> Changes in v3:
> - Return only a boolean for scpsys_domain_is_on()
> - Use regmap_update_bits API when possible.
> - Add some logic to make sure scpsys->domains[id] == NULL or != NULL
> when needed.
> - Return the child node for scpsys_add_one_domain() call.
> - Remove unneded zeroing num_clks variable.
> - Move the soc specific data to separate include files.
>
> Changes in v2:
> - Get base address from parent syscon. We have now a scpsys syscon node
> and a child for the SPM (System Power Manager).
> - Use regmap API to acces de base address.
>
> drivers/soc/mediatek/Kconfig | 13 +
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mt8173-pm-domains.h | 96 +++++
> drivers/soc/mediatek/mtk-pm-domains.c | 455 +++++++++++++++++++++++
> drivers/soc/mediatek/mtk-pm-domains.h | 65 ++++
> 5 files changed, 630 insertions(+)
> create mode 100644 drivers/soc/mediatek/mt8173-pm-domains.h
> create mode 100644 drivers/soc/mediatek/mtk-pm-domains.c
> create mode 100644 drivers/soc/mediatek/mtk-pm-domains.h
[snip]
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> new file mode 100644
> index 000000000000..16503d6db6a8
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020 Collabora Ltd.
> + */
> +#include <linux/clk.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_clk.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/soc/mediatek/infracfg.h>
> +
> +#include "mt8173-pm-domains.h"
> +
> +#define MTK_POLL_DELAY_US 10
> +#define MTK_POLL_TIMEOUT USEC_PER_SEC
> +
> +#define PWR_RST_B_BIT BIT(0)
> +#define PWR_ISO_BIT BIT(1)
> +#define PWR_ON_BIT BIT(2)
> +#define PWR_ON_2ND_BIT BIT(3)
> +#define PWR_CLK_DIS_BIT BIT(4)
> +
> +struct scpsys_domain {
> + struct generic_pm_domain genpd;
> + const struct scpsys_domain_data *data;
> + struct scpsys *scpsys;
> + int num_clks;
> + struct clk_bulk_data *clks;
> + struct regmap *infracfg;
> +};
> +
> +struct scpsys {
> + struct device *dev;
> + struct regmap *base;
> + const struct scpsys_soc_data *soc_data;
> + struct genpd_onecell_data pd_data;
> + struct generic_pm_domain *domains[];
> +};
> +
> +#define to_scpsys_domain(gpd) container_of(gpd, struct scpsys_domain, genpd)
> +
> +static int scpsys_domain_is_on(struct scpsys_domain *pd)

static bool?

> +{
> + struct scpsys *scpsys = pd->scpsys;
> + u32 status, status2;
> +
> + regmap_read(scpsys->base, scpsys->soc_data->pwr_sta_offs, &status);
> + status &= pd->data->sta_mask;
> +
> + regmap_read(scpsys->base, scpsys->soc_data->pwr_sta2nd_offs, &status2);
> + status2 &= pd->data->sta_mask;
> +
> + /* A domain is on when both status bits are set. */
> + return status && status2;
> +}
> +
> +static int scpsys_sram_enable(struct scpsys_domain *pd)
> +{
> + u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> + struct scpsys *scpsys = pd->scpsys;
> + unsigned int tmp;
> +
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, pd->data->sram_pdn_bits, 0);
> +
> + /* Either wait until SRAM_PDN_ACK all 1 or 0 */
> + return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
> + (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
> +static int scpsys_sram_disable(struct scpsys_domain *pd)
> +{
> + u32 pdn_ack = pd->data->sram_pdn_ack_bits;
> + struct scpsys *scpsys = pd->scpsys;
> + unsigned int tmp;
> +
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, pd->data->sram_pdn_bits,
> + pd->data->sram_pdn_bits);
> +
> + /* Either wait until SRAM_PDN_ACK all 1 or 0 */
> + return regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
> + (tmp & pdn_ack) == pdn_ack, MTK_POLL_DELAY_US,
> + MTK_POLL_TIMEOUT);
> +}
> +
> +static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
> +{
> + const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
> +
> + if (!bp_data->bus_prot_mask)
> + return 0;
> +
> + return mtk_infracfg_set_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
> + bp_data->bus_prot_reg_update);
> +}
> +
> +static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> +{
> + const struct scpsys_bus_prot_data *bp_data = &pd->data->bp_infracfg;
> +
> + if (!bp_data->bus_prot_mask)
> + return 0;
> +
> + return mtk_infracfg_clear_bus_protection(pd->infracfg, bp_data->bus_prot_mask,
> + bp_data->bus_prot_reg_update);
> +}
> +
> +static int scpsys_power_on(struct generic_pm_domain *genpd)
> +{
> + struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> + struct scpsys *scpsys = pd->scpsys;
> + unsigned int tmp;

bool tmp?

> + int ret;
> +
> + ret = clk_bulk_enable(pd->num_clks, pd->clks);
> + if (ret)
> + return ret;
> +
> + /* subsys power on */
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_BIT, PWR_ON_BIT);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_2ND_BIT, PWR_ON_2ND_BIT);
> +
> + /* wait until PWR_ACK = 1 */
> + ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp > 0, MTK_POLL_DELAY_US,

`tmp > 0` is a little weird now that scpsys_domain_is_on returns a
boolean. Just use `tmp`.

> + MTK_POLL_TIMEOUT);
> + if (ret < 0)
> + goto err_pwr_ack;
> +
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_CLK_DIS_BIT, 0);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT, 0);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT, PWR_RST_B_BIT);
> +
> + ret = scpsys_sram_enable(pd);
> + if (ret < 0)
> + goto err_pwr_ack;
> +
> + ret = scpsys_bus_protect_disable(pd);
> + if (ret < 0)
> + goto err_pwr_ack;

Do we need to disable the SRAM in this error path?

> +
> + return 0;
> +
> +err_pwr_ack:
> + clk_bulk_disable(pd->num_clks, pd->clks);
> + return ret;
> +}
> +
> +static int scpsys_power_off(struct generic_pm_domain *genpd)
> +{
> + struct scpsys_domain *pd = container_of(genpd, struct scpsys_domain, genpd);
> + struct scpsys *scpsys = pd->scpsys;
> + unsigned int tmp;
> + int ret;
> +
> + ret = scpsys_bus_protect_enable(pd);
> + if (ret < 0)
> + return ret;
> +
> + ret = scpsys_sram_disable(pd);
> + if (ret < 0)
> + return ret;
> +
> + /* subsys power off */
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT, 0);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT, PWR_ISO_BIT);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_CLK_DIS_BIT, PWR_CLK_DIS_BIT);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_2ND_BIT, 0);
> + regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_ON_BIT, 0);
> +
> + /* wait until PWR_ACK = 0 */
> + ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, tmp == 0, MTK_POLL_DELAY_US,

`!tmp` as condition.

> + MTK_POLL_TIMEOUT);
> + if (ret < 0)
> + return ret;
> +
> + clk_bulk_disable(pd->num_clks, pd->clks);
> +
> + return 0;
> +}
> +
> +static struct
> +generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_node *node)
> +{
> + const struct scpsys_domain_data *domain_data;
> + struct scpsys_domain *pd;
> + int i, ret;
> + u32 id;
> +
> + ret = of_property_read_u32(node, "reg", &id);
> + if (ret) {
> + dev_err(scpsys->dev, "%pOF: failed to retrieve domain id from reg: %d\n",
> + node, ret);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (id >= scpsys->soc_data->num_domains) {
> + dev_err(scpsys->dev, "%pOF: invalid domain id %d\n", node, id);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + domain_data = &scpsys->soc_data->domains[id];

See my comment on v2. domain_data = scpsys->soc_data->domains[id]; is
probably what you want (unless I got it completely wrong...)

> + if (!domain_data) {
> + dev_err(scpsys->dev, "%pOF: undefined domain id %d\n", node, id);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pd = devm_kzalloc(scpsys->dev, sizeof(*pd), GFP_KERNEL);
> + if (!pd)
> + return ERR_PTR(-ENOMEM);
> +
> + pd->data = domain_data;
> + pd->scpsys = scpsys;
> +
> + pd->infracfg = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg");
> + if (IS_ERR(pd->infracfg))
> + pd->infracfg = NULL;
> +
> + pd->num_clks = of_clk_get_parent_count(node);
> + if (pd->num_clks > 0) {
> + pd->clks = devm_kcalloc(scpsys->dev, pd->num_clks, sizeof(*pd->clks), GFP_KERNEL);
> + if (!pd->clks)
> + return ERR_PTR(-ENOMEM);
> + }
[snip]