diff mbox

[v13,02/10] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux

Message ID 1492101794-13444-3-git-send-email-peda@axentia.se
State Superseded
Headers show

Commit Message

Peter Rosin April 13, 2017, 4:43 p.m. UTC
Allow specifying that a single multiplexer controller can be used to
control several parallel multiplexers, thus enabling sharing of the
multiplexer controller by different consumers.

Add a binding for a first mux controller in the form of a GPIO based mux
controller.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
 .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 include/dt-bindings/mux/mux.h                      |  16 +++
 4 files changed, 248 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
 create mode 100644 include/dt-bindings/mux/mux.h

Comments

Philipp Zabel April 18, 2017, 10:06 a.m. UTC | #1
On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> Allow specifying that a single multiplexer controller can be used to
> control several parallel multiplexers, thus enabling sharing of the
> multiplexer controller by different consumers.
> 
> Add a binding for a first mux controller in the form of a GPIO based mux
> controller.
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
>  .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  include/dt-bindings/mux/mux.h                      |  16 +++
>  4 files changed, 248 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
>  create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
>  create mode 100644 include/dt-bindings/mux/mux.h
> 
> diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
> new file mode 100644
> index 000000000000..b8f746344d80
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
> @@ -0,0 +1,69 @@
> +GPIO-based multiplexer controller bindings
> +
> +Define what GPIO pins are used to control a multiplexer. Or several
> +multiplexers, if the same pins control more than one multiplexer.
> +
> +Required properties:
> +- compatible : "gpio-mux"
> +- mux-gpios : list of gpios used to control the multiplexer, least
> +	      significant bit first.
> +- #mux-control-cells : <0>
> +* Standard mux-controller bindings as decribed in mux-controller.txt
> +
> +Optional properties:
> +- idle-state : if present, the state the mux will have when idle. The
> +	       special state MUX_IDLE_AS_IS is the default.
> +
> +The multiplexer state is defined as the number represented by the
> +multiplexer GPIO pins, where the first pin is the least significant
> +bit. An active pin is a binary 1, an inactive pin is a binary 0.
> +
> +Example:
> +
> +	mux: mux-controller {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "io-channel-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +
> +		mux-controls = <&mux>;
> +
> +		channels = "sync-1", "in", "out", "sync-2";
> +	};

Could you explain in more detail the reasoning behind this split between
the mux controller and the actual mux?
For SoC internal video bus muxes that are controlled by a register
bitfield, it seems a bit strange to have to split them into two device
tree nodes.

Basically I'm trying to figure out whether a video mux (which has a mux
control plus OF-graph bindings to describe its ports and connections)
would fit into the same category as an adc-mux or i2c-mux, or whether it
would be better to handle them as a specialized form of mux-controller.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin April 18, 2017, 1:36 p.m. UTC | #2
On 2017-04-18 12:06, Philipp Zabel wrote:
> On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
>> Allow specifying that a single multiplexer controller can be used to
>> control several parallel multiplexers, thus enabling sharing of the
>> multiplexer controller by different consumers.
>>
>> Add a binding for a first mux controller in the form of a GPIO based mux
>> controller.
>>
>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
>>  .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
>>  MAINTAINERS                                        |   6 +
>>  include/dt-bindings/mux/mux.h                      |  16 +++
>>  4 files changed, 248 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
>>  create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
>>  create mode 100644 include/dt-bindings/mux/mux.h
>>
>> diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
>> new file mode 100644
>> index 000000000000..b8f746344d80
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
>> @@ -0,0 +1,69 @@
>> +GPIO-based multiplexer controller bindings
>> +
>> +Define what GPIO pins are used to control a multiplexer. Or several
>> +multiplexers, if the same pins control more than one multiplexer.
>> +
>> +Required properties:
>> +- compatible : "gpio-mux"
>> +- mux-gpios : list of gpios used to control the multiplexer, least
>> +	      significant bit first.
>> +- #mux-control-cells : <0>
>> +* Standard mux-controller bindings as decribed in mux-controller.txt
>> +
>> +Optional properties:
>> +- idle-state : if present, the state the mux will have when idle. The
>> +	       special state MUX_IDLE_AS_IS is the default.
>> +
>> +The multiplexer state is defined as the number represented by the
>> +multiplexer GPIO pins, where the first pin is the least significant
>> +bit. An active pin is a binary 1, an inactive pin is a binary 0.
>> +
>> +Example:
>> +
>> +	mux: mux-controller {
>> +		compatible = "gpio-mux";
>> +		#mux-control-cells = <0>;
>> +
>> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
>> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
>> +	};
>> +
>> +	adc-mux {
>> +		compatible = "io-channel-mux";
>> +		io-channels = <&adc 0>;
>> +		io-channel-names = "parent";
>> +
>> +		mux-controls = <&mux>;
>> +
>> +		channels = "sync-1", "in", "out", "sync-2";
>> +	};
> 
> Could you explain in more detail the reasoning behind this split between
> the mux controller and the actual mux?
> For SoC internal video bus muxes that are controlled by a register
> bitfield, it seems a bit strange to have to split them into two device
> tree nodes.

