Message ID | 20230627131040.3418538-1-msp@baylibre.com |
---|---|
Headers | show |
Series | soc: mediatek: MT8365 power support | expand |
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>
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>
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>
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>
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>
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>
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
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
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.
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.
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.
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.
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.
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.
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.