linux-kernel - Re: [PATCH 8/9] ASoC: tlv320aic31xx: Fix the reset GPIO OF name

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: <1374534b-ac17-82ef-7b3b-3c6e4d4c5cc9@ti.com>
Date:   Wed, 8 Nov 2017 17:25:20 -0600
From:   "Andrew F. Davis" <afd@...com>
To:     Mark Brown <broonie@...nel.org>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Benoît Cousson <bcousson@...libre.com>,
        Tony Lindgren <tony@...mide.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        <alsa-devel@...a-project.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 8/9] ASoC: tlv320aic31xx: Fix the reset GPIO OF name

On 11/08/2017 04:18 PM, Mark Brown wrote:
> On Wed, Nov 08, 2017 at 03:53:51PM -0600, Andrew F. Davis wrote:
>> On 11/08/2017 03:36 PM, Mark Brown wrote:
>>> On Wed, Nov 08, 2017 at 03:25:04PM -0600, Andrew F. Davis wrote:
> 
>>> This is obviously an incompatible change in the binding which will break
>>> any production DTs relying on the current behaviour.  You need to keep
>>> support for the existing property.
> 
>> I understand the reasons not to change driver behavior wrt DT, but this
>> driver did not make functional use of this gpio, only going forward will
>> this gpio be used for actually reseting the device (in some patches I
>> will post soon). So I would like to fix this incorrect binding *before*
>> fixing it will cause behavior incompatibilities.
> 
> There is code in the driver to use the GPIO, including in the probe
> where the GPIO is requested and set to high (which will bring it out of
> reset if the default state was low).  At least the probe seems rather
> likely to have a concrete effect.
> 

None of the 4 boards that defined this gpio have this pulled low in the
pin control, boards should have an external pull-up on the reset line.

At any-rate, I'm pushing this fix to allow the driver to use new kernel
frameworks that depend on the correct binding being used. This is not a
case of a badly designed binding or trying to add incompatible
functionality, it is a typo fix. If we have to keep this driver back
using old frameworks and needlessly bloat the code solely for the sake
of compatibility with a typo, then DT "stability" here is causing more
issues than it solves. </rant>

I guess the alternative would be to have of_find_gpio() also consider
prefixing, the 'gpio_suffixes' to 'con_id' and checking for those. That
is rather ugly and probably encourages the spread of this bad binding
property, but whatever the best fix is it cannot be to force bloat into
the driver.

>>> It also doesn't look like a good fix if we're aiming for conformance
>>> with DT naming conventions as unless things changed all GPIO related
>>> properties are supposed to end -gpios even if they can only ever specify
>>> a single GPIO.
> 
>> If that is the new standard I can fix this patch to use -gpios.
> 
> It's always been the standard AFAIK.
> 

Will fix for v2.