mbox series

[v6,0/8] soc: mediatek: MT8365 power support

Message ID 20230627131040.3418538-1-msp@baylibre.com
Headers show
Series soc: mediatek: MT8365 power support | expand

Message

Markus Schneider-Pargmann June 27, 2023, 1:10 p.m. UTC
Hi,

Thanks Angelo for the feedback, this version should fix all issues you
reported as well as the kernel test robot issues.

Thanks for any feedback!

Best,
Markus

Based on v6.4-rc1

Changes in v6:
- Change flags field to be u8 instead of u32
- Use macro concatenation to simplify BUS_PROT macros:
  BUS_PROT_WR(_hwip, ...) etc.
- Use the final bit values for scpsys_bus_prot_flags from the beginning
  of the series.
- Changed scpsys_domain_data->caps to be u16 to accommodate the new flag
  MTK_SCPD_STRICT_BUS_PROTECTION.

Changes in v5:
- Create defines for all registers and bits in mt8365 power domain patch
- Redesign scpsys_bus_prot_data to use flags to store reg_update,
  clr_ack as well as the difference between SMI and INFRACFG. The code
  uses the appropriate regmap depending on the flags.
- The WAY_EN patch now uses two flags, one for inverted operations
  'BUS_PROT_INVERTED' and one to use infracfg-nao for the status flags
  'BUS_PROT_STA_COMPONENT_INFRA_NAO'.

Changes in v4:
- Redesigned WAY_EN patch and split it up in smaller patches.
- Added two documentation patches.
- Added mediatek,infracfg-nao field to the binding.

Changes in v3:
- Mainly redesigned WAY_EN patch to be easier to understand
- Rebased onto v6.0-rc1
- Several other stuff that is described in the individual patches

Changes in v2:
- Updated error handling path for scpsys_power_on()
- Minor updates described in each patch

Previous versions:
v1 - https://lore.kernel.org/linux-mediatek/20220530204214.913251-1-fparent@baylibre.com/
v2 - https://lore.kernel.org/linux-mediatek/20220725081853.1636444-1-msp@baylibre.com/
v3 - https://lore.kernel.org/linux-mediatek/20220822144303.3438467-1-msp@baylibre.com/
v4 - https://lore.kernel.org/linux-arm-kernel/20230105170735.1637416-1-msp@baylibre.com/

Alexandre Bailon (2):
  soc: mediatek: Add support for WAY_EN operations
  soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

Fabien Parent (2):
  dt-bindings: power: Add MT8365 power domains
  soc: mediatek: pm-domains: Add support for MT8365

Markus Schneider-Pargmann (4):
  soc: mediatek: pm-domains: Move bools to a flags field
  soc: mediatek: pm-domains: Split bus_prot_mask
  soc: mediatek: pm-domains: Create bus protection operation functions
  soc: mediatek: pm-domains: Unify configuration for infracfg and smi

 .../power/mediatek,power-controller.yaml      |   6 +
 drivers/soc/mediatek/mt6795-pm-domains.h      |  16 +-
 drivers/soc/mediatek/mt8167-pm-domains.h      |  20 +-
 drivers/soc/mediatek/mt8173-pm-domains.h      |  16 +-
 drivers/soc/mediatek/mt8183-pm-domains.h      | 125 ++++++----
 drivers/soc/mediatek/mt8186-pm-domains.h      | 236 ++++++++++--------
 drivers/soc/mediatek/mt8188-pm-domains.h      | 223 +++++++++++------
 drivers/soc/mediatek/mt8192-pm-domains.h      | 112 ++++++---
 drivers/soc/mediatek/mt8195-pm-domains.h      | 199 +++++++++------
 drivers/soc/mediatek/mt8365-pm-domains.h      | 197 +++++++++++++++
 drivers/soc/mediatek/mtk-pm-domains.c         | 157 ++++++++----
 drivers/soc/mediatek/mtk-pm-domains.h         |  51 ++--
 .../dt-bindings/power/mediatek,mt8365-power.h |  19 ++
 include/linux/soc/mediatek/infracfg.h         |  41 +++
 14 files changed, 972 insertions(+), 446 deletions(-)
 create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
 create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

