Re: [PATCH v6 8/9] dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G mux

[Jonathan Cameron <jic23@kernel.org>]

On 02/01/17 16:01, Peter Rosin wrote:
> On 2017-01-01 12:00, Jonathan Cameron wrote:
>> On 30/11/16 08:17, Peter Rosin wrote:
>>> Analog Devices ADG792A/G is a triple 4:1 mux.
>>>
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> Few comments inline.  Worth adding anything about the gpio (output pins) to
>> the binding at this stage as well?  Would certainly be nice to support
>> them.
> 
> I'll add optional properties "gpio-controller;" and "#gpio-cells = <2>;"
> with the usual interpretation in v7 (but no implementation...) Is that
> enough?
> 
>> Jonathan
>>> ---
>>>  .../devicetree/bindings/misc/mux-adg792a.txt       | 64 ++++++++++++++++++++++
>>>  1 file changed, 64 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/misc/mux-adg792a.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/mux-adg792a.txt b/Documentation/devicetree/bindings/misc/mux-adg792a.txt
>>> new file mode 100644
>>> index 000000000000..4677f9ab1c55
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/mux-adg792a.txt
>>> @@ -0,0 +1,64 @@
>>> +Bindings for Analog Devices ADG792A/G Triple 4:1 Multiplexers
>>> +
>>> +Required properties:
>>> +- compatible : "adi,adg792a" or "adi,adg792g"
>>> +- #mux-control-cells : <0> if parallel, or <1> if not.
>>> +* Standard mux-controller bindings as decribed in mux-controller.txt
>>> +
>>> +Optional properties:
>>> +- adi,parallel : if present, the three muxes are bound together with a single
>>> +  mux controller, controlling all three muxes in parallel.
>>> +- adi,idle-state : if present, array of states the three mux controllers will
>>> +  have when idle (or, if parallel, a single idle-state).
>> Hmm. These are actually a policy decision.  As only one policy will make
>> sense for a given set of hardware probably fine to have it in here I guess.
>> Might be worth adding a note to say this though.
> 
> I don't really know what you want me to add, do you have a suggestion for the
> wording?
> 
>>> +
>>> +Mux controller states 0 through 3 correspond to signals A through D in the
>>> +datasheet. Mux controller states 4 and 5 are only available as possible idle
>>> +states. State 4 represents that nothing is connected, and state 5 represents
>>> +that the mux controller keeps the mux in its previously selected state during
>>> +the idle period. State 5 is the default idle state.
>> I'm never a great fan of magic numbers.  Can we represent this more cleanly by
>> breaking it into multiple properties?
>> Optional:
>> adi,idle-switch-to-channel : switch to this channel when idle.
>> adi,idle-high-impedance : <boolean> the nothing connected state?
>>
>> If neither present leaves it in previous state?
> 
> It's not that easy. adi,idle-state is an array when there are three single
> pole quadruple throw muxes, so there really needs to be a number for each
> desired idle-behavior. Unless you have a better idea for how to describe
> that?
The above with arrays for each of the two parameters?
Though then you need a priority documented - I'd say high impedance overrides
the channel selection if both are present.
> 
> Cheers,
> peda
> 
>>> +
>>> +Example:
>>> +
>>> +	/* three independent mux controllers (of which one is used) */
>>> +	&i2c0 {
>>> +		mux: adg792a@50 {
>>> +			compatible = "adi,adg792a";
>>> +			reg = <0x50>;
>>> +			#mux-control-cells = <1>;
>>> +		};
>>> +	};
>>> +
>>> +	adc-mux {
>>> +		compatible = "iio-mux";
>>> +		io-channels = <&adc 0>;
>>> +		io-channel-names = "parent";
>>> +
>>> +		mux-controls = <&mux 1>;
>>> +
>>> +		channels = "sync-1", "", "out";
>>> +	};
>>> +
>>> +
>>> +	/*
>>> +	 * Three parallel muxes with one mux controller, useful e.g. if
>>> +	 * the adc is differential, thus needing two signals to be muxed
>>> +	 * simultaneously for correct operation.
>>> +	 */
>>> +	&i2c0 {
>>> +		pmux: adg792a@50 {
>>> +			compatible = "adi,adg792a";
>>> +			reg = <0x50>;
>>> +			#mux-control-cells = <0>;
>>> +			adi,parallel;
>>> +		};
>>> +	};
>>> +
>>> +	diff-adc-mux {
>>> +		compatible = "iio-mux";
>>> +		io-channels = <&adc 0>;
>>> +		io-channel-names = "parent";
>>> +
>>> +		mux-controls = <&pmux>;
>>> +
>>> +		channels = "sync-1", "", "out";
>>> +	};
>>>
>>
>