The background for the split is in the cover letter.

Basically, when the same set of gpio lines control several muxes, and
when these muxes are used for unrelated things, you needs some extra
complexity.

The mux controller is what controls those gpio lines, and thus the mux
state for all muxes they are connected to. The consumers refer to the
mux controller and request a certain state. So, the consumers naturally
need to interact or else they would destroy the mux state for each other.
Regarding the device tree layout, perhaps the mux consumers could be
children of the mux controller, but I think that would make the mux
controller more like a bus instead of a class. Anyway, I don't want to
go there again, because I remember it as a messy place. Maybe I could
do better now than I did way back when, but I'm not going willingly
and someone would have to force me.

The benefit of the split is that the mux consumer need no longer concern
itself with if the mux is controlled by gpio lines, an I2C based chip
like the ADG792 or if it is controlled by some mmio register. You can
thus avoid building muxing sub-sub-systems like drivers/i2c/muxes for
every subsystem needing muxing.

The drawback is that you get an extra device tree node for the mux
controller that may not make sense if it is in no way possible to
reuse your driver for HW with a different mux. Which may be the case
for your video case? But for generic stuff like ADC lines and I2C
buses, muxing options are diverse...

> Basically I'm trying to figure out whether a video mux (which has a mux
> control plus OF-graph bindings to describe its ports and connections)
> would fit into the same category as an adc-mux or i2c-mux, or whether it
> would be better to handle them as a specialized form of mux-controller.

