[RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Chintan Vankar]

This series extends mmio-mux driver's capability to configure driver in
with extended property.

In current driver implementation, driver is parsing register's offset,
mask and value from two different device tree property which makes it
complex to specify a specific register or set of registers. Introducing
mux-reg-masks-states will make it easier to specify the same values for
particular register or set of registers.

This series is based on linux next tagged next-20250303.

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

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

Chintan Vankar (2):
  devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for
    new property
  mux: mmio: Extend mmio-mux driver to configure mux with new DT
    property

 .../devicetree/bindings/mux/reg-mux.yaml      |  28 +++-
 drivers/mux/mmio.c                            | 144 ++++++++++++++----
 2 files changed, 141 insertions(+), 31 deletions(-)

-- 
2.34.1

[RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Chintan Vankar]

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

[RFC PATCH v2 2/2] mux: mmio: Extend mmio-mux driver to configure mux with new DT property

[Chintan Vankar]

MMIO mux driver is designed to parse "mux-reg-masks" and "idle-states"
property independently to configure mux registers. 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
register state to "idle-states", that would be more complex for devices
with large memory space.

Add support to extend the mmio mux driver to configure a specific register
or set of register in memory space.

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

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

Changes from v1 to v2:
- Modified driver changes as pointed out by Andrew Davis.

 drivers/mux/mmio.c | 144 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 118 insertions(+), 26 deletions(-)

diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
index 30a952c34365..7253e6305ab8 100644
--- a/drivers/mux/mmio.c
+++ b/drivers/mux/mmio.c
@@ -2,7 +2,7 @@
 /*
  * MMIO register bitfield-controlled multiplexer driver
  *
- * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ * Copyright (C) 2017-2025 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
  */
 
 #include <linux/bitops.h>
@@ -33,10 +33,83 @@ static const struct of_device_id mux_mmio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids);
 
