Hi Matthias, On Fri, Feb 03, 2023 at 01:22:38PM +0100, Matthias Brugger wrote: > > > On 05/01/2023 18:07, Markus Schneider-Pargmann wrote: > > From: Fabien Parent <fparent@xxxxxxxxxxxx> > > > > Add the needed board data to support MT8365 SoC. > > > > Signed-off-by: Fabien Parent <fparent@xxxxxxxxxxxx> > > Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > > --- > > drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++ > > drivers/soc/mediatek/mtk-pm-domains.c | 5 + > > 2 files changed, 152 insertions(+) > > create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h > > > > diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h > > new file mode 100644 > > index 000000000000..8735e833b15b > > --- /dev/null > > +++ b/drivers/soc/mediatek/mt8365-pm-domains.h > > @@ -0,0 +1,147 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > + > > +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H > > +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H > > + > > +#include "mtk-pm-domains.h" > > +#include <dt-bindings/power/mediatek,mt8365-power.h> > > + > > +/* > > + * MT8365 power domain support > > + */ > > + > > +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = { > > + [MT8365_POWER_DOMAIN_MM] = { > > + .name = "mm", > > + .sta_mask = PWR_STATUS_DISP, > > + .ctl_offs = 0x30c, > > + .pwr_sta_offs = 0x0180, > > + .pwr_sta2nd_offs = 0x0184, > > + .sram_pdn_bits = GENMASK(8, 8), > > + .sram_pdn_ack_bits = GENMASK(12, 12), > > + .caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN, > > + .bp_infracfg = { > > + BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258), > > + BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228), > > + BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0), > > + BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28), > > + BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228), > > > BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the > definition. Would it make sense to create a new define like we did with > BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs. > > In any case instead of magic numbers the values should be defined in > include/linux/soc/mediatek/infracfg.h or appropiate header files. Thanks, you are right, I got rid of all the magic numbers and introduced some helper defines as well. Thank you, Markus