[RFC PATCH 0/8] Linux SBI MPXY and RPMI drivers

[RFC PATCH 0/8] Linux SBI MPXY and RPMI drivers

[Anup Patel]

The SBI v3.0 MPXY extension [1] and RPMI v1.0 [2] specifications are
in stable state and under ARC review at the RISC-V International so
as part of the RVI process we would like to receive an early feedback
on the device tree bindings and mailbox drivers hence this series.

Currently, most of the RPMI and MPXY drivers are in OpenSBI whereas
for Linux only has SBI MPXY mailbox controller driver and RPMI clock
driver. This series will be expanded in the future to include some
more RPMI drivers and ACPI support.

These patches can be found in the riscv_sbi_mpxy_mailbox_v1 branch at:
https://github.com/avpatel/linux.git

To test these patches, boot Linux on "virt,rpmi=on" machine with
latest OpenSBI and QEMU from the dev-upstream QEMU branch at:
https://github.com/ventanamicro/qemu.git

[1] https://github.com/riscv-non-isa/riscv-sbi-doc/releases
[2] https://github.com/riscv-non-isa/riscv-rpmi/releases

Anup Patel (7):
  riscv: Add new error codes defined by SBI v3.0
  dt-bindings: mailbox: Add bindings for RPMI shared memory transport
  dt-bindings: mailbox: Add bindings for RISC-V SBI MPXY extension
  RISC-V: Add defines for the SBI message proxy extension
  mailbox: Add common header for RPMI messages sent via mailbox
  mailbox: Add RISC-V SBI message proxy (MPXY) based mailbox driver
  dt-bindings: clock: Add bindings for RISC-V RPMI clock service group

Rahul Pathak (1):
  clk: Add clock driver for the RISC-V RPMI clock service group

 .../bindings/clock/riscv,rpmi-clock.yaml      |  78 ++
 .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 +++
 .../bindings/mailbox/riscv,sbi-mpxy-mbox.yaml |  54 +
 arch/riscv/include/asm/sbi.h                  |  69 ++
 drivers/clk/Kconfig                           |   8 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-rpmi.c                        | 588 +++++++++++
 drivers/mailbox/Kconfig                       |  11 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/riscv-sbi-mpxy-mbox.c         | 979 ++++++++++++++++++
 include/linux/mailbox/riscv-rpmi-message.h    | 218 ++++
 11 files changed, 2143 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml
 create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
 create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml
 create mode 100644 drivers/clk/clk-rpmi.c
 create mode 100644 drivers/mailbox/riscv-sbi-mpxy-mbox.c
 create mode 100644 include/linux/mailbox/riscv-rpmi-message.h

-- 
2.43.0

[RFC PATCH 1/8] riscv: Add new error codes defined by SBI v3.0

[Anup Patel]

The SBI v3.0 defines new error codes so add these new error codes
to the asm/sbi.h for use by newer SBI extensions.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 arch/riscv/include/asm/sbi.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 6c82318065cf..bc35ad750e28 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -418,6 +418,10 @@ enum sbi_ext_nacl_feature {
 #define SBI_ERR_ALREADY_STARTED -7
 #define SBI_ERR_ALREADY_STOPPED -8
 #define SBI_ERR_NO_SHMEM	-9
+#define SBI_ERR_INVALID_STATE	-10
+#define SBI_ERR_BAD_RANGE	-11
+#define SBI_ERR_TIMEOUT		-12
+#define SBI_ERR_IO		-13
 
 extern unsigned long sbi_spec_version;
 struct sbiret {
@@ -504,9 +508,15 @@ static inline int sbi_err_map_linux_errno(int err)
 	case SBI_ERR_DENIED:
 		return -EPERM;
 	case SBI_ERR_INVALID_PARAM:
+	case SBI_ERR_INVALID_STATE:
+	case SBI_ERR_BAD_RANGE:
 		return -EINVAL;
 	case SBI_ERR_INVALID_ADDRESS:
 		return -EFAULT;
+	case SBI_ERR_TIMEOUT:
+		return -ETIMEDOUT;
+	case SBI_ERR_IO:
+		return -EIO;
 	case SBI_ERR_NOT_SUPPORTED:
 	case SBI_ERR_FAILURE:
 	default:
-- 
2.43.0

[RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Anup Patel]

Add device tree bindings for the common RISC-V Platform Management
Interface (RPMI) shared memory transport as a mailbox controller.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 ++++++++++++++++++
 1 file changed, 135 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
new file mode 100644
index 000000000000..8d713ba7ffc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Platform Management Interface (RPMI) shared memory mailbox
+
+maintainers:
+  - Anup Patel <anup@brainfault.org>
+
+description: |
+  The RISC-V Platform Management Interface (RPMI) [1] defines a common shared
+  memory based RPMI transport. This RPMI shared memory transport integrates as
+  mailbox controller in the SBI implementation or supervisor software whereas
+  each RPMI service group is mailbox client in the SBI implementation and
+  supervisor software.
+
+  ===========================================
+  References
+  ===========================================
+
+  [1] RISC-V Platform Management Interface (RPMI)
+      https://github.com/riscv-non-isa/riscv-rpmi/releases
+
+properties:
+  compatible:
+    const: riscv,rpmi-shmem-mbox
+
+  reg:
+    oneOf:
+      - items:
+        - description: A2P request queue base address
+        - description: P2A acknowledgment queue base address
+        - description: P2A request queue base address
+        - description: A2P acknowledgment queue base address
+        - description: A2P doorbell address
+      - items:
+        - description: A2P request queue base address
+        - description: P2A acknowledgment queue base address
+        - description: A2P doorbell address
+
+  reg-names:
+    oneOf:
+      - items:
+        - const: a2p-req
+        - const: p2a-ack
+        - const: p2a-req
+        - const: a2p-ack
+        - const: db-reg
+      - items:
+        - const: a2p-req
+        - const: p2a-ack
+        - const: db-reg
+
+  interrupts:
+    minItems: 1
+    maxItems: 1
+    description:
+      The RPMI shared memory transport supports wired interrupt specified by
+      this property as the P2A doorbell.
+
+  msi-parent:
+    description:
+      The RPMI shared memory transport supports MSI as P2A doorbell and this
+      property specifies the target MSI controller.
+
+  riscv,slot-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 64
+    description:
+      Power-of-2 RPMI slot size of the RPMI shared memory transport.
+
+  riscv,db-mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Update only the register bits of doorbell defined by the mask (32 bit).
+      If this property is not present then mask is assumed to be 0xffffffff.
+
+  riscv,db-value:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Value written to the doorbell register bits (32-bit access) specified
+      by the riscv,db-mask property. If this property is not present then
+      value is assumed to be 0x1.
+
+  "#mbox-cells":
+    const: 1
+    description:
+      The first cell specifies RPMI service group ID.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - riscv,slot-size
+  - "#mbox-cells"
+
+anyOf:
+  - required:
+      - interrupts
+  - required:
+      - msi-parent
+
+additionalProperties: false
+
+examples:
+  - |
+    // Example 1 (RPMI shared memory with only 2 queues):
+    mailbox@10080000 {
+        compatible = "riscv,rpmi-shmem-mbox";
+        reg = <0x00 0x10080000 0x00 0x10000>,
+              <0x00 0x10090000 0x00 0x10000>,
+              <0x00 0x100a0000 0x00 0x4>;
+        reg-names = "a2p-req", "p2a-ack", "db-reg";
+        msi-parent = <&imsic_mlevel>;
+        riscv,slot-size = <64>;
+        #mbox-cells = <1>;
+    };
+  - |
+    // Example 2 (RPMI shared memory with only 4 queues):
+    mailbox@10001000 {
+        compatible = "riscv,rpmi-shmem-mbox";
+        reg = <0x00 0x10001000 0x00 0x800>,
+              <0x00 0x10001800 0x00 0x800>,
+              <0x00 0x10002000 0x00 0x800>,
+              <0x00 0x10002800 0x00 0x800>,
+              <0x00 0x10003000 0x00 0x4>;
+        reg-names = "a2p-req", "p2a-ack", "db-reg";
+        msi-parent = <&imsic_mlevel>;
+        riscv,slot-size = <64>;
+        riscv,db-mask = <0x00008000>;
+        riscv,db-value = <0x00008000>;
+        #mbox-cells = <1>;
+    };
-- 
2.43.0

Re: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Rob Herring (Arm)]

On Mon, 16 Dec 2024 14:18:11 +0530, Anup Patel wrote:
> Add device tree bindings for the common RISC-V Platform Management
> Interface (RPMI) shared memory transport as a mailbox controller.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 ++++++++++++++++++
>  1 file changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml:33:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml:39:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml:46:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml:52:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml: properties:interrupts: 'anyOf' conditional failed, one must be fixed:
	'minItems' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of minItems/maxItems/items
	hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
 	 $id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.example.dtb: mailbox@10080000: reg: 'oneOf' conditional failed, one must be fixed:
	[[0, 268959744], [0, 65536], [0, 269025280], [0, 65536], [0, 269090816], [0, 4]] is too long
	from schema $id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.example.dtb: mailbox@10001000: reg: 'oneOf' conditional failed, one must be fixed:
	[[0, 268439552], [0, 2048], [0, 268441600], [0, 2048], [0, 268443648], [0, 2048], [0, 268445696], [0, 2048], [0, 268447744], [0, 4]] is too long
	from schema $id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241216084817.373131-3-apatel@ventanamicro.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4261 bytes --]

On Mon, Dec 16, 2024 at 02:18:11PM +0530, Anup Patel wrote:
> Add device tree bindings for the common RISC-V Platform Management
> Interface (RPMI) shared memory transport as a mailbox controller.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 ++++++++++++++++++
>  1 file changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> new file mode 100644
> index 000000000000..8d713ba7ffc7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V Platform Management Interface (RPMI) shared memory mailbox
> +
> +maintainers:
> +  - Anup Patel <anup@brainfault.org>
> +
> +description: |
> +  The RISC-V Platform Management Interface (RPMI) [1] defines a common shared
> +  memory based RPMI transport. This RPMI shared memory transport integrates as
> +  mailbox controller in the SBI implementation or supervisor software whereas
> +  each RPMI service group is mailbox client in the SBI implementation and
> +  supervisor software.
> +
> +  ===========================================
> +  References
> +  ===========================================
> +
> +  [1] RISC-V Platform Management Interface (RPMI)
> +      https://github.com/riscv-non-isa/riscv-rpmi/releases
> +
> +properties:
> +  compatible:
> +    const: riscv,rpmi-shmem-mbox
> +
> +  reg:
> +    oneOf:
> +      - items:
> +        - description: A2P request queue base address
> +        - description: P2A acknowledgment queue base address
> +        - description: P2A request queue base address
> +        - description: A2P acknowledgment queue base address
> +        - description: A2P doorbell address
> +      - items:
> +        - description: A2P request queue base address
> +        - description: P2A acknowledgment queue base address
> +        - description: A2P doorbell address
> +
> +  reg-names:
> +    oneOf:
> +      - items:
> +        - const: a2p-req
> +        - const: p2a-ack
> +        - const: p2a-req
> +        - const: a2p-ack
> +        - const: db-reg
> +      - items:
> +        - const: a2p-req
> +        - const: p2a-ack
> +        - const: db-reg
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 1
> +    description:
> +      The RPMI shared memory transport supports wired interrupt specified by
> +      this property as the P2A doorbell.
> +
> +  msi-parent:
> +    description:
> +      The RPMI shared memory transport supports MSI as P2A doorbell and this
> +      property specifies the target MSI controller.
> +
> +  riscv,slot-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 64
> +    description:
> +      Power-of-2 RPMI slot size of the RPMI shared memory transport.
> +
> +  riscv,db-mask:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Update only the register bits of doorbell defined by the mask (32 bit).

For an untested RFC, the bindings here look mostly fine. I'd suggest
renaming "db" to "doorbell" in properties etc, since we can easily
afford the extra characters. Please make sure to actually test the
bindings next time around, looks like all 3 bindings produced warnings.

General comments like not needing to provide minItems: 1 when maxItems
is also 1, and not needing a | unless you have formatting to preserve.

> +      If this property is not present then mask is assumed to be 0xffffffff.

Also, things like this should be handled with a `default: 0xffffffff`,
rather than describing it in text.

I'll give the !rfc bindings a proper review when they appear, but before
that I'll give the code here a go too - thanks for sending the patches.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Anup Patel]

On Tue, Dec 17, 2024 at 12:19 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Dec 16, 2024 at 02:18:11PM +0530, Anup Patel wrote:
> > Add device tree bindings for the common RISC-V Platform Management
> > Interface (RPMI) shared memory transport as a mailbox controller.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 ++++++++++++++++++
> >  1 file changed, 135 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> > new file mode 100644
> > index 000000000000..8d713ba7ffc7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> > @@ -0,0 +1,135 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V Platform Management Interface (RPMI) shared memory mailbox
> > +
> > +maintainers:
> > +  - Anup Patel <anup@brainfault.org>
> > +
> > +description: |
> > +  The RISC-V Platform Management Interface (RPMI) [1] defines a common shared
> > +  memory based RPMI transport. This RPMI shared memory transport integrates as
> > +  mailbox controller in the SBI implementation or supervisor software whereas
> > +  each RPMI service group is mailbox client in the SBI implementation and
> > +  supervisor software.
> > +
> > +  ===========================================
> > +  References
> > +  ===========================================
> > +
> > +  [1] RISC-V Platform Management Interface (RPMI)
> > +      https://github.com/riscv-non-isa/riscv-rpmi/releases
> > +
> > +properties:
> > +  compatible:
> > +    const: riscv,rpmi-shmem-mbox
> > +
> > +  reg:
> > +    oneOf:
> > +      - items:
> > +        - description: A2P request queue base address
> > +        - description: P2A acknowledgment queue base address
> > +        - description: P2A request queue base address
> > +        - description: A2P acknowledgment queue base address
> > +        - description: A2P doorbell address
> > +      - items:
> > +        - description: A2P request queue base address
> > +        - description: P2A acknowledgment queue base address
> > +        - description: A2P doorbell address
> > +
> > +  reg-names:
> > +    oneOf:
> > +      - items:
> > +        - const: a2p-req
> > +        - const: p2a-ack
> > +        - const: p2a-req
> > +        - const: a2p-ack
> > +        - const: db-reg
> > +      - items:
> > +        - const: a2p-req
> > +        - const: p2a-ack
> > +        - const: db-reg
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 1
> > +    description:
> > +      The RPMI shared memory transport supports wired interrupt specified by
> > +      this property as the P2A doorbell.
> > +
> > +  msi-parent:
> > +    description:
> > +      The RPMI shared memory transport supports MSI as P2A doorbell and this
> > +      property specifies the target MSI controller.
> > +
> > +  riscv,slot-size:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 64
> > +    description:
> > +      Power-of-2 RPMI slot size of the RPMI shared memory transport.
> > +
> > +  riscv,db-mask:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Update only the register bits of doorbell defined by the mask (32 bit).
>
> For an untested RFC, the bindings here look mostly fine. I'd suggest
> renaming "db" to "doorbell" in properties etc, since we can easily
> afford the extra characters. Please make sure to actually test the
> bindings next time around, looks like all 3 bindings produced warnings.

Okay, I will fix these issues in the next revision.

>
> General comments like not needing to provide minItems: 1 when maxItems
> is also 1, and not needing a | unless you have formatting to preserve.

Okay, I will update.

>
> > +      If this property is not present then mask is assumed to be 0xffffffff.
>
> Also, things like this should be handled with a `default: 0xffffffff`,
> rather than describing it in text.

Okay, I will update.

>
> I'll give the !rfc bindings a proper review when they appear, but before
> that I'll give the code here a go too - thanks for sending the patches.
>
> Cheers,
> Conor.

Regards,
Anup

RE: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Leyfoon Tan]

> -----Original Message-----
> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Monday, December 16, 2024 4:48 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> <jassisinghbrar@gmail.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> <anup@brainfault.org>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> Subject: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared
> memory transport
> 
[...]

> +
> +examples:
> +  - |
> +    // Example 1 (RPMI shared memory with only 2 queues):
> +    mailbox@10080000 {
> +        compatible = "riscv,rpmi-shmem-mbox";
> +        reg = <0x00 0x10080000 0x00 0x10000>,
> +              <0x00 0x10090000 0x00 0x10000>,
> +              <0x00 0x100a0000 0x00 0x4>;
> +        reg-names = "a2p-req", "p2a-ack", "db-reg";
> +        msi-parent = <&imsic_mlevel>;
> +        riscv,slot-size = <64>;
> +        #mbox-cells = <1>;
> +    };
> +  - |
> +    // Example 2 (RPMI shared memory with only 4 queues):
> +    mailbox@10001000 {
> +        compatible = "riscv,rpmi-shmem-mbox";
> +        reg = <0x00 0x10001000 0x00 0x800>,
> +              <0x00 0x10001800 0x00 0x800>,
> +              <0x00 0x10002000 0x00 0x800>,
> +              <0x00 0x10002800 0x00 0x800>,
> +              <0x00 0x10003000 0x00 0x4>;
> +        reg-names = "a2p-req", "p2a-ack", "db-reg";

reg has 5 entries, missing 2 reg-names?

> +        msi-parent = <&imsic_mlevel>;
> +        riscv,slot-size = <64>;
> +        riscv,db-mask = <0x00008000>;
> +        riscv,db-value = <0x00008000>;
> +        #mbox-cells = <1>;
> +    };
> --
> 2.43.0

Regards
Ley Foon

Re: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Anup Patel]

On Tue, Dec 24, 2024 at 7:49 AM Leyfoon Tan
<leyfoon.tan@starfivetech.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Anup Patel <apatel@ventanamicro.com>
> > Sent: Monday, December 16, 2024 4:48 PM
> > To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> > <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> > <jassisinghbrar@gmail.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> > <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> > Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> > <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> > Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> > <anup@brainfault.org>; linux-clk@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> > kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> > Subject: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared
> > memory transport
> >
> [...]
>
> > +
> > +examples:
> > +  - |
> > +    // Example 1 (RPMI shared memory with only 2 queues):
> > +    mailbox@10080000 {
> > +        compatible = "riscv,rpmi-shmem-mbox";
> > +        reg = <0x00 0x10080000 0x00 0x10000>,
> > +              <0x00 0x10090000 0x00 0x10000>,
> > +              <0x00 0x100a0000 0x00 0x4>;
> > +        reg-names = "a2p-req", "p2a-ack", "db-reg";
> > +        msi-parent = <&imsic_mlevel>;
> > +        riscv,slot-size = <64>;
> > +        #mbox-cells = <1>;
> > +    };
> > +  - |
> > +    // Example 2 (RPMI shared memory with only 4 queues):
> > +    mailbox@10001000 {
> > +        compatible = "riscv,rpmi-shmem-mbox";
> > +        reg = <0x00 0x10001000 0x00 0x800>,
> > +              <0x00 0x10001800 0x00 0x800>,
> > +              <0x00 0x10002000 0x00 0x800>,
> > +              <0x00 0x10002800 0x00 0x800>,
> > +              <0x00 0x10003000 0x00 0x4>;
> > +        reg-names = "a2p-req", "p2a-ack", "db-reg";
>
> reg has 5 entries, missing 2 reg-names?

copy-paste error, thanks for catching.

Regards,
Anup

>
> > +        msi-parent = <&imsic_mlevel>;
> > +        riscv,slot-size = <64>;
> > +        riscv,db-mask = <0x00008000>;
> > +        riscv,db-value = <0x00008000>;
> > +        #mbox-cells = <1>;
> > +    };
> > --
> > 2.43.0
>
> Regards
> Ley Foon

[RFC PATCH 3/8] dt-bindings: mailbox: Add bindings for RISC-V SBI MPXY extension

[Anup Patel]

Add device tree bindings for the RISC-V SBI Message Proxy (MPXY)
extension as a mailbox controller.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 .../bindings/mailbox/riscv,sbi-mpxy-mbox.yaml | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml b/Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml
new file mode 100644
index 000000000000..8a05e089b710
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V SBI Message Proxy (MPXY) extension based mailbox
+
+maintainers:
+  - Anup Patel <anup@brainfault.org>
+
+description: |
+  The RISC-V SBI Message Proxy (MPXY) extension [1] allows supervisor
+  software to send messages through the SBI implementation (M-mode
+  firmware or HS-mode hypervisor). The underlying message protocol
+  and message format used by the supervisor software could be some
+  other standard protocol compatible with the SBI MPXY extension
+  (such as RISC-V Platform Management Interface (RPMI) [2]).
+
+  ===========================================
+  References
+  ===========================================
+
+  [1] RISC-V Supervisor Binary Interface (SBI)
+      https://github.com/riscv-non-isa/riscv-sbi-doc/releases
+
+  [2] RISC-V Platform Management Interface (RPMI)
+      https://github.com/riscv-non-isa/riscv-rpmi/releases
+
+properties:
+  $nodename:
+    const: sbi-mpxy-mbox
+
+  compatible:
+    const: riscv,sbi-mpxy-mbox
+
+  "#mbox-cells":
+    const: 2
+    description:
+      The first cell specifies channel_id of the SBI MPXY channel,
+      the second cell specifies MSG_PROT_ID of the SBI MPXY channel
+
+required:
+  - compatible
+  - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    sbi-mpxy-mbox {
+          compatible = "riscv,sbi-mpxy-mbox";
+          #mbox-cells = <2>;
+    };
-- 
2.43.0

Re: [RFC PATCH 3/8] dt-bindings: mailbox: Add bindings for RISC-V SBI MPXY extension

[Rob Herring (Arm)]

On Mon, 16 Dec 2024 14:18:12 +0530, Anup Patel wrote:
> Add device tree bindings for the RISC-V SBI Message Proxy (MPXY)
> extension as a mailbox controller.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  .../bindings/mailbox/riscv,sbi-mpxy-mbox.yaml | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,sbi-mpxy-mbox.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#'
Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.example.dtb: /example-0/mailbox@10080000: failed to match any schema with compatible: ['riscv,rpmi-shmem-mbox']
Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.example.dtb: /example-1/mailbox@10001000: failed to match any schema with compatible: ['riscv,rpmi-shmem-mbox']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241216084817.373131-4-apatel@ventanamicro.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[RFC PATCH 4/8] RISC-V: Add defines for the SBI message proxy extension

[Anup Patel]

Add defines for the new SBI message proxy extension which is part
of the SBI v3.0 specification.