I did read some earlier thread about your muxing requirements and I got
the impression that you also had HW which controlled the mux with
gpio lines? In that case, the mux subsystem seems like a perfect fit
with a new syscon/mmio/reg based mux driver (or whatever the name should
be, I think I'd go with syscon) pretty much as suggested in your RFC
patches. And then of course reuse the existing gpio-mux driver for the
other case.

The video-mux would fit as a mux consumer just like the iio-mux and the
i2c-mux are mux consumers, with input 0/input 1 being the port that
would be selected with the mux I guess. I don't think there should be a
bunch of video code inside the drivers/mux subdir, for the same reason
there's no iio/i2c code in there.

If I got things wrong when I skimmed whatever I came across, and if the
mmio register is the only mux control option in the stars, it becomes
less obvious... It's of course still possible to hook into the mux
subsystem, but the benefit is questionable. And you do get the extra
device tree node. You could of course also implement a mux driver
outside of drivers/mux and thus make use of the mux api, but it's tiny
and any benefit is truly small.

Cheers,
peda

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel April 19, 2017, 9:17 a.m. UTC | #3
On Tue, 2017-04-18 at 15:36 +0200, Peter Rosin wrote:
> On 2017-04-18 12:06, Philipp Zabel wrote:
> > On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote:
> >> Allow specifying that a single multiplexer controller can be used to
> >> control several parallel multiplexers, thus enabling sharing of the
> >> multiplexer controller by different consumers.
> >>
> >> Add a binding for a first mux controller in the form of a GPIO based mux
> >> controller.
> >>
> >> Acked-by: Jonathan Cameron <jic23@kernel.org>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>  Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
> >>  .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
> >>  MAINTAINERS                                        |   6 +
> >>  include/dt-bindings/mux/mux.h                      |  16 +++
> >>  4 files changed, 248 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
> >>  create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
> >>  create mode 100644 include/dt-bindings/mux/mux.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
> >> new file mode 100644
> >> index 000000000000..b8f746344d80
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
> >> @@ -0,0 +1,69 @@
> >> +GPIO-based multiplexer controller bindings
> >> +
> >> +Define what GPIO pins are used to control a multiplexer. Or several
> >> +multiplexers, if the same pins control more than one multiplexer.
> >> +
> >> +Required properties:
> >> +- compatible : "gpio-mux"
> >> +- mux-gpios : list of gpios used to control the multiplexer, least
> >> +	      significant bit first.
> >> +- #mux-control-cells : <0>
> >> +* Standard mux-controller bindings as decribed in mux-controller.txt
> >> +
> >> +Optional properties:
> >> +- idle-state : if present, the state the mux will have when idle. The
> >> +	       special state MUX_IDLE_AS_IS is the default.
> >> +
> >> +The multiplexer state is defined as the number represented by the
> >> +multiplexer GPIO pins, where the first pin is the least significant
> >> +bit. An active pin is a binary 1, an inactive pin is a binary 0.
> >> +
> >> +Example:
> >> +
> >> +	mux: mux-controller {
> >> +		compatible = "gpio-mux";
> >> +		#mux-control-cells = <0>;
> >> +
> >> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> >> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> >> +	};
> >> +
> >> +	adc-mux {
> >> +		compatible = "io-channel-mux";
> >> +		io-channels = <&adc 0>;
> >> +		io-channel-names = "parent";
> >> +
> >> +		mux-controls = <&mux>;
> >> +
> >> +		channels = "sync-1", "in", "out", "sync-2";
> >> +	};
> > 
> > Could you explain in more detail the reasoning behind this split between
> > the mux controller and the actual mux?
> > For SoC internal video bus muxes that are controlled by a register
> > bitfield, it seems a bit strange to have to split them into two device
> > tree nodes.
> 
> The background for the split is in the cover letter.

Thanks for explaining anyway, I didn't read past the changelog earlier.

[...]
> > Basically I'm trying to figure out whether a video mux (which has a mux
> > control plus OF-graph bindings to describe its ports and connections)
> > would fit into the same category as an adc-mux or i2c-mux, or whether it
> > would be better to handle them as a specialized form of mux-controller.
> 
> I did read some earlier thread about your muxing requirements and I got
> the impression that you also had HW which controlled the mux with
> gpio lines? In that case, the mux subsystem seems like a perfect fit
> with a new syscon/mmio/reg based mux driver (or whatever the name should
> be, I think I'd go with syscon) pretty much as suggested in your RFC
> patches. And then of course reuse the existing gpio-mux driver for the
> other case.

Yes, the requirement on hand is for MMIO controlled SoC internal muxes
for the i.MX6 video capture subsystem, but I'd like to also support GPIO
controlled external muxes to switch between two camera sources on those
boards that have them.

> The video-mux would fit as a mux consumer just like the iio-mux and the
> i2c-mux are mux consumers, with input 0/input 1 being the port that
> would be selected with the mux I guess.

Exactly. An N-input mux would have N+1 ports with port N being the
output.

[...]
> If I got things wrong when I skimmed whatever I came across, and if the
> mmio register is the only mux control option in the stars, it becomes
> less obvious... It's of course still possible to hook into the mux
> subsystem, but the benefit is questionable. And you do get the extra
> device tree node. You could of course also implement a mux driver
> outside of drivers/mux and thus make use of the mux api, but it's tiny
> and any benefit is truly small.

What I wondered mostly is whether it would be a good idea to move the
OF-graph ports into the mux controller node, and let the video capture
device be the consumer of the mux.
But this wouldn't fit well with the clear split between the mux
controller and the actual mux hardware in the mux DT bindings.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin April 19, 2017, 10:41 a.m. UTC | #4
On 2017-04-19 11:17, Philipp Zabel wrote:
> On Tue, 2017-04-18 at 15:36 +0200, Peter Rosin wrote:
>> If I got things wrong when I skimmed whatever I came across, and if the
>> mmio register is the only mux control option in the stars, it becomes
>> less obvious... It's of course still possible to hook into the mux
>> subsystem, but the benefit is questionable. And you do get the extra
>> device tree node. You could of course also implement a mux driver
>> outside of drivers/mux and thus make use of the mux api, but it's tiny
>> and any benefit is truly small.
> 
> What I wondered mostly is whether it would be a good idea to move the
> OF-graph ports into the mux controller node, and let the video capture
> device be the consumer of the mux.
> But this wouldn't fit well with the clear split between the mux
> controller and the actual mux hardware in the mux DT bindings.

I have tried to do something similar. I think. The current
drivers/i2c/muxes/i2c-mux-gpio.c is a good candidate for the same thing
IIUC.

That dedicated driver and the general purpose i2c mux driver does pretty
much the same thing with these two DT snippets:

Dedicated i2c-mux-gpio DT snippet:

	i2c-mux {
		compatible = "i2c-mux-gpio";
		i2c-parent = <&i2c1>;

		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;

		#address-cells = <1>;
		#size-cells = <0>;

		i2c@1 {
			...
		};

		i2c@3 {
			...
		};
	};

General purpose mux DT snippet:

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

		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
	};

	i2c-mux {
		compatible = "i2c-mux";
		i2c-parent = <&i2c1>;

		mux-controls = <&mux>;

		#address-cells = <1>;
		#size-cells = <0>;

		i2c@1 {
			...
		};

		i2c@3 {
			...
		};
	};

