On 2021-11-29 05:36, Aswath Govindraju wrote:

Hi Peter,

On 25/11/21 7:05 pm, Peter Rosin wrote:

quoted

Hi!

You need to have some description on how #mux-control-cells now work.
The previous description is in mux-consumer.yaml and an update there
is needed.

However, I have realized that the adg792a binding uses #mux-control-cells
to indicate if it should expose its three muxes with one mux-control
and operate the muxes in parallel, or if it should be expose three
independent mux-controls. So, the approach in this series to always
have the #mux-control-cells property fixed at <2> when indicating a
state will not work for that binding. And I see no fix for that binding
without adding a new property.

So, I would like a different approach. Since I dislike how mux-controls
-after this series- is not (always) specifying a mux-control like the name
says, but instead optionally a specific state, the new property I would
like to add is #mux-state-cells such that it would always be one more
than #mux-control-cells.

	mux: mux-controller {
		compatible = "gpio-mux";
		#mux-control-cells = <0>;
		#mux-state-cells = <1>;

		mux-gpios = <...>;
	};

	can-phy {
		compatible = "ti,tcan1043";
		...
		mux-states = <&mux 1>;
	};

That solves the naming issue, the unused argument for mux-conrtrollers
that previously had #mux-control-cells = <0>, and the binding for adg792a
need no longer be inconsistent.

Or, how should this be solved? I'm sure there are other options...

I feel that the new approach using mux-state-cells seems to be
overpopulating the device tree nodes, when the state can be represented
using the control cells. I understand that the definition for
mux-controls is to only specify the control line to be used in a given
mux. Can't it now be upgraded to also represent the state at which the
control line has to be set to?

With respect to adg792a, it is inline with the current implementation
and the only change I think would be required in the driver is,

No, that does not work. See below.

When you add cell #2 with the state, the cells variable will end up
as 2 always. Which means that there is no way to alloc one mux
control since "cells ? 3 : 1" will always end up as "3", with no
easy fix.

So, your approach does not work for this driver.

Cheers,
Peter

        if (IS_ERR(mux_chip))

And the following series should be compatible with it. If adg792a driver
is the only issue then would there be any issue with only changing it
and using this implementation?

Thanks,
Aswath

quoted

Cheers,
Peter

On 2021-11-23 09:12, Aswath Govindraju wrote:

quoted

Increase the allowed number of arguments in mux-controls to add support for
passing information regarding the state of the mux to be set, for a given
device.

Signed-off-by: Aswath Govindraju <redacted>
---
 Documentation/devicetree/bindings/mux/gpio-mux.yaml       | 2 +-
 Documentation/devicetree/bindings/mux/mux-controller.yaml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.yaml b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
index 0a7c8d64981a..c810b7df39de 100644
--- a/Documentation/devicetree/bindings/mux/gpio-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.yaml

@@ -26,7 +26,7 @@ properties:
       List of gpios used to control the multiplexer, least significant bit first.
 
   '#mux-control-cells':
-    const: 0
+    enum: [ 0, 1, 2 ]
 
   idle-state:
     default: -1

diff --git a/Documentation/devicetree/bindings/mux/mux-controller.yaml b/Documentation/devicetree/bindings/mux/mux-controller.yaml
index 736a84c3b6a5..0b4b067a97bf 100644
--- a/Documentation/devicetree/bindings/mux/mux-controller.yaml
+++ b/Documentation/devicetree/bindings/mux/mux-controller.yaml

@@ -73,7 +73,7 @@ properties:
     pattern: '^mux-controller(@.*|-[0-9a-f]+)?$'
 
   '#mux-control-cells':
-    enum: [ 0, 1 ]
+    enum: [ 0, 1, 2 ]
 
   idle-state:
     $ref: /schemas/types.yaml#/definitions/int32