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

Groups > linux.kernel > #1536524 > unrolled thread

Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings

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: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux  bindings Rob Herring <robh@kernel.org> - 2016-12-06 00:30 +0100
    Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux  bindings Jonathan Cameron <jic23@kernel.org> - 2016-12-10 19:30 +0100
      Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux  bindings Jonathan Cameron <jic23@kernel.org> - 2016-12-31 17:10 +0100

#1536524 — Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings

On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../bindings/iio/multiplexer/iio-mux.txt           | 40 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 ++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt

I'm still not convinced about this binding, but don't really have more 
comments ATM. Sending 6 versions in 2 weeks or so doesn't really help 
either.

> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
> new file mode 100644
> index 000000000000..8080cf790d82
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
> @@ -0,0 +1,40 @@
> +IIO multiplexer bindings
> +
> +If a multiplexer is used to select which hardware signal is fed to
> +e.g. an ADC channel, these bindings describe that situation.
> +
> +Required properties:
> +- compatible : "iio-mux"

This is a Linuxism. perhaps "adc-mux".

> +- io-channels : Channel node of the parent channel that has multiplexed
> +		input.
> +- io-channel-names : Should be "parent".
> +- #address-cells = <1>;
> +- #size-cells = <0>;
> +- mux-controls : Mux controller node to use for operating the mux
> +- channels : List of strings, labeling the mux controller states.
> +
> +The multiplexer state as described in ../misc/mux-controller.txt
> +
> +For each non-empty string in the channels property, an iio channel will
> +be created. The number of this iio channel is the same as the index into
> +the list of strings in the channels property, and also matches the mux
> +controller state.
> +
> +Example:
> +	mux: mux-controller {
> +		compatible = "mux-gpio";
> +		#mux-control-cells = <0>;
> +
> +		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";
> +
> +		mux-controls = <&mux>;
> +
> +		channels = "sync", "in", system-regulator";
> +	};

[toc] | [next] | [standalone]

#1539884