Co-developed-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 arch/riscv/include/asm/sbi.h | 59 ++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index bc35ad750e28..18817efba430 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -35,6 +35,7 @@ enum sbi_ext_id {
 	SBI_EXT_DBCN = 0x4442434E,
 	SBI_EXT_STA = 0x535441,
 	SBI_EXT_NACL = 0x4E41434C,
+	SBI_EXT_MPXY = 0x4D505859,
 
 	/* Experimentals extensions must lie within this range */
 	SBI_EXT_EXPERIMENTAL_START = 0x08000000,
@@ -401,6 +402,64 @@ enum sbi_ext_nacl_feature {
 #define SBI_NACL_SHMEM_SRET_X(__i)		((__riscv_xlen / 8) * (__i))
 #define SBI_NACL_SHMEM_SRET_X_LAST		31
 
+enum sbi_ext_mpxy_fid {
+	SBI_EXT_MPXY_SET_SHMEM,
+	SBI_EXT_MPXY_GET_CHANNEL_IDS,
+	SBI_EXT_MPXY_READ_ATTRS,
+	SBI_EXT_MPXY_WRITE_ATTRS,
+	SBI_EXT_MPXY_SEND_MSG_WITH_RESP,
+	SBI_EXT_MPXY_SEND_MSG_NO_RESP,
+	SBI_EXT_MPXY_GET_NOTIFICATION_EVENTS,
+};
+
+enum sbi_mpxy_attribute_id {
+	/* Standard channel attributes managed by MPXY framework */
+	SBI_MPXY_ATTR_MSG_PROT_ID		= 0x00000000,
+	SBI_MPXY_ATTR_MSG_PROT_VER		= 0x00000001,
+	SBI_MPXY_ATTR_MSG_MAX_LEN		= 0x00000002,
+	SBI_MPXY_ATTR_MSG_SEND_TIMEOUT		= 0x00000003,
+	SBI_MPXY_ATTR_MSG_COMPLETION_TIMEOUT	= 0x00000004,
+	SBI_MPXY_ATTR_CHANNEL_CAPABILITY	= 0x00000005,
+	SBI_MPXY_ATTR_SSE_EVENT_ID		= 0x00000006,
+	SBI_MPXY_ATTR_MSI_CONTROL		= 0x00000007,
+	SBI_MPXY_ATTR_MSI_ADDR_LO		= 0x00000008,
+	SBI_MPXY_ATTR_MSI_ADDR_HI		= 0x00000009,
+	SBI_MPXY_ATTR_MSI_DATA			= 0x0000000A,
+	SBI_MPXY_ATTR_EVENTS_STATE_CONTROL	= 0x0000000B,
+	SBI_MPXY_ATTR_STD_ATTR_MAX_IDX,
+	/*
+	 * Message protocol specific attributes, managed by
+	 * the message protocol specification.
+	 */
+	SBI_MPXY_ATTR_MSGPROTO_ATTR_START	= 0x80000000,
+	SBI_MPXY_ATTR_MSGPROTO_ATTR_END		= 0xffffffff
+};
+
+/* Possible values of MSG_PROT_ID attribute */
+enum sbi_mpxy_msgproto_id {
+	SBI_MPXY_MSGPROTO_RPMI_ID = 0x0,
+};
+
+/** RPMI message protocol specific MPXY attributes */
+enum sbi_mpxy_rpmi_attribute_id {
+	SBI_MPXY_RPMI_ATTR_SERVICEGROUP_ID = SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
+	SBI_MPXY_RPMI_ATTR_SERVICEGROUP_VERSION,
+	SBI_MPXY_RPMI_ATTR_MAX_ID,
+};
+
+/* Encoding of MSG_PROT_VER attribute */
+#define SBI_MPXY_MSG_PROT_VER_MAJOR(__ver)	(((__ver) >> 16) & 0xffff)
+#define SBI_MPXY_MSG_PROT_VER_MINOR(__ver)	((__ver) & 0xffff)
+#define SBI_MPXY_MSG_PROT_MKVER(__maj, __min)	(((__maj) << 16) | (__min))
+
+/* Capabilities available through CHANNEL_CAPABILITY attribute */
+#define SBI_MPXY_CHAN_CAP_MSI			BIT(0)
+#define SBI_MPXY_CHAN_CAP_SSE			BIT(1)
+#define SBI_MPXY_CHAN_CAP_EVENTS_STATE		BIT(2)
+#define SBI_MPXY_CHAN_CAP_SEND_WITH_RESP	BIT(3)
+#define SBI_MPXY_CHAN_CAP_SEND_WITHOUT_RESP	BIT(4)
+#define SBI_MPXY_CHAN_CAP_GET_NOTIFICATIONS	BIT(5)
+
 /* SBI spec version fields */
 #define SBI_SPEC_VERSION_DEFAULT	0x1
 #define SBI_SPEC_VERSION_MAJOR_SHIFT	24
-- 
2.43.0

[RFC PATCH 5/8] mailbox: Add common header for RPMI messages sent via mailbox

[Anup Patel]

The RPMI based mailbox controller drivers and mailbox cliens need to
share defines related to RPMI messages over mailbox interface so add
a common header for this purpose.

Co-developed-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 include/linux/mailbox/riscv-rpmi-message.h | 202 +++++++++++++++++++++
 1 file changed, 202 insertions(+)
 create mode 100644 include/linux/mailbox/riscv-rpmi-message.h

diff --git a/include/linux/mailbox/riscv-rpmi-message.h b/include/linux/mailbox/riscv-rpmi-message.h
new file mode 100644
index 000000000000..8f4b3a0edbce
--- /dev/null
+++ b/include/linux/mailbox/riscv-rpmi-message.h
@@ -0,0 +1,202 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Ventana Micro Systems Inc.
+ */
+
+#ifndef _LINUX_RISCV_RPMI_MESSAGE_H_
+#define _LINUX_RISCV_RPMI_MESSAGE_H_
+
+#include <linux/mailbox_client.h>
+
+/** RPMI version encode/decode macros */
+#define RPMI_VER_MAJOR(__ver)		(((__ver) >> 16) & 0xffff)
+#define RPMI_VER_MINOR(__ver)		((__ver) & 0xffff)
+#define RPMI_MKVER(__maj, __min)	(((__maj) << 16) | (__min))
+
+/** RPMI message header */
+struct rpmi_message_header {
+	__le16 servicegroup_id;
+	u8 service_id;
+	u8 flags;
+	__le16 datalen;
+	__le16 token;
+};
+
+/** RPMI message */
+struct rpmi_message {
+	struct rpmi_message_header header;
+	u8 data[];
+};
+
+/** RPMI notification event */
+struct rpmi_notification_event {
+	__le16 event_datalen;
+	u8 event_id;
+	u8 reserved;
+	u8 event_data[];
+};
+
+/** RPMI error codes */
+enum rpmi_error_codes {
+	RPMI_SUCCESS			= 0,
+	RPMI_ERR_FAILED			= -1,
+	RPMI_ERR_NOTSUPP		= -2,
+	RPMI_ERR_INVALID_PARAM		= -3,
+	RPMI_ERR_DENIED			= -4,
+	RPMI_ERR_INVALID_ADDR		= -5,
+	RPMI_ERR_ALREADY		= -6,
+	RPMI_ERR_EXTENSION		= -7,
+	RPMI_ERR_HW_FAULT		= -8,
+	RPMI_ERR_BUSY			= -9,
+	RPMI_ERR_INVALID_STATE		= -10,
+	RPMI_ERR_BAD_RANGE		= -11,
+	RPMI_ERR_TIMEOUT		= -12,
+	RPMI_ERR_IO			= -13,
+	RPMI_ERR_NO_DATA		= -14,
+	RPMI_ERR_RESERVED_START		= -15,
+	RPMI_ERR_RESERVED_END		= -127,
+	RPMI_ERR_VENDOR_START		= -128,
+};
+
+static inline int rpmi_to_linux_error(int rpmi_error)
+{
+	switch (rpmi_error) {
+	case RPMI_SUCCESS:
+		return 0;
+	case RPMI_ERR_INVALID_PARAM:
+	case RPMI_ERR_BAD_RANGE:
+	case RPMI_ERR_INVALID_STATE:
+		return -EINVAL;
+	case RPMI_ERR_DENIED:
+		return -EPERM;
+	case RPMI_ERR_INVALID_ADDR:
+	case RPMI_ERR_HW_FAULT:
+		return -EFAULT;
+	case RPMI_ERR_ALREADY:
+		return -EALREADY;
+	case RPMI_ERR_BUSY:
+		return -EBUSY;
+	case RPMI_ERR_TIMEOUT:
+		return -ETIMEDOUT;
+	case RPMI_ERR_IO:
+		return -ECOMM;
+	case RPMI_ERR_FAILED:
+	case RPMI_ERR_NOTSUPP:
+	case RPMI_ERR_NO_DATA:
+	case RPMI_ERR_EXTENSION:
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+/** RPMI linux mailbox attribute IDs */
+enum rpmi_mbox_attribute_id {
+	RPMI_MBOX_ATTR_SPEC_VERSION = 0,
+	RPMI_MBOX_ATTR_MAX_MSG_DATA_SIZE,
+	RPMI_MBOX_ATTR_SERVICEGROUP_ID,
+	RPMI_MBOX_ATTR_SERVICEGROUP_VERSION,
+	RPMI_MBOX_ATTR_MAX_ID,
+};
+
+/** RPMI linux mailbox message types */
+enum rpmi_mbox_message_type {
+	RPMI_MBOX_MSG_TYPE_GET_ATTRIBUTE = 0,
+	RPMI_MBOX_MSG_TYPE_SET_ATTRIBUTE,
+	RPMI_MBOX_MSG_TYPE_SEND_WITH_RESPONSE,
+	RPMI_MBOX_MSG_TYPE_SEND_WITHOUT_RESPONSE,
+	RPMI_MBOX_MSG_TYPE_NOTIFICATION_EVENT,
+	RPMI_MBOX_MSG_MAX_TYPE,
+};
+
+/** RPMI linux mailbox message instance */
+struct rpmi_mbox_message {
+	enum rpmi_mbox_message_type type;
+	union {
+		struct {
+			enum rpmi_mbox_attribute_id id;
+			u32 value;
+		} attr;
+
+		struct {
+			u32 service_id;
+			void *request;
+			unsigned long request_len;
+			void *response;
+			unsigned long max_response_len;
+			unsigned long out_response_len;
+		} data;
+
+		struct rpmi_notification_event *notif_event;
+	};
+	int error;
+};
+
+/** RPMI linux mailbox message helper routines */
+static inline void rpmi_mbox_init_get_attribute(struct rpmi_mbox_message *msg,
+						enum rpmi_mbox_attribute_id id)
+{
+	msg->type = RPMI_MBOX_MSG_TYPE_GET_ATTRIBUTE;
+	msg->attr.id = id;
+	msg->attr.value = 0;
+	msg->error = 0;
+}
+
+static inline void rpmi_mbox_init_set_attribute(struct rpmi_mbox_message *msg,
+						enum rpmi_mbox_attribute_id id,
+						u32 value)
+{
+	msg->type = RPMI_MBOX_MSG_TYPE_SET_ATTRIBUTE;
+	msg->attr.id = id;
+	msg->attr.value = value;
+	msg->error = 0;
+}
+
+static inline void rpmi_mbox_init_send_with_response(struct rpmi_mbox_message *msg,
+						     u32 service_id,
+						     void *request,
+						     unsigned long request_len,
+						     void *response,
+						     unsigned long max_response_len)
+{
+	msg->type = RPMI_MBOX_MSG_TYPE_SEND_WITH_RESPONSE;
+	msg->data.service_id = service_id;
+	msg->data.request = request;
+	msg->data.request_len = request_len;
+	msg->data.response = response;
+	msg->data.max_response_len = max_response_len;
+	msg->data.out_response_len = 0;
+	msg->error = 0;
+}
+
+static inline void rpmi_mbox_init_send_without_response(struct rpmi_mbox_message *msg,
+							u32 service_id,
+							void *request,
+							unsigned long request_len)
+{
+	msg->type = RPMI_MBOX_MSG_TYPE_SEND_WITHOUT_RESPONSE;
+	msg->data.service_id = service_id;
+	msg->data.request = request;
+	msg->data.request_len = request_len;
+	msg->data.response = NULL;
+	msg->data.max_response_len = 0;
+	msg->data.out_response_len = 0;
+	msg->error = 0;
+}
+
+static inline int rpmi_mbox_send_message(struct mbox_chan *chan,
+					 struct rpmi_mbox_message *msg)
+{
+	int ret;
+
+	/* Send message for the underlying mailbox channel */
+	ret = mbox_send_message(chan, msg);
+	if (ret < 0)
+		return ret;
+
+	/* Explicitly signal txdone for mailbox channel */
+	ret = msg->error;
+	mbox_client_txdone(chan, ret);
+	return ret;
+}
+
+#endif /* _LINUX_RISCV_RPMI_MESSAGE_H_ */
-- 
2.43.0

[RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY) based mailbox driver

[Anup Patel]

Add a mailbox controller driver for the new SBI message proxy extension
which is part of the SBI v3.0 specification.

Co-developed-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 drivers/mailbox/Kconfig               |  11 +
 drivers/mailbox/Makefile              |   2 +
 drivers/mailbox/riscv-sbi-mpxy-mbox.c | 979 ++++++++++++++++++++++++++
 3 files changed, 992 insertions(+)
 create mode 100644 drivers/mailbox/riscv-sbi-mpxy-mbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 8ecba7fb999e..5e56d8d3b52b 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -306,4 +306,15 @@ config THEAD_TH1520_MBOX
 	  kernel is running, and E902 core used for power management among other
 	  things.
 
+config RISCV_SBI_MPXY_MBOX
+	tristate "RISC-V SBI Message Proxy (MPXY) Mailbox"
+	depends on RISCV_SBI && OF
+	default RISCV
+	help
+	  Mailbox driver implementation for RISC-V SBI Message Proxy (MPXY)
+	  extension. This mailbox driver is used to send messages to the
+	  remote processor through the SBI implementation (M-mode firmware
+	  or HS-mode hypervisor). Say Y here if you want to have this support.
+	  If unsure say N.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 5f4f5b0ce2cc..c17c2c7a1de5 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -66,3 +66,5 @@ obj-$(CONFIG_QCOM_CPUCP_MBOX)	+= qcom-cpucp-mbox.o
 obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
 
 obj-$(CONFIG_THEAD_TH1520_MBOX)	+= mailbox-th1520.o
+
+obj-$(CONFIG_RISCV_SBI_MPXY_MBOX)	+= riscv-sbi-mpxy-mbox.o
diff --git a/drivers/mailbox/riscv-sbi-mpxy-mbox.c b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
new file mode 100644
index 000000000000..0592df3028f9
--- /dev/null
+++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
@@ -0,0 +1,979 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V SBI Message Proxy (MPXY) mailbox controller driver
+ *
+ * Copyright (C) 2024 Ventana Micro Systems Inc.
+ */
+
+#include <asm/sbi.h>
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/jump_label.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/mm.h>
+#include <linux/msi.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/percpu.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+
+/* ====== SBI MPXY extension data structures ====== */
+
+/* SBI MPXY MSI related channel attributes */
+struct sbi_mpxy_msi_info {
+	/* Lower 32-bits of the MSI target address */
+	u32 msi_addr_lo;
+	/* Upper 32-bits of the MSI target address */
+	u32 msi_addr_hi;
+	/* MSI data value */
+	u32 msi_data;
+};
+
+/*
+ * SBI MPXY standard channel attributes.
+ *
+ * NOTE: The sequence of attribute fields are as-per the
+ * defined sequence in the attribute table in spec (or
+ * as-per the enum sbi_mpxy_attribute_id).
+ */
+struct sbi_mpxy_channel_attrs {
+	/* Message protocol ID */
+	u32 msg_proto_id;
+	/* Message protocol Version */
+	u32 msg_proto_version;
+	/* Message protocol maximum message length */
+	u32 msg_max_len;
+	/* Message protocol message send timeout in microseconds */
+	u32 msg_send_timeout;
+	/* Message protocol message completion timeout in microseconds */
+	u32 msg_completion_timeout;
+	/* Bit array for channel capabilities */
+	u32 capability;
+	/* SSE Event Id */
+	u32 sse_event_id;
+	/* MSI enable/disable control knob */
+	u32 msi_control;
+	/* Channel MSI info */
+	struct sbi_mpxy_msi_info msi_info;
+	/* Events State Control */
+	u32 events_state_ctrl;
+};
+
+/*
+ * RPMI specific SBI MPXY channel attributes.
+ *
+ * NOTE: The sequence of attribute fields are as-per the
+ * defined sequence in the attribute table in spec (or
+ * as-per the enum sbi_mpxy_rpmi_attribute_id).
+ */
+struct sbi_mpxy_rpmi_channel_attrs {
+	/* RPMI service group ID */
+	u32 servicegroup_id;
+	/* RPMI service group version */
+	u32 servicegroup_version;
+};
+
+/* SBI MPXY channel IDs data in shared memory */
+struct sbi_mpxy_channel_ids_data {
+	/* Remaining number of channel ids */
+	__le32 remaining;
+	/* Returned channel ids in current function call */
+	__le32 returned;
+	/* Returned channel id array */
+	__le32 channel_array[];
+};
+
+/* SBI MPXY notification data in shared memory */
+struct sbi_mpxy_notification_data {
+	/* Remaining number of notification events */
+	__le32 remaining;
+	/* Number of notification events returned */
+	__le32 returned;
+	/* Number of notification events lost */
+	__le32 lost;
+	/* Reserved for future use */
+	__le32 reserved;
+	/* Returned channel id array */
+	u8 events_data[];
+};
+
+/* ====== MPXY data structures & helper routines ====== */
+
+/*
+ * Size of per-CPU MPXY shared memory
+ *
+ * Note: This must be multiple of 4096 bytes (PAGE_SIZE)
+ */
+#define MPXY_SHMEM_SIZE		PAGE_SIZE
+
+/* MPXY Per-CPU or local context */
+struct mpxy_local {
+	/* Shared memory base address */
+	void *shmem;
+	/* Shared memory physical address */
+	phys_addr_t shmem_phys_addr;
+	/* Flag representing whether shared memory is active or not */
+	bool shmem_active;
+};
+
+static DEFINE_PER_CPU(struct mpxy_local, mpxy_local);
+static bool mpxy_shmem_init_done;
+
+static int mpxy_get_channel_count(u32 *channel_count)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbi_mpxy_channel_ids_data *sdata = mpxy->shmem;
+	u32 remaining, returned;
+	struct sbiret sret;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!channel_count)
+		return -EINVAL;
+
+	get_cpu();
+
+	/* Get the remaining and returned fields to calculate total */
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_CHANNEL_IDS,
+			 0, 0, 0, 0, 0, 0);
+	if (!sret.error) {
+		remaining = le32_to_cpu(sdata->remaining);
+		returned = le32_to_cpu(sdata->returned);
+		*channel_count = remaining + returned;
+	}
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_get_channel_ids(u32 channel_count, u32 *channel_ids)
+{
+	u32 remaining, returned, sidx, start_index = 0, cidx = 0;
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbi_mpxy_channel_ids_data *sdata = mpxy->shmem;
+	struct sbiret sret;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!channel_count || !channel_ids)
+		return -EINVAL;
+
+	get_cpu();
+
+	do {
+		sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_CHANNEL_IDS,
+				 start_index, 0, 0, 0, 0, 0);
+		if (sret.error)
+			goto done;
+
+		remaining = le32_to_cpu(sdata->remaining);
+		returned = le32_to_cpu(sdata->returned);
+
+		for (sidx = 0; sidx < returned && cidx < channel_count; sidx++) {
+			channel_ids[cidx] = le32_to_cpu(sdata->channel_array[sidx]);
+			cidx += 1;
+		}
+
+		start_index = cidx;
+
+	} while (remaining);
+
+done:
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_read_attrs(u32 channel_id, u32 base_attrid, u32 attr_count,
+			   u32 *attrs_buf)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbiret sret;
+	u32 i;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!attr_count || !attrs_buf)
+		return -EINVAL;
+
+	get_cpu();
+
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_READ_ATTRS,
+			 channel_id, base_attrid, attr_count, 0, 0, 0);
+	if (!sret.error) {
+		for (i = 0; i < attr_count; i++)
+			attrs_buf[i] = le32_to_cpu(((__le32 *)mpxy->shmem)[i]);
+	}
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_write_attrs(u32 channel_id, u32 base_attrid, u32 attr_count,
+			    u32 *attrs_buf)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbiret sret;
+	u32 i;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!attr_count || !attrs_buf)
+		return -EINVAL;
+
+	get_cpu();
+
+	for (i = 0; i < attr_count; i++)
+		((__le32 *)mpxy->shmem)[i] = cpu_to_le32(attrs_buf[i]);
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_WRITE_ATTRS,
+			 channel_id, base_attrid, attr_count, 0, 0, 0);
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
+				       void *tx, unsigned long tx_len,
+				       void *rx, unsigned long max_rx_len,
+				       unsigned long *rx_len)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	unsigned long rx_bytes;
+	struct sbiret sret;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!tx && tx_len)
+		return -EINVAL;
+
+	get_cpu();
+
+	/* Message protocols allowed to have no data in messages */
+	if (tx_len)
+		memcpy(mpxy->shmem, tx, tx_len);
+
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SEND_MSG_WITH_RESP,
+			 channel_id, msg_id, tx_len, 0, 0, 0);
+	if (rx && !sret.error) {
+		rx_bytes = sret.value;
+		rx_bytes = min(max_rx_len, rx_bytes);
+		memcpy(rx, mpxy->shmem, rx_bytes);
+		if (rx_len)
+			*rx_len = rx_bytes;
+	}
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_send_message_without_resp(u32 channel_id, u32 msg_id,
+					  void *tx, unsigned long tx_len)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbiret sret;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!tx && tx_len)
+		return -EINVAL;
+
+	get_cpu();
+
+	/* Message protocols allowed to have no data in messages */
+	if (tx_len)
+		memcpy(mpxy->shmem, tx, tx_len);
+
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SEND_MSG_NO_RESP,
+			 channel_id, msg_id, tx_len, 0, 0, 0);
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_get_notifications(u32 channel_id,
+				  struct sbi_mpxy_notification_data *notif_data,
+				  unsigned long *events_data_len)
+{
+	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
+	struct sbiret sret;
+
+	if (!mpxy->shmem_active)
+		return -ENODEV;
+	if (!notif_data || !events_data_len)
+		return -EINVAL;
+
+	get_cpu();
+
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_GET_NOTIFICATION_EVENTS,
+			 channel_id, 0, 0, 0, 0, 0);
+	if (!sret.error) {
+		memcpy(notif_data, mpxy->shmem, sret.value);
+		notif_data->remaining = le32_to_cpu(notif_data->remaining);
+		notif_data->returned =  le32_to_cpu(notif_data->returned);
+		notif_data->lost =  le32_to_cpu(notif_data->lost);
+		notif_data->reserved =  le32_to_cpu(notif_data->reserved);
+		*events_data_len = sret.value - 16;
+	}
+
+	put_cpu();
+	return sbi_err_map_linux_errno(sret.error);
+}
+
+static int mpxy_cleanup_shmem(unsigned int cpu)
+{
+	struct mpxy_local *mpxy;
+	struct sbiret sret;
+
+	mpxy = per_cpu_ptr(&mpxy_local, cpu);
+	if (!mpxy->shmem_active)
+		return -EINVAL;
+
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SET_SHMEM,
+			 0, -1U, -1U, 0, 0, 0);
+	if (sret.error)
+		return sbi_err_map_linux_errno(sret.error);
+
+	free_pages((unsigned long)mpxy->shmem, get_order(MPXY_SHMEM_SIZE));
+
+	mpxy->shmem = NULL;
+	mpxy->shmem_phys_addr = 0;
+	mpxy->shmem_active = false;
+
+	return 0;
+}
+
+static int mpxy_setup_shmem(unsigned int cpu)
+{
+	struct page *shmem_page;
+	struct mpxy_local *mpxy;
+	struct sbiret sret;
+
+	mpxy = per_cpu_ptr(&mpxy_local, cpu);
+	if (mpxy->shmem_active)
+		return -EINVAL;
+
+	shmem_page = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(MPXY_SHMEM_SIZE));
+	if (!shmem_page)
+		return -ENOMEM;
+
+	/*
+	 * Linux setup of shmem is done in mpxy OVERWRITE mode.
+	 * flags[1:0] = 00b
+	 */
+	sret = sbi_ecall(SBI_EXT_MPXY, SBI_EXT_MPXY_SET_SHMEM,
+			 MPXY_SHMEM_SIZE, page_to_phys(shmem_page), 0, 0, 0, 0);
+	if (sret.error) {
+		free_pages((unsigned long)page_to_virt(shmem_page),
+			   get_order(MPXY_SHMEM_SIZE));
+		return sbi_err_map_linux_errno(sret.error);
+	}
+
+	mpxy->shmem = page_to_virt(shmem_page);
+	mpxy->shmem_phys_addr = page_to_phys(shmem_page);
+	mpxy->shmem_active = true;
+
+	return 0;
+}
+
+/* ====== MPXY mailbox data structures ====== */
+
+/* MPXY mailbox channel */
+struct mpxy_mbox_channel {
+	struct mpxy_mbox *mbox;
+	u32 channel_id;
+	struct sbi_mpxy_channel_attrs attrs;
+	struct sbi_mpxy_rpmi_channel_attrs rpmi_attrs;
+	struct sbi_mpxy_notification_data *notif;
+	u32 max_xfer_len;
+	bool have_events_state;
+	u32 msi_index;
+	u32 msi_irq;
+	bool started;
+};
+
+/* MPXY mailbox */
+struct mpxy_mbox {
+	struct device *dev;
+	u32 channel_count;
+	struct mpxy_mbox_channel *channels;
+	u32 msi_count;
+	struct mpxy_mbox_channel **msi_index_to_channel;
+	struct mbox_controller controller;
+};
+
+/* ====== MPXY RPMI processing ====== */
+
+static int mpxy_mbox_send_rpmi_data(struct mpxy_mbox_channel *mchan,
+				    struct rpmi_mbox_message *msg)
+{
+	int rc = 0;
+
+	switch (msg->type) {
+	case RPMI_MBOX_MSG_TYPE_GET_ATTRIBUTE:
+		switch (msg->attr.id) {
+		case RPMI_MBOX_ATTR_SPEC_VERSION:
+			msg->attr.value = mchan->attrs.msg_proto_version;
+			break;
+		case RPMI_MBOX_ATTR_MAX_MSG_DATA_SIZE:
+			msg->attr.value = mchan->max_xfer_len;
+			break;
+		case RPMI_MBOX_ATTR_SERVICEGROUP_ID:
+			msg->attr.value = mchan->rpmi_attrs.servicegroup_id;
+			break;
+		case RPMI_MBOX_ATTR_SERVICEGROUP_VERSION:
+			msg->attr.value = mchan->rpmi_attrs.servicegroup_version;
+			break;
+		default:
+			rc = -EOPNOTSUPP;
+			break;
+		}
+		break;
+	case RPMI_MBOX_MSG_TYPE_SET_ATTRIBUTE:
+		/* None of the RPMI linux mailbox attributes are writeable */
+		rc = -EOPNOTSUPP;
+		break;
+	case RPMI_MBOX_MSG_TYPE_SEND_WITH_RESPONSE:
+		if ((!msg->data.request && msg->data.request_len) ||
+		    (msg->data.request &&
+		     msg->data.request_len > mchan->max_xfer_len) ||
+		    (!msg->data.response && msg->data.max_response_len)) {
+			rc = -EINVAL;
+			break;
+		}
+		if (!(mchan->attrs.capability & SBI_MPXY_CHAN_CAP_SEND_WITH_RESP)) {
+			rc = -EIO;
+			break;
+		}
+		rc = mpxy_send_message_with_resp(mchan->channel_id,
+						 msg->data.service_id,
+						 msg->data.request,
+						 msg->data.request_len,
+						 msg->data.response,
+						 msg->data.max_response_len,
+						 &msg->data.out_response_len);
+		break;
+	case RPMI_MBOX_MSG_TYPE_SEND_WITHOUT_RESPONSE:
+		if ((!msg->data.request && msg->data.request_len) ||
+		    (msg->data.request &&
+		     msg->data.request_len > mchan->max_xfer_len)) {
+			rc = -EINVAL;
+			break;
+		}
+		if (!(mchan->attrs.capability & SBI_MPXY_CHAN_CAP_SEND_WITHOUT_RESP)) {
+			rc = -EIO;
+			break;
+		}
+		rc = mpxy_send_message_without_resp(mchan->channel_id,
+						    msg->data.service_id,
+						    msg->data.request,
+						    msg->data.request_len);
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+		break;
+	}
+
+	msg->error = rc;
+	return 0;
+}
+
+static void mpxy_mbox_peek_rpmi_data(struct mbox_chan *chan,
+				     struct mpxy_mbox_channel *mchan,
+				     struct sbi_mpxy_notification_data *notif,
+				     unsigned long events_data_len)
+{
+	struct rpmi_notification_event *event;
+	unsigned long pos = 0, event_size;
+	struct rpmi_mbox_message msg;
+
+	while ((pos < events_data_len) && !(pos & 0x3) &&
+	       ((events_data_len - pos) <= sizeof(*event))) {
+		event = (struct rpmi_notification_event *)(notif->events_data + pos);
+		event->event_datalen = le16_to_cpu(event->event_datalen);
+
+		event_size = sizeof(*event) + event->event_datalen;
+		if (event_size > (events_data_len - pos)) {
+			event_size = events_data_len - pos;
+			goto skip_event;
+		}
+		if (event_size & 0x3)
+			goto skip_event;
+
+		msg.type = RPMI_MBOX_MSG_TYPE_NOTIFICATION_EVENT;
+		msg.notif_event = event;
+		msg.error = 0;
+		mbox_chan_received_data(chan, &msg);
+
+skip_event:
+		pos += event_size;
+	}
+}
+
+static int mpxy_mbox_read_rpmi_attrs(struct mpxy_mbox_channel *mchan)
+{
+	return mpxy_read_attrs(mchan->channel_id,
+			       SBI_MPXY_ATTR_MSGPROTO_ATTR_START,
+			       sizeof(mchan->rpmi_attrs) / sizeof(u32),
+			       (u32 *)&mchan->rpmi_attrs);
+}
+
+/* ====== MPXY mailbox callbacks ====== */
+
+static int mpxy_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct mpxy_mbox_channel *mchan = chan->con_priv;
+
+	if (mchan->attrs.msg_proto_id == SBI_MPXY_MSGPROTO_RPMI_ID)
+		return mpxy_mbox_send_rpmi_data(mchan, data);
+
+	return -EOPNOTSUPP;
+}
+
+static bool mpxy_mbox_peek_data(struct mbox_chan *chan)
+{
+	struct mpxy_mbox_channel *mchan = chan->con_priv;
+	struct sbi_mpxy_notification_data *notif = mchan->notif;
+	bool have_notifications = false;
+	unsigned long data_len;
+	int rc;
+
+	while (1) {
+		rc = mpxy_get_notifications(mchan->channel_id, notif, &data_len);
+		if (rc || !notif->returned)
+			break;
+
+		if (mchan->attrs.msg_proto_id == SBI_MPXY_MSGPROTO_RPMI_ID)
+			mpxy_mbox_peek_rpmi_data(chan, mchan, notif, data_len);
+
+		have_notifications = true;
+	}
+
+	return have_notifications;
+}
+
+static irqreturn_t mpxy_mbox_irq_event(int irq, void *dev_id)
+{
+	/* We only have MSI for notification so just wakeup IRQ thread */
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t mpxy_mbox_irq_thread(int irq, void *dev_id)
+{
+	mpxy_mbox_peek_data(dev_id);
+	return IRQ_HANDLED;
+}
+
+static int mpxy_mbox_setup_msi(struct mbox_chan *chan,
+			       struct mpxy_mbox_channel *mchan)
+{
+	struct device *dev = mchan->mbox->dev;
+	int rc;
+
+	/* Do nothing if MSI not supported */
+	if (mchan->msi_irq == U32_MAX)
+		return 0;
+
+	/* Request channel MSI handler */
+	rc = request_threaded_irq(mchan->msi_irq,
+				  mpxy_mbox_irq_event,
+				  mpxy_mbox_irq_thread,
+				  0, dev_name(dev), chan);
+	if (rc) {
+		dev_err(dev, "failed to request MPXY channel 0x%x IRQ\n",
+			mchan->channel_id);
+		return rc;
+	}
+
+	/* Enable channel MSI control */
+	mchan->attrs.msi_control = 1;
+	rc = mpxy_write_attrs(mchan->channel_id, SBI_MPXY_ATTR_MSI_CONTROL,
+			      1, &mchan->attrs.msi_control);
+	if (rc) {
+		dev_err(dev, "enable MSI control failed for MPXY channel 0x%x\n",
+			mchan->channel_id);
+		free_irq(mchan->msi_irq, chan);
+		return rc;
+	}
+
+	return 0;
+}
+
+static void mpxy_mbox_cleanup_msi(struct mbox_chan *chan,
+				  struct mpxy_mbox_channel *mchan)
+{
+	struct device *dev = mchan->mbox->dev;
+	int rc;
+
+	/* Do nothing if MSI not supported */
+	if (mchan->msi_irq == U32_MAX)
+		return;
+
+	/* Disable channel MSI control */
+	mchan->attrs.msi_control = 0;
+	rc = mpxy_write_attrs(mchan->channel_id, SBI_MPXY_ATTR_MSI_CONTROL,
+			      1, &mchan->attrs.msi_control);
+	if (rc) {
+		dev_err(dev, "disable MSI control failed for MPXY channel 0x%x\n",
+			mchan->channel_id);
+	}
+
+	/* Free channel MSI handler */
+	free_irq(mchan->msi_irq, chan);
+}
+
+static int mpxy_mbox_setup_events(struct mpxy_mbox_channel *mchan)
+{
+	struct device *dev = mchan->mbox->dev;
+	int rc;
+
+	/* Do nothing if events state not supported */
+	if (!mchan->have_events_state)
+		return 0;
+
+	/* Enable channel events state */
+	mchan->attrs.events_state_ctrl = 1;
+	rc = mpxy_write_attrs(mchan->channel_id, SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
+			      1, &mchan->attrs.events_state_ctrl);
+	if (rc) {
+		dev_err(dev, "enable events state failed for MPXY channel 0x%x\n",
+			mchan->channel_id);
+		return rc;
+	}
+
+	return 0;
+}
+
+static void mpxy_mbox_cleanup_events(struct mpxy_mbox_channel *mchan)
+{
+	struct device *dev = mchan->mbox->dev;
+	int rc;
+
+	/* Do nothing if events state not supported */
+	if (!mchan->have_events_state)
+		return;
+
+	/* Disable channel events state */
+	mchan->attrs.events_state_ctrl = 0;
+	rc = mpxy_write_attrs(mchan->channel_id, SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
+			      1, &mchan->attrs.events_state_ctrl);
+	if (rc) {
+		dev_err(dev, "disbable events state failed for MPXY channel 0x%x\n",
+			mchan->channel_id);
+	}
+}
+
+static int mpxy_mbox_startup(struct mbox_chan *chan)
+{
+	struct mpxy_mbox_channel *mchan = chan->con_priv;
+	int rc;
+
+	if (mchan->started)
+		return -EALREADY;
+
+	/* Setup channel MSI */
+	rc = mpxy_mbox_setup_msi(chan, mchan);
+	if (rc)
+		return rc;
+
+	/* Setup channel notification events */
+	rc = mpxy_mbox_setup_events(mchan);
+	if (rc) {
+		mpxy_mbox_cleanup_msi(chan, mchan);
+		return rc;
+	}
+
+	/* Mark the channel as started */
+	mchan->started = true;
+
+	return 0;
+}
+
+static void mpxy_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct mpxy_mbox_channel *mchan = chan->con_priv;
+
+	if (!mchan->started)
+		return;
+
+	/* Mark the channel as stopped */
+	mchan->started = false;
+
+	/* Cleanup channel notification events */
+	mpxy_mbox_cleanup_events(mchan);
+
+	/* Cleanup channel MSI */
+	mpxy_mbox_cleanup_msi(chan, mchan);
+}
+
+static const struct mbox_chan_ops mpxy_mbox_ops = {
+	.send_data = mpxy_mbox_send_data,
+	.peek_data = mpxy_mbox_peek_data,
+	.startup = mpxy_mbox_startup,
+	.shutdown = mpxy_mbox_shutdown,
+};
+
+/* ====== MPXY platform driver ===== */
+
+static void mpxy_mbox_msi_write(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct device *dev = msi_desc_to_dev(desc);
+	struct mpxy_mbox *mbox = dev_get_drvdata(dev);
+	struct mpxy_mbox_channel *mchan;
+	struct sbi_mpxy_msi_info *minfo;
+	int rc;
+
+	mchan = mbox->msi_index_to_channel[desc->msi_index];
+	if (!mchan) {
+		dev_warn(dev, "MPXY channel not available for MSI index %d\n",
+			 desc->msi_index);
+		return;
+	}
+
+	minfo = &mchan->attrs.msi_info;
+	minfo->msi_addr_lo = msg->address_lo;
+	minfo->msi_addr_hi = msg->address_hi;
+	minfo->msi_data = msg->data;
+
+	rc = mpxy_write_attrs(mchan->channel_id, SBI_MPXY_ATTR_MSI_ADDR_LO,
+			      sizeof(*minfo) / sizeof(u32), (u32 *)minfo);
+	if (rc) {
+		dev_warn(dev, "failed to write MSI info for MPXY channel 0x%x\n",
+			 mchan->channel_id);
+	}
+}
+
+static struct mbox_chan *mpxy_mbox_of_xlate(struct mbox_controller *ctlr,
+					    const struct of_phandle_args *pa)
+{
+	struct mpxy_mbox *mbox = container_of(ctlr, struct mpxy_mbox, controller);
+	struct mpxy_mbox_channel *mchan;
+	u32 i;
+
+	if (pa->args_count != 2)
+		return ERR_PTR(-EINVAL);
+
+	for (i = 0; i < mbox->channel_count; i++) {
+		mchan = &mbox->channels[i];
+		if (mchan->channel_id == pa->args[0] &&
+		    mchan->attrs.msg_proto_id == pa->args[1])
+			return &mbox->controller.chans[i];
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
+static int mpxy_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mpxy_mbox_channel *mchan;
+	struct mpxy_mbox *mbox;
+	int i, msi_idx, rc;
+	u32 *channel_ids;
+
+	/*
+	 * Initialize MPXY shared memory only once. This also ensures
+	 * that SBI MPXY mailbox is probed only once.
+	 */
+	if (mpxy_shmem_init_done) {
+		dev_err(dev, "SBI MPXY mailbox already initialized\n");
+		return -EALREADY;
+	}
+
+	/* Probe for SBI MPXY extension */
+	if (sbi_spec_version < sbi_mk_version(1, 0) ||
+	    sbi_probe_extension(SBI_EXT_MPXY) <= 0) {
+		dev_info(dev, "SBI MPXY extension not available\n");
+		return -ENODEV;
+	}
+
+	/* Setup cpuhp notifier for per-CPU MPXY shared memory */
+	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/sbi-mpxy-shmem",
+			  mpxy_setup_shmem, mpxy_cleanup_shmem);
+
+	/* Mark as MPXY shared memory initialization done */
+	mpxy_shmem_init_done = true;
+
+	/* Allocate mailbox instance */
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+	mbox->dev = dev;
+	platform_set_drvdata(pdev, mbox);
+
+	/* Find-out of number of channels */
+	rc = mpxy_get_channel_count(&mbox->channel_count);
+	if (rc) {
+		dev_err(dev, "failed to get number of MPXY channels\n");
+		return rc;
+	}
+	if (!mbox->channel_count) {
+		dev_err(dev, "no MPXY channels available\n");
+		return -ENODEV;
+	}
+
+	/* Allocate and fetch all channel IDs */
+	channel_ids = devm_kcalloc(dev, mbox->channel_count,
+				   sizeof(*channel_ids), GFP_KERNEL);
+	if (!channel_ids)
+		return -ENOMEM;
+	rc = mpxy_get_channel_ids(mbox->channel_count, channel_ids);
+	if (rc) {
+		dev_err(dev, "failed to get number of MPXY channels\n");
+		return rc;
+	}
+
+	/* Populate all channels */
+	mbox->channels = devm_kcalloc(dev, mbox->channel_count,
+				      sizeof(*mbox->channels), GFP_KERNEL);
+	if (!mbox->channels)
+		return -ENOMEM;
+	for (i = 0; i < mbox->channel_count; i++) {
+		mchan = &mbox->channels[i];
+		mchan->mbox = mbox;
+		mchan->channel_id = channel_ids[i];
+
+		rc = mpxy_read_attrs(mchan->channel_id, SBI_MPXY_ATTR_MSG_PROT_ID,
+				     sizeof(mchan->attrs) / sizeof(u32),
+				     (u32 *)&mchan->attrs);
+		if (rc) {
+			dev_err(dev, "read attributes failed for MPXY channel 0x%x\n",
+				mchan->channel_id);
+			return rc;
+		}
+
+		if (mchan->attrs.msg_proto_id == SBI_MPXY_MSGPROTO_RPMI_ID) {
+			rc = mpxy_mbox_read_rpmi_attrs(mchan);
+			if (rc) {
+				dev_err(dev,
+					"read RPMI attributes failed for MPXY channel 0x%x\n",
+					mchan->channel_id);
+				return rc;
+			}
+		}
+
+		mchan->notif = devm_kzalloc(dev, MPXY_SHMEM_SIZE, GFP_KERNEL);
+		if (!mchan->notif)
+			return -ENOMEM;
+
+		mchan->max_xfer_len = min(MPXY_SHMEM_SIZE, mchan->attrs.msg_max_len);
+
+		if ((mchan->attrs.capability & SBI_MPXY_CHAN_CAP_GET_NOTIFICATIONS) &&
+		    (mchan->attrs.capability & SBI_MPXY_CHAN_CAP_EVENTS_STATE))
+			mchan->have_events_state = true;
+
+		if (mchan->have_events_state &&
+		    (mchan->attrs.capability & SBI_MPXY_CHAN_CAP_MSI))
+			mchan->msi_index = mbox->msi_count++;
+		else
+			mchan->msi_index = U32_MAX;
+		mchan->msi_irq = U32_MAX;
+	}
+
+	/* Free-up channel IDs */
+	devm_kfree(dev, channel_ids);
+
+	/* Initialize mailbox controller */
+	mbox->controller.txdone_irq = false;
+	mbox->controller.txdone_poll = false;
+	mbox->controller.ops = &mpxy_mbox_ops;
+	mbox->controller.dev = dev;
+	mbox->controller.num_chans = mbox->channel_count;
+	mbox->controller.of_xlate = mpxy_mbox_of_xlate;
+	mbox->controller.chans = devm_kcalloc(dev, mbox->channel_count,
+					      sizeof(*mbox->controller.chans),
+					      GFP_KERNEL);
+	if (!mbox->controller.chans)
+		return -ENOMEM;
+	for (i = 0; i < mbox->channel_count; i++)
+		mbox->controller.chans[i].con_priv = &mbox->channels[i];
+
+	/* Set the MSI domain if not available */
+	if (!dev_get_msi_domain(dev)) {
+		/*
+		 * The device MSI domain for OF devices is only set at the
+		 * time of populating/creating OF device. If the device MSI
+		 * domain is discovered later after the OF device is created
+		 * then we need to set it explicitly before using any platform
+		 * MSI functions.
+		 */
+		if (is_of_node(dev->fwnode))
+			of_msi_configure(dev, to_of_node(dev->fwnode));
+	}
+
+	/* Setup MSIs for mailbox (if required) */
+	if (mbox->msi_count) {
+		mbox->msi_index_to_channel = devm_kcalloc(dev, mbox->msi_count,
+							  sizeof(*mbox->msi_index_to_channel),
+							  GFP_KERNEL);
+		if (!mbox->msi_index_to_channel)
+			return -ENOMEM;
+
+		for (msi_idx = 0; msi_idx < mbox->msi_count; msi_idx++) {
+			for (i = 0; i < mbox->channel_count; i++) {
+				mchan = &mbox->channels[i];
+				if (mchan->msi_index == msi_idx) {
+					mbox->msi_index_to_channel[msi_idx] = mchan;
+					break;
+				}
+			}
+		}
+
+		rc = platform_device_msi_init_and_alloc_irqs(dev, mbox->msi_count,
+							     mpxy_mbox_msi_write);
+		if (rc) {
+			dev_err(dev, "Failed to allocate %d MSIs\n", mbox->msi_count);
+			return rc;
+		}
+
+		for (i = 0; i < mbox->channel_count; i++) {
+			mchan = &mbox->channels[i];
+			if (mchan->msi_index == U32_MAX)
+				continue;
+			mchan->msi_irq = msi_get_virq(dev, mchan->msi_index);
+		}
+	}
+
+	/* Register mailbox controller */
+	rc = devm_mbox_controller_register(dev, &mbox->controller);
+	if (rc) {
+		dev_err(dev, "Registering SBI MPXY mailbox failed\n");
+		if (mbox->msi_count)
+			platform_device_msi_free_irqs_all(dev);
+		return rc;
+	}
+
+	dev_info(dev, "mailbox registered with %d channels\n",
+		 mbox->channel_count);
+	return 0;
+}
+
+static void mpxy_mbox_remove(struct platform_device *pdev)
+{
+	struct mpxy_mbox *mbox = platform_get_drvdata(pdev);
+
+	if (mbox->msi_count)
+		platform_device_msi_free_irqs_all(mbox->dev);
+}
+
+static const struct of_device_id mpxy_mbox_of_match[] = {
+	{.compatible = "riscv,sbi-mpxy-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mpxy_mbox_of_match);
+
+static struct platform_driver mpxy_mbox_driver = {
+	.driver = {
+		.name = "riscv-sbi-mpxy-mbox",
+		.of_match_table = mpxy_mbox_of_match,
+	},
+	.probe = mpxy_mbox_probe,
+	.remove = mpxy_mbox_remove,
+};
+module_platform_driver(mpxy_mbox_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Anup Patel <apatel@ventanamicro.com>");
+MODULE_DESCRIPTION("RISC-V SBI MPXY mailbox controller driver");
-- 
2.43.0

RE: [RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY) based mailbox driver

[Leyfoon Tan]

> -----Original Message-----
> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Monday, December 16, 2024 4:48 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> <jassisinghbrar@gmail.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> <anup@brainfault.org>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> Subject: [RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY)
> based mailbox driver
> 
> Add a mailbox controller driver for the new SBI message proxy extension
> which is part of the SBI v3.0 specification.
> 
> Co-developed-by: Rahul Pathak <rpathak@ventanamicro.com>
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  drivers/mailbox/Kconfig               |  11 +
>  drivers/mailbox/Makefile              |   2 +
>  drivers/mailbox/riscv-sbi-mpxy-mbox.c | 979
> ++++++++++++++++++++++++++
>  3 files changed, 992 insertions(+)
>  create mode 100644 drivers/mailbox/riscv-sbi-mpxy-mbox.c
> 

[...]

> sbi-mpxy-mbox.c
> new file mode 100644
> index 000000000000..0592df3028f9
> --- /dev/null
> +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> @@ -0,0 +1,979 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V SBI Message Proxy (MPXY) mailbox controller driver
> + *
> + * Copyright (C) 2024 Ventana Micro Systems Inc.
> + */
> +
> +#include <asm/sbi.h>
> +#include <linux/cpu.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/jump_label.h>
> +#include <linux/kernel.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/mailbox/riscv-rpmi-message.h>

> +#include <linux/mm.h>
> +#include <linux/msi.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
Sorting include header files based on alphanumeric.

> +#include <linux/percpu.h>
> +#include <linux/platform_device.h>
> +#include <linux/smp.h>
> +
> +/* ====== SBI MPXY extension data structures ====== */
> +
> +/* SBI MPXY MSI related channel attributes */ struct sbi_mpxy_msi_info
> +{
> +	/* Lower 32-bits of the MSI target address */
> +	u32 msi_addr_lo;
> +	/* Upper 32-bits of the MSI target address */
> +	u32 msi_addr_hi;
> +	/* MSI data value */
> +	u32 msi_data;
> +};
> +
> +/*
> + * SBI MPXY standard channel attributes.
> + *
> + * NOTE: The sequence of attribute fields are as-per the
> + * defined sequence in the attribute table in spec (or
> + * as-per the enum sbi_mpxy_attribute_id).
> + */
> +struct sbi_mpxy_channel_attrs {
> +	/* Message protocol ID */
> +	u32 msg_proto_id;
> +	/* Message protocol Version */
Don't need capital letter for "version" .

> +	u32 msg_proto_version;
> +	/* Message protocol maximum message length */
> +	u32 msg_max_len;
> +	/* Message protocol message send timeout in microseconds */
> +	u32 msg_send_timeout;
> +	/* Message protocol message completion timeout in microseconds */
> +	u32 msg_completion_timeout;
> +	/* Bit array for channel capabilities */
> +	u32 capability;
> +	/* SSE Event Id */
Same for 'event'.
> +	u32 sse_event_id;
> +	/* MSI enable/disable control knob */
> +	u32 msi_control;
> +	/* Channel MSI info */
> +	struct sbi_mpxy_msi_info msi_info;
> +	/* Events State Control */
Same here

> +	u32 events_state_ctrl;
> +};
> +
> +/*


[...]

> +
> +static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
> +				       void *tx, unsigned long tx_len,
> +				       void *rx, unsigned long max_rx_len,
> +				       unsigned long *rx_len)
> +{
> +	struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
> +	unsigned long rx_bytes;
> +	struct sbiret sret;
> +
> +	if (!mpxy->shmem_active)
> +		return -ENODEV;
> +	if (!tx && tx_len)
> +		return -EINVAL;
> +
> +	get_cpu();
> +
> +	/* Message protocols allowed to have no data in messages */
> +	if (tx_len)
> +		memcpy(mpxy->shmem, tx, tx_len);
> +
> +	sret = sbi_ecall(SBI_EXT_MPXY,
> SBI_EXT_MPXY_SEND_MSG_WITH_RESP,
> +			 channel_id, msg_id, tx_len, 0, 0, 0);
> +	if (rx && !sret.error) {
> +		rx_bytes = sret.value;
> +		rx_bytes = min(max_rx_len, rx_bytes);

Caller should know if the rx_bytes is larger than max_rx_len?

> +		memcpy(rx, mpxy->shmem, rx_bytes);
> +		if (rx_len)
> +			*rx_len = rx_bytes;
> +	}
> +
> +	put_cpu();
> +	return sbi_err_map_linux_errno(sret.error);
> +}
> +

[...]

> +
> +static int mpxy_mbox_setup_msi(struct mbox_chan *chan,
> +			       struct mpxy_mbox_channel *mchan) {
> +	struct device *dev = mchan->mbox->dev;
> +	int rc;
> +
> +	/* Do nothing if MSI not supported */
> +	if (mchan->msi_irq == U32_MAX)
> +		return 0;
> +
> +	/* Request channel MSI handler */
> +	rc = request_threaded_irq(mchan->msi_irq,
> +				  mpxy_mbox_irq_event,
> +				  mpxy_mbox_irq_thread,
> +				  0, dev_name(dev), chan);
> +	if (rc) {
> +		dev_err(dev, "failed to request MPXY channel 0x%x IRQ\n",
> +			mchan->channel_id);
> +		return rc;
> +	}
> +
> +	/* Enable channel MSI control */
> +	mchan->attrs.msi_control = 1;
> +	rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_MSI_CONTROL,
> +			      1, &mchan->attrs.msi_control);
> +	if (rc) {
> +		dev_err(dev, "enable MSI control failed for MPXY channel
> 0x%x\n",
> +			mchan->channel_id);
> +		free_irq(mchan->msi_irq, chan);

Set mchan->attrs.msi_control = 0 if failed?

> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mpxy_mbox_cleanup_msi(struct mbox_chan *chan,
> +				  struct mpxy_mbox_channel *mchan)
> +{
> +	struct device *dev = mchan->mbox->dev;
> +	int rc;
> +
> +	/* Do nothing if MSI not supported */
> +	if (mchan->msi_irq == U32_MAX)


Should check if(!mchan->attrs.msi_control) instead of mchan->msi_irq?


> +		return;
> +
> +	/* Disable channel MSI control */
> +	mchan->attrs.msi_control = 0;
> +	rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_MSI_CONTROL,
> +			      1, &mchan->attrs.msi_control);
> +	if (rc) {
> +		dev_err(dev, "disable MSI control failed for MPXY channel
> 0x%x\n",
> +			mchan->channel_id);
> +	}
> +
> +	/* Free channel MSI handler */
> +	free_irq(mchan->msi_irq, chan);
> +}
> +
> +static int mpxy_mbox_setup_events(struct mpxy_mbox_channel *mchan) {
> +	struct device *dev = mchan->mbox->dev;
> +	int rc;
> +
> +	/* Do nothing if events state not supported */
> +	if (!mchan->have_events_state)
> +		return 0;
> +
> +	/* Enable channel events state */
> +	mchan->attrs.events_state_ctrl = 1;
> +	rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> +			      1, &mchan->attrs.events_state_ctrl);
> +	if (rc) {
> +		dev_err(dev, "enable events state failed for MPXY channel
> 0x%x\n",
> +			mchan->channel_id);

Should set mchan->attrs.events_state_ctrl = 0; ?

> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static void mpxy_mbox_cleanup_events(struct mpxy_mbox_channel
> *mchan) {
> +	struct device *dev = mchan->mbox->dev;
> +	int rc;
> +
> +	/* Do nothing if events state not supported */
> +	if (!mchan->have_events_state)
Check also if (!mchan->attrs.events_state_ctrl)?

> +		return;
> +
> +	/* Disable channel events state */
> +	mchan->attrs.events_state_ctrl = 0;
> +	rc = mpxy_write_attrs(mchan->channel_id,
> SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> +			      1, &mchan->attrs.events_state_ctrl);
> +	if (rc) {
> +		dev_err(dev, "disbable events state failed for MPXY channel

Typo ' disbable'.

> 0x%x\n",
> +			mchan->channel_id);
> +	}
> +}
> +


[...]


> +
> +static int mpxy_mbox_probe(struct platform_device *pdev) {
> +	struct device *dev = &pdev->dev;
> +	struct mpxy_mbox_channel *mchan;
> +	struct mpxy_mbox *mbox;
> +	int i, msi_idx, rc;
> +	u32 *channel_ids;
> +
> +	/*
> +	 * Initialize MPXY shared memory only once. This also ensures
> +	 * that SBI MPXY mailbox is probed only once.
> +	 */
> +	if (mpxy_shmem_init_done) {
> +		dev_err(dev, "SBI MPXY mailbox already initialized\n");
> +		return -EALREADY;
> +	}
> +
> +	/* Probe for SBI MPXY extension */
> +	if (sbi_spec_version < sbi_mk_version(1, 0) ||
> +	    sbi_probe_extension(SBI_EXT_MPXY) <= 0) {
> +		dev_info(dev, "SBI MPXY extension not available\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Setup cpuhp notifier for per-CPU MPXY shared memory */
> +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/sbi-mpxy-
> shmem",
> +			  mpxy_setup_shmem, mpxy_cleanup_shmem);
> +
> +	/* Mark as MPXY shared memory initialization done */
> +	mpxy_shmem_init_done = true;
> +
> +	/* Allocate mailbox instance */
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +	mbox->dev = dev;
> +	platform_set_drvdata(pdev, mbox);
> +
> +	/* Find-out of number of channels */
> +	rc = mpxy_get_channel_count(&mbox->channel_count);
> +	if (rc) {
> +		dev_err(dev, "failed to get number of MPXY channels\n");
Suggest print 'rc' value when error. Same for other error messages below.

> +		return rc;
> +	}
> +	if (!mbox->channel_count) {
> +		dev_err(dev, "no MPXY channels available\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Allocate and fetch all channel IDs */
> +	channel_ids = devm_kcalloc(dev, mbox->channel_count,
> +				   sizeof(*channel_ids), GFP_KERNEL);
> +	if (!channel_ids)
> +		return -ENOMEM;
> +	rc = mpxy_get_channel_ids(mbox->channel_count, channel_ids);
> +	if (rc) {
> +		dev_err(dev, "failed to get number of MPXY channels\n");
> +		return rc;
> +	}
> +

[...]

Regards
Ley Foon

Re: [RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY) based mailbox driver

[Anup Patel]

On Tue, Dec 24, 2024 at 8:50 AM Leyfoon Tan
<leyfoon.tan@starfivetech.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Anup Patel <apatel@ventanamicro.com>
> > Sent: Monday, December 16, 2024 4:48 PM
> > To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> > <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> > <jassisinghbrar@gmail.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> > <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> > Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> > <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> > Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> > <anup@brainfault.org>; linux-clk@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> > kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> > Subject: [RFC PATCH 6/8] mailbox: Add RISC-V SBI message proxy (MPXY)
> > based mailbox driver
> >
> > Add a mailbox controller driver for the new SBI message proxy extension
> > which is part of the SBI v3.0 specification.
> >
> > Co-developed-by: Rahul Pathak <rpathak@ventanamicro.com>
> > Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  drivers/mailbox/Kconfig               |  11 +
> >  drivers/mailbox/Makefile              |   2 +
> >  drivers/mailbox/riscv-sbi-mpxy-mbox.c | 979
> > ++++++++++++++++++++++++++
> >  3 files changed, 992 insertions(+)
> >  create mode 100644 drivers/mailbox/riscv-sbi-mpxy-mbox.c
> >
>
> [...]
>
> > sbi-mpxy-mbox.c
> > new file mode 100644
> > index 000000000000..0592df3028f9
> > --- /dev/null
> > +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> > @@ -0,0 +1,979 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RISC-V SBI Message Proxy (MPXY) mailbox controller driver
> > + *
> > + * Copyright (C) 2024 Ventana Micro Systems Inc.
> > + */
> > +
> > +#include <asm/sbi.h>
> > +#include <linux/cpu.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/jump_label.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mailbox_controller.h>
> > +#include <linux/mailbox/riscv-rpmi-message.h>
>
> > +#include <linux/mm.h>
> > +#include <linux/msi.h>
> > +#include <linux/module.h>
> > +#include <linux/of_irq.h>
> Sorting include header files based on alphanumeric.

Okay, I will update.

>
> > +#include <linux/percpu.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/smp.h>
> > +
> > +/* ====== SBI MPXY extension data structures ====== */
> > +
> > +/* SBI MPXY MSI related channel attributes */ struct sbi_mpxy_msi_info
> > +{
> > +     /* Lower 32-bits of the MSI target address */
> > +     u32 msi_addr_lo;
> > +     /* Upper 32-bits of the MSI target address */
> > +     u32 msi_addr_hi;
> > +     /* MSI data value */
> > +     u32 msi_data;
> > +};
> > +
> > +/*
> > + * SBI MPXY standard channel attributes.
> > + *
> > + * NOTE: The sequence of attribute fields are as-per the
> > + * defined sequence in the attribute table in spec (or
> > + * as-per the enum sbi_mpxy_attribute_id).
> > + */
> > +struct sbi_mpxy_channel_attrs {
> > +     /* Message protocol ID */
> > +     u32 msg_proto_id;
> > +     /* Message protocol Version */
> Don't need capital letter for "version" .

Okay, I will update.

>
> > +     u32 msg_proto_version;
> > +     /* Message protocol maximum message length */
> > +     u32 msg_max_len;
> > +     /* Message protocol message send timeout in microseconds */
> > +     u32 msg_send_timeout;
> > +     /* Message protocol message completion timeout in microseconds */
> > +     u32 msg_completion_timeout;
> > +     /* Bit array for channel capabilities */
> > +     u32 capability;
> > +     /* SSE Event Id */
> Same for 'event'.

Okay, I will update.

> > +     u32 sse_event_id;
> > +     /* MSI enable/disable control knob */
> > +     u32 msi_control;
> > +     /* Channel MSI info */
> > +     struct sbi_mpxy_msi_info msi_info;
> > +     /* Events State Control */
> Same here

Okay, I will update.

>
> > +     u32 events_state_ctrl;
> > +};
> > +
> > +/*
>
>
> [...]
>
> > +
> > +static int mpxy_send_message_with_resp(u32 channel_id, u32 msg_id,
> > +                                    void *tx, unsigned long tx_len,
> > +                                    void *rx, unsigned long max_rx_len,
> > +                                    unsigned long *rx_len)
> > +{
> > +     struct mpxy_local *mpxy = this_cpu_ptr(&mpxy_local);
> > +     unsigned long rx_bytes;
> > +     struct sbiret sret;
> > +
> > +     if (!mpxy->shmem_active)
> > +             return -ENODEV;
> > +     if (!tx && tx_len)
> > +             return -EINVAL;
> > +
> > +     get_cpu();
> > +
> > +     /* Message protocols allowed to have no data in messages */
> > +     if (tx_len)
> > +             memcpy(mpxy->shmem, tx, tx_len);
> > +
> > +     sret = sbi_ecall(SBI_EXT_MPXY,
> > SBI_EXT_MPXY_SEND_MSG_WITH_RESP,
> > +                      channel_id, msg_id, tx_len, 0, 0, 0);
> > +     if (rx && !sret.error) {
> > +             rx_bytes = sret.value;
> > +             rx_bytes = min(max_rx_len, rx_bytes);
>
> Caller should know if the rx_bytes is larger than max_rx_len?

Good catch.

It is better to just fail when rx_bytes > max_rx_len so that
caller can deal with an undersized rx buffer.

I will update accordingly.

>
> > +             memcpy(rx, mpxy->shmem, rx_bytes);
> > +             if (rx_len)
> > +                     *rx_len = rx_bytes;
> > +     }
> > +
> > +     put_cpu();
> > +     return sbi_err_map_linux_errno(sret.error);
> > +}
> > +
>
> [...]
>
> > +
> > +static int mpxy_mbox_setup_msi(struct mbox_chan *chan,
> > +                            struct mpxy_mbox_channel *mchan) {
> > +     struct device *dev = mchan->mbox->dev;
> > +     int rc;
> > +
> > +     /* Do nothing if MSI not supported */
> > +     if (mchan->msi_irq == U32_MAX)
> > +             return 0;
> > +
> > +     /* Request channel MSI handler */
> > +     rc = request_threaded_irq(mchan->msi_irq,
> > +                               mpxy_mbox_irq_event,
> > +                               mpxy_mbox_irq_thread,
> > +                               0, dev_name(dev), chan);
> > +     if (rc) {
> > +             dev_err(dev, "failed to request MPXY channel 0x%x IRQ\n",
> > +                     mchan->channel_id);
> > +             return rc;
> > +     }
> > +
> > +     /* Enable channel MSI control */
> > +     mchan->attrs.msi_control = 1;
> > +     rc = mpxy_write_attrs(mchan->channel_id,
> > SBI_MPXY_ATTR_MSI_CONTROL,
> > +                           1, &mchan->attrs.msi_control);
> > +     if (rc) {
> > +             dev_err(dev, "enable MSI control failed for MPXY channel
> > 0x%x\n",
> > +                     mchan->channel_id);
> > +             free_irq(mchan->msi_irq, chan);
>
> Set mchan->attrs.msi_control = 0 if failed?

Okay, I will update.

>
> > +             return rc;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void mpxy_mbox_cleanup_msi(struct mbox_chan *chan,
> > +                               struct mpxy_mbox_channel *mchan)
> > +{
> > +     struct device *dev = mchan->mbox->dev;
> > +     int rc;
> > +
> > +     /* Do nothing if MSI not supported */
> > +     if (mchan->msi_irq == U32_MAX)
>
>
> Should check if(!mchan->attrs.msi_control) instead of mchan->msi_irq?

Actually, we should check both over here as well as in
mpxy_mbox_setup_msi() to avoid inappropriate calls
to these functions.

>
>
> > +             return;
> > +
> > +     /* Disable channel MSI control */
> > +     mchan->attrs.msi_control = 0;
> > +     rc = mpxy_write_attrs(mchan->channel_id,
> > SBI_MPXY_ATTR_MSI_CONTROL,
> > +                           1, &mchan->attrs.msi_control);
> > +     if (rc) {
> > +             dev_err(dev, "disable MSI control failed for MPXY channel
> > 0x%x\n",
> > +                     mchan->channel_id);
> > +     }
> > +
> > +     /* Free channel MSI handler */
> > +     free_irq(mchan->msi_irq, chan);
> > +}
> > +
> > +static int mpxy_mbox_setup_events(struct mpxy_mbox_channel *mchan) {
> > +     struct device *dev = mchan->mbox->dev;
> > +     int rc;
> > +
> > +     /* Do nothing if events state not supported */
> > +     if (!mchan->have_events_state)
> > +             return 0;
> > +
> > +     /* Enable channel events state */
> > +     mchan->attrs.events_state_ctrl = 1;
> > +     rc = mpxy_write_attrs(mchan->channel_id,
> > SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> > +                           1, &mchan->attrs.events_state_ctrl);
> > +     if (rc) {
> > +             dev_err(dev, "enable events state failed for MPXY channel
> > 0x%x\n",
> > +                     mchan->channel_id);
>
> Should set mchan->attrs.events_state_ctrl = 0; ?

Okay, I will update.

>
> > +             return rc;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void mpxy_mbox_cleanup_events(struct mpxy_mbox_channel
> > *mchan) {
> > +     struct device *dev = mchan->mbox->dev;
> > +     int rc;
> > +
> > +     /* Do nothing if events state not supported */
> > +     if (!mchan->have_events_state)
> Check also if (!mchan->attrs.events_state_ctrl)?

Okay, I will update.

>
> > +             return;
> > +
> > +     /* Disable channel events state */
> > +     mchan->attrs.events_state_ctrl = 0;
> > +     rc = mpxy_write_attrs(mchan->channel_id,
> > SBI_MPXY_ATTR_EVENTS_STATE_CONTROL,
> > +                           1, &mchan->attrs.events_state_ctrl);
> > +     if (rc) {
> > +             dev_err(dev, "disbable events state failed for MPXY channel
>
> Typo ' disbable'.

Okay, I will update.

>
> > 0x%x\n",
> > +                     mchan->channel_id);
> > +     }
> > +}
> > +
>
>
> [...]
>
>
> > +
> > +static int mpxy_mbox_probe(struct platform_device *pdev) {
> > +     struct device *dev = &pdev->dev;
> > +     struct mpxy_mbox_channel *mchan;
> > +     struct mpxy_mbox *mbox;
> > +     int i, msi_idx, rc;
> > +     u32 *channel_ids;
> > +
> > +     /*
> > +      * Initialize MPXY shared memory only once. This also ensures
> > +      * that SBI MPXY mailbox is probed only once.
> > +      */
> > +     if (mpxy_shmem_init_done) {
> > +             dev_err(dev, "SBI MPXY mailbox already initialized\n");
> > +             return -EALREADY;
> > +     }
> > +
> > +     /* Probe for SBI MPXY extension */
> > +     if (sbi_spec_version < sbi_mk_version(1, 0) ||
> > +         sbi_probe_extension(SBI_EXT_MPXY) <= 0) {
> > +             dev_info(dev, "SBI MPXY extension not available\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Setup cpuhp notifier for per-CPU MPXY shared memory */
> > +     cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "riscv/sbi-mpxy-
> > shmem",
> > +                       mpxy_setup_shmem, mpxy_cleanup_shmem);
> > +
> > +     /* Mark as MPXY shared memory initialization done */
> > +     mpxy_shmem_init_done = true;
> > +
> > +     /* Allocate mailbox instance */
> > +     mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > +     if (!mbox)
> > +             return -ENOMEM;
> > +     mbox->dev = dev;
> > +     platform_set_drvdata(pdev, mbox);
> > +
> > +     /* Find-out of number of channels */
> > +     rc = mpxy_get_channel_count(&mbox->channel_count);
> > +     if (rc) {
> > +             dev_err(dev, "failed to get number of MPXY channels\n");
> Suggest print 'rc' value when error. Same for other error messages below.

Okay, I will update.

>
> > +             return rc;
> > +     }
> > +     if (!mbox->channel_count) {
> > +             dev_err(dev, "no MPXY channels available\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Allocate and fetch all channel IDs */
> > +     channel_ids = devm_kcalloc(dev, mbox->channel_count,
> > +                                sizeof(*channel_ids), GFP_KERNEL);
> > +     if (!channel_ids)
> > +             return -ENOMEM;
> > +     rc = mpxy_get_channel_ids(mbox->channel_count, channel_ids);
> > +     if (rc) {
> > +             dev_err(dev, "failed to get number of MPXY channels\n");
> > +             return rc;
> > +     }
> > +
>
> [...]
>
> Regards
> Ley Foon

Regards,
Anup

[RFC PATCH 7/8] dt-bindings: clock: Add bindings for RISC-V RPMI clock service group

[Anup Patel]

Add device tree bindings for the clock service group defined by the
RISC-V platform management interface (RPMI) specification.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 .../bindings/clock/riscv,rpmi-clock.yaml      | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml b/Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml
new file mode 100644
index 000000000000..9dd98e390708
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/riscv,rpmi-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V RPMI clock service group based clock controller
+
+maintainers:
+  - Anup Patel <anup@brainfault.org>
+
+description: |
+  The RISC-V Platform Management Interface (RPMI) [1] defines a
+  messaging protocol which is modular and extensible. The supervisor
+  software can send/receive RPMI messages via SBI MPXY extension [2]
+  or some dedicated supervisor-mode RPMI transport.
+
+  The RPMI specification [1] defines clock service group for accessing
+  system clocks managed by a platform microcontroller.
+
+  ===========================================
+  References
+  ===========================================
+
+  [1] RISC-V Platform Management Interface (RPMI)
+      https://github.com/riscv-non-isa/riscv-rpmi/releases
+
+  [2] RISC-V Supervisor Binary Interface (SBI)
+      https://github.com/riscv-non-isa/riscv-sbi-doc/releases
+
+properties:
+  compatible:
+    oneOf:
+      - description:
+          Intended for use by the SBI implementation in machine mode or
+          software in supervisor mode.
+        const: riscv,rpmi-clock
+
+      - description:
+          Intended for use by the SBI implementation in machine mode.
+        const: riscv,rpmi-mpxy-clock
+
+  mboxes:
+    minItems: 1
+    maxItems: 1
+    description: |
+      Mailbox channel of the underlying RPMI transport or SBI message proxy.
+
+  riscv,sbi-mpxy-channel-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The SBI MPXY channel id to be used for providing RPMI access to
+      the supervisor software. This property is mandatory when using
+      riscv,rpmi-mpxy-clock compatible string.
+
+  "#clock-cells":
+    const: 1
+    description:
+      This property is mandatory when using riscv,rpmi-clock compatible string.
+
+required:
+  - compatible
+  - mboxes
+
+additionalProperties: false
+
+examples:
+  - |
+    mpxy_mbox: sbi-mpxy-mbox {
+          compatible = "riscv,sbi-mpxy-mbox";
+          #mbox-cells = <2>;
+    };
+    rpmi-clk {
+        compatible = "riscv,rpmi-clock";
+        mboxes = <&mpxy_mbox 0x1000 0x0>;
+        #clock-cells = <1>;
+    };
+...
-- 
2.43.0

Re: [RFC PATCH 7/8] dt-bindings: clock: Add bindings for RISC-V RPMI clock service group

[Rob Herring (Arm)]

On Mon, 16 Dec 2024 14:18:16 +0530, Anup Patel wrote:
> Add device tree bindings for the clock service group defined by the
> RISC-V platform management interface (RPMI) specification.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  .../bindings/clock/riscv,rpmi-clock.yaml      | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml: properties:mboxes: 'anyOf' conditional failed, one must be fixed:
	'minItems' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of minItems/maxItems/items
	hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/mailbox.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/riscv,rpmi-clock.yaml: properties:mboxes: 'anyOf' conditional failed, one must be fixed:
	'minItems' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of minItems/maxItems/items
	hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/mailbox.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241216084817.373131-8-apatel@ventanamicro.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock service group

[Anup Patel]

From: Rahul Pathak <rpathak@ventanamicro.com>

The RPMI specification defines a clock service group which can be
accessed via SBI MPXY extension or dedicated S-mode RPMI transport.

Add mailbox client based clock driver for the RISC-V RPMI clock
service group.

Co-developed-by: Anup Patel <apatel@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
---
 drivers/clk/Kconfig                        |   8 +
 drivers/clk/Makefile                       |   1 +
 drivers/clk/clk-rpmi.c                     | 588 +++++++++++++++++++++
 include/linux/mailbox/riscv-rpmi-message.h |  16 +
 4 files changed, 613 insertions(+)
 create mode 100644 drivers/clk/clk-rpmi.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 713573b6c86c..874cea2ab21c 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -493,6 +493,14 @@ config COMMON_CLK_SP7021
 	  Not all features of the PLL are currently supported
 	  by the driver.
 
+config COMMON_CLK_RPMI
+	tristate "Clock driver based on RISC-V RPMI"
+	depends on MAILBOX || COMPILE_TEST
+	default RISCV
+	help
+	  Support for clocks based on the clock service group defined by
+	  the RISC-V platform management interface (RPMI) specification.
+
 source "drivers/clk/actions/Kconfig"
 source "drivers/clk/analogbits/Kconfig"
 source "drivers/clk/baikal-t1/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index bf4bd45adc3a..95ef59e439bf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
 obj-$(CONFIG_COMMON_CLK_VC7)		+= clk-versaclock7.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
+obj-$(CONFIG_COMMON_CLK_RPMI)		+= clk-rpmi.o
 
 # please keep this section sorted lexicographically by directory path name
 obj-y					+= actions/
diff --git a/drivers/clk/clk-rpmi.c b/drivers/clk/clk-rpmi.c
new file mode 100644
index 000000000000..ed8e32527d3d
--- /dev/null
+++ b/drivers/clk/clk-rpmi.c
@@ -0,0 +1,588 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V MPXY Based Clock Driver
+ *
+ * Copyright (C) 2024 Ventana Micro Systems Ltd.
+ */
+
+#include <linux/io.h>
+#include <linux/mm.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mailbox/riscv-rpmi-message.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk-provider.h>
+
+#define RPMI_CLK_MAX_NUM_RATES		16
+#define RPMI_CLK_NAME_LEN		16
+
+#define GET_RATE_LO_U32(rate_u64)	((u32)rate_u64)
+#define GET_RATE_HI_U32(rate_u64)	((u32)((u64)(rate_u64) >> 32))
+#define GET_RATE_U64(hi_u32, lo_u32)	((u64)(hi_u32) << 32 | (lo_u32))
+
+enum rpmi_clk_config {
+	RPMI_CLK_DISABLE = 0,
+	RPMI_CLK_ENABLE = 1,
+};
+
+enum rpmi_clk_type {
+	RPMI_CLK_DISCRETE = 0,
+	RPMI_CLK_LINEAR = 1,
+	RPMI_CLK_TYPE_MAX_IDX,
+};
+
+struct rpmi_clk_context {
+	struct device *dev;
+	struct mbox_chan *chan;
+	struct mbox_client client;
+};
+
+union rpmi_clk_rate {
+	struct {
+		u32 lo;
+		u32 hi;
+	} discrete[RPMI_CLK_MAX_NUM_RATES];
+	struct {
+		u32 min_lo;
+		u32 min_hi;
+		u32 max_lo;
+		u32 max_hi;
+		u32 step_lo;
+		u32 step_hi;
+	} linear;
+};
+
+union rpmi_clk_rates {
+	u64 discrete[RPMI_CLK_MAX_NUM_RATES];
+	struct {
+		u64 min;
+		u64 max;
+		u64 step;
+	} linear;
+};
+
+struct rpmi_clk {
+	struct rpmi_clk_context *context;
+	u32 id;
+	u32 num_rates;
+	u32 transition_latency;
+	enum rpmi_clk_type type;
+	union rpmi_clk_rates *rates;
+	char name[RPMI_CLK_NAME_LEN];
+	struct clk_hw hw;
+};
+
+#define to_rpmi_clk(clk)	container_of(clk, struct rpmi_clk, hw)
+
+struct rpmi_get_num_clocks_rx {
+	s32 status;
+	u32 num_clocks;
+};
+
+struct rpmi_get_attrs_tx {
+	u32 clkid;
+};
+
+struct rpmi_get_attrs_rx {
+	s32 status;
+	u32 flags;
+	u32 num_rates;
+	u32 transition_latency;
+	char name[RPMI_CLK_NAME_LEN];
+};
+
+struct rpmi_get_supp_rates_tx {
+	u32 clkid;
+	u32 clk_rate_idx;
+};
+
+struct rpmi_get_supp_rates_rx {
+	u32 status;
+	u32 flags;
+	u32 remaining;
+	u32 returned;
+	union rpmi_clk_rate rates;
+};
+
+struct rpmi_get_rate_tx {
+	u32 clkid;
+};
+
+struct rpmi_get_rate_rx {
+	u32 status;
+	u32 lo;
+	u32 hi;
+};
+
+struct rpmi_set_rate_tx {
+	u32 clkid;
+	u32 flags;
+	u32 lo;
+	u32 hi;
+};
+
+struct rpmi_set_rate_rx {
+	u32 status;
+};
+
+struct rpmi_set_config_tx {
+	u32 clkid;
+	u32 config;
+};
+
+struct rpmi_set_config_rx {
+	u32 status;
+};
+
+static int rpmi_clk_get_num_clocks(struct rpmi_clk_context *context)
+{
+	struct rpmi_get_num_clocks_rx rx;
+	struct rpmi_mbox_message msg;
+	int ret;
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_NUM_CLOCKS,
+					  NULL, 0, &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rpmi_to_linux_error(rx.status);
+
+	return rx.num_clocks;
+}
+
+/**
+ * Get the RPMI Clock Attributes.
+ * These attributes belong to a particular clock(clkid)
+ * which are different from the MPXY channel attributes.
+ */
+static int rpmi_clk_get_attrs(u32 clkid, struct rpmi_clk *rpmi_clk)
+{
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_mbox_message msg;
+	struct rpmi_get_attrs_tx tx;
+	struct rpmi_get_attrs_rx rx;
+	u8 format;
+	int ret;
+
+	tx.clkid = cpu_to_le32(clkid);
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_ATTRIBUTES,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rpmi_to_linux_error(rx.status);
+
+	rpmi_clk->id = clkid;
+	rpmi_clk->num_rates = rx.num_rates;
+	rpmi_clk->transition_latency = rx.transition_latency;
+	strscpy(rpmi_clk->name, rx.name, RPMI_CLK_NAME_LEN);
+
+	format = rx.flags & 1U;
+	if (format >= RPMI_CLK_TYPE_MAX_IDX)
+		return -EINVAL;
+
+	rpmi_clk->type = format;
+
+	return 0;
+}
+
+static int rpmi_clk_get_supported_rates(u32 clkid, struct rpmi_clk *rpmi_clk)
+{
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_get_supp_rates_tx tx;
+	struct rpmi_get_supp_rates_rx rx;
+	struct rpmi_mbox_message msg;
+	size_t clk_rate_idx = 0;
+	int ret, rateidx, j;
+
+	tx.clkid = cpu_to_le32(clkid);
+	tx.clk_rate_idx = 0;
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_SUPPORTED_RATES,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rpmi_to_linux_error(rx.status);
+	if (!rx.returned)
+		return -EINVAL;
+
+	if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
+		for (rateidx = 0; rateidx < rx.returned; rateidx++) {
+			rpmi_clk->rates->discrete[rateidx] =
+					GET_RATE_U64(rx.rates.discrete[rateidx].hi,
+						     rx.rates.discrete[rateidx].lo);
+		}
+
+		while (rx.remaining) {
+			clk_rate_idx += rx.returned;
+			tx.clk_rate_idx = clk_rate_idx;
+
+			rpmi_mbox_init_send_with_response(&msg,
+							  RPMI_CLK_SRV_GET_SUPPORTED_RATES,
+							  &tx, sizeof(tx), &rx, sizeof(rx));
+			ret = rpmi_mbox_send_message(context->chan, &msg);
+			if (ret)
+				return ret;
+
+			for (j = 0; j < rx.returned; j++) {
+				if (rateidx >= (clk_rate_idx + rx.returned))
+					break;
+				rpmi_clk->rates->discrete[rateidx++] =
+					GET_RATE_U64(rx.rates.discrete[j].hi,
+						     rx.rates.discrete[j].lo);
+			}
+		}
+	} else if (rpmi_clk->type == RPMI_CLK_LINEAR) {
+		rpmi_clk->rates->linear.min =
+				GET_RATE_U64(rx.rates.linear.min_hi,
+					     rx.rates.linear.min_lo);
+		rpmi_clk->rates->linear.max =
+				GET_RATE_U64(rx.rates.linear.max_hi,
+					     rx.rates.linear.max_lo);
+		rpmi_clk->rates->linear.step =
+				GET_RATE_U64(rx.rates.linear.step_hi,
+					     rx.rates.linear.step_lo);
+	}
+
+	return 0;
+}
+
+static unsigned long rpmi_clk_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_mbox_message msg;
+	struct rpmi_get_rate_tx tx;
+	struct rpmi_get_rate_rx rx;
+	int ret;
+
+	tx.clkid = cpu_to_le32(rpmi_clk->id);
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_RATE,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rx.status;
+
+	return GET_RATE_U64(rx.hi, rx.lo);
+}
+
+static long rpmi_clk_round_rate(struct clk_hw *hw,
+				unsigned long rate,
+				unsigned long *parent_rate)
+{
+	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
+	u64 fmin, fmax, ftmp;
+
+	if (rpmi_clk->type == RPMI_CLK_DISCRETE)
+		return rate;
+
+	fmin = rpmi_clk->rates->linear.min;
+	fmax = rpmi_clk->rates->linear.max;
+
+	if (rate <= fmin)
+		return fmin;
+	else if (rate >=  fmax)
+		return fmax;
+
+	ftmp = rate - fmin;
+	ftmp += rpmi_clk->rates->linear.step - 1;
+	do_div(ftmp, rpmi_clk->rates->linear.step);
+
+	return ftmp * rpmi_clk->rates->linear.step + fmin;
+}
+
+static int rpmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_mbox_message msg;
+	struct rpmi_set_rate_tx tx;
+	struct rpmi_set_rate_rx rx;
+	int ret;
+
+	tx.clkid = cpu_to_le32(rpmi_clk->id);
+	tx.lo = cpu_to_le32(GET_RATE_LO_U32(rate));
+	tx.hi = cpu_to_le32(GET_RATE_HI_U32(rate));
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_RATE,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rpmi_to_linux_error(rx.status);
+
+	return 0;
+}
+
+static int rpmi_clk_enable(struct clk_hw *hw)
+{
+	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_mbox_message msg;
+	struct rpmi_set_config_tx tx;
+	struct rpmi_set_config_rx rx;
+	int ret;
+
+	tx.config = cpu_to_le32(RPMI_CLK_ENABLE);
+	tx.clkid = cpu_to_le32(rpmi_clk->id);
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret)
+		return ret;
+	if (rx.status)
+		return rpmi_to_linux_error(rx.status);
+
+	return 0;
+}
+
+static void rpmi_clk_disable(struct clk_hw *hw)
+{
+	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
+	struct rpmi_clk_context *context = rpmi_clk->context;
+	struct rpmi_mbox_message msg;
+	struct rpmi_set_config_tx tx;
+	struct rpmi_set_config_rx rx;
+	int ret;
+
+	tx.config = cpu_to_le32(RPMI_CLK_DISABLE);
+	tx.clkid = cpu_to_le32(rpmi_clk->id);
+
+	rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
+					  &tx, sizeof(tx), &rx, sizeof(rx));
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret || rx.status)
+		pr_err("Failed to disable clk-%u\n", rpmi_clk->id);
+}
+
+static const struct clk_ops rpmi_clk_ops = {
+	.recalc_rate = rpmi_clk_recalc_rate,
+	.round_rate = rpmi_clk_round_rate,
+	.set_rate = rpmi_clk_set_rate,
+	.prepare = rpmi_clk_enable,
+	.unprepare = rpmi_clk_disable,
+};
+
+static struct clk_hw *rpmi_clk_enumerate(struct rpmi_clk_context *context, u32 clkid)
+{
+	struct device *dev = context->dev;
+	unsigned long min_rate, max_rate;
+	union rpmi_clk_rates *rates;
+	struct rpmi_clk *rpmi_clk;
+	struct clk_init_data init;
+	struct clk_hw *clk_hw;
+	int ret;
+
+	rates = devm_kzalloc(dev, sizeof(union rpmi_clk_rates), GFP_KERNEL);
+	if (!rates)
+		return ERR_PTR(-ENOMEM);
+
+	rpmi_clk = devm_kzalloc(dev, sizeof(struct rpmi_clk), GFP_KERNEL);
+	if (!rpmi_clk)
+		return ERR_PTR(-ENOMEM);
+	rpmi_clk->context = context;
+	rpmi_clk->rates = rates;
+
+	ret = rpmi_clk_get_attrs(clkid, rpmi_clk);
+	if (ret) {
+		dev_err(dev, "Failed to get clk-%u attributes\n", clkid);
+		return ERR_PTR(ret);
+	}
+
+	ret = rpmi_clk_get_supported_rates(clkid, rpmi_clk);
+	if (ret) {
+		dev_err(dev, "Get supported rates failed for clk-%u, %d\n",
+			clkid, ret);
+		return ERR_PTR(ret);
+	}
+
+	init.flags = CLK_GET_RATE_NOCACHE;
+	init.num_parents = 0;
+	init.ops = &rpmi_clk_ops;
+	init.name = rpmi_clk->name;
+	clk_hw = &rpmi_clk->hw;
+	clk_hw->init = &init;
+
+	ret = devm_clk_hw_register(dev, clk_hw);
+	if (ret) {
+		dev_err(dev, "Unable to register clk-%u\n", clkid);
+		return ERR_PTR(ret);
+	}
+
+	if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
+		min_rate = rpmi_clk->rates->discrete[0];
+		max_rate = rpmi_clk->rates->discrete[rpmi_clk->num_rates -  1];
+	} else {
+		min_rate = rpmi_clk->rates->linear.min;
+		max_rate = rpmi_clk->rates->linear.max;
+	}
+
+	clk_hw_set_rate_range(clk_hw, min_rate, max_rate);
+
+	return NULL;
+}
+
+static void rpmi_clk_receive_message(struct mbox_client *cl, void *msg)
+{
+	/* Nothing to do here. */
+}
+
+static int rpmi_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct clk_hw_onecell_data *clk_data;
+	struct rpmi_clk_context *context;
+	struct rpmi_mbox_message msg;
+	int ret, num_clocks, i;
+	struct clk_hw *hw_ptr;
+
+	/* Allocate RPMI clock context */
+	context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
+	if (!context)
+		return -ENOMEM;
+	context->dev = dev;
+	platform_set_drvdata(pdev, context);
+
+	/* Setup mailbox client */
+	context->client.dev		= context->dev;
+	context->client.rx_callback	= rpmi_clk_receive_message;
+	context->client.tx_block	= false;
+	context->client.knows_txdone	= true;
+	context->client.tx_tout		= 0;
+
+	/* Request mailbox channel */
+	context->chan = mbox_request_channel(&context->client, 0);
+	if (IS_ERR(context->chan))
+		return PTR_ERR(context->chan);
+
+	/* Validate RPMI specification version */
+	rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SPEC_VERSION);
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret) {
+		dev_err(dev, "Failed to get spec version\n");
+		goto fail_free_channel;
+	}
+	if (msg.attr.value < RPMI_MKVER(1, 0)) {
+		dev_err(dev,
+			"msg protocol version mismatch, expected 0x%x, found 0x%x\n",
+			RPMI_MKVER(1, 0), msg.attr.value);
+		ret = -EINVAL;
+		goto fail_free_channel;
+	}
+
+	/* Validate clock service group ID */
+	rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_ID);
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret) {
+		dev_err(dev, "Failed to get service group ID\n");
+		goto fail_free_channel;
+	}
+	if (msg.attr.value != RPMI_SRVGRP_CLOCK) {
+		dev_err(dev,
+			"service group match failed, expected 0x%x, found 0x%x\n",
+			RPMI_SRVGRP_CLOCK, msg.attr.value);
+		ret = -EINVAL;
+		goto fail_free_channel;
+	}
+
+	/* Validate clock service group version */
+	rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_VERSION);
+	ret = rpmi_mbox_send_message(context->chan, &msg);
+	if (ret) {
+		dev_err(dev, "Failed to get service group version\n");
+		goto fail_free_channel;
+	}
+	if (msg.attr.value < RPMI_MKVER(1, 0)) {
+		dev_err(dev,
+			"service group version failed, expected 0x%x, found 0x%x\n",
+			RPMI_MKVER(1, 0), msg.attr.value);
+		ret = -EINVAL;
+		goto fail_free_channel;
+	}
+
+	/* Find-out number of clocks */
+	num_clocks = rpmi_clk_get_num_clocks(context);
+	if (!num_clocks) {
+		dev_err(dev, "No clocks found\n");
+		ret = -ENODEV;
+		goto fail_free_channel;
+	}
+
+	/* Allocate clock data */
+	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clocks),
+				GFP_KERNEL);
+	if (!clk_data) {
+		ret = -ENOMEM;
+		goto fail_free_channel;
+	}
+	clk_data->num = num_clocks;
+
+	/* Setup clock data */
+	for (i = 0; i < clk_data->num; i++) {
+		hw_ptr = rpmi_clk_enumerate(context, i);
+		if (IS_ERR(hw_ptr)) {
+			dev_err(dev, "failed to register clk-%d\n", i);
+			ret = PTR_ERR(hw_ptr);
+			goto fail_free_channel;
+		}
+		clk_data->hws[i] = hw_ptr;
+	}
+
+	/* Register clock HW provider */
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
+	if (ret) {
+		dev_err(dev, "failed to register clock HW provider\n");
+		goto fail_free_channel;
+	}
+
+	dev_info(dev, "clk HW provider registered with %d clocks\n",
+		 num_clocks);
+	return 0;
+
+fail_free_channel:
+	mbox_free_channel(context->chan);
+	return ret;
+}
+
+static void rpmi_clk_remove(struct platform_device *pdev)
+{
+	struct rpmi_clk_context *context = platform_get_drvdata(pdev);
+
+	mbox_free_channel(context->chan);
+}
+
+static const struct of_device_id rpmi_clk_of_match[] = {
+	{ .compatible = "riscv,rpmi-clock" },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, rpmi_clk_of_match);
+
+static struct platform_driver rpmi_clk_driver = {
+	.driver = {
+		.name = "riscv-rpmi-clock",
+		.of_match_table = rpmi_clk_of_match,
+	},
+	.probe = rpmi_clk_probe,
+	.remove = rpmi_clk_remove,
+};
+module_platform_driver(rpmi_clk_driver);
+
+MODULE_AUTHOR("Rahul Pathak <rpathak@ventanamicro.com>");
+MODULE_DESCRIPTION("Clock Driver based on RPMI message protocol");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mailbox/riscv-rpmi-message.h b/include/linux/mailbox/riscv-rpmi-message.h
index 8f4b3a0edbce..4e9478c4c0a3 100644
--- a/include/linux/mailbox/riscv-rpmi-message.h
+++ b/include/linux/mailbox/riscv-rpmi-message.h
@@ -89,6 +89,22 @@ static inline int rpmi_to_linux_error(int rpmi_error)
 	}
 }
 