Comments

AngeloGioacchino Del Regno July 4, 2023, 10:25 a.m. UTC | #1
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> bus_prot_mask is used for all operations, set clear and acknowledge. In
> preparation of m8365 power domain support split this one mask into two,
> one mask for set and clear, another one for acknowledge.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mtk-pm-domains.c | 24 ++++++++++++++----------
>   drivers/soc/mediatek/mtk-pm-domains.h | 14 ++++++++------
>   2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index aa9ab413479e..c801fa763e89 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -123,18 +123,20 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
>   	int i, ret;
>   
>   	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> -		u32 val, mask = bpd[i].bus_prot_mask;
> +		u32 val;
> +		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> +		u32 sta_mask = bpd[i].bus_prot_sta_mask;
>   
> -		if (!mask)
> +		if (!set_clr_mask)
>   			break;
>   
>   		if (bpd[i].flags & BUS_PROT_REG_UPDATE)
> -			regmap_set_bits(regmap, bpd[i].bus_prot_set, mask);
> +			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
>   		else
> -			regmap_write(regmap, bpd[i].bus_prot_set, mask);
> +			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
>   
>   		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> -					       val, (val & mask) == mask,
> +					       val, (val & sta_mask) == sta_mask,
>   					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>   		if (ret)
>   			return ret;
> @@ -160,21 +162,23 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>   	int i, ret;
>   
>   	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> -		u32 val, mask = bpd[i].bus_prot_mask;
> +		u32 val;
> +		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> +		u32 sta_mask = bpd[i].bus_prot_sta_mask;

Only one nitpick: val should be declared after the others....
That's just for readability and nothing else, obviously.

Anyway, after that change:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:25 a.m. UTC | #2
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> To simplify the macros, use a flags field for simple bools. This is in
> preparation for more flags.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:26 a.m. UTC | #3
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> Separate the register access used for bus protection enable/disable into
> their own functions. These will be used later for WAY_EN support.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:26 a.m. UTC | #4
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> Use flags to distinguish between infracfg and smi subsystem for a bus
> protection configuration. It simplifies enabling/disabling and prepares
> the driver for the use of another regmap for mt8365.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:27 a.m. UTC | #5
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> From: Alexandre Bailon <abailon@baylibre.com>
> 
> This updates the power domain to support WAY_EN operations. WAY_EN
> operations on mt8365 are using a different component to check for the
> acknowledgment, namely the infracfg-nao component. Also to enable a way
> it the bit needs to be cleared while disabling a way needs a bit to be
> set. To support these two operations two flags are added,
> BUS_PROT_INVERTED and BUS_PROT_STA_COMPONENT_INFRA_NAO. Additionally
> another regmap is created if the INFRA_NAO capability is set.
> 
> This operation is required by the mt8365 for the MM power domain.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>


Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:27 a.m. UTC | #6
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> From: Alexandre Bailon <abailon@baylibre.com>
> 
> This adds support for MTK_SCPD_STRICT_BUS_PROTECTION capability. It is a
> strict bus protection policy that requires the bus protection to be
> disabled before accessing the bus.
> This is required by the mt8365, for the MM power domain.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno July 4, 2023, 10:31 a.m. UTC | #7
Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add the needed board data to support MT8365 SoC.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>



..snip..


> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index 07f67b3d8e97..f853397697b5 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -2,6 +2,47 @@
>   #ifndef __SOC_MEDIATEK_INFRACFG_H
>   #define __SOC_MEDIATEK_INFRACFG_H
>   
> +#define MT8365_INFRA_TOPAXI_PROTECTEN_STA1				0x228
> +#define MT8365_INFRA_TOPAXI_PROTECTEN_SET				0x2a0
> +#define MT8365_INFRA_TOPAXI_PROTECTEN_CLR				0x2a4
> +# define MT8365_INFRA_TOPAXI_PROTECTEN_MM_M0				BIT(1)

Personally, I like this kind of indentation, but more like

#define REGADDRESS
  #define REGMASK

instead of

#define REGADDRESS
# define REGMASK