I would love to find a way to cleanly get the mux framework to handle
the first DT as well, and thus being able to obsolete the dedicated
i2c-mux-gpio driver. I have not figured out how to accomplish that
without abusing the driver-model to a point that it's not working.
Help with that task is dearly appreciated.

What I have stumbled on, I think, is that two drivers needs to be
instantiated from the same DT node. At the same time, I need the
mux framework to handle the current out-of-node thing with a
phandle as well, so that several mux consumers can share a common
mux controller. My understanding of these matters are apparently not
deep enough...

I think you would like a DT that looks more like the first DT
snippet but still enjoy the flexibility of the mux framework and w/o
implementing a (another) full muxing sub-sub-system like the i2c
sub-system has done. Correct?

Cheers,
peda

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel April 19, 2017, 11:05 a.m. UTC | #5
On Wed, 2017-04-19 at 12:41 +0200, Peter Rosin wrote:
> On 2017-04-19 11:17, Philipp Zabel wrote:
> > On Tue, 2017-04-18 at 15:36 +0200, Peter Rosin wrote:
> >> If I got things wrong when I skimmed whatever I came across, and if the
> >> mmio register is the only mux control option in the stars, it becomes
> >> less obvious... It's of course still possible to hook into the mux
> >> subsystem, but the benefit is questionable. And you do get the extra
> >> device tree node. You could of course also implement a mux driver
> >> outside of drivers/mux and thus make use of the mux api, but it's tiny
> >> and any benefit is truly small.
> > 
> > What I wondered mostly is whether it would be a good idea to move the
> > OF-graph ports into the mux controller node, and let the video capture
> > device be the consumer of the mux.
> > But this wouldn't fit well with the clear split between the mux
> > controller and the actual mux hardware in the mux DT bindings.
> 
> I have tried to do something similar. I think. The current
> drivers/i2c/muxes/i2c-mux-gpio.c is a good candidate for the same thing
> IIUC.
>
> That dedicated driver and the general purpose i2c mux driver does pretty
> much the same thing with these two DT snippets:
> 
> Dedicated i2c-mux-gpio DT snippet:
> 
> 	i2c-mux {
> 		compatible = "i2c-mux-gpio";
> 		i2c-parent = <&i2c1>;
> 
> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		i2c@1 {
> 			...
> 		};
> 
> 		i2c@3 {
> 			...
> 		};
> 	};
> 
> General purpose mux DT snippet:
> 
> 	mux: mux-controller {
> 		compatible = "gpio-mux";
> 		#mux-control-cells = <0>;
> 
> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> 	};
> 
> 	i2c-mux {
> 		compatible = "i2c-mux";
> 		i2c-parent = <&i2c1>;
> 
> 		mux-controls = <&mux>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		i2c@1 {
> 			...
> 		};
> 
> 		i2c@3 {
> 			...
> 		};
> 	};