+/** RPMI service group IDs */
+#define RPMI_SRVGRP_CLOCK		0x00007
+
+/** RPMI clock service IDs */
+enum rpmi_clock_service_id {
+	RPMI_CLK_SRV_ENABLE_NOTIFICATION = 0x01,
+	RPMI_CLK_SRV_GET_NUM_CLOCKS = 0x02,
+	RPMI_CLK_SRV_GET_ATTRIBUTES = 0x03,
+	RPMI_CLK_SRV_GET_SUPPORTED_RATES = 0x04,
+	RPMI_CLK_SRV_SET_CONFIG = 0x05,
+	RPMI_CLK_SRV_GET_CONFIG = 0x06,
+	RPMI_CLK_SRV_SET_RATE = 0x07,
+	RPMI_CLK_SRV_GET_RATE = 0x08,
+	RPMI_CLK_SRV_ID_MAX_COUNT,
+};
+
 /** RPMI linux mailbox attribute IDs */
 enum rpmi_mbox_attribute_id {
 	RPMI_MBOX_ATTR_SPEC_VERSION = 0,
-- 
2.43.0

Re: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock service group

[Stephen Boyd]

Quoting Anup Patel (2024-12-16 00:48:17)
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index bf4bd45adc3a..95ef59e439bf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -107,6 +107,7 @@ obj-$(CONFIG_COMMON_CLK_VC5)                += clk-versaclock5.o
>  obj-$(CONFIG_COMMON_CLK_VC7)           += clk-versaclock7.o
>  obj-$(CONFIG_COMMON_CLK_WM831X)                += clk-wm831x.o
>  obj-$(CONFIG_COMMON_CLK_XGENE)         += clk-xgene.o
> +obj-$(CONFIG_COMMON_CLK_RPMI)          += clk-rpmi.o

Keep this sorted by filename.

>  
>  # please keep this section sorted lexicographically by directory path name
>  obj-y                                  += actions/
> diff --git a/drivers/clk/clk-rpmi.c b/drivers/clk/clk-rpmi.c
> new file mode 100644
> index 000000000000..ed8e32527d3d
> --- /dev/null
> +++ b/drivers/clk/clk-rpmi.c
> @@ -0,0 +1,588 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V MPXY Based Clock Driver
> + *
> + * Copyright (C) 2024 Ventana Micro Systems Ltd.
> + */
> +
> +#include <linux/io.h>

Is this include used?

> +#include <linux/mm.h>

Is this include used?

> +#include <linux/of.h>

Is this include used?

> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/mailbox/riscv-rpmi-message.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>

Did you mean mod_devicetable.h?

> +#include <linux/clk-provider.h>

Please sort includes alphabetically.

> +
> +#define RPMI_CLK_MAX_NUM_RATES         16
> +#define RPMI_CLK_NAME_LEN              16
> +
> +#define GET_RATE_LO_U32(rate_u64)      ((u32)rate_u64)
> +#define GET_RATE_HI_U32(rate_u64)      ((u32)((u64)(rate_u64) >> 32))

Use upper_32_bits() and lower_32_bits() instead.

> +#define GET_RATE_U64(hi_u32, lo_u32)   ((u64)(hi_u32) << 32 | (lo_u32))

I couldn't find a macro for this one which is kinda surprising.

> +
> +enum rpmi_clk_config {
> +       RPMI_CLK_DISABLE = 0,
> +       RPMI_CLK_ENABLE = 1,
> +};
> +
> +enum rpmi_clk_type {
> +       RPMI_CLK_DISCRETE = 0,
> +       RPMI_CLK_LINEAR = 1,
> +       RPMI_CLK_TYPE_MAX_IDX,
> +};
> +
> +struct rpmi_clk_context {
> +       struct device *dev;
> +       struct mbox_chan *chan;
> +       struct mbox_client client;
> +};
> +
> +union rpmi_clk_rate {
> +       struct {
> +               u32 lo;
> +               u32 hi;
> +       } discrete[RPMI_CLK_MAX_NUM_RATES];
> +       struct {
> +               u32 min_lo;
> +               u32 min_hi;
> +               u32 max_lo;
> +               u32 max_hi;
> +               u32 step_lo;
> +               u32 step_hi;
> +       } linear;
> +};
> +
> +union rpmi_clk_rates {
> +       u64 discrete[RPMI_CLK_MAX_NUM_RATES];
> +       struct {
> +               u64 min;
> +               u64 max;
> +               u64 step;
> +       } linear;
> +};
> +
> +struct rpmi_clk {
> +       struct rpmi_clk_context *context;
> +       u32 id;
> +       u32 num_rates;
> +       u32 transition_latency;
> +       enum rpmi_clk_type type;
> +       union rpmi_clk_rates *rates;
> +       char name[RPMI_CLK_NAME_LEN];
> +       struct clk_hw hw;
> +};
> +
> +#define to_rpmi_clk(clk)       container_of(clk, struct rpmi_clk, hw)
> +
> +struct rpmi_get_num_clocks_rx {
> +       s32 status;
> +       u32 num_clocks;
> +};
> +
> +struct rpmi_get_attrs_tx {
> +       u32 clkid;

Maybe just 'id', unless the spec calls this 'clkid'?

> +};
> +
> +struct rpmi_get_attrs_rx {
> +       s32 status;
> +       u32 flags;
> +       u32 num_rates;
> +       u32 transition_latency;
> +       char name[RPMI_CLK_NAME_LEN];
> +};
> +
> +struct rpmi_get_supp_rates_tx {
> +       u32 clkid;
> +       u32 clk_rate_idx;
> +};
> +
> +struct rpmi_get_supp_rates_rx {
> +       u32 status;
> +       u32 flags;
> +       u32 remaining;
> +       u32 returned;
> +       union rpmi_clk_rate rates;
> +};
> +
> +struct rpmi_get_rate_tx {
> +       u32 clkid;
> +};
> +
> +struct rpmi_get_rate_rx {
> +       u32 status;
> +       u32 lo;
> +       u32 hi;
> +};
> +
> +struct rpmi_set_rate_tx {
> +       u32 clkid;
> +       u32 flags;
> +       u32 lo;
> +       u32 hi;
> +};
> +
> +struct rpmi_set_rate_rx {
> +       u32 status;
> +};
> +
> +struct rpmi_set_config_tx {
> +       u32 clkid;
> +       u32 config;
> +};
> +
> +struct rpmi_set_config_rx {
> +       u32 status;
> +};
> +
> +static int rpmi_clk_get_num_clocks(struct rpmi_clk_context *context)
> +{
> +       struct rpmi_get_num_clocks_rx rx;
> +       struct rpmi_mbox_message msg;
> +       int ret;
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_NUM_CLOCKS,
> +                                         NULL, 0, &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rpmi_to_linux_error(rx.status);
> +
> +       return rx.num_clocks;
> +}
> +
> +/**

This isn't kernel-doc so either remove the extra '*' or write
kernel-doc.

> + * Get the RPMI Clock Attributes.
> + * These attributes belong to a particular clock(clkid)
> + * which are different from the MPXY channel attributes.
> + */
> +static int rpmi_clk_get_attrs(u32 clkid, struct rpmi_clk *rpmi_clk)
> +{
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_mbox_message msg;
> +       struct rpmi_get_attrs_tx tx;
> +       struct rpmi_get_attrs_rx rx;
> +       u8 format;
> +       int ret;
> +
> +       tx.clkid = cpu_to_le32(clkid);

Run sparse and fix errors please. I assume that the mailbox interactions
need to have __le32 marked structs.

> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_ATTRIBUTES,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rpmi_to_linux_error(rx.status);
> +
> +       rpmi_clk->id = clkid;
> +       rpmi_clk->num_rates = rx.num_rates;
> +       rpmi_clk->transition_latency = rx.transition_latency;
> +       strscpy(rpmi_clk->name, rx.name, RPMI_CLK_NAME_LEN);
> +
> +       format = rx.flags & 1U;
> +       if (format >= RPMI_CLK_TYPE_MAX_IDX)

How is this possible?

> +               return -EINVAL;
> +
> +       rpmi_clk->type = format;
> +
> +       return 0;
> +}
> +
> +static int rpmi_clk_get_supported_rates(u32 clkid, struct rpmi_clk *rpmi_clk)
> +{
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_get_supp_rates_tx tx;
> +       struct rpmi_get_supp_rates_rx rx;
> +       struct rpmi_mbox_message msg;
> +       size_t clk_rate_idx = 0;
> +       int ret, rateidx, j;
> +
> +       tx.clkid = cpu_to_le32(clkid);
> +       tx.clk_rate_idx = 0;
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rpmi_to_linux_error(rx.status);
> +       if (!rx.returned)
> +               return -EINVAL;
> +
> +       if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> +               for (rateidx = 0; rateidx < rx.returned; rateidx++) {
> +                       rpmi_clk->rates->discrete[rateidx] =
> +                                       GET_RATE_U64(rx.rates.discrete[rateidx].hi,
> +                                                    rx.rates.discrete[rateidx].lo);
> +               }
> +
> +               while (rx.remaining) {
> +                       clk_rate_idx += rx.returned;
> +                       tx.clk_rate_idx = clk_rate_idx;
> +
> +                       rpmi_mbox_init_send_with_response(&msg,
> +                                                         RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> +                                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +                       ret = rpmi_mbox_send_message(context->chan, &msg);
> +                       if (ret)
> +                               return ret;
> +
> +                       for (j = 0; j < rx.returned; j++) {
> +                               if (rateidx >= (clk_rate_idx + rx.returned))
> +                                       break;
> +                               rpmi_clk->rates->discrete[rateidx++] =
> +                                       GET_RATE_U64(rx.rates.discrete[j].hi,
> +                                                    rx.rates.discrete[j].lo);
> +                       }
> +               }
> +       } else if (rpmi_clk->type == RPMI_CLK_LINEAR) {
> +               rpmi_clk->rates->linear.min =
> +                               GET_RATE_U64(rx.rates.linear.min_hi,
> +                                            rx.rates.linear.min_lo);
> +               rpmi_clk->rates->linear.max =
> +                               GET_RATE_U64(rx.rates.linear.max_hi,
> +                                            rx.rates.linear.max_lo);
> +               rpmi_clk->rates->linear.step =
> +                               GET_RATE_U64(rx.rates.linear.step_hi,
> +                                            rx.rates.linear.step_lo);
> +       }
> +
> +       return 0;
> +}
> +
> +static unsigned long rpmi_clk_recalc_rate(struct clk_hw *hw,
> +                                         unsigned long parent_rate)
> +{
> +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_mbox_message msg;
> +       struct rpmi_get_rate_tx tx;
> +       struct rpmi_get_rate_rx rx;
> +       int ret;
> +
> +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_RATE,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rx.status;
> +
> +       return GET_RATE_U64(rx.hi, rx.lo);
> +}
> +
> +static long rpmi_clk_round_rate(struct clk_hw *hw,
> +                               unsigned long rate,
> +                               unsigned long *parent_rate)
> +{
> +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +       u64 fmin, fmax, ftmp;
> +
> +       if (rpmi_clk->type == RPMI_CLK_DISCRETE)
> +               return rate;
> +
> +       fmin = rpmi_clk->rates->linear.min;
> +       fmax = rpmi_clk->rates->linear.max;
> +
> +       if (rate <= fmin)
> +               return fmin;
> +       else if (rate >=  fmax)
> +               return fmax;
> +
> +       ftmp = rate - fmin;
> +       ftmp += rpmi_clk->rates->linear.step - 1;
> +       do_div(ftmp, rpmi_clk->rates->linear.step);
> +
> +       return ftmp * rpmi_clk->rates->linear.step + fmin;
> +}
> +
> +static int rpmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +                            unsigned long parent_rate)
> +{
> +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_mbox_message msg;
> +       struct rpmi_set_rate_tx tx;
> +       struct rpmi_set_rate_rx rx;
> +       int ret;
> +
> +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> +       tx.lo = cpu_to_le32(GET_RATE_LO_U32(rate));
> +       tx.hi = cpu_to_le32(GET_RATE_HI_U32(rate));
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_RATE,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rpmi_to_linux_error(rx.status);
> +
> +       return 0;
> +}
> +
> +static int rpmi_clk_enable(struct clk_hw *hw)
> +{
> +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_mbox_message msg;
> +       struct rpmi_set_config_tx tx;
> +       struct rpmi_set_config_rx rx;
> +       int ret;
> +
> +       tx.config = cpu_to_le32(RPMI_CLK_ENABLE);
> +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret)
> +               return ret;
> +       if (rx.status)
> +               return rpmi_to_linux_error(rx.status);
> +
> +       return 0;
> +}
> +
> +static void rpmi_clk_disable(struct clk_hw *hw)
> +{
> +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +       struct rpmi_clk_context *context = rpmi_clk->context;
> +       struct rpmi_mbox_message msg;
> +       struct rpmi_set_config_tx tx;
> +       struct rpmi_set_config_rx rx;
> +       int ret;
> +
> +       tx.config = cpu_to_le32(RPMI_CLK_DISABLE);
> +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
> +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret || rx.status)
> +               pr_err("Failed to disable clk-%u\n", rpmi_clk->id);
> +}
> +
> +static const struct clk_ops rpmi_clk_ops = {
> +       .recalc_rate = rpmi_clk_recalc_rate,
> +       .round_rate = rpmi_clk_round_rate,

Please implement determine_rate instead.

> +       .set_rate = rpmi_clk_set_rate,
> +       .prepare = rpmi_clk_enable,
> +       .unprepare = rpmi_clk_disable,
> +};
> +
> +static struct clk_hw *rpmi_clk_enumerate(struct rpmi_clk_context *context, u32 clkid)
> +{
> +       struct device *dev = context->dev;
> +       unsigned long min_rate, max_rate;
> +       union rpmi_clk_rates *rates;
> +       struct rpmi_clk *rpmi_clk;
> +       struct clk_init_data init;

Use init = { } to initialize everything to zero.

> +       struct clk_hw *clk_hw;
> +       int ret;
> +
> +       rates = devm_kzalloc(dev, sizeof(union rpmi_clk_rates), GFP_KERNEL);
> +       if (!rates)
> +               return ERR_PTR(-ENOMEM);
> +
> +       rpmi_clk = devm_kzalloc(dev, sizeof(struct rpmi_clk), GFP_KERNEL);
> +       if (!rpmi_clk)
> +               return ERR_PTR(-ENOMEM);
> +       rpmi_clk->context = context;
> +       rpmi_clk->rates = rates;
> +
> +       ret = rpmi_clk_get_attrs(clkid, rpmi_clk);
> +       if (ret) {
> +               dev_err(dev, "Failed to get clk-%u attributes\n", clkid);

Please use dev_err_probe() and helpers.

> +               return ERR_PTR(ret);
> +       }
> +
> +       ret = rpmi_clk_get_supported_rates(clkid, rpmi_clk);
> +       if (ret) {
> +               dev_err(dev, "Get supported rates failed for clk-%u, %d\n",
> +                       clkid, ret);
> +               return ERR_PTR(ret);
> +       }
> +
> +       init.flags = CLK_GET_RATE_NOCACHE;
> +       init.num_parents = 0;
> +       init.ops = &rpmi_clk_ops;
> +       init.name = rpmi_clk->name;
> +       clk_hw = &rpmi_clk->hw;
> +       clk_hw->init = &init;
> +
> +       ret = devm_clk_hw_register(dev, clk_hw);
> +       if (ret) {
> +               dev_err(dev, "Unable to register clk-%u\n", clkid);
> +               return ERR_PTR(ret);
> +       }
> +
> +       if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> +               min_rate = rpmi_clk->rates->discrete[0];
> +               max_rate = rpmi_clk->rates->discrete[rpmi_clk->num_rates -  1];
> +       } else {
> +               min_rate = rpmi_clk->rates->linear.min;
> +               max_rate = rpmi_clk->rates->linear.max;
> +       }
> +
> +       clk_hw_set_rate_range(clk_hw, min_rate, max_rate);
> +
> +       return NULL;