...but this doesn't count, because this header doesn't follow *either* formats,
not my preferred, nor yours: this means that, for consistency, you have to follow
what's in there already, so you have to change that to

#define REGADDRESS
#define REGMASK

...so please change that to retain consistency across the infracfg.h header,
after which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

P.S.: I'm sorry for not noticing this in v5.

Cheers,
Angelo
Markus Schneider-Pargmann July 10, 2023, 8:34 a.m. UTC | #8
Hi Angelo,

On Tue, Jul 04, 2023 at 12:31:23PM +0200, AngeloGioacchino Del Regno wrote:
> Il 27/06/23 15:10, Markus Schneider-Pargmann ha scritto:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > Add the needed board data to support MT8365 SoC.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> 
> 
> 
> ..snip..
> 
> 
> > diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> > index 07f67b3d8e97..f853397697b5 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -2,6 +2,47 @@
> >   #ifndef __SOC_MEDIATEK_INFRACFG_H
> >   #define __SOC_MEDIATEK_INFRACFG_H
> > +#define MT8365_INFRA_TOPAXI_PROTECTEN_STA1				0x228
> > +#define MT8365_INFRA_TOPAXI_PROTECTEN_SET				0x2a0
> > +#define MT8365_INFRA_TOPAXI_PROTECTEN_CLR				0x2a4
> > +# define MT8365_INFRA_TOPAXI_PROTECTEN_MM_M0				BIT(1)
> 
> Personally, I like this kind of indentation, but more like
> 
> #define REGADDRESS
>  #define REGMASK
> 
> instead of
> 
> #define REGADDRESS
> # define REGMASK
> 
> ...but this doesn't count, because this header doesn't follow *either* formats,
> not my preferred, nor yours: this means that, for consistency, you have to follow
> what's in there already, so you have to change that to
> 
> #define REGADDRESS
> #define REGMASK
> 
> ...so please change that to retain consistency across the infracfg.h header,
> after which:
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Thank you for your review. I fixed both issues you pointed out for the
next version.

Best,
Markus
Alexandre Mergnat July 10, 2023, 3:53 p.m. UTC | #9
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> To simplify the macros, use a flags field for simple bools. This is in
> preparation for more flags.
Alexandre Mergnat July 10, 2023, 3:53 p.m. UTC | #10
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> bus_prot_mask is used for all operations, set clear and acknowledge. In
> preparation of m8365 power domain support split this one mask into two,
> one mask for set and clear, another one for acknowledge.
Alexandre Mergnat July 10, 2023, 3:53 p.m. UTC | #11
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> Separate the register access used for bus protection enable/disable into
> their own functions. These will be used later for WAY_EN support.
Alexandre Mergnat July 10, 2023, 3:54 p.m. UTC | #12
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> Use flags to distinguish between infracfg and smi subsystem for a bus
> protection configuration. It simplifies enabling/disabling and prepares
> the driver for the use of another regmap for mt8365.
Alexandre Mergnat July 10, 2023, 3:54 p.m. UTC | #13
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> This updates the power domain to support WAY_EN operations. WAY_EN
> operations on mt8365 are using a different component to check for the
> acknowledgment, namely the infracfg-nao component. Also to enable a way
> it the bit needs to be cleared while disabling a way needs a bit to be
> set. To support these two operations two flags are added,
> BUS_PROT_INVERTED and BUS_PROT_STA_COMPONENT_INFRA_NAO. Additionally
> another regmap is created if the INFRA_NAO capability is set.
> 
> This operation is required by the mt8365 for the MM power domain.
Alexandre Mergnat July 10, 2023, 3:54 p.m. UTC | #14
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> This adds support for MTK_SCPD_STRICT_BUS_PROTECTION capability. It is a
> strict bus protection policy that requires the bus protection to be
> disabled before accessing the bus.
> This is required by the mt8365, for the MM power domain.
Alexandre Mergnat July 10, 2023, 3:55 p.m. UTC | #15
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>


On 27/06/2023 15:10, Markus Schneider-Pargmann wrote:
> Add the needed board data to support MT8365 SoC.