[PATCH 0/2] Add support for new property 'mux-reg-masks-state' for mmio mux

[PATCH 0/2] Add support for new property 'mux-reg-masks-state' for mmio mux

[Chintan Vankar]

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

In current driver implementation (mmio.c), driver is parsing register's
offset, mask and value from two different device tree property. In order
to configure mux register, these two properties should be synchronized
with each other, and this constraint makes it complex to modify or add 
further entries for that memory region. To simplify this, add support for 
a new property, "mux-reg-masks-state", which allows offset, mask, and 
value to be specified together as a tuple, and remove the above
constraint.

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

Link to RFC series:
https://lore.kernel.org/r/20250304102306.2977836-1-c-vankar@ti.com/

Chintan Vankar (2):
  devicetree: bindings: mux: reg-mux: Add support for new property
    'mux-reg-masks-state'
  mux: mmio: Extend mmio-mux driver to configure mux with
    mux-reg-masks-state

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

-- 
2.34.1

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

[Chintan Vankar]

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'

[Krzysztof Kozlowski]

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

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

[Rob Herring]

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

[PATCH 2/2] mux: mmio: Extend mmio-mux driver to configure mux with mux-reg-masks-state

[Chintan Vankar]

MMIO mux driver is designed to parse "mux-reg-masks" and "idle-states"
property independently to configure mux. The current design is complex for
the devices with larger memory space, which requires synchronization
between the two properties.

Extend mmio-mux driver to support a single property, "mux-reg-masks-state"
which configures mux registers without above constraint.

Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---
 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 9993ce38a818..5ce9c16fd431 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>
@@ -39,10 +39,83 @@ static const struct regmap_config mux_mmio_regmap_cfg = {
 	.reg_stride = 4,
 };
 
+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;
@@ -70,15 +143,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));
@@ -90,19 +164,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;
@@ -126,16 +206,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: [PATCH 2/2] mux: mmio: Extend mmio-mux driver to configure mux with mux-reg-masks-state

[Krzysztof Kozlowski]

On 05/06/2025 08:34, Chintan Vankar wrote:
> MMIO mux driver is designed to parse "mux-reg-masks" and "idle-states"
> property independently to configure mux. The current design is complex for
> the devices with larger memory space, which requires synchronization
> between the two properties.
> 
> Extend mmio-mux driver to support a single property, "mux-reg-masks-state"
> which configures mux registers without above constraint.
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
>  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 9993ce38a818..5ce9c16fd431 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>

Why are you updating someone's copyrights?


Best regards,
Krzysztof

Re: [PATCH 2/2] mux: mmio: Extend mmio-mux driver to configure mux with mux-reg-masks-state

[Chintan Vankar]

Hello Krzysztof,

On 05/06/25 12:06, Krzysztof Kozlowski wrote:
> On 05/06/2025 08:34, Chintan Vankar wrote:
>> MMIO mux driver is designed to parse "mux-reg-masks" and "idle-states"
>> property independently to configure mux. The current design is complex for
>> the devices with larger memory space, which requires synchronization
>> between the two properties.
>>
>> Extend mmio-mux driver to support a single property, "mux-reg-masks-state"
>> which configures mux registers without above constraint.
>>
>> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
>> ---
>>   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 9993ce38a818..5ce9c16fd431 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>
> 
> Why are you updating someone's copyrights?
> 

Yes, I forgot to modify from the RFC series. I will keep it unmodified
in the next version.

Regards,
Chintan.

> 
> Best regards,
> Krzysztof