diff mbox series

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

Message ID 20221121065144.3667658-2-ping.bai@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add nxp bbnsm module support | expand

Commit Message

Jacky Bai Nov. 21, 2022, 6:51 a.m. UTC
Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).

Signed-off-by: Jacky Bai <ping.bai@nxp.com>
---
 .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103 ++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml

Comments

Krzysztof Kozlowski Nov. 21, 2022, 9:09 a.m. UTC | #1
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@nxp.com>
> ---
>  .../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@nxp.com>
> +
> +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.

> +
> +      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
Krzysztof Kozlowski Nov. 21, 2022, 9:18 a.m. UTC | #2
Also few nits:

On 21/11/2022 07:51, Jacky Bai wrote:
> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).

Subject: drop second, redundant "bindings".

> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  .../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

Drop "bindings"

> +
> +maintainers:
> +  - Jacky Bai <ping.bai@nxp.com>

Best regards,
Krzysztof
Alexandre Belloni Nov. 21, 2022, 9:27 a.m. UTC | #3
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@nxp.com>
> > ---
> >  .../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@nxp.com>
> > +
> > +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
>
Jacky Bai Nov. 21, 2022, 10:26 a.m. UTC | #4
> 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@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? ^_^

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

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

Ok, will fix it.

Thx

BR

> > +        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
Jacky Bai Nov. 21, 2022, 10:30 a.m. UTC | #5
> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> Also few nits:
> 
> On 21/11/2022 07:51, Jacky Bai wrote:
> > Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
> 
> Subject: drop second, redundant "bindings".
> 

Do you mean the redundant 'bindings' in below title line?

> > +
> > +title: NXP Battery-Backed Non-Secure Module bindings
> 
> Drop "bindings"
> 

Ok will drop it in v2.

BR

> > +
> > +maintainers:
> > +  - Jacky Bai <ping.bai@nxp.com>
> 
> Best regards,
> Krzysztof
Jacky Bai Nov. 21, 2022, 10:33 a.m. UTC | #6
> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> 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@nxp.com>
> > > ---
> > >  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
> ++++++++++++++++++
> > >  1 file changed, 103 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml

...

> > > +
> > > +title: NXP Battery-Backed Non-Secure Module bindings
> > > +
> > > +maintainers:
> > > +  - Jacky Bai <ping.bai@nxp.com>
> > > +
> > > +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

The RTC counter will be reset to 0 after PoR reset, do we still need to add
this property?

BR
> 
> > > +
> > > +      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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin
> .com%2F&amp;data=05%7C01%7Cping.bai%40nxp.com%7Cd188bbb7b6ec40
> 5c481f08dacba2af8b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638046196834682924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> C%7C&amp;sdata=L%2FTZNaG01NTrKvbKwz9%2FNFEFQ6JqdnsOIzUydww1D
> ZU%3D&amp;reserved=0
Alexandre Belloni Nov. 21, 2022, 11:10 a.m. UTC | #7
On 21/11/2022 10:33:15+0000, Jacky Bai wrote:
> > Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> > bbnsm
> > 
> > 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@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
> > ++++++++++++++++++
> > > >  1 file changed, 103 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
> 
> ...
> 
> > > > +
> > > > +title: NXP Battery-Backed Non-Secure Module bindings
> > > > +
> > > > +maintainers:
> > > > +  - Jacky Bai <ping.bai@nxp.com>
> > > > +
> > > > +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
> 
> The RTC counter will be reset to 0 after PoR reset, do we still need to add
> this property?
> 

Is this really an RTC then?

> BR
> > 
> > > > +
> > > > +      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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin
> > .com%2F&amp;data=05%7C01%7Cping.bai%40nxp.com%7Cd188bbb7b6ec40
> > 5c481f08dacba2af8b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> > 7C638046196834682924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> > wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> > C%7C&amp;sdata=L%2FTZNaG01NTrKvbKwz9%2FNFEFQ6JqdnsOIzUydww1D
> > ZU%3D&amp;reserved=0
Lee Jones Nov. 21, 2022, 12:28 p.m. UTC | #8
My email client takes around 20s to open your emails.

