Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1525467 > unrolled thread

Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for  mux-gpio Rob Herring <robh@kernel.org> - 2016-11-18 16:40 +0100
    Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for  mux-gpio Jonathan Cameron <jic23@kernel.org> - 2016-11-19 16:30 +0100

#1525467 — Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio

On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
> ---
>  .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> new file mode 100644
> index 000000000000..73699a37824f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> @@ -0,0 +1,79 @@
> +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.

I think this makes sense in your case, but I think it is too complicated 
for a non-shared case. Perhaps mux-gpios should be used directly (i.e. 
in the adc-mux node) and control-muxes only used for the shared case.

Part of me feels like you are working around in DT the GPIO subsystem 
limitation that it can't share GPIO lines. Either this could be fixed 
in some way in the GPIO subsystem, or the mux subsys could deal with it. 
You just have to look up if you already have a mux registered for the 
same GPIOs. Of course, that may make the mux subsys pretty much GPIO 
only, but I'm having a hard time thinking how you would have shared 
muxes that are not GPIO controlled. Any other control would be 
integrated into the mux itself.

> +
> +Required properties:
> +- compatible : "mux-gpio"
> +- mux-gpios : list of gpios used to control the multiplexer, least
> +	      significant bit first.
> +
> +Optional properties:
> +- idle-state : if present, the state the mux will have when idle.

Needs some detail on what the value is. One bit per gpio? One cell per 
gpio?

> +
> +Example:
> +	control_mux: control-adc-mux {
> +		compatible = "mux-gpio";
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "iio-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +
> +		control-muxes = <&control_mux>;

This is a common name? It should be in a common mux binding doc.

> +		control-mux-names = "mux";

I think this can be dropped at least until you have more than 1.

> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		sync-1@0 {
> +			reg = <0>;
> +		};
> +
> +		in@1 {
> +			reg = <1>;
> +		};
> +
> +		out@2 {
> +			reg = <2>;
> +		};
> +
> +		sync-2@3 {
> +			reg = <3>;
> +		};
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux-simple,mux-locked";
> +		i2c-parent = <&i2c1>;
> +
> +		control-muxes = <&control_mux>;
> +		control-mux-names = "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 {
> +				/* ... */
> +			};
> +		};
> +	};
> -- 
> 2.1.4
> 

[toc] | [next] | [standalone]

#1526012

On 18/11/16 16:59, Peter Rosin wrote:
> On 2016-11-18 16:35, Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
>>> ---
>>>  .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
>>>  1 file changed, 79 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> new file mode 100644
>>> index 000000000000..73699a37824f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> @@ -0,0 +1,79 @@
>>> +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.
>>
>> I think this makes sense in your case, but I think it is too complicated 
>> for a non-shared case. Perhaps mux-gpios should be used directly (i.e. 
>> in the adc-mux node) and control-muxes only used for the shared case.
>>
>> Part of me feels like you are working around in DT the GPIO subsystem 
>> limitation that it can't share GPIO lines. Either this could be fixed 
>> in some way in the GPIO subsystem, or the mux subsys could deal with it. 
>> You just have to look up if you already have a mux registered for the 
>> same GPIOs. Of course, that may make the mux subsys pretty much GPIO 
>> only, but I'm having a hard time thinking how you would have shared 
>> muxes that are not GPIO controlled. Any other control would be 
>> integrated into the mux itself.
> 
> But if someone wants to mux an adc line with a mux that is some kind of
> integrated i2c device, you'd have to add extra code to the iio muxer
> driver to handle that case. Or fork it. Or build something like the
> i2c muxer infrastructure and separate out the mux control in small
> drivers and handle the generic iio muxing centrally. But then someone
> else uses that i2c device to instead mux an i2c bus, and you'd end up
> with code duplication when that same muxer control code is added under
> drivers/i2c/muxes.
> 
> With the proposed solution, this is unified.
> 
> I'd just hate to see drivers for muxers added under drivers/i2c/muxes
> that do little more that control a mux that happens to be used to mux
> an i2c bus, but are generic muxers that could equally well mux something
> else. Even if the control is integrated into the mux, what the mux is
> actually used for should perhaps not determine where its driver should
> live.
> 
> Anyway, I don't know what to make with your suggestion, I just don't
> see the path forward (not enough experience with the kernel and/or gpio
> code). And it would be a limited solution (GPIO only,a s you say) so it
> doesn't feel right.
Also worth pointing out here the possibility of multi pole muxes...
Relays are ultimately muxes as well (be it slow ones ;)

A quick google fed me:
TI SN74LS153 for example.  This one is digital only though...

Analog option (in both senses) is:
http://www.analog.com/media/en/technical-documentation/data-sheets/ADG888.pdf

So these 'look' the same as two single muxes wired to the same GPIOs.

> 
> Is there perhaps some way to keep the complicated shared case work as
> is (or equivalently, the exact details are not important), and also
> provide a simpler in-node thingy to glue a mux control to a consumer
> w/o pointing to it with a phandle, but still have the same mux driver
> handle both cases? No, I'm not a devicetree guru, so I don't see a
> solution for that either, but maybe someone else does?
> 
> Perhaps the consumer could look for the mux control in first the
> phandle, as in my proposal. If not found, it could also look for
> a mux provider bound to child node.
> 
> 	adc-mux {
> 		compatible = "iio-mux";
> 		io-channels = <&adc 0>;
> 		io-channel-names = "parent";
> 
> 		mux-control {
> 			compatible = "mux-gpio";
> 			mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> 				    <&pioA 1 GPIO_ACTIVE_HIGH>;
> 		};
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		sync-1@0 {
> 			reg = <0>;
> 		};
> 		/* ... */
> 	};
> 
> Or perhaps look in a parent node:
> 
> 	mux-control {
> 		compatible = "mux-gpio";
> 		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> 			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> 
> 		adc-mux {
> 			compatible = "iio-mux";
> 			io-channels = <&adc 0>;
> 			io-channel-names = "parent";
> 
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			sync-1@0 {
> 				reg = <0>;
> 			};
> 			/* ... */
> 		};
> 	};
> 
> With the last suggestion, you could have multiple children of the
> mux-control node for the complicated case where it controls more
> than one mux. Not too bad? Hmm, what does the driver for the
> mux-control node have to do to have drivers tied to its children?
> 
> Maybe this last layout should be the only thing supported? Good
> enough for me anyway...
> 
> Cheers,
> Peter
> 
> PS. I will take care of the other comments for the next round.
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web