[PATCH 1/2] devicetree: bindings: mux: reg-mux: Add support for new property 'mux-reg-masks-state'

Chintan Vankar posted 2 patches 6 months, 2 weeks ago
[PATCH 1/2] devicetree: bindings: mux: reg-mux: Add support for new property 'mux-reg-masks-state'
Posted by Chintan Vankar 6 months, 2 weeks ago
The DT binding for reg-mux currenly requires specifying register offset
and masks in the "mux-reg-masks" property, while corresponding register
values are defined in the "idle-states" property. This approach imposes a
constraint where "mux-reg-masks" and "idle-states" must remain
synchroniszed, adding complexity when configuring specific registers or a
set of registers with large memory spaces.

Add support of a new property "mux-reg-masks-state" to remove this
constraint, allowing offset, mask and value to be specified together as a
tuple.

Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---
 .../devicetree/bindings/mux/reg-mux.yaml      | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mux/reg-mux.yaml b/Documentation/devicetree/bindings/mux/reg-mux.yaml
index dc4be092fc2f..b029e2f28df0 100644
--- a/Documentation/devicetree/bindings/mux/reg-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/reg-mux.yaml
@@ -32,11 +32,39 @@ properties:
         - description: pre-shifted bitfield mask
     description: Each entry pair describes a single mux control.
 
-  idle-states: true
+  idle-states:
+    description: Each entry describes mux register state.
+
+  mux-reg-masks-state:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    items:
+      items:
+        - description: register offset
+        - description: pre-shifted bitfield mask
+        - description: register value to be set
+    description: This property is an extension of mux-reg-masks which
+                 allows specifying register offset, mask and register
+                 value to be set.
+
+allOf:
+  - oneOf:
+      - required: [mux-reg-masks]
+      - required: [mux-reg-masks-state]
+
+  - if:
+      required: [mux-reg-masks-state]
+    then:
+      not:
+        required: [idle-states]
+
+  - if:
+      required: [mux-reg-masks]
+    then:
+      properties:
+        idle-states: true
 
 required:
   - compatible
-  - mux-reg-masks
   - '#mux-control-cells'
 
 additionalProperties: false
-- 
2.34.1
Re: [PATCH 1/2] devicetree: bindings: mux: reg-mux: Add support for new property 'mux-reg-masks-state'
Posted by Rob Herring 5 months, 3 weeks ago
On Thu, Jun 05, 2025 at 12:04:21PM +0530, Chintan Vankar wrote:
> The DT binding for reg-mux currenly requires specifying register offset
> and masks in the "mux-reg-masks" property, while corresponding register
> values are defined in the "idle-states" property. This approach imposes a
> constraint where "mux-reg-masks" and "idle-states" must remain
> synchroniszed, adding complexity when configuring specific registers or a
> set of registers with large memory spaces.

Sorry, but I don't follow why there's complexity. Having to support 2 
different ways to express the same thing adds complexity we have to 
maintain forever. I prefer to impose the complexity on the .dts than 
maintainers.

Rob
Re: [PATCH 1/2] devicetree: bindings: mux: reg-mux: Add support for new property 'mux-reg-masks-state'
Posted by Krzysztof Kozlowski 6 months, 2 weeks ago
On 05/06/2025 08:34, Chintan Vankar wrote:
> The DT binding for reg-mux currenly requires specifying register offset
> and masks in the "mux-reg-masks" property, while corresponding register
> values are defined in the "idle-states" property. This approach imposes a
> constraint where "mux-reg-masks" and "idle-states" must remain
> synchroniszed, adding complexity when configuring specific registers or a

Typo

> set of registers with large memory spaces.
> 
> Add support of a new property "mux-reg-masks-state" to remove this
> constraint, allowing offset, mask and value to be specified together as a
> tuple.
> 

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters


> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
>  .../devicetree/bindings/mux/reg-mux.yaml      | 32 +++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mux/reg-mux.yaml b/Documentation/devicetree/bindings/mux/reg-mux.yaml
> index dc4be092fc2f..b029e2f28df0 100644
> --- a/Documentation/devicetree/bindings/mux/reg-mux.yaml
> +++ b/Documentation/devicetree/bindings/mux/reg-mux.yaml
> @@ -32,11 +32,39 @@ properties:
>          - description: pre-shifted bitfield mask
>      description: Each entry pair describes a single mux control.
>  
> -  idle-states: true
> +  idle-states:
> +    description: Each entry describes mux register state.
> +
> +  mux-reg-masks-state:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    items:
> +      items:
> +        - description: register offset
> +        - description: pre-shifted bitfield mask
> +        - description: register value to be set
> +    description: This property is an extension of mux-reg-masks which

How one is supposed to use both (since this is extension)?

> +                 allows specifying register offset, mask and register
> +                 value to be set.

Anyway, this is getting too complex (next will be
mux-reg-masks-enable-state and mux-reg-masks-foo-state), just define
your own device schema.

> +
> +allOf:
> +  - oneOf:
> +      - required: [mux-reg-masks]
> +      - required: [mux-reg-masks-state]

So not an extension but replacement?

> +
> +  - if:
> +      required: [mux-reg-masks-state]
> +    then:
> +      not:
> +        required: [idle-states]

You cannot un-require a field. Where was it made required?

> +
> +  - if:
> +      required: [mux-reg-masks]
> +    then:
> +      properties:
> +        idle-states: true

This is no-op. I have no clue what you wanted to express here.


>  
>  required:
>    - compatible
> -  - mux-reg-masks
>    - '#mux-control-cells'
>  
>  additionalProperties: false


Best regards,
Krzysztof