+static int reg_mux_get_controllers(const struct device_node *np, char *prop_name)
+{
+	int ret;
+
+	ret = of_property_count_u32_elems(np, prop_name);
+	if (ret == 0 || ret % 2)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static int reg_mux_get_controllers_extended(const struct device_node *np, char *prop_name)
+{
+	int ret;
+
+	ret = of_property_count_u32_elems(np, prop_name);
+	if (ret == 0 || ret % 3)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static int reg_mux_parse_dt(const struct device_node *np, bool *mux_reg_masks_state,
+			    int *num_fields)
+{
+	int ret;
+
+	if (of_property_present(np, "mux-reg-masks-state")) {
+		*mux_reg_masks_state = true;
+		ret = reg_mux_get_controllers_extended(np, "mux-reg-masks-state");
+		if (ret < 0)
+			return ret;
+		*num_fields = ret / 3;
+	} else {
+		ret = reg_mux_get_controllers(np, "mux-reg-masks");
+		if (ret < 0)
+			return ret;
+		*num_fields = ret / 2;
+	}
+	return ret;
+}
+
+static int mux_reg_set_parameters(const struct device_node *np, char *prop_name, u32 *reg,
+				  u32 *mask, int index)
+{
+	int ret;
+
+	ret = of_property_read_u32_index(np, prop_name, 2 * index, reg);
+	if (!ret)
+		ret = of_property_read_u32_index(np, prop_name, 2 * index + 1, mask);
+
+	return ret;
+}
+
+static int mux_reg_set_parameters_extended(const struct device_node *np, char *prop_name, u32 *reg,
+					   u32 *mask, u32 *state, int index)
+{
+	int ret;
+
+	ret = of_property_read_u32_index(np, prop_name, 3 * index, reg);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32_index(np, prop_name, 3 * index + 1, mask);
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32_index(np, prop_name, 3 * index + 2, state);
+
+	return ret;
+}
+
 static int mux_mmio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	bool mux_reg_masks_state = false;
 	struct regmap_field **fields;
 	struct mux_chip *mux_chip;
 	struct regmap *regmap;
@@ -59,15 +132,16 @@ static int mux_mmio_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(regmap),
 				     "failed to get regmap\n");
 
-	ret = of_property_count_u32_elems(np, "mux-reg-masks");
-	if (ret == 0 || ret % 2)
-		ret = -EINVAL;
+	ret = reg_mux_parse_dt(np, &mux_reg_masks_state, &num_fields);
 	if (ret < 0) {
-		dev_err(dev, "mux-reg-masks property missing or invalid: %d\n",
-			ret);
+		if (mux_reg_masks_state)
+			dev_err(dev, "mux-reg-masks-state property missing or invalid: %d\n",
+				ret);
+		else
+			dev_err(dev, "mux-reg-masks property missing or invalid: %d\n",
+				ret);
 		return ret;
 	}
-	num_fields = ret / 2;
 
 	mux_chip = devm_mux_chip_alloc(dev, num_fields, num_fields *
 				       sizeof(*fields));
@@ -79,19 +153,25 @@ static int mux_mmio_probe(struct platform_device *pdev)
 	for (i = 0; i < num_fields; i++) {
 		struct mux_control *mux = &mux_chip->mux[i];
 		struct reg_field field;
-		s32 idle_state = MUX_IDLE_AS_IS;
+		s32 state, idle_state = MUX_IDLE_AS_IS;
 		u32 reg, mask;
 		int bits;
 
-		ret = of_property_read_u32_index(np, "mux-reg-masks",
-						 2 * i, &reg);
-		if (!ret)
-			ret = of_property_read_u32_index(np, "mux-reg-masks",
-							 2 * i + 1, &mask);
-		if (ret < 0) {
-			dev_err(dev, "bitfield %d: failed to read mux-reg-masks property: %d\n",
-				i, ret);
-			return ret;
+		if (!mux_reg_masks_state) {
+			ret = mux_reg_set_parameters(np, "mux-reg-masks", &reg, &mask, i);
+			if (ret < 0) {
+				dev_err(dev, "bitfield %d: failed to read mux-reg-masks property: %d\n",
+					i, ret);
+				return ret;
+			}
+		} else {
+			ret = mux_reg_set_parameters_extended(np, "mux-reg-masks-state", &reg,
+							      &mask, &state, i);
+			if (ret < 0) {
+				dev_err(dev, "bitfield %d: failed to read custom-states property: %d\n",
+					i, ret);
+				return ret;
+			}
 		}
 
 		field.reg = reg;
@@ -115,16 +195,28 @@ static int mux_mmio_probe(struct platform_device *pdev)
 		bits = 1 + field.msb - field.lsb;
 		mux->states = 1 << bits;
 
-		of_property_read_u32_index(np, "idle-states", i,
-					   (u32 *)&idle_state);
-		if (idle_state != MUX_IDLE_AS_IS) {
-			if (idle_state < 0 || idle_state >= mux->states) {
-				dev_err(dev, "bitfield: %d: out of range idle state %d\n",
-					i, idle_state);
-				return -EINVAL;
+		if (!mux_reg_masks_state) {
+			of_property_read_u32_index(np, "idle-states", i,
+						   (u32 *)&idle_state);
+			if (idle_state != MUX_IDLE_AS_IS) {
+				if (idle_state < 0 || idle_state >= mux->states) {
+					dev_err(dev, "bitfield: %d: out of range idle state %d\n",
+						i, idle_state);
+					return -EINVAL;
+				}
+
+				mux->idle_state = idle_state;
+			}
+		} else {
+			if (state != MUX_IDLE_AS_IS) {
+				if (state < 0 || state >= mux->states) {
+					dev_err(dev, "bitfield: %d: out of range idle state %d\n",
+						i, state);
+					return -EINVAL;
+				}
+
+				mux->idle_state = state;
 			}
-
-			mux->idle_state = idle_state;
 		}
 	}
 
-- 
2.34.1

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Vankar, Chintan]

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
>   

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Rob Herring]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Vankar, Chintan]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Rob Herring]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Vankar, Chintan]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Rob Herring]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Vankar, Chintan]

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

Re: [RFC PATCH v2 1/2] devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for new property

[Chintan Vankar]

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

Re: [RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Chintan Vankar]

Hello Peter,

I am trying to implement a driver for hardware module called Timesync
Router which is present on almost all the SoCs of Texas Instruments and
I need your advise to implement it.

Timesync Router provides a mechanism to mux M input to N outputs, where
all M inputs are selectable to be driven per N output.

                          ________________________
                         |    Timesync INTR       +---->dma_local_events
                         |                        |
  Device sync events----->                        +---->pcie_cpts_hw_push
                         |                        |
           cpts_genf----->                        +---->cpts_hw_push
                         |________________________|


Diagram shows a very concise view of Timesync Router. It receives
signals from multiple modules and routes the same on the other side. To
configure the functionality, we need to program output registers of
Timesync Router to configure it with the input signal. One of the
application of Timesync Router is to generate a PPS signal for CPTS
module. Timesync Router receives periodic signals generated by CPTS
module as shown "cpts_genf" in diagram and it can be routed via Timesync
Router as a Hardware Push Events as shown "cpts_hw_push" in diagram.

The functionality of Timesync Router seems very much identical to the
mux-controller, specifically mmio driver present in the mux subsystem.
I have also posted a detailed explanation on how can we modify mmio
driver which can work as a generic driver for the hardware module
identical to Timesync Router at here:
https://lore.kernel.org/r/1ce1fc6b-fc16-4fb7-9f68-57b495aa5eae@ti.com/

