On 21/11/2022 10:09:40+0100, Krzysztof Kozlowski wrote: > 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> > > --- > > .../devicetree/bindings/mfd/nxp,bbnsm.yaml | 103 ++++++++++++++++++ > > 1 file changed, 103 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml > > > > diff --git a/Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml b/Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml > > new file mode 100644 > > index 000000000000..b3f22b0daea6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml > > @@ -0,0 +1,103 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/nxp,bbnsm.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NXP Battery-Backed Non-Secure Module bindings > > + > > +maintainers: > > + - Jacky Bai <ping.bai@xxxxxxx> > > + > > +description: | > > + NXP BBNSM serves as non-volatile logic and storage for the system. > > + it Intergrates RTC & ON/OFF control. > > + The RTC can retain its state and continues counting even when the > > + main chip is power down. A time alarm is generated once the most > > + significant 32 bits of the real-time counter match the value in the > > + Time Alarm register. > > + The ON/OFF logic inside the BBNSM allows for connecting directly to > > + a PMIC or other voltage regulator device. both smart PMIC mode and > > + Dumb PMIC mode supported. > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - nxp,bbnsm > > + - const: syscon > > + - const: simple-mfd > > + > > + reg: > > + maxItems: 1 > > + > > + rtc: > > + type: object > > + > > + properties: > > + compatible: > > + const: nxp,bbnsm-rtc > > > Missing ref to rtc.yaml. > This is also missing start-year > > + > > + regmap: > > Use vendor prefix, descriptive name and description. Where is the type > of 'regmap' defined? > > > + 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. > > > + > > + 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. > > > + > > + required: > > + - compatible > > + - regmap > > + - interrupts > > + > > + additionalProperties: false > > + > > + pwrkey: > > + type: object > > + $ref: /schemas/input/input.yaml# > > + > > + properties: > > + compatible: > > + const: nxp,bbnsm-pwrkey > > + > > + regmap: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + linux,code: true > > + > > + required: > > + - compatible > > + - regmap > > + - interrupts > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - rtc > > + - pwrkey > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + bbnsm: bbnsm@44440000 { > > + compatible = "nxp,bbnsm", "syscon", "simple-mfd"; > > + reg = <0x44440000 0x10000>; > > + > > + bbnsm_rtc: rtc { > > + compatible = "nxp,bbnsm-rtc"; > > Use 4 spaces for example indentation. > > > + regmap = <&bbnsm>; > > + interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + > > + bbnsm_pwrkey: pwrkey { > > + compatible = "nxp,bbnsm-pwrkey"; > > + regmap = <&bbnsm>; > > + interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; > > + linux,code = <KEY_POWER>; > > + }; > > + }; > > Best regards, > Krzysztof > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com