diff mbox series

[v6,1/7] dt-bindings: firmware: add i.MX95 SCMI Extension protocol

Message ID 20240718-imx95-bbm-misc-v2-v6-1-18f008e16e9d@nxp.com (mailing list archive)
State Superseded
Headers show
Series firmware: support i.MX95 SCMI BBM/MISC Extenstion | expand

Commit Message

Peng Fan (OSS) July 18, 2024, 7:41 a.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

Add i.MX SCMI Extension protocols bindings for:
- Battery Backed Module(BBM) Protocol
  This contains persistent storage (GPR), an RTC, and the ON/OFF button.
  The protocol can also provide access to similar functions implemented via
  external board components.
- MISC Protocol.
  This includes controls that are misc settings/actions that must be exposed
  from the SM to agents. They are device specific and are usually define to
  access bit fields in various mix block control modules, IOMUX_GPR, and
  other GPR/CSR owned by the SM.

Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml     |  5 ++-
 .../bindings/firmware/nxp,imx95-scmi.yaml          | 43 ++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski July 21, 2024, 1:36 p.m. UTC | #1 
On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX SCMI Extension protocols bindings for:
> - Battery Backed Module(BBM) Protocol
>   This contains persistent storage (GPR), an RTC, and the ON/OFF button.
>   The protocol can also provide access to similar functions implemented via
>   external board components.
> - MISC Protocol.
>   This includes controls that are misc settings/actions that must be exposed
>   from the SM to agents. They are device specific and are usually define to
>   access bit fields in various mix block control modules, IOMUX_GPR, and
>   other GPR/CSR owned by the SM.
> 
> Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>

Why quotes? Which tools did you use?

Best regards,
Krzysztof
Peng Fan July 31, 2024, 12:18 p.m. UTC | #2 
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX SCMI Extension protocols bindings for:
> > - Battery Backed Module(BBM) Protocol
> >   This contains persistent storage (GPR), an RTC, and the ON/OFF
> button.
> >   The protocol can also provide access to similar functions
> implemented via
> >   external board components.
> > - MISC Protocol.
> >   This includes controls that are misc settings/actions that must be
> exposed
> >   from the SM to agents. They are device specific and are usually
> define to
> >   access bit fields in various mix block control modules, IOMUX_GPR,
> and
> >   other GPR/CSR owned by the SM.
> >
> > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> 
> Why quotes? Which tools did you use?

I could not recall why have this. I will drop and resend the patchset.

Thanks,
Peng.

> 
> Best regards,
> Krzysztof
>
Sudeep Holla July 31, 2024, 12:35 p.m. UTC | #3 
On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> > Extension protocol
> > 
> > On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Add i.MX SCMI Extension protocols bindings for:
> > > - Battery Backed Module(BBM) Protocol
> > >   This contains persistent storage (GPR), an RTC, and the ON/OFF
> > button.
> > >   The protocol can also provide access to similar functions
> > implemented via
> > >   external board components.
> > > - MISC Protocol.
> > >   This includes controls that are misc settings/actions that must be
> > exposed
> > >   from the SM to agents. They are device specific and are usually
> > define to
> > >   access bit fields in various mix block control modules, IOMUX_GPR,
> > and
> > >   other GPR/CSR owned by the SM.
> > >
> > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> > 
> > Why quotes? Which tools did you use?
> 
> I could not recall why have this. I will drop and resend the patchset.
> 

Resend only if you have any other comments addressed, no spin just for this
one please.
Peng Fan July 31, 2024, 12:49 p.m. UTC | #4 
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> SCMI
> > > Extension protocol
> > >
> > > On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add i.MX SCMI Extension protocols bindings for:
> > > > - Battery Backed Module(BBM) Protocol
> > > >   This contains persistent storage (GPR), an RTC, and the ON/OFF
> > > button.
> > > >   The protocol can also provide access to similar functions
> > > implemented via
> > > >   external board components.
> > > > - MISC Protocol.
> > > >   This includes controls that are misc settings/actions that must
> > > > be
> > > exposed
> > > >   from the SM to agents. They are device specific and are usually
> > > define to
> > > >   access bit fields in various mix block control modules,
> > > > IOMUX_GPR,
> > > and
> > > >   other GPR/CSR owned by the SM.
> > > >
> > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> > >
> > > Why quotes? Which tools did you use?
> >
> > I could not recall why have this. I will drop and resend the patchset.
> >
> 
> Resend only if you have any other comments addressed, no spin just
> for this one please.

