[PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp bbnsm

[Krzysztof Kozlowski]

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 at nxp.com>
>>> ---
> 
> ...
> 
>>> +
>>> +    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