DT-binding of reg-mux is defined in such a way that one need to provide
register offset and mask in a "mux-reg-masks" property and corresponding
register value in "idle-states" property. This constraint forces to define
these values in such a way that "mux-reg-masks" and "idle-states" must be
in sync with each other. This implementation would be more complex if
specific register or set of registers need to be configured which has
large memory space. Introduce a new property "mux-reg-masks-state" which
allow to specify offset, mask and value as a tuple in a single property.

Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---

Link to v1:
https://lore.kernel.org/r/20250227202206.2551305-2-c-vankar@ti.com/

Changes from v1 to v2:
- Updated dt-bindings for the required conditions as suggested by Conor
  Dooley and Andrew Davis.

 .../devicetree/bindings/mux/reg-mux.yaml      | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mux/reg-mux.yaml b/Documentation/devicetree/bindings/mux/reg-mux.yaml
index dc4be092fc2f..5255e4a06920 100644
--- a/Documentation/devicetree/bindings/mux/reg-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/reg-mux.yaml
@@ -32,12 +32,30 @@ 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.
 
-required:
-  - compatible
-  - mux-reg-masks
-  - '#mux-control-cells'
+  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 in a single property.
+
+allOf:
+  - not:
+      required: [mux-reg-masks, mux-reg-masks-state]
+
+  - if:
+      required:
+        - mux-reg-masks-state
+    then:
+      properties:
+        idle-states: false
 
 additionalProperties: false
 
-- 
2.34.1

On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
> DT-binding of reg-mux is defined in such a way that one need to provide
> register offset and mask in a "mux-reg-masks" property and corresponding
> register value in "idle-states" property. This constraint forces to define
> these values in such a way that "mux-reg-masks" and "idle-states" must be
> in sync with each other. This implementation would be more complex if
> specific register or set of registers need to be configured which has
> large memory space. Introduce a new property "mux-reg-masks-state" which
> allow to specify offset, mask and value as a tuple in a single property.

Maybe in hindsight that would have been better, but having 2 ways to 
specify the same thing that we have to maintain forever is not an 
improvement.

No one is making you use this binding. If you have a large number of 
muxes, then maybe you should use a specific binding.

Rob

Hello Rob,

On 3/4/2025 9:09 PM, Rob Herring wrote:
> On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
>> DT-binding of reg-mux is defined in such a way that one need to provide
>> register offset and mask in a "mux-reg-masks" property and corresponding
>> register value in "idle-states" property. This constraint forces to define
>> these values in such a way that "mux-reg-masks" and "idle-states" must be
>> in sync with each other. This implementation would be more complex if
>> specific register or set of registers need to be configured which has
>> large memory space. Introduce a new property "mux-reg-masks-state" which
>> allow to specify offset, mask and value as a tuple in a single property.
> 
> Maybe in hindsight that would have been better, but having 2 ways to
> specify the same thing that we have to maintain forever is not an
> improvement.
> 
> No one is making you use this binding. If you have a large number of
> muxes, then maybe you should use a specific binding.
> 

Thank you for reviewing the patch. The reason behind choosing mux
subsystem is working and implementation of mmio driver. As we can see
that implementing this new property in mux-controller is almost
identical to mmio driver, and it would make it easier to define and 
extend mux-controller's functionality. If we introduce the new driver
than that would be most likely a clone of mmio driver.

Let me know if implementation would be accepted by adding a new
compatible for it.


Regards,
Chintan.

> Rob

