> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp > bbnsm > > On 21/11/2022 11:26, Jacky Bai wrote: > >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for > >> nxp bbnsm > >> > >> On 21/11/2022 07:51, Jacky Bai wrote: > >>> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module). > >>> > >>> Signed-off-by: Jacky Bai <ping.bai@xxxxxxx> > >>> --- > > > > ... > > > >>> + > >>> + properties: > >>> + compatible: > >>> + const: nxp,bbnsm-rtc > >> > >> > >> Missing ref to rtc.yaml. > > > > Ok will include in v2. > >> > >>> + > >>> + regmap: > >> > >> Use vendor prefix, descriptive name and description. Where is the > >> type of 'regmap' defined? > > > > Type is missed. Will add a description and type define if necessary. > > > >> > >>> + maxItems: 1 > >> > >> I don't think this is correct. Rob explained the simple-mfd means > >> children > > do > >> not depend on anything from the parent, but taking a regmap from its > > parent > >> contradicts it. > > > > For this BBNSM module, basically, it provides two sperate & different > > function: RTC and ON/OFF button control. But no separate register > > offset range for each of these functions. For example, the interrupt > > enable control, Interrupt status and basic function control are mixed > > in the same registers' > > different bit. > > > > Any good suggestion on how to handle such case? ^_^ > > The solution for more complex common parts, dedicated device driver (MFD > driver) with its own binding. However I understand why it would be overshoot > here. > Is it ok to keep current implementation rather than reimplement a new dedicate MFD wrapper driver? BR > > > >> > >>> + > >>> + interrupts: > >>> + maxItems: 1 > >> > >> You have same interrupt and same address space used by two devices. > >> > >> Both arguments (depending on parent regmap, sharing interrupt) > >> suggests that this is one device block, so describing it with > >> simple-mfd is quite unflexible. > >> > > > > It is depends on how SoC integrates this BBNSM module. From the BBNSM > > side, it has separate IRQ lines for RTC function and ON/OFF function. > > Different IRQ lines can be used for RTC and ON/OFF button. But in > > current i.MX93 SoC, those interrupts are ORed together at SoC level. > > That's why same interrupt in the example. > > It's fine then. > > Best regards, > Krzysztof