Please fix that.

    [-- Begin signature information --]
    Problem signature from: CN=nxa19010,OU=Developers,OU=Managed Users,OU=CN,OU=NXP,DC=wbi,DC=nxp,DC=com
                   aka: <ping.bai@nxp.com>
               created: Mon 21 Nov 2022 10:26:32 GMT
               expires: Mon 08 Apr 2024 10:15:04 BST
    [-- End signature information --]
Jacky Bai Nov. 21, 2022, 1:45 p.m. UTC | #9
> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> On 21/11/2022 10:33:15+0000, Jacky Bai wrote:
> > > Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding
> > > for nxp bbnsm
> > >
> > > 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@nxp.com>
> > > > > ---
> > > > >  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
> > > ++++++++++++++++++
> > > > >  1 file changed, 103 insertions(+)  create mode 100644
> > > > > Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
> >
> > ...
> >
> > > > > +
> > > > > +title: NXP Battery-Backed Non-Secure Module bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Jacky Bai <ping.bai@nxp.com>
> > > > > +
> > > > > +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
> >
> > The RTC counter will be reset to 0 after PoR reset, do we still need
> > to add this property?
> >
> 
> Is this really an RTC then?

Sorry, I think I misunderstand your previous comment. The 'start-year' is used to expand the rtc range,
I will add this property in V2. Thx.

BR
> 
> > BR
> > >
> > > > > +
> > > > > +      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
> > > >
> > >
> > > --
Krzysztof Kozlowski Nov. 22, 2022, 7:59 a.m. UTC | #10
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@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
Krzysztof Kozlowski Nov. 22, 2022, 7:59 a.m. UTC | #11
On 21/11/2022 11:30, Jacky Bai wrote:
>> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
>> bbnsm
>>
>> Also few nits:
>>
>> On 21/11/2022 07:51, Jacky Bai wrote:
>>> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
>>
>> Subject: drop second, redundant "bindings".
>>
> 
> Do you mean the redundant 'bindings' in below title line?

No, I meant subject.


Best regards,
Krzysztof
Alexandre Belloni Nov. 22, 2022, 1:16 p.m. UTC | #12
On 21/11/2022 13:45:27+0000, Jacky Bai wrote:
> > > > > Missing ref to rtc.yaml.
> > > > >
> > > >
> > > > This is also missing start-year
> > >
> > > The RTC counter will be reset to 0 after PoR reset, do we still need
> > > to add this property?
> > >
> > 
> > Is this really an RTC then?
> 
> Sorry, I think I misunderstand your previous comment. The 'start-year' is used to expand the rtc range,
> I will add this property in V2. Thx.
> 

I think my question is still valid, if the counter is reset to 0 on POR,
what is the use case?
Rob Herring (Arm) Nov. 22, 2022, 8:28 p.m. UTC | #13
On Mon, 21 Nov 2022 14:51:41 +0800, Jacky Bai wrote:
> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103 ++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dts:28.27-28 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221121065144.3667658-2-ping.bai@nxp.com

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command.
Jacky Bai Nov. 23, 2022, 7:43 a.m. UTC | #14
> 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@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.
>

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
Jacky Bai Nov. 23, 2022, 7:50 a.m. UTC | #15
Hi Alexandre,

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> On 21/11/2022 13:45:27+0000, Jacky Bai wrote:
> > > > > > Missing ref to rtc.yaml.
> > > > > >
> > > > >
> > > > > This is also missing start-year
> > > >
> > > > The RTC counter will be reset to 0 after PoR reset, do we still
> > > > need to add this property?
> > > >
> > >
> > > Is this really an RTC then?
> >
> > Sorry, I think I misunderstand your previous comment. The 'start-year'
> > is used to expand the rtc range, I will add this property in V2. Thx.
> >
> 
> I think my question is still valid, if the counter is reset to 0 on POR, what is the
> use case?
> 