Sorry, I pushed the button too quick to send out v7(just correct 
this R-b tag and rebased to linux-next) before checking my inbox.

Regards,
Peng.

> 
> --
> Regards,
> Sudeep
Sudeep Holla July 31, 2024, 12:58 p.m. UTC | #5 
On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> > Extension protocol
> >
> > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> > SCMI
> > > > Extension protocol
> > > >
> > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > Add i.MX SCMI Extension protocols bindings for:
> > > > > - Battery Backed Module(BBM) Protocol
> > > > >   This contains persistent storage (GPR), an RTC, and the ON/OFF
> > > > button.
> > > > >   The protocol can also provide access to similar functions
> > > > implemented via
> > > > >   external board components.
> > > > > - MISC Protocol.
> > > > >   This includes controls that are misc settings/actions that must
> > > > > be
> > > > exposed
> > > > >   from the SM to agents. They are device specific and are usually
> > > > define to
> > > > >   access bit fields in various mix block control modules,
> > > > > IOMUX_GPR,
> > > > and
> > > > >   other GPR/CSR owned by the SM.
> > > > >
> > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> > > >
> > > > Why quotes? Which tools did you use?
> > >
> > > I could not recall why have this. I will drop and resend the patchset.
> > >
> >
> > Resend only if you have any other comments addressed, no spin just
> > for this one please.
>
> Sorry, I pushed the button too quick to send out v7(just correct
> this R-b tag and rebased to linux-next) before checking my inbox.
>

Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after the
last version of any of your patch set without any version bump before I
can look at it. Otherwise it is quite impossible to match your speed of
patch respinning and the whole review process gets complicated to follow.

Also it is bit sad that you thought it needs to be re-spinned just for this
which can be easily fixed when applying. Also I haven't even started looking
at this series for the reason I mentioned above.

--
Regards,
Sudeep
Peng Fan July 31, 2024, 1:29 p.m. UTC | #6 
Hi Sudeep,

> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> SCMI
> > > Extension protocol
> > >
> > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> > > SCMI
> > > > > Extension protocol
> > > > >
> > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > Add i.MX SCMI Extension protocols bindings for:
> > > > > > - Battery Backed Module(BBM) Protocol
> > > > > >   This contains persistent storage (GPR), an RTC, and the
> > > > > > ON/OFF
> > > > > button.
> > > > > >   The protocol can also provide access to similar functions
> > > > > implemented via
> > > > > >   external board components.
> > > > > > - MISC Protocol.
> > > > > >   This includes controls that are misc settings/actions that
> > > > > > must be
> > > > > exposed
> > > > > >   from the SM to agents. They are device specific and are
> > > > > > usually
> > > > > define to
> > > > > >   access bit fields in various mix block control modules,
> > > > > > IOMUX_GPR,
> > > > > and
> > > > > >   other GPR/CSR owned by the SM.
> > > > > >
> > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> > > > >
> > > > > Why quotes? Which tools did you use?
> > > >
> > > > I could not recall why have this. I will drop and resend the
> patchset.
> > > >
> > >
> > > Resend only if you have any other comments addressed, no spin
> just
> > > for this one please.
> >
> > Sorry, I pushed the button too quick to send out v7(just correct this
> > R-b tag and rebased to linux-next) before checking my inbox.
> >
> 
> Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after
> the last version of any of your patch set without any version bump
> before I can look at it. Otherwise it is quite impossible to match your
> speed of patch respinning and the whole review process gets
> complicated to follow.

I think you might be busy. So just after addressing Cristian's
comments, I post a new version. Then I think Cristian's R-b
is good enough for the patch to be queued into your tree.
So I did not wait for your reply on previous patchset.

I usually wait more than one week, near two weeks. If no comments,
I will ping.

> 
> Also it is bit sad that you thought it needs to be re-spinned just for this
> which can be easily fixed when applying. Also I haven't even started
> looking at this series for the reason I mentioned above.

I did not intend to bring trouble for your reviewing. For future scmi
related patches that I do, I will wait for two weeks to collect
comments. If no comments, I will post a ping(if you have a patchwork
queue to check, that would be better). I will wait for your
reply before post a new version. But if Cristian's comments are
enough for me to do new version after two weeks time window,
that would also be good.

