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

[Jonathan Cameron]

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@xxxxxxxxxx>
>>>>> ---
>>>>>  .../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
>