The RTC can keep running without losing the state if the power rail to it is not cut off.
It has a battery backed supply even if the main power rail is off when device shutdown.
I thought the 'start-year' is to indicate the initial value of the RTC count after first power up.

BR
> 
> --
Jacky Bai Nov. 23, 2022, 7:54 a.m. UTC | #16
Hi Rob,

> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> 
> On Mon, 21 Nov 2022 14:51:41 +0800, Jacky Bai wrote:
> > Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
> >
> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > ---
> >  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
> ++++++++++++++++++
> >  1 file changed, 103 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
> >
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Error:
> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dts:28.27-28
> syntax error FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:406:
> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
> 

This error should be related to the 'interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;'
Do we need to change it a magic number define?

BR
Jacky Bai
> doc reference errors (make refcheckdocs):
> 
...
> 
> This check can fail if there are any dependencies. The base for a patch series
> is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command.
Krzysztof Kozlowski Nov. 23, 2022, 7:58 a.m. UTC | #17
On 23/11/2022 08:43, Jacky Bai wrote:
>> 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@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.
>>
> 
> Is it ok to keep current implementation rather than reimplement a new dedicate MFD wrapper driver?

Yes

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 23, 2022, 9:31 a.m. UTC | #18
On 23/11/2022 08:54, Jacky Bai wrote:
> Hi Rob,
> 
>> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
>> bbnsm
>>
>>
>> On Mon, 21 Nov 2022 14:51:41 +0800, Jacky Bai wrote:
>>> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
>>>
>>> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
>>> ---
>>>  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
>> ++++++++++++++++++
>>>  1 file changed, 103 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m
>> dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> Error:
>> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dts:28.27-28
>> syntax error FATAL ERROR: Unable to parse input tree
>> make[1]: *** [scripts/Makefile.lib:406:
>> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dtb] Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> make: *** [Makefile:1492: dt_binding_check] Error 2
>>
> 
> This error should be related to the 'interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;'
> Do we need to change it a magic number define?

You should include a proper header. Look at other bindings.

Best regards,
Krzysztof
Jacky Bai Nov. 23, 2022, 9:35 a.m. UTC | #19
> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for nxp
> bbnsm
> 
> On 23/11/2022 08:54, Jacky Bai wrote:
> > Hi Rob,
> >
> >> Subject: Re: [PATCH 1/4] dt-bindings: mfd: nxp,bbnsm: Add binding for
> >> nxp bbnsm
> >>
> >>
> >> On Mon, 21 Nov 2022 14:51:41 +0800, Jacky Bai wrote:
> >>> Add binding for NXP BBNSM(Battery-Backed Non-Secure Module).
> >>>
> >>> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> >>> ---
> >>>  .../devicetree/bindings/mfd/nxp,bbnsm.yaml    | 103
> >> ++++++++++++++++++
> >>>  1 file changed, 103 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/mfd/nxp,bbnsm.yaml
> >>>
> >>
> >> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> >> dt_binding_check'
> >> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >>
> >> yamllint warnings/errors:
> >>
> >> dtschema/dtc warnings/errors:
> >> Error:
> >>
> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dts:28.27-28
> >> syntax error FATAL ERROR: Unable to parse input tree
> >> make[1]: *** [scripts/Makefile.lib:406:
> >> Documentation/devicetree/bindings/mfd/nxp,bbnsm.example.dtb] Error 1
> >> make[1]: *** Waiting for unfinished jobs....
> >> make: *** [Makefile:1492: dt_binding_check] Error 2
> >>
> >
> > This error should be related to the 'interrupts = <GIC_SPI 73
> IRQ_TYPE_LEVEL_HIGH>;'
> > Do we need to change it a magic number define?
> 
> You should include a proper header. Look at other bindings.
> 

Great thx. I missed the header file including . Will fix in V2. ^_^

BR
> Best regards,
> Krzysztof
diff mbox series

Patch

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@nxp.com>
+
+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
+
+      regmap:
+        maxItems: 1
+
+      interrupts:
+        maxItems: 1
+
+    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";
+        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>;
+       };
+    };