I hope the upper approach is good for you.

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep
Peng Fan Aug. 6, 2024, 2:04 p.m. UTC | #7 
> Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On Wed, Jul 31, 2024 at 12:49:59PM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> SCMI
> > > Extension protocol
> > >
> > > On Wed, Jul 31, 2024 at 12:18:28PM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH v6 1/7] dt-bindings: firmware: add i.MX95
> > > SCMI
> > > > > Extension protocol
> > > > >
> > > > > On 18/07/2024 09:41, Peng Fan (OSS) wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > Add i.MX SCMI Extension protocols bindings for:
> > > > > > - Battery Backed Module(BBM) Protocol
> > > > > >   This contains persistent storage (GPR), an RTC, and the
> > > > > > ON/OFF
> > > > > button.
> > > > > >   The protocol can also provide access to similar functions
> > > > > implemented via
> > > > > >   external board components.
> > > > > > - MISC Protocol.
> > > > > >   This includes controls that are misc settings/actions that
> > > > > > must be
> > > > > exposed
> > > > > >   from the SM to agents. They are device specific and are
> > > > > > usually
> > > > > define to
> > > > > >   access bit fields in various mix block control modules,
> > > > > > IOMUX_GPR,
> > > > > and
> > > > > >   other GPR/CSR owned by the SM.
> > > > > >
> > > > > > Reviewed-by: "Rob Herring (Arm)" <robh@kernel.org>
> > > > >
> > > > > Why quotes? Which tools did you use?
> > > >
> > > > I could not recall why have this. I will drop and resend the
> patchset.
> > > >
> > >
> > > Resend only if you have any other comments addressed, no spin
> just
> > > for this one please.
> >
> > Sorry, I pushed the button too quick to send out v7(just correct this
> > R-b tag and rebased to linux-next) before checking my inbox.
> >
> 

I just rechecked the whole series patch version history from the
cover-letter. And I have to respond again to your reply.

> Just makes me wonder if I should wait for 3-4 pings + 2-3 weeks after
> the last version of any of your patch set without any version bump
> before I can look at it. 

I hope not. I think I not did rapid version respin.

Otherwise it is quite impossible to match your
> speed of patch respinning and the whole review process gets
> complicated to follow.

I'd argue for this.

If you have read the cover-letter.
https://lore.kernel.org/all/20240731-imx95-bbm-misc-v2-v7-0-a41394365602@nxp.com/

The patch version timeline is as below:
v1: 2024-2-2
v2: 2024-4-5
v3: 2024-4-12
v4: 2024-5-24
v5: 2024-6-21
v6: 2024-7-18
v7: 2024-7-31

v2->v3 is 1 week, I admit this is short period.

as you said, you not look into this patchset. It is almost 6 months, I not think
it is a rapid patch version respin except that I did a quick update in v7 with
just a minor R-b tag update after I reply in .

> 
> Also it is bit sad that you thought it needs to be re-spinned just for this
> which can be easily fixed when applying. 

I admit it is not good to just update R-b with a new version. But.. 

Also I haven't even started
> looking at this series for the reason I mentioned above.
> 

It is 6 months.. if just because I missed your 20mins reply, you think
the whole patchset should be delayed or else, that is not fair,
there must be some misunderstanding here.

Thanks,
Peng.

> --
> Regards,
> Sudeep
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 4d823f3b1f0e..47f0487e35de 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -22,6 +22,9 @@  description: |
 
   [0] https://developer.arm.com/documentation/den0056/latest
 
+anyOf:
+  - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
+
 properties:
   $nodename:
     const: scmi
@@ -284,7 +287,7 @@  properties:
     required:
       - reg
 
-additionalProperties: false
+unevaluatedProperties: false
 
 $defs:
   protocol-node:
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
new file mode 100644
index 000000000000..1a95010a546b
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
@@ -0,0 +1,43 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
+
+maintainers:
+  - Peng Fan <peng.fan@nxp.com>
+
+properties:
+  protocol@81:
+    $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        const: 0x81
+
+  protocol@84:
+    $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        const: 0x84
+
+      nxp,ctrl-ids:
+        description:
+          Each entry consists of 2 integers, represents the ctrl id and the value
+        items:
+          items:
+            - description: the ctrl id index
+              enum: [0, 1, 2, 3, 4, 5, 6, 7, 0x8000, 0x8001, 0x8002, 0x8003,
+                     0x8004, 0x8005, 0x8006, 0x8007]
+            - description: the value assigned to the ctrl id
+        minItems: 1
+        maxItems: 16
+        $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+additionalProperties: true