Re: [PATCH v3 07/16] soc: mediatek: pm-domains: Add extra sram control

[Nicolas Boichat]

On Tue, Oct 27, 2020 at 1:55 AM Enric Balletbo i Serra
<enric.balletbo@xxxxxxxxxxxxx> wrote:
>
> From: Matthias Brugger <mbrugger@xxxxxxxx>
>
> For some power domains like vpu_core on MT8183 whose sram need to do clock
> and internal isolation while power on/off sram. We add a cap
> "MTK_SCPD_SRAM_ISO" to judge if we need to do the extra sram isolation
> control or not.
>
> Signed-off-by: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>
> Signed-off-by: Matthias Brugger <mbrugger@xxxxxxxx>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Nit, split readl(ctl_addr) | pd->data->sram_pdn_bits in two lines.
> - Use regmap API
>
>  drivers/soc/mediatek/mtk-pm-domains.c | 25 +++++++++++++++++++++++--
>  drivers/soc/mediatek/mtk-pm-domains.h |  1 +
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 006eb7571d32..82f6d937ed93 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -24,6 +24,8 @@
>  #define PWR_ON_BIT                     BIT(2)
>  #define PWR_ON_2ND_BIT                 BIT(3)
>  #define PWR_CLK_DIS_BIT                        BIT(4)
> +#define PWR_SRAM_CLKISO_BIT            BIT(5)
> +#define PWR_SRAM_ISOINT_B_BIT          BIT(6)
>
>  struct scpsys_domain {
>         struct generic_pm_domain genpd;
> @@ -65,12 +67,24 @@ 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;
> +       int ret;
>
>         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);
> +       ret = regmap_read_poll_timeout(scpsys->base, pd->data->ctl_offs, tmp,
> +                                      (tmp & pdn_ack) == 0, MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO)) {
> +               regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_SRAM_ISOINT_B_BIT,
> +                                  PWR_SRAM_ISOINT_B_BIT);

regmap_set_bits?

> +               udelay(1);
> +               regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_SRAM_CLKISO_BIT, 0);

regmap_clear_bits?

But then I'm afraid we'll want to modify it everywhere for consistency.

I only noticed here as the first call spans over 2 lines..

> +       }
> +
> +       return 0;
>  }
>
>  static int scpsys_sram_disable(struct scpsys_domain *pd)
> @@ -79,6 +93,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
>         struct scpsys *scpsys = pd->scpsys;
>         unsigned int tmp;
>
> +       if (MTK_SCPD_CAPS(pd, MTK_SCPD_SRAM_ISO)) {
> +               regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_SRAM_CLKISO_BIT,
> +                                  PWR_SRAM_CLKISO_BIT);
> +               udelay(1);
> +               regmap_update_bits(scpsys->base, pd->data->ctl_offs, PWR_SRAM_ISOINT_B_BIT, 0);
> +       }
> +
>         regmap_update_bits(scpsys->base, pd->data->ctl_offs, pd->data->sram_pdn_bits,
>                            pd->data->sram_pdn_bits);
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index 7b1abcfc4ba3..4152b96c1b29 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -5,6 +5,7 @@
>
>  #define MTK_SCPD_ACTIVE_WAKEUP         BIT(0)
>  #define MTK_SCPD_FWAIT_SRAM            BIT(1)
> +#define MTK_SCPD_SRAM_ISO              BIT(2)
>  #define MTK_SCPD_CAPS(_scpd, _x)       ((_scpd)->data->caps & (_x))
>
>  #define SPM_VDE_PWR_CON                        0x0210
> --
> 2.28.0
>