On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>
> Hello Rob,
>
> On 3/4/2025 9:09 PM, Rob Herring wrote:
> > On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
> >> DT-binding of reg-mux is defined in such a way that one need to provide
> >> register offset and mask in a "mux-reg-masks" property and corresponding
> >> register value in "idle-states" property. This constraint forces to define
> >> these values in such a way that "mux-reg-masks" and "idle-states" must be
> >> in sync with each other. This implementation would be more complex if
> >> specific register or set of registers need to be configured which has
> >> large memory space. Introduce a new property "mux-reg-masks-state" which
> >> allow to specify offset, mask and value as a tuple in a single property.
> >
> > Maybe in hindsight that would have been better, but having 2 ways to
> > specify the same thing that we have to maintain forever is not an
> > improvement.
> >
> > No one is making you use this binding. If you have a large number of
> > muxes, then maybe you should use a specific binding.
> >
>
> Thank you for reviewing the patch. The reason behind choosing mux
> subsystem is working and implementation of mmio driver. As we can see
> that implementing this new property in mux-controller is almost
> identical to mmio driver, and it would make it easier to define and
> extend mux-controller's functionality. If we introduce the new driver
> than that would be most likely a clone of mmio driver.

I'm talking about the binding, not the driver. They are independent.
Generic drivers are great. I love them. Generic bindings, not so much.

> Let me know if implementation would be accepted by adding a new
> compatible for it.

Adding a new compatible to the mmio driver? Certainly. That happens
all the time.

I also didn't say don't use this binding as-is. That's fine too.

Rob

Hello Rob,

On 3/5/2025 2:10 AM, Rob Herring wrote:
> On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>>
>> Hello Rob,
>>
>> On 3/4/2025 9:09 PM, Rob Herring wrote:
>>> On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
>>>> DT-binding of reg-mux is defined in such a way that one need to provide
>>>> register offset and mask in a "mux-reg-masks" property and corresponding
>>>> register value in "idle-states" property. This constraint forces to define
>>>> these values in such a way that "mux-reg-masks" and "idle-states" must be
>>>> in sync with each other. This implementation would be more complex if
>>>> specific register or set of registers need to be configured which has
>>>> large memory space. Introduce a new property "mux-reg-masks-state" which
>>>> allow to specify offset, mask and value as a tuple in a single property.
>>>
>>> Maybe in hindsight that would have been better, but having 2 ways to
>>> specify the same thing that we have to maintain forever is not an
>>> improvement.
>>>
>>> No one is making you use this binding. If you have a large number of
>>> muxes, then maybe you should use a specific binding.
>>>
>>
>> Thank you for reviewing the patch. The reason behind choosing mux
>> subsystem is working and implementation of mmio driver. As we can see
>> that implementing this new property in mux-controller is almost
>> identical to mmio driver, and it would make it easier to define and
>> extend mux-controller's functionality. If we introduce the new driver
>> than that would be most likely a clone of mmio driver.
> 
> I'm talking about the binding, not the driver. They are independent.
> Generic drivers are great. I love them. Generic bindings, not so much.
> 
>> Let me know if implementation would be accepted by adding a new
>> compatible for it.
> 
> Adding a new compatible to the mmio driver? Certainly. That happens
> all the time.
> 
> I also didn't say don't use this binding as-is. That's fine too.
> 

Can you please review the following binding:

oneOf:
   - required: [ mux-reg-masks ]
   - required: [ mux-reg-masks-state ]

allOf:
   - if:
       required:
         - mux-reg-masks-state
     then:
       properties:
         idle-states: false

required:
   - compatible
   - '#mux-control-cells'

I think it won't disturb the current bindings and keep backward
compatibility with existing implementation.


Regards,
Chintan.


> Rob