Yes, replace i2c-mux with video-mux and the i2c@x nodes with port@x
nodes, and this is very close to what I am thinking about.

> I would love to find a way to cleanly get the mux framework to handle
> the first DT as well, and thus being able to obsolete the dedicated
> i2c-mux-gpio driver. I have not figured out how to accomplish that
> without abusing the driver-model to a point that it's not working.
> Help with that task is dearly appreciated.
> 
> What I have stumbled on, I think, is that two drivers needs to be
> instantiated from the same DT node. At the same time, I need the
> mux framework to handle the current out-of-node thing with a
> phandle as well, so that several mux consumers can share a common
> mux controller. My understanding of these matters are apparently not
> deep enough...

Not necessarily, if the framework could export a function to create a
gpio/mmio mux_chip on a given device and the gpio-mux and *-mux-gpio
drivers just reuse that.

> I think you would like a DT that looks more like the first DT
> snippet but still enjoy the flexibility of the mux framework and w/o
> implementing a (another) full muxing sub-sub-system like the i2c
> sub-system has done. Correct?

Correct.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin April 19, 2017, 11:23 a.m. UTC | #6
On 2017-04-19 13:05, Philipp Zabel wrote:
> On Wed, 2017-04-19 at 12:41 +0200, Peter Rosin wrote:
>> On 2017-04-19 11:17, Philipp Zabel wrote:
>>> On Tue, 2017-04-18 at 15:36 +0200, Peter Rosin wrote:
>>>> If I got things wrong when I skimmed whatever I came across, and if the
>>>> mmio register is the only mux control option in the stars, it becomes
>>>> less obvious... It's of course still possible to hook into the mux
>>>> subsystem, but the benefit is questionable. And you do get the extra
>>>> device tree node. You could of course also implement a mux driver
>>>> outside of drivers/mux and thus make use of the mux api, but it's tiny
>>>> and any benefit is truly small.
>>>
>>> What I wondered mostly is whether it would be a good idea to move the
>>> OF-graph ports into the mux controller node, and let the video capture
>>> device be the consumer of the mux.
>>> But this wouldn't fit well with the clear split between the mux
>>> controller and the actual mux hardware in the mux DT bindings.
>>
>> I have tried to do something similar. I think. The current
>> drivers/i2c/muxes/i2c-mux-gpio.c is a good candidate for the same thing
>> IIUC.
>>
>> That dedicated driver and the general purpose i2c mux driver does pretty
>> much the same thing with these two DT snippets:
>>
>> Dedicated i2c-mux-gpio DT snippet:
>>
>> 	i2c-mux {
>> 		compatible = "i2c-mux-gpio";
>> 		i2c-parent = <&i2c1>;
>>
>> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
>>
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		i2c@1 {
>> 			...
>> 		};
>>
>> 		i2c@3 {
>> 			...
>> 		};
>> 	};
>>
>> General purpose mux DT snippet:
>>
>> 	mux: mux-controller {
>> 		compatible = "gpio-mux";
>> 		#mux-control-cells = <0>;
>>
>> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
>> 	};
>>
>> 	i2c-mux {
>> 		compatible = "i2c-mux";
>> 		i2c-parent = <&i2c1>;
>>
>> 		mux-controls = <&mux>;
>>
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		i2c@1 {
>> 			...
>> 		};
>>
>> 		i2c@3 {
>> 			...
>> 		};
>> 	};
> 
> Yes, replace i2c-mux with video-mux and the i2c@x nodes with port@x
> nodes, and this is very close to what I am thinking about.
> 
>> I would love to find a way to cleanly get the mux framework to handle
>> the first DT as well, and thus being able to obsolete the dedicated
>> i2c-mux-gpio driver. I have not figured out how to accomplish that
>> without abusing the driver-model to a point that it's not working.
>> Help with that task is dearly appreciated.
>>
>> What I have stumbled on, I think, is that two drivers needs to be
>> instantiated from the same DT node. At the same time, I need the
>> mux framework to handle the current out-of-node thing with a
>> phandle as well, so that several mux consumers can share a common
>> mux controller. My understanding of these matters are apparently not
>> deep enough...
> 
> Not necessarily, if the framework could export a function to create a
> gpio/mmio mux_chip on a given device and the gpio-mux and *-mux-gpio
> drivers just reuse that.

