On Wed, Mar 15, 2023, at 08:28, Jacky Huang wrote: > From: Jacky Huang <ychuang3@xxxxxxxxxxx> > > The system manager is a set of registers used for power control, > multi-function pin control, USB phy control, IP reset, and other > miscellaneous controls. It also contains some registers that > provide SoC information and status. > > Signed-off-by: Jacky Huang <ychuang3@xxxxxxxxxxx> > --- > include/linux/mfd/ma35d1-sys.h | 95 ++++++++++++++++++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 include/linux/mfd/ma35d1-sys.h > > diff --git a/include/linux/mfd/ma35d1-sys.h b/include/linux/mfd/ma35d1-sys.h > new file mode 100644 > index 000000000000..dcd85231125d > --- /dev/null > +++ b/include/linux/mfd/ma35d1-sys.h > + > +#define REG_SYS_PDID (0x000) /* Product and Device Identifier */ > +#define REG_SYS_PWRONOTP (0x004) /* Power-on Setting OTP Source */ > +#define REG_SYS_PWRONPIN (0x008) /* Power-on Setting Pin Source */ > +#define REG_SYS_RSTSTS (0x010) /* Reset Source Active Status */ ... It is a bit odd to have a header file in include/linux/mfd/ but only have the register numbers in there, and not an actual drivers/mfd/ driver to go along with them. I think what we often do is to just list the individual register numbers in the drivers that need them and not have the central header at all. On the other hand, I can see it's useful to have this documented in one place, and we clearly don't want to add a driver if none is needed. Maybe Lee has a suggestion for how he'd like to handle this. > +void ma35d1_reg_lock(void); > +void ma35d1_reg_unlock(void); These look like they were left over from an earlier version of the code. Since you use the regmap framework, I think this will take care of the locking for you. Arnd