Why does it return NULL?

> +}
> +
> +static void rpmi_clk_receive_message(struct mbox_client *cl, void *msg)
> +{
> +       /* Nothing to do here. */
> +}
> +
> +static int rpmi_clk_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct clk_hw_onecell_data *clk_data;
> +       struct rpmi_clk_context *context;
> +       struct rpmi_mbox_message msg;
> +       int ret, num_clocks, i;
> +       struct clk_hw *hw_ptr;
> +
> +       /* Allocate RPMI clock context */
> +       context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> +       if (!context)
> +               return -ENOMEM;
> +       context->dev = dev;
> +       platform_set_drvdata(pdev, context);
> +
> +       /* Setup mailbox client */
> +       context->client.dev             = context->dev;
> +       context->client.rx_callback     = rpmi_clk_receive_message;
> +       context->client.tx_block        = false;
> +       context->client.knows_txdone    = true;
> +       context->client.tx_tout         = 0;
> +
> +       /* Request mailbox channel */
> +       context->chan = mbox_request_channel(&context->client, 0);
> +       if (IS_ERR(context->chan))
> +               return PTR_ERR(context->chan);
> +
> +       /* Validate RPMI specification version */
> +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SPEC_VERSION);
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret) {
> +               dev_err(dev, "Failed to get spec version\n");

Use dev_err_probe()

> +               goto fail_free_channel;
> +       }
> +       if (msg.attr.value < RPMI_MKVER(1, 0)) {
> +               dev_err(dev,
> +                       "msg protocol version mismatch, expected 0x%x, found 0x%x\n",
> +                       RPMI_MKVER(1, 0), msg.attr.value);
> +               ret = -EINVAL;
> +               goto fail_free_channel;
> +       }
> +
> +       /* Validate clock service group ID */
> +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_ID);
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret) {
> +               dev_err(dev, "Failed to get service group ID\n");
> +               goto fail_free_channel;
> +       }
> +       if (msg.attr.value != RPMI_SRVGRP_CLOCK) {
> +               dev_err(dev,
> +                       "service group match failed, expected 0x%x, found 0x%x\n",
> +                       RPMI_SRVGRP_CLOCK, msg.attr.value);
> +               ret = -EINVAL;
> +               goto fail_free_channel;
> +       }
> +
> +       /* Validate clock service group version */
> +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_VERSION);
> +       ret = rpmi_mbox_send_message(context->chan, &msg);
> +       if (ret) {
> +               dev_err(dev, "Failed to get service group version\n");
> +               goto fail_free_channel;
> +       }
> +       if (msg.attr.value < RPMI_MKVER(1, 0)) {
> +               dev_err(dev,
> +                       "service group version failed, expected 0x%x, found 0x%x\n",
> +                       RPMI_MKVER(1, 0), msg.attr.value);
> +               ret = -EINVAL;
> +               goto fail_free_channel;
> +       }
> +
> +       /* Find-out number of clocks */
> +       num_clocks = rpmi_clk_get_num_clocks(context);
> +       if (!num_clocks) {
> +               dev_err(dev, "No clocks found\n");
> +               ret = -ENODEV;
> +               goto fail_free_channel;
> +       }
> +
> +       /* Allocate clock data */
> +       clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clocks),
> +                               GFP_KERNEL);
> +       if (!clk_data) {
> +               ret = -ENOMEM;
> +               goto fail_free_channel;
> +       }
> +       clk_data->num = num_clocks;
> +
> +       /* Setup clock data */
> +       for (i = 0; i < clk_data->num; i++) {
> +               hw_ptr = rpmi_clk_enumerate(context, i);
> +               if (IS_ERR(hw_ptr)) {
> +                       dev_err(dev, "failed to register clk-%d\n", i);
> +                       ret = PTR_ERR(hw_ptr);
> +                       goto fail_free_channel;
> +               }
> +               clk_data->hws[i] = hw_ptr;
> +       }
> +
> +       /* Register clock HW provider */
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> +       if (ret) {
> +               dev_err(dev, "failed to register clock HW provider\n");
> +               goto fail_free_channel;
> +       }
> +
> +       dev_info(dev, "clk HW provider registered with %d clocks\n",
> +                num_clocks);