I have also tried to implement this module with irq subsystem:
https://lore.kernel.org/r/20250205160119.136639-1-c-vankar@ti.com/, for
which I received a response from the Thomas Gleixner that why it cannot
be included in the irq subsystem:
https://lore.kernel.org/r/87ikp8jph9.ffs@tglx/.

After receiving feedback on the Interrupt Router implementation, I tried
to implement it as a mux-controller which seems more relevant subsystem
for Timesync Router. Can you please advise me whether it can be included
in the mux-controller subsystem or not ?

Regards,
Chintan.




On 04/03/25 15:53, Chintan Vankar wrote:
> This series extends mmio-mux driver's capability to configure driver in
> with extended property.
> 
> In current driver implementation, driver is parsing register's offset,
> mask and value from two different device tree property which makes it
> complex to specify a specific register or set of registers. Introducing
> mux-reg-masks-states will make it easier to specify the same values for
> particular register or set of registers.
> 
> This series is based on linux next tagged next-20250303.
> 
> Link to v1:
> https://lore.kernel.org/r/20250227202206.2551305-1-c-vankar@ti.com/
> 
> Changes from v1 to v2:
> - Updated dt-bindings for the required conditions as suggested by Conor
>    Dooley and Andrew Davis.
> - Modified driver changes as pointed out by Andrew Davis.
> 
> Chintan Vankar (2):
>    devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for
>      new property
>    mux: mmio: Extend mmio-mux driver to configure mux with new DT
>      property
> 
>   .../devicetree/bindings/mux/reg-mux.yaml      |  28 +++-
>   drivers/mux/mmio.c                            | 144 ++++++++++++++----
>   2 files changed, 141 insertions(+), 31 deletions(-)
> 

Re: [RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Vankar, Chintan]

Hello Greg,

I have tried to implement Timesync Router node to the suitable
Subsystems (Interrupt controller and Mux-controller). Thomas
has provided a feedback with a reason why Timesync Router is not
suitable for irqchip. But I didn't get a proper feedback for mux-
controller subsystem.

Can you please help me deciding in which subsystem I should implement
it, if not mux-controller can it go in drivers/misc ?

Regards,
Chintan.

On 5/20/2025 10:59 AM, Chintan Vankar wrote:
> Hello Peter,
> 
> I am trying to implement a driver for hardware module called Timesync
> Router which is present on almost all the SoCs of Texas Instruments and
> I need your advise to implement it.
> 
> Timesync Router provides a mechanism to mux M input to N outputs, where
> all M inputs are selectable to be driven per N output.
> 
>                           ________________________
>                          |    Timesync INTR       +---->dma_local_events
>                          |                        |
>   Device sync events----->                        +---->pcie_cpts_hw_push
>                          |                        |
>            cpts_genf----->                        +---->cpts_hw_push
>                          |________________________|
> 
> 
> Diagram shows a very concise view of Timesync Router. It receives
> signals from multiple modules and routes the same on the other side. To
> configure the functionality, we need to program output registers of
> Timesync Router to configure it with the input signal. One of the
> application of Timesync Router is to generate a PPS signal for CPTS
> module. Timesync Router receives periodic signals generated by CPTS
> module as shown "cpts_genf" in diagram and it can be routed via Timesync
> Router as a Hardware Push Events as shown "cpts_hw_push" in diagram.
> 
> The functionality of Timesync Router seems very much identical to the
> mux-controller, specifically mmio driver present in the mux subsystem.
> I have also posted a detailed explanation on how can we modify mmio
> driver which can work as a generic driver for the hardware module
> identical to Timesync Router at here:
> https://lore.kernel.org/r/1ce1fc6b-fc16-4fb7-9f68-57b495aa5eae@ti.com/
> 
> I have also tried to implement this module with irq subsystem:
> https://lore.kernel.org/r/20250205160119.136639-1-c-vankar@ti.com/, for
> which I received a response from the Thomas Gleixner that why it cannot
> be included in the irq subsystem:
> https://lore.kernel.org/r/87ikp8jph9.ffs@tglx/.
> 
> After receiving feedback on the Interrupt Router implementation, I tried
> to implement it as a mux-controller which seems more relevant subsystem
> for Timesync Router. Can you please advise me whether it can be included
> in the mux-controller subsystem or not ?
> 
> Regards,
> Chintan.
> 
> 
> 
> 
> On 04/03/25 15:53, Chintan Vankar wrote:
>> This series extends mmio-mux driver's capability to configure driver in
>> with extended property.
>>
>> In current driver implementation, driver is parsing register's offset,
>> mask and value from two different device tree property which makes it
>> complex to specify a specific register or set of registers. Introducing
>> mux-reg-masks-states will make it easier to specify the same values for
>> particular register or set of registers.
>>
>> This series is based on linux next tagged next-20250303.
>>
>> Link to v1:
>> https://lore.kernel.org/r/20250227202206.2551305-1-c-vankar@ti.com/
>>
>> Changes from v1 to v2:
>> - Updated dt-bindings for the required conditions as suggested by Conor
>>    Dooley and Andrew Davis.
>> - Modified driver changes as pointed out by Andrew Davis.
>>
>> Chintan Vankar (2):
>>    devicetree: bindings: mux: reg-mux: Update bindings for reg-mux for
>>      new property
>>    mux: mmio: Extend mmio-mux driver to configure mux with new DT
>>      property
>>
>>   .../devicetree/bindings/mux/reg-mux.yaml      |  28 +++-
>>   drivers/mux/mmio.c                            | 144 ++++++++++++++----
>>   2 files changed, 141 insertions(+), 31 deletions(-)
>>

