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. > >> >>> + >>> + 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