Remove this info print please.

> +       return 0;
> +
> +fail_free_channel:
> +       mbox_free_channel(context->chan);
> +       return ret;
> +}
> +
> +static void rpmi_clk_remove(struct platform_device *pdev)
> +{
> +       struct rpmi_clk_context *context = platform_get_drvdata(pdev);
> +
> +       mbox_free_channel(context->chan);
> +}
> +
> +static const struct of_device_id rpmi_clk_of_match[] = {
> +       { .compatible = "riscv,rpmi-clock" },
> +       { },

Nitpick: Drop comma so nothing can come after.

> +};
> +

Nitpick: Drop extra newline.

> +MODULE_DEVICE_TABLE(of, rpmi_clk_of_match);
> +
> +static struct platform_driver rpmi_clk_driver = {
> +       .driver = {
> +               .name = "riscv-rpmi-clock",
> +               .of_match_table = rpmi_clk_of_match,
> +       },
> +       .probe = rpmi_clk_probe,
> +       .remove = rpmi_clk_remove,
> +};
> +module_platform_driver(rpmi_clk_driver);
> +
> +MODULE_AUTHOR("Rahul Pathak <rpathak@ventanamicro.com>");
> +MODULE_DESCRIPTION("Clock Driver based on RPMI message protocol");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mailbox/riscv-rpmi-message.h b/include/linux/mailbox/riscv-rpmi-message.h
> index 8f4b3a0edbce..4e9478c4c0a3 100644
> --- a/include/linux/mailbox/riscv-rpmi-message.h
> +++ b/include/linux/mailbox/riscv-rpmi-message.h
> @@ -89,6 +89,22 @@ static inline int rpmi_to_linux_error(int rpmi_error)
>         }
>  }
>  
> +/** RPMI service group IDs */
> +#define RPMI_SRVGRP_CLOCK              0x00007
> +
> +/** RPMI clock service IDs */
> +enum rpmi_clock_service_id {
> +       RPMI_CLK_SRV_ENABLE_NOTIFICATION = 0x01,
> +       RPMI_CLK_SRV_GET_NUM_CLOCKS = 0x02,
> +       RPMI_CLK_SRV_GET_ATTRIBUTES = 0x03,
> +       RPMI_CLK_SRV_GET_SUPPORTED_RATES = 0x04,
> +       RPMI_CLK_SRV_SET_CONFIG = 0x05,
> +       RPMI_CLK_SRV_GET_CONFIG = 0x06,
> +       RPMI_CLK_SRV_SET_RATE = 0x07,
> +       RPMI_CLK_SRV_GET_RATE = 0x08,
> +       RPMI_CLK_SRV_ID_MAX_COUNT,
> +};
> +

