On Thu, Oct 11, 2018 at 11:46:59AM +0200, Marek Szyprowski wrote:
> On 2018-10-11 11:29, Linus Walleij wrote:
> > On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski

> >> I've just noticed that this patch causes regression on Samsung
> >> Exynos4412-based Trats2 board. Conversion to GPIO descriptor breaks
> >> operation when regulators used shared GPIO:  sii9234 i2c driver
> >> is not able to get vcc33mhl regulator (it uses shared GPIO enable
> >> line with vsil12 regulator).

> > So I guess this means that this physical GPIO line will enable the
> > vcc33mhl and the vsil12 regulators at the same time?

> Right. It is so common case, that regulator core has special code for
> handling shared enable GPIO.

Yes, and we've discussed it several times already.

> > I was aware of the usecase "several consumers takes the same
> > GPIO line" (Mark told me several times...) so it was in the back of
> > my mind, but it's just hard to see when we were gonna run into it.
> > So it is the fixed regulators, on Samsung boards, I see.

> I don't think this is Samsung specific. I saw similar solution on various
> other boards too. Sharing enable gpio is rather common thing. The issue
> happens if there are separate drivers for each hw block and they need to
> enable it from their code.

It's really common, yeah - things like controlling the power for an
entire chip with one GPIO for all the regulators supplying that chip for
example.  I think what you're seeing here is that Samsung are among the
most active testers of -next (thanks!) rather than that their hardware
is particularly weird.

> > So I was thinking to introduce
> > gpiod_get_nonexclusive() to explicitly handle this case for the few
> > systems that use shared GPIO lines, let me see what I can do.

> The old interface also didn't allow sharing GPIO easily, so regulator
> core has special code for shared enable gpio. However I still have no
> idea how to do this cleanly using descriptor.

Yes, it's been discussed several times and I thought Linus had some idea
for it - IIRC it was a gpiod_is_equal() or something.  You can also do
this by converting descriptors back to numbers and comparing the numbers
but obviously the numbers are supposed to be being removed.