On 06/12/16 09:18, Peter Rosin wrote:
> On 2016-12-06 00:26, Rob Herring wrote:
>> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>> ---
>>>  .../bindings/iio/multiplexer/iio-mux.txt           | 40 ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  6 ++++
>>>  2 files changed, 46 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>
>> I'm still not convinced about this binding, but don't really have more 
>> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help 
>> either.
> 
> Sorry about the noise, I'll try to be more careful going forward. On
> the flip side, I haven't touched the code since v6.
> 
> I don't see how bindings that are as flexible as the current (and
> original) phandle link between the mux consumer and the mux controller
> would look, and at the same time be simpler to understand. You need
> to be able to refer to a mux controller from several mux consumers, and
> you need to support several mux controllers in one node (the ADG792A
> case). And, AFAICT, the complex case wasn't really the problem, it was
> that it is overly complex to describe the simple case of one mux
> consumer and one mux controller. But in your comment for v2 [1] you
> said that I was working around limitations with shared GPIO pins. But
> solving that in the GPIO subsystem would not solve all that the
> phandle approach is solving, since you would not have support for
> ADG792A (or other non-GPIO controlled muxes). So, I think listing
> the gpio pins inside the mux consumer node is a non-starter, the mux
> controller has to live in its own node with its own compatible.
> 
> Would you be happier if I managed to marry the phandle approach with
> the option of having the mux controller as a child node of the mux
> consumer for the simple case?
> 
> I added an example at the end of this message (the same as the first
> example in v4 [2], at least in principle) for easy comparison between
> the phandle and the controller-in-child-node approaches. I can't say
> that I personally find the difference all that significant, and do not
> think it is worth it. As I see it, the "simple option" would just muddy
> the waters...
> 
> [1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
> [2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
> 
>>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>> new file mode 100644
>>> index 000000000000..8080cf790d82
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>> @@ -0,0 +1,40 @@
>>> +IIO multiplexer bindings
>>> +
>>> +If a multiplexer is used to select which hardware signal is fed to
>>> +e.g. an ADC channel, these bindings describe that situation.
>>> +
>>> +Required properties:
>>> +- compatible : "iio-mux"
>>
>> This is a Linuxism. perhaps "adc-mux".
> 
> No, that's not general enough, it could just as well be used to mux a
> temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
> "io-channel-mux" is better? That matches the io-channels property used to
> refer to the parent channel.
analog-mux maybe? Makes more sense out of context (though with io-channels defined on
the next line you have plenty of context here ;)
> 
>>> +- io-channels : Channel node of the parent channel that has multiplexed
>>> +		input.
>>> +- io-channel-names : Should be "parent".
>>> +- #address-cells = <1>;
>>> +- #size-cells = <0>;
>>> +- mux-controls : Mux controller node to use for operating the mux
>>> +- channels : List of strings, labeling the mux controller states.
>>> +
>>> +The multiplexer state as described in ../misc/mux-controller.txt
>>> +
>>> +For each non-empty string in the channels property, an iio channel will
>>> +be created. The number of this iio channel is the same as the index into
>>> +the list of strings in the channels property, and also matches the mux
>>> +controller state.
>>> +
>>> +Example:
>>> +	mux: mux-controller {
>>> +		compatible = "mux-gpio";
>>> +		#mux-control-cells = <0>;
>>> +
>>> +		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";
>>> +
>>> +		mux-controls = <&mux>;
>>> +
>>> +		channels = "sync", "in", system-regulator";
>>> +	};
> 
> Describing the same as above, but with the mux controller as a child
> node.
> 
> 	adc-mux {
> 		compatible = "iio-mux";
> 		io-channels = <&adc 0>;
> 		io-channel-names = "parent";
> 
> 		channels = "sync", "in", system-regulator";
> 
> 		mux-controller {
> 			compatible = "mux-gpio";
> 
> 			mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> 				    <&pioA 1 GPIO_ACTIVE_HIGH>;
> 		};
> 	};
> 
> Cheers,
> Peter
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]

#1548813

On 12/12/16 12:18, Peter Rosin wrote:
> On 2016-12-10 19:21, Jonathan Cameron wrote:
>> On 06/12/16 09:18, Peter Rosin wrote:
>>> On 2016-12-06 00:26, Rob Herring wrote:
>>>> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>> ---
>>>>>  .../bindings/iio/multiplexer/iio-mux.txt           | 40 ++++++++++++++++++++++
>>>>>  MAINTAINERS                                        |  6 ++++
>>>>>  2 files changed, 46 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>
>>>> I'm still not convinced about this binding, but don't really have more 
>>>> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help 
>>>> either.
>>>
>>> Sorry about the noise, I'll try to be more careful going forward. On
>>> the flip side, I haven't touched the code since v6.
>>>
>>> I don't see how bindings that are as flexible as the current (and
>>> original) phandle link between the mux consumer and the mux controller
>>> would look, and at the same time be simpler to understand. You need
>>> to be able to refer to a mux controller from several mux consumers, and
>>> you need to support several mux controllers in one node (the ADG792A
>>> case). And, AFAICT, the complex case wasn't really the problem, it was
>>> that it is overly complex to describe the simple case of one mux
>>> consumer and one mux controller. But in your comment for v2 [1] you
>>> said that I was working around limitations with shared GPIO pins. But
>>> solving that in the GPIO subsystem would not solve all that the
>>> phandle approach is solving, since you would not have support for
>>> ADG792A (or other non-GPIO controlled muxes). So, I think listing
>>> the gpio pins inside the mux consumer node is a non-starter, the mux
>>> controller has to live in its own node with its own compatible.
>>>
>>> Would you be happier if I managed to marry the phandle approach with
>>> the option of having the mux controller as a child node of the mux
>>> consumer for the simple case?
>>>
>>> I added an example at the end of this message (the same as the first
>>> example in v4 [2], at least in principle) for easy comparison between
>>> the phandle and the controller-in-child-node approaches. I can't say
>>> that I personally find the difference all that significant, and do not
>>> think it is worth it. As I see it, the "simple option" would just muddy
>>> the waters...
>>>
>>> [1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
>>> [2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>> new file mode 100644
>>>>> index 000000000000..8080cf790d82
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>> @@ -0,0 +1,40 @@
>>>>> +IIO multiplexer bindings
>>>>> +
>>>>> +If a multiplexer is used to select which hardware signal is fed to
>>>>> +e.g. an ADC channel, these bindings describe that situation.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : "iio-mux"
>>>>
>>>> This is a Linuxism. perhaps "adc-mux".
>>>
>>> No, that's not general enough, it could just as well be used to mux a
>>> temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
>>> "io-channel-mux" is better? That matches the io-channels property used to
>>> refer to the parent channel.
>> analog-mux maybe? Makes more sense out of context (though with io-channels defined on
>> the next line you have plenty of context here ;)
> 
> Not that I care all that much about the name, but that doesn't really
> fit if you take e.g. an IIO_INDEX channel. That sounds entirely non-analog
> to me, but what do I know? Maybe that example doesn't make sense for some
> reason, but I can't help but think that there will be some non-analog
> channel in the future, if there isn't one already.
> 
> So, my preference is io-channel-mux, as that matches the previous dt
> naming for what is muxed. But that's just my opinion, if I'm told that
> it should be something else, then that's ok.
io-channel-mux works fine for me. It's some sort of input / output channel
and we are muxing it ;)
> 
> I'm more worried about other aspects, such as how to get reviewers and who
> is going to take the core mux patches and what tree they are going to be
> merged into etc. That is, if this series is going anywhere at all or if
> someone is going to put up a road-block for some reason...
Whilst it is meeting some resistance, I'm not seeing any absolute blockers
(people tend to be rather explicit about that).  The binding is still causing
the most friction I think and it may be that it just needs some more time for
Rob to mull it over. It's a fiddly thing to describe, so was never going
to drop straight in!

The core mux patches probably need to go one of a few possible routes.

1. Directly as a pull to linus with a good collection of Acks.
2. Via Greg KH perhaps as generic driver infrastructure, or Andrew Morton
as being in the category no one else will take.
3. Via either me or Wolfram (as a separate immutable branch) on the basis it's
core stuff but the users currently are IIO and I2C.

In any case, this needs at the very least Acks from Rob, Wolfram and myself.
Others would be most welcome, Arnd and/or Greg might be persuaded to take
a look for example...

Happy New Year,

Jonathan
> 
> Cheers,
> peda
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web