What is the benefit of the enum vs. just having a #define?

Re: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock service group

[Rahul Pathak]

Hi Stephen,

On Wed, Dec 18, 2024 at 1:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Anup Patel (2024-12-16 00:48:17)
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index bf4bd45adc3a..95ef59e439bf 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -107,6 +107,7 @@ obj-$(CONFIG_COMMON_CLK_VC5)                += clk-versaclock5.o
> >  obj-$(CONFIG_COMMON_CLK_VC7)           += clk-versaclock7.o
> >  obj-$(CONFIG_COMMON_CLK_WM831X)                += clk-wm831x.o
> >  obj-$(CONFIG_COMMON_CLK_XGENE)         += clk-xgene.o
> > +obj-$(CONFIG_COMMON_CLK_RPMI)          += clk-rpmi.o
>
> Keep this sorted by filename.

Sure, I will update.

>
> >
> >  # please keep this section sorted lexicographically by directory path name
> >  obj-y                                  += actions/
> > diff --git a/drivers/clk/clk-rpmi.c b/drivers/clk/clk-rpmi.c
> > new file mode 100644
> > index 000000000000..ed8e32527d3d
> > --- /dev/null
> > +++ b/drivers/clk/clk-rpmi.c
> > @@ -0,0 +1,588 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RISC-V MPXY Based Clock Driver
> > + *
> > + * Copyright (C) 2024 Ventana Micro Systems Ltd.
> > + */
> > +
> > +#include <linux/io.h>
>
> Is this include used?
>
> > +#include <linux/mm.h>
>
> Is this include used?
>
> > +#include <linux/of.h>
>
> Is this include used?
>
> > +#include <linux/err.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/mailbox/riscv-rpmi-message.h>
> > +#include <linux/mailbox_client.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
>
> Did you mean mod_devicetable.h?
>
> > +#include <linux/clk-provider.h>
>
> Please sort includes alphabetically.

I will correct all the headers and sort them.

>
> > +
> > +#define RPMI_CLK_MAX_NUM_RATES         16
> > +#define RPMI_CLK_NAME_LEN              16
> > +
> > +#define GET_RATE_LO_U32(rate_u64)      ((u32)rate_u64)
> > +#define GET_RATE_HI_U32(rate_u64)      ((u32)((u64)(rate_u64) >> 32))
>
> Use upper_32_bits() and lower_32_bits() instead.

Sure, I will update.

>
> > +#define GET_RATE_U64(hi_u32, lo_u32)   ((u64)(hi_u32) << 32 | (lo_u32))
>
> I couldn't find a macro for this one which is kinda surprising.
>
> > +
> > +enum rpmi_clk_config {
> > +       RPMI_CLK_DISABLE = 0,
> > +       RPMI_CLK_ENABLE = 1,
> > +};
> > +
> > +enum rpmi_clk_type {
> > +       RPMI_CLK_DISCRETE = 0,
> > +       RPMI_CLK_LINEAR = 1,
> > +       RPMI_CLK_TYPE_MAX_IDX,
> > +};
> > +
> > +struct rpmi_clk_context {
> > +       struct device *dev;
> > +       struct mbox_chan *chan;
> > +       struct mbox_client client;
> > +};
> > +
> > +union rpmi_clk_rate {
> > +       struct {
> > +               u32 lo;
> > +               u32 hi;
> > +       } discrete[RPMI_CLK_MAX_NUM_RATES];
> > +       struct {
> > +               u32 min_lo;
> > +               u32 min_hi;
> > +               u32 max_lo;
> > +               u32 max_hi;
> > +               u32 step_lo;
> > +               u32 step_hi;
> > +       } linear;
> > +};
> > +
> > +union rpmi_clk_rates {
> > +       u64 discrete[RPMI_CLK_MAX_NUM_RATES];
> > +       struct {
> > +               u64 min;
> > +               u64 max;
> > +               u64 step;
> > +       } linear;
> > +};
> > +
> > +struct rpmi_clk {
> > +       struct rpmi_clk_context *context;
> > +       u32 id;
> > +       u32 num_rates;
> > +       u32 transition_latency;
> > +       enum rpmi_clk_type type;
> > +       union rpmi_clk_rates *rates;
> > +       char name[RPMI_CLK_NAME_LEN];
> > +       struct clk_hw hw;
> > +};
> > +
> > +#define to_rpmi_clk(clk)       container_of(clk, struct rpmi_clk, hw)
> > +
> > +struct rpmi_get_num_clocks_rx {
> > +       s32 status;
> > +       u32 num_clocks;
> > +};
> > +
> > +struct rpmi_get_attrs_tx {
> > +       u32 clkid;
>
> Maybe just 'id', unless the spec calls this 'clkid'?

Spec calls it Clock ID, and in short used as clkid.
The word ID is used for other things as well so I think using clkid
here is better.

>
> > +};
> > +
> > +struct rpmi_get_attrs_rx {
> > +       s32 status;
> > +       u32 flags;
> > +       u32 num_rates;
> > +       u32 transition_latency;
> > +       char name[RPMI_CLK_NAME_LEN];
> > +};
> > +
> > +struct rpmi_get_supp_rates_tx {
> > +       u32 clkid;
> > +       u32 clk_rate_idx;
> > +};
> > +
> > +struct rpmi_get_supp_rates_rx {
> > +       u32 status;
> > +       u32 flags;
> > +       u32 remaining;
> > +       u32 returned;
> > +       union rpmi_clk_rate rates;
> > +};
> > +
> > +struct rpmi_get_rate_tx {
> > +       u32 clkid;
> > +};
> > +
> > +struct rpmi_get_rate_rx {
> > +       u32 status;
> > +       u32 lo;
> > +       u32 hi;
> > +};
> > +
> > +struct rpmi_set_rate_tx {
> > +       u32 clkid;
> > +       u32 flags;
> > +       u32 lo;
> > +       u32 hi;
> > +};
> > +
> > +struct rpmi_set_rate_rx {
> > +       u32 status;
> > +};
> > +
> > +struct rpmi_set_config_tx {
> > +       u32 clkid;
> > +       u32 config;
> > +};
> > +
> > +struct rpmi_set_config_rx {
> > +       u32 status;
> > +};
> > +
> > +static int rpmi_clk_get_num_clocks(struct rpmi_clk_context *context)
> > +{
> > +       struct rpmi_get_num_clocks_rx rx;
> > +       struct rpmi_mbox_message msg;
> > +       int ret;
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_NUM_CLOCKS,
> > +                                         NULL, 0, &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rpmi_to_linux_error(rx.status);
> > +
> > +       return rx.num_clocks;
> > +}
> > +
> > +/**
>
> This isn't kernel-doc so either remove the extra '*' or write
> kernel-doc.

Sure, I will remove that wherever necessary.

>
> > + * Get the RPMI Clock Attributes.
> > + * These attributes belong to a particular clock(clkid)
> > + * which are different from the MPXY channel attributes.
> > + */
> > +static int rpmi_clk_get_attrs(u32 clkid, struct rpmi_clk *rpmi_clk)
> > +{
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_mbox_message msg;
> > +       struct rpmi_get_attrs_tx tx;
> > +       struct rpmi_get_attrs_rx rx;
> > +       u8 format;
> > +       int ret;
> > +
> > +       tx.clkid = cpu_to_le32(clkid);
>
> Run sparse and fix errors please. I assume that the mailbox interactions
> need to have __le32 marked structs.

Sure.

>
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_ATTRIBUTES,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rpmi_to_linux_error(rx.status);
> > +
> > +       rpmi_clk->id = clkid;
> > +       rpmi_clk->num_rates = rx.num_rates;
> > +       rpmi_clk->transition_latency = rx.transition_latency;
> > +       strscpy(rpmi_clk->name, rx.name, RPMI_CLK_NAME_LEN);
> > +
> > +       format = rx.flags & 1U;
> > +       if (format >= RPMI_CLK_TYPE_MAX_IDX)
>
> How is this possible?

There is a mistake in getting the format from flags which makes the
"if" condition useless.
The format is supposed to be 2 bit wide in the flags.
The spec currently only defines 0b00 and 0b01, rest are
reserved and if condition will be necessary. i will correct this


>
> > +               return -EINVAL;
> > +
> > +       rpmi_clk->type = format;
> > +
> > +       return 0;
> > +}
> > +
> > +static int rpmi_clk_get_supported_rates(u32 clkid, struct rpmi_clk *rpmi_clk)
> > +{
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_get_supp_rates_tx tx;
> > +       struct rpmi_get_supp_rates_rx rx;
> > +       struct rpmi_mbox_message msg;
> > +       size_t clk_rate_idx = 0;
> > +       int ret, rateidx, j;
> > +
> > +       tx.clkid = cpu_to_le32(clkid);
> > +       tx.clk_rate_idx = 0;
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rpmi_to_linux_error(rx.status);
> > +       if (!rx.returned)
> > +               return -EINVAL;
> > +
> > +       if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> > +               for (rateidx = 0; rateidx < rx.returned; rateidx++) {
> > +                       rpmi_clk->rates->discrete[rateidx] =
> > +                                       GET_RATE_U64(rx.rates.discrete[rateidx].hi,
> > +                                                    rx.rates.discrete[rateidx].lo);
> > +               }
> > +
> > +               while (rx.remaining) {
> > +                       clk_rate_idx += rx.returned;
> > +                       tx.clk_rate_idx = clk_rate_idx;
> > +
> > +                       rpmi_mbox_init_send_with_response(&msg,
> > +                                                         RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> > +                                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +                       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +                       if (ret)
> > +                               return ret;
> > +
> > +                       for (j = 0; j < rx.returned; j++) {
> > +                               if (rateidx >= (clk_rate_idx + rx.returned))
> > +                                       break;
> > +                               rpmi_clk->rates->discrete[rateidx++] =
> > +                                       GET_RATE_U64(rx.rates.discrete[j].hi,
> > +                                                    rx.rates.discrete[j].lo);
> > +                       }
> > +               }
> > +       } else if (rpmi_clk->type == RPMI_CLK_LINEAR) {
> > +               rpmi_clk->rates->linear.min =
> > +                               GET_RATE_U64(rx.rates.linear.min_hi,
> > +                                            rx.rates.linear.min_lo);
> > +               rpmi_clk->rates->linear.max =
> > +                               GET_RATE_U64(rx.rates.linear.max_hi,
> > +                                            rx.rates.linear.max_lo);
> > +               rpmi_clk->rates->linear.step =
> > +                               GET_RATE_U64(rx.rates.linear.step_hi,
> > +                                            rx.rates.linear.step_lo);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static unsigned long rpmi_clk_recalc_rate(struct clk_hw *hw,
> > +                                         unsigned long parent_rate)
> > +{
> > +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_mbox_message msg;
> > +       struct rpmi_get_rate_tx tx;
> > +       struct rpmi_get_rate_rx rx;
> > +       int ret;
> > +
> > +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_GET_RATE,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rx.status;
> > +
> > +       return GET_RATE_U64(rx.hi, rx.lo);
> > +}
> > +
> > +static long rpmi_clk_round_rate(struct clk_hw *hw,
> > +                               unsigned long rate,
> > +                               unsigned long *parent_rate)
> > +{
> > +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +       u64 fmin, fmax, ftmp;
> > +
> > +       if (rpmi_clk->type == RPMI_CLK_DISCRETE)
> > +               return rate;
> > +
> > +       fmin = rpmi_clk->rates->linear.min;
> > +       fmax = rpmi_clk->rates->linear.max;
> > +
> > +       if (rate <= fmin)
> > +               return fmin;
> > +       else if (rate >=  fmax)
> > +               return fmax;
> > +
> > +       ftmp = rate - fmin;
> > +       ftmp += rpmi_clk->rates->linear.step - 1;
> > +       do_div(ftmp, rpmi_clk->rates->linear.step);
> > +
> > +       return ftmp * rpmi_clk->rates->linear.step + fmin;
> > +}
> > +
> > +static int rpmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> > +                            unsigned long parent_rate)
> > +{
> > +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_mbox_message msg;
> > +       struct rpmi_set_rate_tx tx;
> > +       struct rpmi_set_rate_rx rx;
> > +       int ret;
> > +
> > +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +       tx.lo = cpu_to_le32(GET_RATE_LO_U32(rate));
> > +       tx.hi = cpu_to_le32(GET_RATE_HI_U32(rate));
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_RATE,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rpmi_to_linux_error(rx.status);
> > +
> > +       return 0;
> > +}
> > +
> > +static int rpmi_clk_enable(struct clk_hw *hw)
> > +{
> > +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_mbox_message msg;
> > +       struct rpmi_set_config_tx tx;
> > +       struct rpmi_set_config_rx rx;
> > +       int ret;
> > +
> > +       tx.config = cpu_to_le32(RPMI_CLK_ENABLE);
> > +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret)
> > +               return ret;
> > +       if (rx.status)
> > +               return rpmi_to_linux_error(rx.status);
> > +
> > +       return 0;
> > +}
> > +
> > +static void rpmi_clk_disable(struct clk_hw *hw)
> > +{
> > +       struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +       struct rpmi_clk_context *context = rpmi_clk->context;
> > +       struct rpmi_mbox_message msg;
> > +       struct rpmi_set_config_tx tx;
> > +       struct rpmi_set_config_rx rx;
> > +       int ret;
> > +
> > +       tx.config = cpu_to_le32(RPMI_CLK_DISABLE);
> > +       tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +       rpmi_mbox_init_send_with_response(&msg, RPMI_CLK_SRV_SET_CONFIG,
> > +                                         &tx, sizeof(tx), &rx, sizeof(rx));
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret || rx.status)
> > +               pr_err("Failed to disable clk-%u\n", rpmi_clk->id);
> > +}
> > +
> > +static const struct clk_ops rpmi_clk_ops = {
> > +       .recalc_rate = rpmi_clk_recalc_rate,
> > +       .round_rate = rpmi_clk_round_rate,
>
> Please implement determine_rate instead.

Sure. I will update

>
> > +       .set_rate = rpmi_clk_set_rate,
> > +       .prepare = rpmi_clk_enable,
> > +       .unprepare = rpmi_clk_disable,
> > +};
> > +
> > +static struct clk_hw *rpmi_clk_enumerate(struct rpmi_clk_context *context, u32 clkid)
> > +{
> > +       struct device *dev = context->dev;
> > +       unsigned long min_rate, max_rate;
> > +       union rpmi_clk_rates *rates;
> > +       struct rpmi_clk *rpmi_clk;
> > +       struct clk_init_data init;
>
> Use init = { } to initialize everything to zero.

Sure, I will update

>
> > +       struct clk_hw *clk_hw;
> > +       int ret;
> > +
> > +       rates = devm_kzalloc(dev, sizeof(union rpmi_clk_rates), GFP_KERNEL);
> > +       if (!rates)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       rpmi_clk = devm_kzalloc(dev, sizeof(struct rpmi_clk), GFP_KERNEL);
> > +       if (!rpmi_clk)
> > +               return ERR_PTR(-ENOMEM);
> > +       rpmi_clk->context = context;
> > +       rpmi_clk->rates = rates;
> > +
> > +       ret = rpmi_clk_get_attrs(clkid, rpmi_clk);
> > +       if (ret) {
> > +               dev_err(dev, "Failed to get clk-%u attributes\n", clkid);
>
> Please use dev_err_probe() and helpers.

Sure, I will update

>
> > +               return ERR_PTR(ret);
> > +       }
> > +
> > +       ret = rpmi_clk_get_supported_rates(clkid, rpmi_clk);
> > +       if (ret) {
> > +               dev_err(dev, "Get supported rates failed for clk-%u, %d\n",
> > +                       clkid, ret);
> > +               return ERR_PTR(ret);
> > +       }
> > +
> > +       init.flags = CLK_GET_RATE_NOCACHE;
> > +       init.num_parents = 0;
> > +       init.ops = &rpmi_clk_ops;
> > +       init.name = rpmi_clk->name;
> > +       clk_hw = &rpmi_clk->hw;
> > +       clk_hw->init = &init;
> > +
> > +       ret = devm_clk_hw_register(dev, clk_hw);
> > +       if (ret) {
> > +               dev_err(dev, "Unable to register clk-%u\n", clkid);
> > +               return ERR_PTR(ret);
> > +       }
> > +
> > +       if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> > +               min_rate = rpmi_clk->rates->discrete[0];
> > +               max_rate = rpmi_clk->rates->discrete[rpmi_clk->num_rates -  1];
> > +       } else {
> > +               min_rate = rpmi_clk->rates->linear.min;
> > +               max_rate = rpmi_clk->rates->linear.max;
> > +       }
> > +
> > +       clk_hw_set_rate_range(clk_hw, min_rate, max_rate);
> > +
> > +       return NULL;
>
> Why does it return NULL?