I've been up that creek. Why should the gpio mux be special cased?
That's not clean, the implication is that all mux consumers need
to handle the gpio case and have a special compatible for that
case etc. Then someone thinks the DT should look equally "clean" for
some i2c based mux, and the weeds start piling up. This is exactly
what we don't want. We want the mux consumer drivers to be totally
agnostic about the fact that they happen to use a gpio mux.

Cheers,
peda

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel April 19, 2017, 4:34 p.m. UTC | #7
On Wed, 2017-04-19 at 13:23 +0200, Peter Rosin wrote:
> On 2017-04-19 13:05, Philipp Zabel wrote:
> > On Wed, 2017-04-19 at 12:41 +0200, Peter Rosin wrote:
> >> On 2017-04-19 11:17, Philipp Zabel wrote:
> >>> On Tue, 2017-04-18 at 15:36 +0200, Peter Rosin wrote:
> >>>> If I got things wrong when I skimmed whatever I came across, and if the
> >>>> mmio register is the only mux control option in the stars, it becomes
> >>>> less obvious... It's of course still possible to hook into the mux
> >>>> subsystem, but the benefit is questionable. And you do get the extra
> >>>> device tree node. You could of course also implement a mux driver
> >>>> outside of drivers/mux and thus make use of the mux api, but it's tiny
> >>>> and any benefit is truly small.
> >>>
> >>> What I wondered mostly is whether it would be a good idea to move the
> >>> OF-graph ports into the mux controller node, and let the video capture
> >>> device be the consumer of the mux.
> >>> But this wouldn't fit well with the clear split between the mux
> >>> controller and the actual mux hardware in the mux DT bindings.
> >>
> >> I have tried to do something similar. I think. The current
> >> drivers/i2c/muxes/i2c-mux-gpio.c is a good candidate for the same thing
> >> IIUC.
> >>
> >> That dedicated driver and the general purpose i2c mux driver does pretty
> >> much the same thing with these two DT snippets:
> >>
> >> Dedicated i2c-mux-gpio DT snippet:
> >>
> >> 	i2c-mux {
> >> 		compatible = "i2c-mux-gpio";
> >> 		i2c-parent = <&i2c1>;
> >>
> >> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> >>
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >>
> >> 		i2c@1 {
> >> 			...
> >> 		};
> >>
> >> 		i2c@3 {
> >> 			...
> >> 		};
> >> 	};
> >>
> >> General purpose mux DT snippet:
> >>
> >> 	mux: mux-controller {
> >> 		compatible = "gpio-mux";
> >> 		#mux-control-cells = <0>;
> >>
> >> 		mux-gpios = <&gpio1 22 0 &gpio1 23 0>;
> >> 	};
> >>
> >> 	i2c-mux {
> >> 		compatible = "i2c-mux";
> >> 		i2c-parent = <&i2c1>;
> >>
> >> 		mux-controls = <&mux>;
> >>
> >> 		#address-cells = <1>;
> >> 		#size-cells = <0>;
> >>
> >> 		i2c@1 {
> >> 			...
> >> 		};
> >>
> >> 		i2c@3 {
> >> 			...
> >> 		};
> >> 	};
> > 
> > Yes, replace i2c-mux with video-mux and the i2c@x nodes with port@x
> > nodes, and this is very close to what I am thinking about.
> > 
> >> I would love to find a way to cleanly get the mux framework to handle
> >> the first DT as well, and thus being able to obsolete the dedicated
> >> i2c-mux-gpio driver. I have not figured out how to accomplish that
> >> without abusing the driver-model to a point that it's not working.
> >> Help with that task is dearly appreciated.
> >>
> >> What I have stumbled on, I think, is that two drivers needs to be
> >> instantiated from the same DT node. At the same time, I need the
> >> mux framework to handle the current out-of-node thing with a
> >> phandle as well, so that several mux consumers can share a common
> >> mux controller. My understanding of these matters are apparently not
> >> deep enough...
> > 
> > Not necessarily, if the framework could export a function to create a
> > gpio/mmio mux_chip on a given device and the gpio-mux and *-mux-gpio
> > drivers just reuse that.
> 
> I've been up that creek. Why should the gpio mux be special cased?

