Re: [PATCH 2/3] dt-bindings: spmi: Add SmartDV SPMI controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On Tue, Nov 03, 2026 at 09:56:06AM +0530, vamshi gajjela wrote:
> From: Vamshi Gajjela <vamshigajjela@google.com>
> 
> Document the device tree bindings for the SmartDV IP Solutions MIPI SPMI
> master controller ("smartdv,spmi").
> 
> This controller IP is integrated in Google Tensor SoCs starting with the
> G5 generation (codename: laguna, canonically written as "lga"), so add
> "google,lga-spmi" with a fallback to "smartdv,spmi".
> 
> Signed-off-by: Vamshi Gajjela <vamshigajjela@google.com>
> ---
>  .../bindings/spmi/smartdv,spmi.yaml           | 126 ++++++++++++++++++
>  MAINTAINERS                                   |   6 +
>  2 files changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spmi/smartdv,spmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spmi/smartdv,spmi.yaml b/Documentation/devicetree/bindings/spmi/smartdv,spmi.yaml
> new file mode 100644
> index 000000000000..e0512e60d932
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spmi/smartdv,spmi.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spmi/smartdv,spmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SmartDV IP Solutions SPMI Controller
> +
> +maintainers:
> +  - Vamshi Gajjela <vamshigajjela@google.com>
> +
> +description:
> +  SmartDV IP Solutions MIPI System Power Management Interface (SPMI) master
> +  controller supporting MIPI SPMI Specification versions 1.0 and 2.0.
> +
> +allOf:
> +  - $ref: spmi.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: google,lga-spmi
> +          - const: smartdv,spmi
> +      - const: smartdv,spmi
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description:
> +      Core clock to the controller from which SPMI SCL is derived. Optional if
> +      the controller uses a free-running clock and clock-frequency is specified.

Don't repeat constraints in free form text. Just create proper
constraints.

> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  clock-frequency:
> +    description:
> +      Input core clock frequency in Hz when the controller uses a free-running
> +      clock not described in clocks.

Why clock would not be described in clocks? From where does it come?

> +
> +  smartdv,master-arb-enable:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +    description:
> +      Enable master bus arbitration phase (0 = disable, 1 = enable).
> +
> +  smartdv,master-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 0
> +    description:
> +      Master ID for the controller on the SPMI bus.

smartdv,bus-id, I guess

> +
> +  smartdv,spmi-version:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +    description:
> +      MIPI SPMI specification version used for transfers (0 for MIPI SPMI v1.0,
> +      1 for MIPI SPMI v2.0).

And why would that be board level configuration?

> +
> +  smartdv,arb-retry-limit:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +    default: 32
> +    description:
> +      Maximum number of times the master retries a command when losing bus
> +      arbitration.
> +
> +  smartdv,prescaler:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +    default: 0
> +    description:
> +      Clock divider value to generate SPMI SCL from the input core clock. Writing
> +      1 divides the system clock by 2; more generally, a non-zero value n
> +      divides the system clock by 2 * n.
> +
> +  smartdv,read-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +    default: 32
> +    description:
> +      Number of SPMI SCL clock cycles to pause during bus turnaround on read
> +      commands to allow the target device to fetch data.
> +
> +  smartdv,arb-scl-delay:

Except Conor's comments, some of the properties here should use proper
units.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 0
> +    description:
> +      Delay between arbitration and Sequence Start Condition (SSC) when the
> +      controller is the Bus Owner Master (BOM).
> +
> +required:
> +  - compatible
> +  - reg

Best regards,
Krzysztof