On Wed, Mar 5, 2025 at 3:43 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>
> Hello Rob,
>
> On 3/5/2025 2:10 AM, Rob Herring wrote:
> > On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@ti.com> wrote:
> >>
> >> Hello Rob,
> >>
> >> On 3/4/2025 9:09 PM, Rob Herring wrote:
> >>> On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
> >>>> DT-binding of reg-mux is defined in such a way that one need to provide
> >>>> register offset and mask in a "mux-reg-masks" property and corresponding
> >>>> register value in "idle-states" property. This constraint forces to define
> >>>> these values in such a way that "mux-reg-masks" and "idle-states" must be
> >>>> in sync with each other. This implementation would be more complex if
> >>>> specific register or set of registers need to be configured which has
> >>>> large memory space. Introduce a new property "mux-reg-masks-state" which
> >>>> allow to specify offset, mask and value as a tuple in a single property.
> >>>
> >>> Maybe in hindsight that would have been better, but having 2 ways to
> >>> specify the same thing that we have to maintain forever is not an
> >>> improvement.
> >>>
> >>> No one is making you use this binding. If you have a large number of
> >>> muxes, then maybe you should use a specific binding.
> >>>
> >>
> >> Thank you for reviewing the patch. The reason behind choosing mux
> >> subsystem is working and implementation of mmio driver. As we can see
> >> that implementing this new property in mux-controller is almost
> >> identical to mmio driver, and it would make it easier to define and
> >> extend mux-controller's functionality. If we introduce the new driver
> >> than that would be most likely a clone of mmio driver.
> >
> > I'm talking about the binding, not the driver. They are independent.
> > Generic drivers are great. I love them. Generic bindings, not so much.
> >
> >> Let me know if implementation would be accepted by adding a new
> >> compatible for it.
> >
> > Adding a new compatible to the mmio driver? Certainly. That happens
> > all the time.
> >
> > I also didn't say don't use this binding as-is. That's fine too.
> >
>
> Can you please review the following binding:
>
> oneOf:
>    - required: [ mux-reg-masks ]
>    - required: [ mux-reg-masks-state ]
>
> allOf:
>    - if:
>        required:
>          - mux-reg-masks-state
>      then:
>        properties:
>          idle-states: false
>
> required:
>    - compatible
>    - '#mux-control-cells'
>
> I think it won't disturb the current bindings and keep backward
> compatibility with existing implementation.

Wasn't that the case before? There's nothing really different here.

Rob

Hello Rob,

On 3/6/2025 3:44 AM, Rob Herring wrote:
> On Wed, Mar 5, 2025 at 3:43 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>>
>> Hello Rob,
>>
>> On 3/5/2025 2:10 AM, Rob Herring wrote:
>>> On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>>>>
>>>> Hello Rob,
>>>>
>>>> On 3/4/2025 9:09 PM, Rob Herring wrote:
>>>>> On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
>>>>>> DT-binding of reg-mux is defined in such a way that one need to provide
>>>>>> register offset and mask in a "mux-reg-masks" property and corresponding
>>>>>> register value in "idle-states" property. This constraint forces to define
>>>>>> these values in such a way that "mux-reg-masks" and "idle-states" must be
>>>>>> in sync with each other. This implementation would be more complex if
>>>>>> specific register or set of registers need to be configured which has
>>>>>> large memory space. Introduce a new property "mux-reg-masks-state" which
>>>>>> allow to specify offset, mask and value as a tuple in a single property.
>>>>>
>>>>> Maybe in hindsight that would have been better, but having 2 ways to
>>>>> specify the same thing that we have to maintain forever is not an
>>>>> improvement.
>>>>>
>>>>> No one is making you use this binding. If you have a large number of
>>>>> muxes, then maybe you should use a specific binding.
>>>>>
>>>>
>>>> Thank you for reviewing the patch. The reason behind choosing mux
>>>> subsystem is working and implementation of mmio driver. As we can see
>>>> that implementing this new property in mux-controller is almost
>>>> identical to mmio driver, and it would make it easier to define and
>>>> extend mux-controller's functionality. If we introduce the new driver
>>>> than that would be most likely a clone of mmio driver.
>>>
>>> I'm talking about the binding, not the driver. They are independent.
>>> Generic drivers are great. I love them. Generic bindings, not so much.
>>>
>>>> Let me know if implementation would be accepted by adding a new
>>>> compatible for it.
>>>
>>> Adding a new compatible to the mmio driver? Certainly. That happens
>>> all the time.
>>>
>>> I also didn't say don't use this binding as-is. That's fine too.
>>>
>>
>> Can you please review the following binding:
>>
>> oneOf:
>>     - required: [ mux-reg-masks ]
>>     - required: [ mux-reg-masks-state ]
>>
>> allOf:
>>     - if:
>>         required:
>>           - mux-reg-masks-state
>>       then:
>>         properties:
>>           idle-states: false
>>
>> required:
>>     - compatible
>>     - '#mux-control-cells'
>>
>> I think it won't disturb the current bindings and keep backward
>> compatibility with existing implementation.
> 
> Wasn't that the case before? There's nothing really different here.
> 

No, the binding before was not considering "mux-reg-masks" as required
property which was breaking actual dt-binding. In this binding, one of
the properties from "mux-reg-masks" or "mux-reg-masks-state" is required
which keeps the binding as it is unless someone wants to use
"mux-reg-masks-state".

Regards,
Chintan.

> Rob

Hello Rob,

The reason to introduce new bindings for mux-controller is to make use
of it to implement Timesync Router. Timesync Router module provides a
mechanism to mux M interrupt inputs to N interrupt outputs, where all M
inputs are selectable to be driven per N ouput.

More details about Timesync Router can be found in section 11.3.2.1 of
TRM at: https://www.ti.com/lit/ug/spruiu1d/spruiu1d.pdf

Timesync Router is identical to mux-controller, but the issue to use
mux-controller subsystem for it is it's current binding. As it is
discussed in the series, drawback of this approach is, while configuring
mux-controller one need to specify every register of memory space with
offset and mask in "mux-reg-masks" and "idle-states" property, it would
be complex for the devices with large memory space. To overcome this
limitation, new bindings are introduced, which allows to define all
these values in the same property, making it easy to define and extend
the mux-controller node.

We tried to implement Timesync Router by modeling it as an Interrupt
Router. But that is not accepted, RFC series for that is at:
https://lore.kernel.org/r/20250205160119.136639-1-c-vankar@ti.com/.

Also we tried to implement it as a new driver in MUX subsystem, but that
is almost identical to "mmio.c" driver, which I posted it as this
series.

I want your suggestion on whether a new bindings can be acceptable and
we can use the same to implement Timesync Router, if not can you please
advice me on how can I implement it ?

Regards,
Chintan.

On 06/03/25 04:00, Vankar, Chintan wrote:
> Hello Rob,
> 
> On 3/6/2025 3:44 AM, Rob Herring wrote:
>> On Wed, Mar 5, 2025 at 3:43 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>>>
>>> Hello Rob,
>>>
>>> On 3/5/2025 2:10 AM, Rob Herring wrote:
>>>> On Tue, Mar 4, 2025 at 1:03 PM Vankar, Chintan <c-vankar@ti.com> wrote:
>>>>>
>>>>> Hello Rob,
>>>>>
>>>>> On 3/4/2025 9:09 PM, Rob Herring wrote:
>>>>>> On Tue, Mar 04, 2025 at 03:53:05PM +0530, Chintan Vankar wrote:
>>>>>>> DT-binding of reg-mux is defined in such a way that one need to 
>>>>>>> provide
>>>>>>> register offset and mask in a "mux-reg-masks" property and 
>>>>>>> corresponding
>>>>>>> register value in "idle-states" property. This constraint forces 
>>>>>>> to define
>>>>>>> these values in such a way that "mux-reg-masks" and "idle-states" 
>>>>>>> must be
>>>>>>> in sync with each other. This implementation would be more 
>>>>>>> complex if
>>>>>>> specific register or set of registers need to be configured which 
>>>>>>> has
>>>>>>> large memory space. Introduce a new property 
>>>>>>> "mux-reg-masks-state" which
>>>>>>> allow to specify offset, mask and value as a tuple in a single 
>>>>>>> property.
>>>>>>
>>>>>> Maybe in hindsight that would have been better, but having 2 ways to
>>>>>> specify the same thing that we have to maintain forever is not an
>>>>>> improvement.
>>>>>>
>>>>>> No one is making you use this binding. If you have a large number of
>>>>>> muxes, then maybe you should use a specific binding.
>>>>>>
>>>>>
>>>>> Thank you for reviewing the patch. The reason behind choosing mux
>>>>> subsystem is working and implementation of mmio driver. As we can see
>>>>> that implementing this new property in mux-controller is almost
>>>>> identical to mmio driver, and it would make it easier to define and
>>>>> extend mux-controller's functionality. If we introduce the new driver
>>>>> than that would be most likely a clone of mmio driver.
>>>>
>>>> I'm talking about the binding, not the driver. They are independent.
>>>> Generic drivers are great. I love them. Generic bindings, not so much.
>>>>
>>>>> Let me know if implementation would be accepted by adding a new
>>>>> compatible for it.
>>>>
>>>> Adding a new compatible to the mmio driver? Certainly. That happens
>>>> all the time.
>>>>
>>>> I also didn't say don't use this binding as-is. That's fine too.
>>>>
>>>
>>> Can you please review the following binding:
>>>
>>> oneOf:
>>>     - required: [ mux-reg-masks ]
>>>     - required: [ mux-reg-masks-state ]
>>>
>>> allOf:
>>>     - if:
>>>         required:
>>>           - mux-reg-masks-state
>>>       then:
>>>         properties:
>>>           idle-states: false
>>>
>>> required:
>>>     - compatible
>>>     - '#mux-control-cells'
>>>
>>> I think it won't disturb the current bindings and keep backward
>>> compatibility with existing implementation.
>>
>> Wasn't that the case before? There's nothing really different here.
>>
> 
> No, the binding before was not considering "mux-reg-masks" as required
> property which was breaking actual dt-binding. In this binding, one of
> the properties from "mux-reg-masks" or "mux-reg-masks-state" is required
> which keeps the binding as it is unless someone wants to use
> "mux-reg-masks-state".
> 
> Regards,
> Chintan.
> 
>> Rob

Hello All,

On 3/4/2025 3:53 PM, Chintan Vankar wrote:
> DT-binding of reg-mux is defined in such a way that one need to provide
> register offset and mask in a "mux-reg-masks" property and corresponding
> register value in "idle-states" property. This constraint forces to define
> these values in such a way that "mux-reg-masks" and "idle-states" must be
> in sync with each other. This implementation would be more complex if
> specific register or set of registers need to be configured which has
> large memory space. Introduce a new property "mux-reg-masks-state" which
> allow to specify offset, mask and value as a tuple in a single property.
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
> 
> Link to v1:
> https://lore.kernel.org/r/20250227202206.2551305-2-c-vankar@ti.com/
> 
> Changes from v1 to v2:
> - Updated dt-bindings for the required conditions as suggested by Conor
>    Dooley and Andrew Davis.
> 
>   .../devicetree/bindings/mux/reg-mux.yaml      | 28 +++++++++++++++----
>   1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mux/reg-mux.yaml b/Documentation/devicetree/bindings/mux/reg-mux.yaml
> index dc4be092fc2f..5255e4a06920 100644
> --- a/Documentation/devicetree/bindings/mux/reg-mux.yaml
> +++ b/Documentation/devicetree/bindings/mux/reg-mux.yaml
> @@ -32,12 +32,30 @@ 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.
>   
> -required:
> -  - compatible
> -  - mux-reg-masks
> -  - '#mux-control-cells'

Accidentally, I have removed above "required:" section, will update it
when I post next version.

Regards,
Chintan.

> +  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 in a single property.
> +
> +allOf:
> +  - not:
> +      required: [mux-reg-masks, mux-reg-masks-state]
> +
> +  - if:
> +      required:
> +        - mux-reg-masks-state
> +    then:
> +      properties:
> +        idle-states: false
>   
>   additionalProperties: false
>