Yes, it's a typo. I will correct this.

>
> > +}
> > +
> > +static void rpmi_clk_receive_message(struct mbox_client *cl, void *msg)
> > +{
> > +       /* Nothing to do here. */
> > +}
> > +
> > +static int rpmi_clk_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct clk_hw_onecell_data *clk_data;
> > +       struct rpmi_clk_context *context;
> > +       struct rpmi_mbox_message msg;
> > +       int ret, num_clocks, i;
> > +       struct clk_hw *hw_ptr;
> > +
> > +       /* Allocate RPMI clock context */
> > +       context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> > +       if (!context)
> > +               return -ENOMEM;
> > +       context->dev = dev;
> > +       platform_set_drvdata(pdev, context);
> > +
> > +       /* Setup mailbox client */
> > +       context->client.dev             = context->dev;
> > +       context->client.rx_callback     = rpmi_clk_receive_message;
> > +       context->client.tx_block        = false;
> > +       context->client.knows_txdone    = true;
> > +       context->client.tx_tout         = 0;
> > +
> > +       /* Request mailbox channel */
> > +       context->chan = mbox_request_channel(&context->client, 0);
> > +       if (IS_ERR(context->chan))
> > +               return PTR_ERR(context->chan);
> > +
> > +       /* Validate RPMI specification version */
> > +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SPEC_VERSION);
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret) {
> > +               dev_err(dev, "Failed to get spec version\n");
>
> Use dev_err_probe()

Sure, I will update

>
> > +               goto fail_free_channel;
> > +       }
> > +       if (msg.attr.value < RPMI_MKVER(1, 0)) {
> > +               dev_err(dev,
> > +                       "msg protocol version mismatch, expected 0x%x, found 0x%x\n",
> > +                       RPMI_MKVER(1, 0), msg.attr.value);
> > +               ret = -EINVAL;
> > +               goto fail_free_channel;
> > +       }
> > +
> > +       /* Validate clock service group ID */
> > +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_ID);
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret) {
> > +               dev_err(dev, "Failed to get service group ID\n");
> > +               goto fail_free_channel;
> > +       }
> > +       if (msg.attr.value != RPMI_SRVGRP_CLOCK) {
> > +               dev_err(dev,
> > +                       "service group match failed, expected 0x%x, found 0x%x\n",
> > +                       RPMI_SRVGRP_CLOCK, msg.attr.value);
> > +               ret = -EINVAL;
> > +               goto fail_free_channel;
> > +       }
> > +
> > +       /* Validate clock service group version */
> > +       rpmi_mbox_init_get_attribute(&msg, RPMI_MBOX_ATTR_SERVICEGROUP_VERSION);
> > +       ret = rpmi_mbox_send_message(context->chan, &msg);
> > +       if (ret) {
> > +               dev_err(dev, "Failed to get service group version\n");
> > +               goto fail_free_channel;
> > +       }
> > +       if (msg.attr.value < RPMI_MKVER(1, 0)) {
> > +               dev_err(dev,
> > +                       "service group version failed, expected 0x%x, found 0x%x\n",
> > +                       RPMI_MKVER(1, 0), msg.attr.value);
> > +               ret = -EINVAL;
> > +               goto fail_free_channel;
> > +       }
> > +
> > +       /* Find-out number of clocks */
> > +       num_clocks = rpmi_clk_get_num_clocks(context);
> > +       if (!num_clocks) {
> > +               dev_err(dev, "No clocks found\n");
> > +               ret = -ENODEV;
> > +               goto fail_free_channel;
> > +       }
> > +
> > +       /* Allocate clock data */
> > +       clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clocks),
> > +                               GFP_KERNEL);
> > +       if (!clk_data) {
> > +               ret = -ENOMEM;
> > +               goto fail_free_channel;
> > +       }
> > +       clk_data->num = num_clocks;
> > +
> > +       /* Setup clock data */
> > +       for (i = 0; i < clk_data->num; i++) {
> > +               hw_ptr = rpmi_clk_enumerate(context, i);
> > +               if (IS_ERR(hw_ptr)) {
> > +                       dev_err(dev, "failed to register clk-%d\n", i);
> > +                       ret = PTR_ERR(hw_ptr);
> > +                       goto fail_free_channel;
> > +               }
> > +               clk_data->hws[i] = hw_ptr;
> > +       }
> > +
> > +       /* Register clock HW provider */
> > +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> > +       if (ret) {
> > +               dev_err(dev, "failed to register clock HW provider\n");
> > +               goto fail_free_channel;
> > +       }
> > +
> > +       dev_info(dev, "clk HW provider registered with %d clocks\n",
> > +                num_clocks);
>
> Remove this info print please.

Sure, I will update.

>
> > +       return 0;
> > +
> > +fail_free_channel:
> > +       mbox_free_channel(context->chan);
> > +       return ret;
> > +}
> > +
> > +static void rpmi_clk_remove(struct platform_device *pdev)
> > +{
> > +       struct rpmi_clk_context *context = platform_get_drvdata(pdev);
> > +
> > +       mbox_free_channel(context->chan);
> > +}
> > +
> > +static const struct of_device_id rpmi_clk_of_match[] = {
> > +       { .compatible = "riscv,rpmi-clock" },
> > +       { },
>
> Nitpick: Drop comma so nothing can come after.

Sure, I will update.

>
> > +};
> > +
>
> Nitpick: Drop extra newline.

Sure, I will update.

>
> > +MODULE_DEVICE_TABLE(of, rpmi_clk_of_match);
> > +
> > +static struct platform_driver rpmi_clk_driver = {
> > +       .driver = {
> > +               .name = "riscv-rpmi-clock",
> > +               .of_match_table = rpmi_clk_of_match,
> > +       },
> > +       .probe = rpmi_clk_probe,
> > +       .remove = rpmi_clk_remove,
> > +};
> > +module_platform_driver(rpmi_clk_driver);
> > +
> > +MODULE_AUTHOR("Rahul Pathak <rpathak@ventanamicro.com>");
> > +MODULE_DESCRIPTION("Clock Driver based on RPMI message protocol");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mailbox/riscv-rpmi-message.h b/include/linux/mailbox/riscv-rpmi-message.h
> > index 8f4b3a0edbce..4e9478c4c0a3 100644
> > --- a/include/linux/mailbox/riscv-rpmi-message.h
> > +++ b/include/linux/mailbox/riscv-rpmi-message.h
> > @@ -89,6 +89,22 @@ static inline int rpmi_to_linux_error(int rpmi_error)
> >         }
> >  }
> >
> > +/** RPMI service group IDs */
> > +#define RPMI_SRVGRP_CLOCK              0x00007
> > +
> > +/** RPMI clock service IDs */
> > +enum rpmi_clock_service_id {
> > +       RPMI_CLK_SRV_ENABLE_NOTIFICATION = 0x01,
> > +       RPMI_CLK_SRV_GET_NUM_CLOCKS = 0x02,
> > +       RPMI_CLK_SRV_GET_ATTRIBUTES = 0x03,
> > +       RPMI_CLK_SRV_GET_SUPPORTED_RATES = 0x04,
> > +       RPMI_CLK_SRV_SET_CONFIG = 0x05,
> > +       RPMI_CLK_SRV_GET_CONFIG = 0x06,
> > +       RPMI_CLK_SRV_SET_RATE = 0x07,
> > +       RPMI_CLK_SRV_GET_RATE = 0x08,
> > +       RPMI_CLK_SRV_ID_MAX_COUNT,
> > +};
> > +
>
> What is the benefit of the enum vs. just having a #define?

#define can achieve the same thing as enum does.
We have kept the same format for such defines everywhere.
For example a counterpart implementation of RPMI clock will
be in Platform microcontroller firmware and also in Qemu for
emulation which came first that's why I used the same.

I will prefer to use the common format everywhere unless
the driver requires use of #define instead of enum.

RE: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock service group

[Leyfoon Tan]

> -----Original Message-----
> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Monday, December 16, 2024 4:48 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> <jassisinghbrar@gmail.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> <anup@brainfault.org>; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> Subject: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock
> service group
> 
> From: Rahul Pathak <rpathak@ventanamicro.com>
> 
> The RPMI specification defines a clock service group which can be accessed via
> SBI MPXY extension or dedicated S-mode RPMI transport.
> 
> Add mailbox client based clock driver for the RISC-V RPMI clock service group.
> 
> Co-developed-by: Anup Patel <apatel@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> ---


>  obj-y					+= actions/
> diff --git a/drivers/clk/clk-rpmi.c b/drivers/clk/clk-rpmi.c new file mode
> 100644 index 000000000000..ed8e32527d3d
> --- /dev/null
> +++ b/drivers/clk/clk-rpmi.c
> @@ -0,0 +1,588 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V MPXY Based Clock Driver
> + *
> + * Copyright (C) 2024 Ventana Micro Systems Ltd.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/mm.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/mailbox/riscv-rpmi-message.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk-provider.h>

Sorting header files in alphabetical order.

> +
> +#define RPMI_CLK_MAX_NUM_RATES		16
This macro only used for discrete clocks, so suggest change to RPMI_CLK_MAX_DISCRETE_NUM_RATES.
Is 16 too few?

> +#define RPMI_CLK_NAME_LEN		16
> +
> +#define GET_RATE_LO_U32(rate_u64)	((u32)rate_u64)
> +#define GET_RATE_HI_U32(rate_u64)	((u32)((u64)(rate_u64) >> 32))
> +#define GET_RATE_U64(hi_u32, lo_u32)	((u64)(hi_u32) << 32 |
> (lo_u32))
> +
> +enum rpmi_clk_config {
> +	RPMI_CLK_DISABLE = 0,
> +	RPMI_CLK_ENABLE = 1,
> +};
> +

[...]

> +static int rpmi_clk_get_supported_rates(u32 clkid, struct rpmi_clk
> +*rpmi_clk) {
> +	struct rpmi_clk_context *context = rpmi_clk->context;
> +	struct rpmi_get_supp_rates_tx tx;
> +	struct rpmi_get_supp_rates_rx rx;
> +	struct rpmi_mbox_message msg;
> +	size_t clk_rate_idx = 0;
> +	int ret, rateidx, j;
> +
> +	tx.clkid = cpu_to_le32(clkid);
> +	tx.clk_rate_idx = 0;
> +
> +	rpmi_mbox_init_send_with_response(&msg,
> RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> +					  &tx, sizeof(tx), &rx, sizeof(rx));
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret)
> +		return ret;
> +	if (rx.status)
> +		return rpmi_to_linux_error(rx.status);
> +	if (!rx.returned)
> +		return -EINVAL;
> +
> +	if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> +		for (rateidx = 0; rateidx < rx.returned; rateidx++) {

Need to check RPMI_CLK_MAX_NUM_RATES limit as well.

> +			rpmi_clk->rates->discrete[rateidx] =
> +
> 	GET_RATE_U64(rx.rates.discrete[rateidx].hi,
> +
> rx.rates.discrete[rateidx].lo);
> +		}
> +
> +		while (rx.remaining) {
> +			clk_rate_idx += rx.returned;
> +			tx.clk_rate_idx = clk_rate_idx;
> +
> +			rpmi_mbox_init_send_with_response(&msg,
> +
> RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> +							  &tx, sizeof(tx), &rx,
> sizeof(rx));
> +			ret = rpmi_mbox_send_message(context->chan,
> &msg);
> +			if (ret)
> +				return ret;

Need check if (rx.status) here?

> +
> +			for (j = 0; j < rx.returned; j++) {
Same here, check RPMI_CLK_MAX_NUM_RATES.

> +				if (rateidx >= (clk_rate_idx + rx.returned))
> +					break;
> +				rpmi_clk->rates->discrete[rateidx++] =
> +					GET_RATE_U64(rx.rates.discrete[j].hi,
> +						     rx.rates.discrete[j].lo);
> +			}
> +		}
> +	} else if (rpmi_clk->type == RPMI_CLK_LINEAR) {
> +		rpmi_clk->rates->linear.min =
> +				GET_RATE_U64(rx.rates.linear.min_hi,
> +					     rx.rates.linear.min_lo);
> +		rpmi_clk->rates->linear.max =
> +				GET_RATE_U64(rx.rates.linear.max_hi,
> +					     rx.rates.linear.max_lo);
> +		rpmi_clk->rates->linear.step =
> +				GET_RATE_U64(rx.rates.linear.step_hi,
> +					     rx.rates.linear.step_lo);
> +	}
> +
> +	return 0;
> +}
> +
> +static unsigned long rpmi_clk_recalc_rate(struct clk_hw *hw,
> +					  unsigned long parent_rate)
> +{
> +	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +	struct rpmi_clk_context *context = rpmi_clk->context;
> +	struct rpmi_mbox_message msg;
> +	struct rpmi_get_rate_tx tx;
> +	struct rpmi_get_rate_rx rx;
> +	int ret;
> +
> +	tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +	rpmi_mbox_init_send_with_response(&msg,
> RPMI_CLK_SRV_GET_RATE,
> +					  &tx, sizeof(tx), &rx, sizeof(rx));
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret)
> +		return ret;
> +	if (rx.status)
> +		return rx.status;
> +
> +	return GET_RATE_U64(rx.hi, rx.lo);
> +}
> +
> +static long rpmi_clk_round_rate(struct clk_hw *hw,
> +				unsigned long rate,
> +				unsigned long *parent_rate)
> +{
> +	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +	u64 fmin, fmax, ftmp;
> +
> +	if (rpmi_clk->type == RPMI_CLK_DISCRETE)
> +		return rate;
> +
> +	fmin = rpmi_clk->rates->linear.min;
> +	fmax = rpmi_clk->rates->linear.max;
> +
> +	if (rate <= fmin)
> +		return fmin;
> +	else if (rate >=  fmax)
> +		return fmax;
> +
> +	ftmp = rate - fmin;
> +	ftmp += rpmi_clk->rates->linear.step - 1;
> +	do_div(ftmp, rpmi_clk->rates->linear.step);
> +
> +	return ftmp * rpmi_clk->rates->linear.step + fmin; }
> +
> +static int rpmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +			     unsigned long parent_rate)
> +{
> +	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +	struct rpmi_clk_context *context = rpmi_clk->context;
> +	struct rpmi_mbox_message msg;
> +	struct rpmi_set_rate_tx tx;
> +	struct rpmi_set_rate_rx rx;
> +	int ret;
> +
> +	tx.clkid = cpu_to_le32(rpmi_clk->id);
> +	tx.lo = cpu_to_le32(GET_RATE_LO_U32(rate));
> +	tx.hi = cpu_to_le32(GET_RATE_HI_U32(rate));
> +
> +	rpmi_mbox_init_send_with_response(&msg,
> RPMI_CLK_SRV_SET_RATE,
> +					  &tx, sizeof(tx), &rx, sizeof(rx));
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret)
> +		return ret;
> +	if (rx.status)
> +		return rpmi_to_linux_error(rx.status);
> +
> +	return 0;
> +}
> +
> +static int rpmi_clk_enable(struct clk_hw *hw) {
> +	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +	struct rpmi_clk_context *context = rpmi_clk->context;
> +	struct rpmi_mbox_message msg;
> +	struct rpmi_set_config_tx tx;
> +	struct rpmi_set_config_rx rx;
> +	int ret;
> +
> +	tx.config = cpu_to_le32(RPMI_CLK_ENABLE);
> +	tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +	rpmi_mbox_init_send_with_response(&msg,
> RPMI_CLK_SRV_SET_CONFIG,
> +					  &tx, sizeof(tx), &rx, sizeof(rx));
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret)
> +		return ret;
> +	if (rx.status)
> +		return rpmi_to_linux_error(rx.status);
> +
> +	return 0;
> +}
> +
> +static void rpmi_clk_disable(struct clk_hw *hw) {
> +	struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> +	struct rpmi_clk_context *context = rpmi_clk->context;
> +	struct rpmi_mbox_message msg;
> +	struct rpmi_set_config_tx tx;
> +	struct rpmi_set_config_rx rx;
> +	int ret;
> +
> +	tx.config = cpu_to_le32(RPMI_CLK_DISABLE);
> +	tx.clkid = cpu_to_le32(rpmi_clk->id);
> +
> +	rpmi_mbox_init_send_with_response(&msg,
> RPMI_CLK_SRV_SET_CONFIG,
> +					  &tx, sizeof(tx), &rx, sizeof(rx));
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret || rx.status)
> +		pr_err("Failed to disable clk-%u\n", rpmi_clk->id); }
> +
> +static const struct clk_ops rpmi_clk_ops = {
> +	.recalc_rate = rpmi_clk_recalc_rate,
> +	.round_rate = rpmi_clk_round_rate,
> +	.set_rate = rpmi_clk_set_rate,
> +	.prepare = rpmi_clk_enable,
> +	.unprepare = rpmi_clk_disable,
> +};
> +
> +static struct clk_hw *rpmi_clk_enumerate(struct rpmi_clk_context
> +*context, u32 clkid) {
> +	struct device *dev = context->dev;
> +	unsigned long min_rate, max_rate;
> +	union rpmi_clk_rates *rates;
> +	struct rpmi_clk *rpmi_clk;
> +	struct clk_init_data init;
> +	struct clk_hw *clk_hw;
> +	int ret;
> +
> +	rates = devm_kzalloc(dev, sizeof(union rpmi_clk_rates), GFP_KERNEL);
> +	if (!rates)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rpmi_clk = devm_kzalloc(dev, sizeof(struct rpmi_clk), GFP_KERNEL);
> +	if (!rpmi_clk)
> +		return ERR_PTR(-ENOMEM);
> +	rpmi_clk->context = context;
> +	rpmi_clk->rates = rates;
> +
> +	ret = rpmi_clk_get_attrs(clkid, rpmi_clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to get clk-%u attributes\n", clkid);
> +		return ERR_PTR(ret);
> +	}
> +
> +	ret = rpmi_clk_get_supported_rates(clkid, rpmi_clk);
> +	if (ret) {
> +		dev_err(dev, "Get supported rates failed for clk-%u, %d\n",
> +			clkid, ret);
> +		return ERR_PTR(ret);
> +	}
> +
> +	init.flags = CLK_GET_RATE_NOCACHE;
> +	init.num_parents = 0;
> +	init.ops = &rpmi_clk_ops;
> +	init.name = rpmi_clk->name;
> +	clk_hw = &rpmi_clk->hw;
> +	clk_hw->init = &init;
> +
> +	ret = devm_clk_hw_register(dev, clk_hw);
> +	if (ret) {
> +		dev_err(dev, "Unable to register clk-%u\n", clkid);
> +		return ERR_PTR(ret);
> +	}
> +
> +	if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> +		min_rate = rpmi_clk->rates->discrete[0];
> +		max_rate = rpmi_clk->rates->discrete[rpmi_clk->num_rates -
> 1];
> +	} else {
> +		min_rate = rpmi_clk->rates->linear.min;
> +		max_rate = rpmi_clk->rates->linear.max;
> +	}
> +
> +	clk_hw_set_rate_range(clk_hw, min_rate, max_rate);
> +
> +	return NULL;
> +}
> +
> +static void rpmi_clk_receive_message(struct mbox_client *cl, void *msg)
> +{
> +	/* Nothing to do here. */
> +}
> +
> +static int rpmi_clk_probe(struct platform_device *pdev) {
> +	struct device *dev = &pdev->dev;
> +	struct clk_hw_onecell_data *clk_data;
> +	struct rpmi_clk_context *context;
> +	struct rpmi_mbox_message msg;
> +	int ret, num_clocks, i;
> +	struct clk_hw *hw_ptr;
> +
> +	/* Allocate RPMI clock context */
> +	context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> +	if (!context)
> +		return -ENOMEM;
> +	context->dev = dev;
> +	platform_set_drvdata(pdev, context);
> +
> +	/* Setup mailbox client */
> +	context->client.dev		= context->dev;
> +	context->client.rx_callback	= rpmi_clk_receive_message;
> +	context->client.tx_block	= false;
> +	context->client.knows_txdone	= true;
> +	context->client.tx_tout		= 0;
> +
> +	/* Request mailbox channel */
> +	context->chan = mbox_request_channel(&context->client, 0);
> +	if (IS_ERR(context->chan))
> +		return PTR_ERR(context->chan);
> +
> +	/* Validate RPMI specification version */
> +	rpmi_mbox_init_get_attribute(&msg,
> RPMI_MBOX_ATTR_SPEC_VERSION);
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret) {
> +		dev_err(dev, "Failed to get spec version\n");
> +		goto fail_free_channel;
> +	}
> +	if (msg.attr.value < RPMI_MKVER(1, 0)) {
> +		dev_err(dev,
> +			"msg protocol version mismatch, expected 0x%x,
> found 0x%x\n",
> +			RPMI_MKVER(1, 0), msg.attr.value);
> +		ret = -EINVAL;
> +		goto fail_free_channel;
> +	}
> +
> +	/* Validate clock service group ID */
> +	rpmi_mbox_init_get_attribute(&msg,
> RPMI_MBOX_ATTR_SERVICEGROUP_ID);
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret) {
> +		dev_err(dev, "Failed to get service group ID\n");
> +		goto fail_free_channel;
> +	}
> +	if (msg.attr.value != RPMI_SRVGRP_CLOCK) {
> +		dev_err(dev,
> +			"service group match failed, expected 0x%x, found
> 0x%x\n",
> +			RPMI_SRVGRP_CLOCK, msg.attr.value);
> +		ret = -EINVAL;
> +		goto fail_free_channel;
> +	}
> +
> +	/* Validate clock service group version */
> +	rpmi_mbox_init_get_attribute(&msg,
> RPMI_MBOX_ATTR_SERVICEGROUP_VERSION);
> +	ret = rpmi_mbox_send_message(context->chan, &msg);
> +	if (ret) {
> +		dev_err(dev, "Failed to get service group version\n");
> +		goto fail_free_channel;
> +	}
> +	if (msg.attr.value < RPMI_MKVER(1, 0)) {
> +		dev_err(dev,
> +			"service group version failed, expected 0x%x, found
> 0x%x\n",
> +			RPMI_MKVER(1, 0), msg.attr.value);
> +		ret = -EINVAL;
> +		goto fail_free_channel;
> +	}
> +
> +	/* Find-out number of clocks */
> +	num_clocks = rpmi_clk_get_num_clocks(context);
> +	if (!num_clocks) {
> +		dev_err(dev, "No clocks found\n");
> +		ret = -ENODEV;
> +		goto fail_free_channel;
> +	}
> +
> +	/* Allocate clock data */
> +	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clocks),
> +				GFP_KERNEL);
> +	if (!clk_data) {
> +		ret = -ENOMEM;
> +		goto fail_free_channel;
> +	}
> +	clk_data->num = num_clocks;
> +
> +	/* Setup clock data */
> +	for (i = 0; i < clk_data->num; i++) {
> +		hw_ptr = rpmi_clk_enumerate(context, i);
> +		if (IS_ERR(hw_ptr)) {
> +			dev_err(dev, "failed to register clk-%d\n", i);
> +			ret = PTR_ERR(hw_ptr);
> +			goto fail_free_channel;
> +		}
> +		clk_data->hws[i] = hw_ptr;
> +	}
> +
> +	/* Register clock HW provider */
> +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> clk_data);
> +	if (ret) {
> +		dev_err(dev, "failed to register clock HW provider\n");
> +		goto fail_free_channel;
> +	}
> +
> +	dev_info(dev, "clk HW provider registered with %d clocks\n",
> +		 num_clocks);
> +	return 0;
> +
> +fail_free_channel:
> +	mbox_free_channel(context->chan);
> +	return ret;
> +}
> +
> +static void rpmi_clk_remove(struct platform_device *pdev) {
> +	struct rpmi_clk_context *context = platform_get_drvdata(pdev);
> +
> +	mbox_free_channel(context->chan);
> +}
> +
> +static const struct of_device_id rpmi_clk_of_match[] = {
> +	{ .compatible = "riscv,rpmi-clock" },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, rpmi_clk_of_match);
> +
> +static struct platform_driver rpmi_clk_driver = {
> +	.driver = {
> +		.name = "riscv-rpmi-clock",
> +		.of_match_table = rpmi_clk_of_match,
> +	},
> +	.probe = rpmi_clk_probe,
> +	.remove = rpmi_clk_remove,
> +};
> +module_platform_driver(rpmi_clk_driver);
> +
> +MODULE_AUTHOR("Rahul Pathak <rpathak@ventanamicro.com>");
> +MODULE_DESCRIPTION("Clock Driver based on RPMI message protocol");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mailbox/riscv-rpmi-message.h
> b/include/linux/mailbox/riscv-rpmi-message.h
> index 8f4b3a0edbce..4e9478c4c0a3 100644
> --- a/include/linux/mailbox/riscv-rpmi-message.h
> +++ b/include/linux/mailbox/riscv-rpmi-message.h
> @@ -89,6 +89,22 @@ static inline int rpmi_to_linux_error(int rpmi_error)
>  	}
>  }
> 
> +/** RPMI service group IDs */
> +#define RPMI_SRVGRP_CLOCK		0x00007
> +
> +/** RPMI clock service IDs */
> +enum rpmi_clock_service_id {
> +	RPMI_CLK_SRV_ENABLE_NOTIFICATION = 0x01,
> +	RPMI_CLK_SRV_GET_NUM_CLOCKS = 0x02,
> +	RPMI_CLK_SRV_GET_ATTRIBUTES = 0x03,
> +	RPMI_CLK_SRV_GET_SUPPORTED_RATES = 0x04,
> +	RPMI_CLK_SRV_SET_CONFIG = 0x05,
> +	RPMI_CLK_SRV_GET_CONFIG = 0x06,
> +	RPMI_CLK_SRV_SET_RATE = 0x07,
> +	RPMI_CLK_SRV_GET_RATE = 0x08,
> +	RPMI_CLK_SRV_ID_MAX_COUNT,
> +};
> +
>  /** RPMI linux mailbox attribute IDs */  enum rpmi_mbox_attribute_id {
>  	RPMI_MBOX_ATTR_SPEC_VERSION = 0,
> --
> 2.43.0

Re: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock service group

[Rahul Pathak]

Hi Leyfoon,

On Tue, Dec 24, 2024 at 11:50 AM Leyfoon Tan
<leyfoon.tan@starfivetech.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Anup Patel <apatel@ventanamicro.com>
> > Sent: Monday, December 16, 2024 4:48 PM
> > To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> > <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jassi Brar
> > <jassisinghbrar@gmail.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>; Paul Walmsley
> > <paul.walmsley@sifive.com>; Sunil V L <sunilvl@ventanamicro.com>; Rahul
> > Pathak <rpathak@ventanamicro.com>; Leyfoon Tan
> > <leyfoon.tan@starfivetech.com>; Atish Patra <atishp@atishpatra.org>;
> > Andrew Jones <ajones@ventanamicro.com>; Anup Patel
> > <anup@brainfault.org>; linux-clk@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-riscv@lists.infradead.org; linux-
> > kernel@vger.kernel.org; Anup Patel <apatel@ventanamicro.com>
> > Subject: [RFC PATCH 8/8] clk: Add clock driver for the RISC-V RPMI clock
> > service group
> >
> > From: Rahul Pathak <rpathak@ventanamicro.com>
> >
> > The RPMI specification defines a clock service group which can be accessed via
> > SBI MPXY extension or dedicated S-mode RPMI transport.
> >
> > Add mailbox client based clock driver for the RISC-V RPMI clock service group.
> >
> > Co-developed-by: Anup Patel <apatel@ventanamicro.com>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> > ---
>
>
> >  obj-y                                        += actions/
> > diff --git a/drivers/clk/clk-rpmi.c b/drivers/clk/clk-rpmi.c new file mode
> > 100644 index 000000000000..ed8e32527d3d
> > --- /dev/null
> > +++ b/drivers/clk/clk-rpmi.c
> > @@ -0,0 +1,588 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RISC-V MPXY Based Clock Driver
> > + *
> > + * Copyright (C) 2024 Ventana Micro Systems Ltd.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/mm.h>
> > +#include <linux/of.h>
> > +#include <linux/err.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/mailbox/riscv-rpmi-message.h>
> > +#include <linux/mailbox_client.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/clk-provider.h>
>
> Sorting header files in alphabetical order.

Already taken care of. also removed which are not needed.
>
> > +
> > +#define RPMI_CLK_MAX_NUM_RATES               16
> This macro only used for discrete clocks, so suggest change to RPMI_CLK_MAX_DISCRETE_NUM_RATES.
> Is 16 too few?

I will change the name. We can keep 16 for now and
may change later if we find this insufficient.

>
> > +#define RPMI_CLK_NAME_LEN            16
> > +
> > +#define GET_RATE_LO_U32(rate_u64)    ((u32)rate_u64)
> > +#define GET_RATE_HI_U32(rate_u64)    ((u32)((u64)(rate_u64) >> 32))
> > +#define GET_RATE_U64(hi_u32, lo_u32) ((u64)(hi_u32) << 32 |
> > (lo_u32))
> > +
> > +enum rpmi_clk_config {
> > +     RPMI_CLK_DISABLE = 0,
> > +     RPMI_CLK_ENABLE = 1,
> > +};
> > +
>
> [...]
>
> > +static int rpmi_clk_get_supported_rates(u32 clkid, struct rpmi_clk
> > +*rpmi_clk) {
> > +     struct rpmi_clk_context *context = rpmi_clk->context;
> > +     struct rpmi_get_supp_rates_tx tx;
> > +     struct rpmi_get_supp_rates_rx rx;
> > +     struct rpmi_mbox_message msg;
> > +     size_t clk_rate_idx = 0;
> > +     int ret, rateidx, j;
> > +
> > +     tx.clkid = cpu_to_le32(clkid);
> > +     tx.clk_rate_idx = 0;
> > +
> > +     rpmi_mbox_init_send_with_response(&msg,
> > RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> > +                                       &tx, sizeof(tx), &rx, sizeof(rx));
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret)
> > +             return ret;
> > +     if (rx.status)
> > +             return rpmi_to_linux_error(rx.status);
> > +     if (!rx.returned)
> > +             return -EINVAL;
> > +
> > +     if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> > +             for (rateidx = 0; rateidx < rx.returned; rateidx++) {
>
> Need to check RPMI_CLK_MAX_NUM_RATES limit as well.

Sure, i will update

>
> > +                     rpmi_clk->rates->discrete[rateidx] =
> > +
> >       GET_RATE_U64(rx.rates.discrete[rateidx].hi,
> > +
> > rx.rates.discrete[rateidx].lo);
> > +             }
> > +
> > +             while (rx.remaining) {
> > +                     clk_rate_idx += rx.returned;
> > +                     tx.clk_rate_idx = clk_rate_idx;
> > +
> > +                     rpmi_mbox_init_send_with_response(&msg,
> > +
> > RPMI_CLK_SRV_GET_SUPPORTED_RATES,
> > +                                                       &tx, sizeof(tx), &rx,
> > sizeof(rx));
> > +                     ret = rpmi_mbox_send_message(context->chan,
> > &msg);
> > +                     if (ret)
> > +                             return ret;
>
> Need check if (rx.status) here?

Sure, i will update

>
> > +
> > +                     for (j = 0; j < rx.returned; j++) {
> Same here, check RPMI_CLK_MAX_NUM_RATES.

Sure, i will update

>
> > +                             if (rateidx >= (clk_rate_idx + rx.returned))
> > +                                     break;
> > +                             rpmi_clk->rates->discrete[rateidx++] =
> > +                                     GET_RATE_U64(rx.rates.discrete[j].hi,
> > +                                                  rx.rates.discrete[j].lo);
> > +                     }
> > +             }
> > +     } else if (rpmi_clk->type == RPMI_CLK_LINEAR) {
> > +             rpmi_clk->rates->linear.min =
> > +                             GET_RATE_U64(rx.rates.linear.min_hi,
> > +                                          rx.rates.linear.min_lo);
> > +             rpmi_clk->rates->linear.max =
> > +                             GET_RATE_U64(rx.rates.linear.max_hi,
> > +                                          rx.rates.linear.max_lo);
> > +             rpmi_clk->rates->linear.step =
> > +                             GET_RATE_U64(rx.rates.linear.step_hi,
> > +                                          rx.rates.linear.step_lo);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static unsigned long rpmi_clk_recalc_rate(struct clk_hw *hw,
> > +                                       unsigned long parent_rate)
> > +{
> > +     struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +     struct rpmi_clk_context *context = rpmi_clk->context;
> > +     struct rpmi_mbox_message msg;
> > +     struct rpmi_get_rate_tx tx;
> > +     struct rpmi_get_rate_rx rx;
> > +     int ret;
> > +
> > +     tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +     rpmi_mbox_init_send_with_response(&msg,
> > RPMI_CLK_SRV_GET_RATE,
> > +                                       &tx, sizeof(tx), &rx, sizeof(rx));
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret)
> > +             return ret;
> > +     if (rx.status)
> > +             return rx.status;
> > +
> > +     return GET_RATE_U64(rx.hi, rx.lo);
> > +}
> > +
> > +static long rpmi_clk_round_rate(struct clk_hw *hw,
> > +                             unsigned long rate,
> > +                             unsigned long *parent_rate)
> > +{
> > +     struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +     u64 fmin, fmax, ftmp;
> > +
> > +     if (rpmi_clk->type == RPMI_CLK_DISCRETE)
> > +             return rate;
> > +
> > +     fmin = rpmi_clk->rates->linear.min;
> > +     fmax = rpmi_clk->rates->linear.max;
> > +
> > +     if (rate <= fmin)
> > +             return fmin;
> > +     else if (rate >=  fmax)
> > +             return fmax;
> > +
> > +     ftmp = rate - fmin;
> > +     ftmp += rpmi_clk->rates->linear.step - 1;
> > +     do_div(ftmp, rpmi_clk->rates->linear.step);
> > +
> > +     return ftmp * rpmi_clk->rates->linear.step + fmin; }
> > +
> > +static int rpmi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> > +                          unsigned long parent_rate)
> > +{
> > +     struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +     struct rpmi_clk_context *context = rpmi_clk->context;
> > +     struct rpmi_mbox_message msg;
> > +     struct rpmi_set_rate_tx tx;
> > +     struct rpmi_set_rate_rx rx;
> > +     int ret;
> > +
> > +     tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +     tx.lo = cpu_to_le32(GET_RATE_LO_U32(rate));
> > +     tx.hi = cpu_to_le32(GET_RATE_HI_U32(rate));
> > +
> > +     rpmi_mbox_init_send_with_response(&msg,
> > RPMI_CLK_SRV_SET_RATE,
> > +                                       &tx, sizeof(tx), &rx, sizeof(rx));
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret)
> > +             return ret;
> > +     if (rx.status)
> > +             return rpmi_to_linux_error(rx.status);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rpmi_clk_enable(struct clk_hw *hw) {
> > +     struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +     struct rpmi_clk_context *context = rpmi_clk->context;
> > +     struct rpmi_mbox_message msg;
> > +     struct rpmi_set_config_tx tx;
> > +     struct rpmi_set_config_rx rx;
> > +     int ret;
> > +
> > +     tx.config = cpu_to_le32(RPMI_CLK_ENABLE);
> > +     tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +     rpmi_mbox_init_send_with_response(&msg,
> > RPMI_CLK_SRV_SET_CONFIG,
> > +                                       &tx, sizeof(tx), &rx, sizeof(rx));
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret)
> > +             return ret;
> > +     if (rx.status)
> > +             return rpmi_to_linux_error(rx.status);
> > +
> > +     return 0;
> > +}
> > +
> > +static void rpmi_clk_disable(struct clk_hw *hw) {
> > +     struct rpmi_clk *rpmi_clk = to_rpmi_clk(hw);
> > +     struct rpmi_clk_context *context = rpmi_clk->context;
> > +     struct rpmi_mbox_message msg;
> > +     struct rpmi_set_config_tx tx;
> > +     struct rpmi_set_config_rx rx;
> > +     int ret;
> > +
> > +     tx.config = cpu_to_le32(RPMI_CLK_DISABLE);
> > +     tx.clkid = cpu_to_le32(rpmi_clk->id);
> > +
> > +     rpmi_mbox_init_send_with_response(&msg,
> > RPMI_CLK_SRV_SET_CONFIG,
> > +                                       &tx, sizeof(tx), &rx, sizeof(rx));
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret || rx.status)
> > +             pr_err("Failed to disable clk-%u\n", rpmi_clk->id); }
> > +
> > +static const struct clk_ops rpmi_clk_ops = {
> > +     .recalc_rate = rpmi_clk_recalc_rate,
> > +     .round_rate = rpmi_clk_round_rate,
> > +     .set_rate = rpmi_clk_set_rate,
> > +     .prepare = rpmi_clk_enable,
> > +     .unprepare = rpmi_clk_disable,
> > +};
> > +
> > +static struct clk_hw *rpmi_clk_enumerate(struct rpmi_clk_context
> > +*context, u32 clkid) {
> > +     struct device *dev = context->dev;
> > +     unsigned long min_rate, max_rate;
> > +     union rpmi_clk_rates *rates;
> > +     struct rpmi_clk *rpmi_clk;
> > +     struct clk_init_data init;
> > +     struct clk_hw *clk_hw;
> > +     int ret;
> > +
> > +     rates = devm_kzalloc(dev, sizeof(union rpmi_clk_rates), GFP_KERNEL);
> > +     if (!rates)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     rpmi_clk = devm_kzalloc(dev, sizeof(struct rpmi_clk), GFP_KERNEL);
> > +     if (!rpmi_clk)
> > +             return ERR_PTR(-ENOMEM);
> > +     rpmi_clk->context = context;
> > +     rpmi_clk->rates = rates;
> > +
> > +     ret = rpmi_clk_get_attrs(clkid, rpmi_clk);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to get clk-%u attributes\n", clkid);
> > +             return ERR_PTR(ret);
> > +     }
> > +
> > +     ret = rpmi_clk_get_supported_rates(clkid, rpmi_clk);
> > +     if (ret) {
> > +             dev_err(dev, "Get supported rates failed for clk-%u, %d\n",
> > +                     clkid, ret);
> > +             return ERR_PTR(ret);
> > +     }
> > +
> > +     init.flags = CLK_GET_RATE_NOCACHE;
> > +     init.num_parents = 0;
> > +     init.ops = &rpmi_clk_ops;
> > +     init.name = rpmi_clk->name;
> > +     clk_hw = &rpmi_clk->hw;
> > +     clk_hw->init = &init;
> > +
> > +     ret = devm_clk_hw_register(dev, clk_hw);
> > +     if (ret) {
> > +             dev_err(dev, "Unable to register clk-%u\n", clkid);
> > +             return ERR_PTR(ret);
> > +     }
> > +
> > +     if (rpmi_clk->type == RPMI_CLK_DISCRETE) {
> > +             min_rate = rpmi_clk->rates->discrete[0];
> > +             max_rate = rpmi_clk->rates->discrete[rpmi_clk->num_rates -
> > 1];
> > +     } else {
> > +             min_rate = rpmi_clk->rates->linear.min;
> > +             max_rate = rpmi_clk->rates->linear.max;
> > +     }
> > +
> > +     clk_hw_set_rate_range(clk_hw, min_rate, max_rate);
> > +
> > +     return NULL;
> > +}
> > +
> > +static void rpmi_clk_receive_message(struct mbox_client *cl, void *msg)
> > +{
> > +     /* Nothing to do here. */
> > +}
> > +
> > +static int rpmi_clk_probe(struct platform_device *pdev) {
> > +     struct device *dev = &pdev->dev;
> > +     struct clk_hw_onecell_data *clk_data;
> > +     struct rpmi_clk_context *context;
> > +     struct rpmi_mbox_message msg;
> > +     int ret, num_clocks, i;
> > +     struct clk_hw *hw_ptr;
> > +
> > +     /* Allocate RPMI clock context */
> > +     context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> > +     if (!context)
> > +             return -ENOMEM;
> > +     context->dev = dev;
> > +     platform_set_drvdata(pdev, context);
> > +
> > +     /* Setup mailbox client */
> > +     context->client.dev             = context->dev;
> > +     context->client.rx_callback     = rpmi_clk_receive_message;
> > +     context->client.tx_block        = false;
> > +     context->client.knows_txdone    = true;
> > +     context->client.tx_tout         = 0;
> > +
> > +     /* Request mailbox channel */
> > +     context->chan = mbox_request_channel(&context->client, 0);
> > +     if (IS_ERR(context->chan))
> > +             return PTR_ERR(context->chan);
> > +
> > +     /* Validate RPMI specification version */
> > +     rpmi_mbox_init_get_attribute(&msg,
> > RPMI_MBOX_ATTR_SPEC_VERSION);
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to get spec version\n");
> > +             goto fail_free_channel;
> > +     }
> > +     if (msg.attr.value < RPMI_MKVER(1, 0)) {
> > +             dev_err(dev,
> > +                     "msg protocol version mismatch, expected 0x%x,
> > found 0x%x\n",
> > +                     RPMI_MKVER(1, 0), msg.attr.value);
> > +             ret = -EINVAL;
> > +             goto fail_free_channel;
> > +     }
> > +
> > +     /* Validate clock service group ID */
> > +     rpmi_mbox_init_get_attribute(&msg,
> > RPMI_MBOX_ATTR_SERVICEGROUP_ID);
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to get service group ID\n");
> > +             goto fail_free_channel;
> > +     }
> > +     if (msg.attr.value != RPMI_SRVGRP_CLOCK) {
> > +             dev_err(dev,
> > +                     "service group match failed, expected 0x%x, found
> > 0x%x\n",
> > +                     RPMI_SRVGRP_CLOCK, msg.attr.value);
> > +             ret = -EINVAL;
> > +             goto fail_free_channel;
> > +     }
> > +
> > +     /* Validate clock service group version */
> > +     rpmi_mbox_init_get_attribute(&msg,
> > RPMI_MBOX_ATTR_SERVICEGROUP_VERSION);
> > +     ret = rpmi_mbox_send_message(context->chan, &msg);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to get service group version\n");
> > +             goto fail_free_channel;
> > +     }
> > +     if (msg.attr.value < RPMI_MKVER(1, 0)) {
> > +             dev_err(dev,
> > +                     "service group version failed, expected 0x%x, found
> > 0x%x\n",
> > +                     RPMI_MKVER(1, 0), msg.attr.value);
> > +             ret = -EINVAL;
> > +             goto fail_free_channel;
> > +     }
> > +
> > +     /* Find-out number of clocks */
> > +     num_clocks = rpmi_clk_get_num_clocks(context);
> > +     if (!num_clocks) {
> > +             dev_err(dev, "No clocks found\n");
> > +             ret = -ENODEV;
> > +             goto fail_free_channel;
> > +     }
> > +
> > +     /* Allocate clock data */
> > +     clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, num_clocks),
> > +                             GFP_KERNEL);
> > +     if (!clk_data) {
> > +             ret = -ENOMEM;
> > +             goto fail_free_channel;
> > +     }
> > +     clk_data->num = num_clocks;
> > +
> > +     /* Setup clock data */
> > +     for (i = 0; i < clk_data->num; i++) {
> > +             hw_ptr = rpmi_clk_enumerate(context, i);
> > +             if (IS_ERR(hw_ptr)) {
> > +                     dev_err(dev, "failed to register clk-%d\n", i);
> > +                     ret = PTR_ERR(hw_ptr);
> > +                     goto fail_free_channel;
> > +             }
> > +             clk_data->hws[i] = hw_ptr;
> > +     }
> > +
> > +     /* Register clock HW provider */
> > +     ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> > clk_data);
> > +     if (ret) {
> > +             dev_err(dev, "failed to register clock HW provider\n");
> > +             goto fail_free_channel;
> > +     }
> > +
> > +     dev_info(dev, "clk HW provider registered with %d clocks\n",
> > +              num_clocks);
> > +     return 0;
> > +
> > +fail_free_channel:
> > +     mbox_free_channel(context->chan);
> > +     return ret;
> > +}
> > +
> > +static void rpmi_clk_remove(struct platform_device *pdev) {
> > +     struct rpmi_clk_context *context = platform_get_drvdata(pdev);
> > +
> > +     mbox_free_channel(context->chan);
> > +}
> > +
> > +static const struct of_device_id rpmi_clk_of_match[] = {
> > +     { .compatible = "riscv,rpmi-clock" },
> > +     { },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, rpmi_clk_of_match);
> > +
> > +static struct platform_driver rpmi_clk_driver = {
> > +     .driver = {
> > +             .name = "riscv-rpmi-clock",
> > +             .of_match_table = rpmi_clk_of_match,
> > +     },
> > +     .probe = rpmi_clk_probe,
> > +     .remove = rpmi_clk_remove,
> > +};
> > +module_platform_driver(rpmi_clk_driver);
> > +
> > +MODULE_AUTHOR("Rahul Pathak <rpathak@ventanamicro.com>");
> > +MODULE_DESCRIPTION("Clock Driver based on RPMI message protocol");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mailbox/riscv-rpmi-message.h
> > b/include/linux/mailbox/riscv-rpmi-message.h
> > index 8f4b3a0edbce..4e9478c4c0a3 100644
> > --- a/include/linux/mailbox/riscv-rpmi-message.h
> > +++ b/include/linux/mailbox/riscv-rpmi-message.h
> > @@ -89,6 +89,22 @@ static inline int rpmi_to_linux_error(int rpmi_error)
> >       }
> >  }
> >
> > +/** RPMI service group IDs */
> > +#define RPMI_SRVGRP_CLOCK            0x00007
> > +
> > +/** RPMI clock service IDs */
> > +enum rpmi_clock_service_id {
> > +     RPMI_CLK_SRV_ENABLE_NOTIFICATION = 0x01,
> > +     RPMI_CLK_SRV_GET_NUM_CLOCKS = 0x02,
> > +     RPMI_CLK_SRV_GET_ATTRIBUTES = 0x03,
> > +     RPMI_CLK_SRV_GET_SUPPORTED_RATES = 0x04,
> > +     RPMI_CLK_SRV_SET_CONFIG = 0x05,
> > +     RPMI_CLK_SRV_GET_CONFIG = 0x06,
> > +     RPMI_CLK_SRV_SET_RATE = 0x07,
> > +     RPMI_CLK_SRV_GET_RATE = 0x08,
> > +     RPMI_CLK_SRV_ID_MAX_COUNT,
> > +};
> > +
> >  /** RPMI linux mailbox attribute IDs */  enum rpmi_mbox_attribute_id {
> >       RPMI_MBOX_ATTR_SPEC_VERSION = 0,
> > --
> > 2.43.0
>