Re: [RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Greg Kroah-Hartman]

On Fri, May 30, 2025 at 10:35:24PM +0530, Vankar, Chintan wrote:
> Hello Greg,
> 
> I have tried to implement Timesync Router node to the suitable
> Subsystems (Interrupt controller and Mux-controller). Thomas
> has provided a feedback with a reason why Timesync Router is not
> suitable for irqchip. But I didn't get a proper feedback for mux-
> controller subsystem.

What do you mean "proper feedback"?

> Can you please help me deciding in which subsystem I should implement
> it, if not mux-controller can it go in drivers/misc ?

Why not mux?  What's preventing that from happening?  Why would misc be
better?

thanks,

greg k-h

Re: [RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Vankar, Chintan]

Hello Greg,

On 5/31/2025 11:22 AM, Greg Kroah-Hartman wrote:
> On Fri, May 30, 2025 at 10:35:24PM +0530, Vankar, Chintan wrote:
>> Hello Greg,
>>
>> I have tried to implement Timesync Router node to the suitable
>> Subsystems (Interrupt controller and Mux-controller). Thomas
>> has provided a feedback with a reason why Timesync Router is not
>> suitable for irqchip. But I didn't get a proper feedback for mux-
>> controller subsystem.
> 
> What do you mean "proper feedback"?
> 

By proper feedback, I meant, from the comments I was not able to figure
out whether Timesync Router will be acceptable in the "mux-controller"
subsystem or not.

>> Can you please help me deciding in which subsystem I should implement
>> it, if not mux-controller can it go in drivers/misc ?
> 
> Why not mux?  What's preventing that from happening?  Why would misc be
> better?
> 

Sure, if mux-controller subsystem is acceptable, I will implement the
Timesync Router with that and post a series.

I thought of misc, when mux-controller subsystem is not acceptable and I
could not find any other subsystem which is suitable for Timesync
Router.

Regards,
Chintan.

> thanks,
> 
> greg k-h

Re: [RFC PATCH v2 0/2] Extend mmio-mux driver to configure mux with new DT property

[Greg Kroah-Hartman]

On Sat, May 31, 2025 at 03:07:14PM +0530, Vankar, Chintan wrote:
> Hello Greg,
> 
> On 5/31/2025 11:22 AM, Greg Kroah-Hartman wrote:
> > On Fri, May 30, 2025 at 10:35:24PM +0530, Vankar, Chintan wrote:
> > > Hello Greg,
> > > 
> > > I have tried to implement Timesync Router node to the suitable
> > > Subsystems (Interrupt controller and Mux-controller). Thomas
> > > has provided a feedback with a reason why Timesync Router is not
> > > suitable for irqchip. But I didn't get a proper feedback for mux-
> > > controller subsystem.
> > 
> > What do you mean "proper feedback"?
> > 
> 
> By proper feedback, I meant, from the comments I was not able to figure
> out whether Timesync Router will be acceptable in the "mux-controller"
> subsystem or not.

Did you submit a real patch to do so?  Note, I know I do not read "RFC"
patches for the most part as that implies the submitter does not feel it
is ready to be merged, when I have other patches that submitters _do_
feel are ready to be merged that are still left to review.

> > > Can you please help me deciding in which subsystem I should implement
> > > it, if not mux-controller can it go in drivers/misc ?
> > 
> > Why not mux?  What's preventing that from happening?  Why would misc be
> > better?
> > 
> 
> Sure, if mux-controller subsystem is acceptable, I will implement the
> Timesync Router with that and post a series.

Try it and see!  We don't normally do "what if I did this" type of
review, we want to see patches that actually work.

thanks,

greg k-h