linux-kernel - Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20181011094701eucas1p2177080d98481b4d4ebcfad9960d7dd97~chNSImIbW1044610446eucas1p2Q@eucas1p2.samsung.com>
Date:   Thu, 11 Oct 2018 11:46:59 +0200
From:   Marek Szyprowski <m.szyprowski@...sung.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Janusz Krzysztofik <jmkrzyszt@...il.com>,
        Alexander Shiyan <shc_work@...l.ru>,
        Haojian Zhuang <haojian.zhuang@...il.com>,
        Aaro Koskinen <aaro.koskinen@....fi>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Robert Jarzmik <robert.jarzmik@...e.fr>,
        Philipp Zabel <philipp.zabel@...il.com>,
        Daniel Mack <zonque@...il.com>,
        Marc Zyngier <marc.zyngier@....com>,
        jacopo <jacopo@...ndi.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor
 only

Hi Linus,

On 2018-10-11 11:29, Linus Walleij wrote:
> On Thu, Oct 11, 2018 at 11:01 AM Marek Szyprowski
> <m.szyprowski@...sung.com> wrote:
>
>> 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.

>> This issue has been already pointed in case of commits:
>> 37fa23dbccbd97663acc085bd79246f427e603a1
>> d1dae72fab2c377ff463742eefd8ac0f9e99b7b9
>> ab4d11e2c2329cf7cb7be31ff22489aae4dee5dc
> A big sorry for my ignorance, I guess the information overload
> on the mailing list just makes me miss the important points.
> I'll try to be better, sadly I constantly fail to keep everything
> in mind and constantly break things like this.
>
>> Maybe it would be better to first solve the handling of shared enable
>> GPIO in the descriptor-based interface before converting more regulators
>> and stepping into this issue again?
> I am trying to solve it, but I just don't have systems to reproduce all
> kinds of things. It's a bit stressful since this is one of those runtime
> things that is hard to test when devising a patch for systems I don't
> have.
>
> I am kind of relying on system maintainers to test things.
>
> 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.

> Anyways. Let's try to fix it now then instead of me constantly
> hitting this and breaking it. It happens because it is just unintuitive
> so I share your frustration.

I try to test linux-next daily to catch such thing as early as possible
and help others to fix their code. Feel free add me on cc: so I will
test it.

> I don't want to generally make it possible for a gpio line to be
> retrieved nonexclusively. We need the semantics to help people
> do the right thing.
>
> 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.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland