Re: [PATCH v3 05/16] soc: mediatek: pm_domains: Make bus protection generic

2020-10-29 Thread Matthias Brugger




On 27/10/2020 13:57, Fabien Parent wrote:

-   ret = mtk_infracfg_set_bus_protection(pd->infracfg,
- bpd[i].bus_prot_mask,
- 
bpd[i].bus_prot_reg_update);


[snip]


-   ret = mtk_infracfg_clear_bus_protection(pd->infracfg,
-   bpd[i].bus_prot_mask,
-   
bpd[i].bus_prot_reg_update);


Since you got rid of all the dependencies to mtk-infracfg.c, maybe you
can also remove the "depends on MTK_INFRACFG" in the Kconfig.



We still need that file for the old driver.

Regards,
Matthias


Re: [PATCH v3 05/16] soc: mediatek: pm_domains: Make bus protection generic

2020-10-27 Thread Fabien Parent
> -   ret = mtk_infracfg_set_bus_protection(pd->infracfg,
> - bpd[i].bus_prot_mask,
> - 
> bpd[i].bus_prot_reg_update);

[snip]

> -   ret = mtk_infracfg_clear_bus_protection(pd->infracfg,
> -   bpd[i].bus_prot_mask,
> -   
> bpd[i].bus_prot_reg_update);

Since you got rid of all the dependencies to mtk-infracfg.c, maybe you
can also remove the "depends on MTK_INFRACFG" in the Kconfig.


Re: [PATCH v3 05/16] soc: mediatek: pm_domains: Make bus protection generic

2020-10-26 Thread Nicolas Boichat
On Tue, Oct 27, 2020 at 1:55 AM Enric Balletbo i Serra
 wrote:
>
> From: Matthias Brugger 
>
> Bus protection is not exclusively done by calling the infracfg misc driver.
> Make the calls for setting and clearing the bus protection generic so
> that we can use other blocks for it as well.
>
> Signed-off-by: Matthias Brugger 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/soc/mediatek/mtk-infracfg.c   |  5 ---
>  drivers/soc/mediatek/mtk-pm-domains.c | 53 +--
>  include/linux/soc/mediatek/infracfg.h |  5 +++
>  3 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-infracfg.c 
> b/drivers/soc/mediatek/mtk-infracfg.c
> index 4a123796aad3..0590b68e0d78 100644
> --- a/drivers/soc/mediatek/mtk-infracfg.c
> +++ b/drivers/soc/mediatek/mtk-infracfg.c
> @@ -12,11 +12,6 @@
>  #define MTK_POLL_DELAY_US   10
>  #define MTK_POLL_TIMEOUT(jiffies_to_usecs(HZ))
>
> -#define INFRA_TOPAXI_PROTECTEN 0x0220
> -#define INFRA_TOPAXI_PROTECTSTA1   0x0228
> -#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
> -#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
> -
>  /**
>   * mtk_infracfg_set_bus_protection - enable bus protection
>   * @infracfg: The infracfg regmap
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c 
> b/drivers/soc/mediatek/mtk-pm-domains.c
> index 2121e05cb9a4..92c61e59255b 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -87,18 +87,24 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> MTK_POLL_TIMEOUT);
>  }
>
> -static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
> +static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data 
> *bpd, struct regmap *regmap)
>  {
> -   const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
> int i, ret;
>
> for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> -   if (!bpd[i].bus_prot_mask)
> +   u32 val, mask = bpd[i].bus_prot_mask;
> +
> +   if (!mask)
> break;
>
> -   ret = mtk_infracfg_set_bus_protection(pd->infracfg,
> - bpd[i].bus_prot_mask,
> - 
> bpd[i].bus_prot_reg_update);
> +   if (bpd[i].bus_prot_reg_update)
> +   regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, 
> mask, mask);
> +   else
> +   regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_SET, 
> mask);
> +
> +   ret = regmap_read_poll_timeout(regmap, 
> INFRA_TOPAXI_PROTECTSTA1,
> +  val, (val & mask) == mask,
> +  MTK_POLL_DELAY_US, 
> MTK_POLL_TIMEOUT);
> if (ret)
> return ret;
> }
> @@ -106,18 +112,34 @@ static int scpsys_bus_protect_enable(struct 
> scpsys_domain *pd)
> return 0;
>  }
>
> -static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> +static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>  {
> const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;
> +   int ret;
> +
> +   ret = _scpsys_bus_protect_enable(bpd, pd->infracfg);
> +   return ret;
> +}
> +
> +static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data 
> *bpd,
> +  struct regmap *regmap)
> +{
> int i, ret;
>
> for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> -   if (!bpd[i].bus_prot_mask)
> +   u32 val, mask = bpd[i].bus_prot_mask;
> +
> +   if (!mask)
> return 0;
>
> -   ret = mtk_infracfg_clear_bus_protection(pd->infracfg,
> -   bpd[i].bus_prot_mask,
> -   
> bpd[i].bus_prot_reg_update);
> +   if (bpd[i].bus_prot_reg_update)
> +   regmap_update_bits(regmap, INFRA_TOPAXI_PROTECTEN, 
> mask, 0);
> +   else
> +   regmap_write(regmap, INFRA_TOPAXI_PROTECTEN_CLR, 
> mask);
> +
> +   ret = regmap_read_poll_timeout(regmap, 
> INFRA_TOPAXI_PROTECTSTA1,
> +  val, !(val & mask),
> +  MTK_POLL_DELAY_US, 
> MTK_POLL_TIMEOUT);
> if (ret)
> return ret;
> }
> @@ -125,6 +147,15 @@ static int scpsys_bus_protect_disable(struct 
> scpsys_domain *pd)
> return 0;
>  }
>
> +static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> +{
> +   const struct scpsys_bus_prot_data *bpd = pd->data->bp_infracfg;

More of a nit: The next patch gets rid of this line, so maybe you
don't need