Message-ID: <67bc8a1c-7067-700f-2b69-12a76d91b2ba@axentia.se>
Date:   Mon, 30 Jan 2017 09:02:52 +0100
From:   Peter Rosin <peda@...ntia.se>
To:     Rob Herring <robh@...nel.org>
CC:     Jonathan Cameron <jic23@...nel.org>,
        <linux-kernel@...r.kernel.org>, Wolfram Sang <wsa@...-dreams.de>,
        Mark Rutland <mark.rutland@....com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        <linux-i2c@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-iio@...r.kernel.org>, <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v8 12/12] mux: support simplified bindings for single-user
 gpio mux

On 2017-01-27 16:52, Rob Herring wrote:
> On Mon, Jan 23, 2017 at 11:24:18AM +0100, Peter Rosin wrote:
>> On 2017-01-22 14:30, Jonathan Cameron wrote:
>>> On 18/01/17 15:57, Peter Rosin wrote:
>>>> Allow bindings for a GPIO controlled mux to be specified in the
>>>> mux consumer node.
>>>>
>>>> Signed-off-by: Peter Rosin <peda@...ntia.se>
>>> Code is good as far as I am concerned. Only question is whether this
>>
>> Hmmm, now that I think some more about it, the code supporting the
>> simplified binding (patch 12/12) is a bit fishy in one respect.
>>
>> A driver that calls mux_control_get and gets a mux_control that happens
>> to be backed by an implicit mux chip (i.e. using the simplified binding)
>> will not be able to reverse the resource allocation with less than a
>> complete destruction of itself. Now, this is likely not a problem in
>> most cases, but I bet it will creep up at the most inopportune time. And
>> your remark that I'm the one that has to maintain this makes me dislike
>> this concept...
>>
>> I.e. mux_control_put *should* reverse mux_control_get, but this simply
>> does not happen for the implicit mux chips, as implicit mux chips are
>> not put away until the owning device is put away.
> 
> I think this is because you aren't creating a device in this case. Nodes 
> in DT are not the only way to create devices. Drivers can create a child 
> device when they find mux-gpios property.

Yes, but even with such a child device, a flag is needed somewhere that
triggers cleanup when the mux_control is put away. And then it is possible
to cleanup w/o the help of a child device. I wrote some code for this when
I realized the problem, and it looks simple enough, but I haven't tested
it yet, so who knows... It is attached (patch to be applied on top of 12/12)
if anyone cares.

>> Every time I have tried to come up with a way to implement the simplified
>> bindings I seem to hit one of these subtleties.
>>
>>> is worth the hassle given the normal bindings don't give that high
>>> a burden in complexity!
> 
> I was going to change my mind here, but we already have "mux-gpios" as a 
> binding at least for i2c-gpio-mux. So really the question is do we want 
> to support that here?

I think my preference is to drop the simplified binding, but I can also
live with it. But as there appears to be no strong feelings, let's just
drop it. It is always possible to add it later. Ok?

>> I am missing an ack from Rob though.
>>
>>> I don't really care either way:)
>>
>> But Rob seems to care, this series just has to find a way to get out of
>> his too-much-churn-will-look-at-it-later list. I sadly don't know how to
>> pull that trick...
> 
> By complaining that I'm putting it off... :) I guess I'm okay with this 
> series in general. I will reply on the specific patches today.

Great, it appears that I'm quite the magician. :-) Thanks!

Cheers,
peda


View attachment "0001-mux-fix-cleanup-for-simplified-bindings.patch" of type "text/plain" (2403 bytes)

Powered by blists - more mailing lists