Hi Matthias, On Tue, 2022-01-18 at 14:17 +0100, Matthias Brugger wrote: > > On 18/01/2022 10:25, Johnson Wang wrote: > > Hi Matthias, > > > > On Fri, 2022-01-14 at 16:46 +0100, Matthias Brugger wrote: > > > > > > On 07/01/2022 11:46, Johnson Wang wrote: > > > > MT8186 are highly integrated SoC and use PMIC_MT6366 for > > > > power management. This patch adds pwrap master driver to > > > > access PMIC_MT6366. > > > > > > > > > > It seems this new arbiter is significantly different from the > > > version > > > 1. Please > > > explain that in the commit message. > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@xxxxxxxxxxxx> > > > > --- > > > > drivers/soc/mediatek/mtk-pmic-wrap.c | 72 > > > > ++++++++++++++++++++++++++++ > > > > 1 file changed, 72 insertions(+) > > > > > > > > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c > > > > b/drivers/soc/mediatek/mtk-pmic-wrap.c > > > > index 952bc554f443..78866ebf7f04 100644 > > > > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c > > > > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c > > > > @@ -30,6 +30,7 @@ > > > > #define PWRAP_GET_WACS_REQ(x) (((x) >> 19) & > > > > 0x00000001) > > > > #define PWRAP_STATE_SYNC_IDLE0 BIT(20) > > > > #define PWRAP_STATE_INIT_DONE0 BIT(21) > > > > +#define PWRAP_STATE_INIT_DONE0_V2 BIT(22) > > > > > > That's a strange name, does it come from the datasheet > > > description? > > > > Thanks for your review. > > > > No, there is only PWRAP_STATE_INIT_DONE0 in MT8186 datasheet. > > However, it's the 22nd bit in MT8186 and the 21st bit in other > > SoCs. > > So we changed its name to avoid redefinition of > > PWRAP_STATE_INIT_DONE0. > > > > Could you give us some suggestion on proper definition naming? > > Do you think PWRAP_STATE_INIT_DONE0_MT8186 will be a better choice? > > > > Is this a difference that only will show up on the PMIC-wrapper of > MT8186 or > will other SoCs include the same IP? If not, then > PWRAP_STATE_INIT_DONE0_MT8186 > should be fine. Otherwise we would need a better name. > In fact, we don't know whether following SoCs will include the same IP in the future. Can we just replace _V2 with _MT8186 this time or please give us some suggestion on it. Thanks for your response. > > > > > > > #define PWRAP_STATE_INIT_DONE1 BIT(15) > > > > > > > > /* macro for WACS FSM */ > > > > @@ -77,6 +78,8 @@ > > > > #define PWRAP_CAP_INT1_EN BIT(3) > > > > #define PWRAP_CAP_WDT_SRC1 BIT(4) > > > > #define PWRAP_CAP_ARB BIT(5) > > > > +#define PWRAP_CAP_MONITOR_V2 BIT(6) > > > > > > Not used capability, please delete. > > > > > > > > > Regards, > > > Matthias > > > > PWRAP_CAP_MONITOR_V2 is not used right now. > > We can remove it in next version. > > But this capability will be added when we need it. > > > > That's OK, we should add capability definitions once they are added > to the > driver, not before that. > > Thanks, > Matthias