You are right, this does not scale.

> That's not clean, the implication is that all mux consumers need
> to handle the gpio case and have a special compatible for that
> case etc. Then someone thinks the DT should look equally "clean" for
> some i2c based mux, and the weeds start piling up. This is exactly
> what we don't want. We want the mux consumer drivers to be totally
> agnostic about the fact that they happen to use a gpio mux.

If you want to have i2c-mux-gpio and i2c-mux compatibles, and a single
driver to handle them both, it must at least match both compatibles, so
it can't be completely agnostic.

Why not then have it call

	if (/* compatible == "i2c-mux" */)
		mux = devm_mux_control_get(dev, NULL);
	else /* if (compatible == "i2c-mux-gpio/mmio/etc.") */
		mux = devm_mux_control_create(dev);

? The mux framework core could hold a list of those <usage>-mux-<type>
compatibles and dispatch creation of the correct mux (or mux platform
device, if necessary).

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
new file mode 100644
index 000000000000..b8f746344d80
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
@@ -0,0 +1,69 @@ 
+GPIO-based multiplexer controller bindings
+
+Define what GPIO pins are used to control a multiplexer. Or several
+multiplexers, if the same pins control more than one multiplexer.
+
+Required properties:
+- compatible : "gpio-mux"
+- mux-gpios : list of gpios used to control the multiplexer, least
+	      significant bit first.
+- #mux-control-cells : <0>
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle. The
+	       special state MUX_IDLE_AS_IS is the default.
+
+The multiplexer state is defined as the number represented by the
+multiplexer GPIO pins, where the first pin is the least significant
+bit. An active pin is a binary 1, an inactive pin is a binary 0.
+
+Example:
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
diff --git a/Documentation/devicetree/bindings/mux/mux-controller.txt b/Documentation/devicetree/bindings/mux/mux-controller.txt
new file mode 100644
index 000000000000..4f47e4bd2fa0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/mux-controller.txt
@@ -0,0 +1,157 @@ 
+Common multiplexer controller bindings
+======================================
+
+A multiplexer (or mux) controller will have one, or several, consumer devices
+that uses the mux controller. Thus, a mux controller can possibly control
+several parallel multiplexers. Presumably there will be at least one
+multiplexer needed by each consumer, but a single mux controller can of course
+control several multiplexers for a single consumer.
+
+A mux controller provides a number of states to its consumers, and the state
+space is a simple zero-based enumeration. I.e. 0-1 for a 2-way multiplexer,
+0-7 for an 8-way multiplexer, etc.
+
+
+Consumers
+---------
+
+Mux controller consumers should specify a list of mux controllers that they
+want to use with a property containing a 'mux-ctrl-list':
+
+	mux-ctrl-list ::= <single-mux-ctrl> [mux-ctrl-list]
+	single-mux-ctrl ::= <mux-ctrl-phandle> [mux-ctrl-specifier]
+	mux-ctrl-phandle : phandle to mux controller node
+	mux-ctrl-specifier : array of #mux-control-cells specifying the
+			     given mux controller (controller specific)
+
+Mux controller properties should be named "mux-controls". The exact meaning of
+each mux controller property must be documented in the device tree binding for
+each consumer. An optional property "mux-control-names" may contain a list of
+strings to label each of the mux controllers listed in the "mux-controls"
+property.
+
+Drivers for devices that use more than a single mux controller can use the
+"mux-control-names" property to map the name of the requested mux controller
+to an index into the list given by the "mux-controls" property.
+
+mux-ctrl-specifier typically encodes the chip-relative mux controller number.
+If the mux controller chip only provides a single mux controller, the
+mux-ctrl-specifier can typically be left out.
+
+Example:
+
+	/* One consumer of a 2-way mux controller (one GPIO-line) */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+		mux-control-names = "adc";
+
+		channels = "sync", "in";
+	};
+
+Note that in the example above, specifying the "mux-control-names" is redundant
+because there is only one mux controller in the list. However, if the driver
+for the consumer node in fact asks for a named mux controller, that name is of
+course still required.
+
+	/*
+	 * Two consumers (one for an ADC line and one for an i2c bus) of
+	 * parallel 4-way multiplexers controlled by the same two GPIO-lines.
+	 */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
+
+
+Mux controller nodes
+--------------------
+
+Mux controller nodes must specify the number of cells used for the
+specifier using the '#mux-control-cells' property.
+
+Optionally, mux controller nodes can also specify the state the mux should
+have when it is idle. The idle-state property is used for this. If the
+idle-state is not present, the mux controller is typically left as is when
+it is idle. For multiplexer chips that expose several mux controllers, the
+idle-state property is an array with one idle state for each mux controller.
+
+The special value (-1) may be used to indicate that the mux should be left
+as is when it is idle. This is the default, but can still be useful for
+mux controller chips with more than one mux controller, particularly when
+there is a need to "step past" a mux controller and set some other idle
+state for a mux controller with a higher index.
+
+Some mux controllers have the ability to disconnect the input/output of the
+multiplexer. Using this disconnected high-impedance state as the idle state
+is indicated with idle state (-2).
+
+These constants are available in
+
+      #include <dt-bindings/mux/mux.h>
+
+as MUX_IDLE_AS_IS (-1) and MUX_IDLE_DISCONNECT (-2).
+
+An example mux controller node look like this (the adg972a chip is a triple
+4-way multiplexer):
+
+	mux: mux-controller@50 {
+		compatible = "adi,adg792a";
+		reg = <0x50>;
+		#mux-control-cells = <1>;
+
+		idle-state = <MUX_IDLE_DISCONNECT MUX_IDLE_AS_IS 2>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index c265a5fe4848..7fc06739c8ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8558,6 +8558,12 @@  S:	Orphan
 F:	drivers/mmc/host/mmc_spi.c
 F:	include/linux/spi/mmc_spi.h
 
+MULTIPLEXER SUBSYSTEM
+M:	Peter Rosin <peda@axentia.se>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mux/
+F:	include/linux/dt-bindings/mux/
+
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
 S:	Maintained
diff --git a/include/dt-bindings/mux/mux.h b/include/dt-bindings/mux/mux.h
new file mode 100644
index 000000000000..c8e855c4a609
--- /dev/null
+++ b/include/dt-bindings/mux/mux.h
@@ -0,0 +1,16 @@ 
+/*
+ * This header provides constants for most Multiplexer bindings.
+ *
+ * Most Multiplexer bindings specify an idle state. In most cases, the
+ * the multiplexer can be left as is when idle, and in some cases it can
+ * disconnect the input/output and leave the multiplexer in a high
+ * impedance state.
+ */
+
+#ifndef _DT_BINDINGS_MUX_MUX_H
+#define _DT_BINDINGS_MUX_MUX_H
+
+#define MUX_IDLE_AS_IS      (-1)
+#define MUX_IDLE_DISCONNECT (-2)
+
+#endif