On 11/09/2017 02:26 PM, Benoît Thébaudeau wrote:
> Dear Andrew F. Davis,
>
> On Wed, Nov 8, 2017 at 10:24 PM, Andrew F. Davis <afd@xxxxxx> wrote:
>> The property used to specify a GPIO intended for reset is "reset-gpio",
>> this binding uses "gpio-reset", as almost all other bindings use the
>> former name this use of the latter is certainly not intended and
>> was a typo. It is not compatible with newer methods used to fetch
>> GPIO pins and to prevent the spread of this error to other bindings
>> lets fix this here.
>>
>> We also standardize the pin as active-low, different device trees have
>> marked the GPIO different ways, luckily the driver currently uses the
>> low-level GPIO set function which does not respect the active-low flag,
>> but future changes may change this. This is an active-low reset, mark
>> it as such.
>>
>> Lastly, add an example of use for this property.
>>
>> Fixes: c24fdc886fde ("ASoC: tlv320aic3x: Add device tree bindings")
>>
>> Signed-off-by: Andrew F. Davis <afd@xxxxxx>
>> ---
>> Documentation/devicetree/bindings/sound/tlv320aic3x.txt | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
>> index ba5b45c483f5..9e8eaa08ce90 100644
>> --- a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
>> +++ b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
>> @@ -17,7 +17,7 @@ Required properties:
>>
>> Optional properties:
>>
>> -- gpio-reset - gpio pin number used for codec reset
>> +- reset-gpio - GPIO specification for the active low RESET input.
>> - ai3x-gpio-func - <array of 2 int> - AIC3X_GPIO1 & AIC3X_GPIO2 Functionality
>> - Not supported on tlv320aic3104
>> - ai3x-micbias-vg - MicBias Voltage required.
>> @@ -61,10 +61,14 @@ The pins can be used in referring sound node's audio-routing property.
>>
>> Example:
>>
>> +#include <dt-bindings/gpio/gpio.h>
>> +
>> tlv320aic3x: tlv320aic3x@1b {
>> compatible = "ti,tlv320aic3x";
>> reg = <0x1b>;
>>
>> + reset-gpio = <&gpio1 17 GPIO_ACTIVE_LOW>;
>> +
>> AVDD-supply = <®ulator>;
>> IOVDD-supply = <®ulator>;
>> DRVDD-supply = <®ulator>;
>
> According to Documentation/devicetree/bindings/gpio/gpio.txt:
> "GPIO properties should be named "[<name>-]gpios", with <name> being the purpose
> of this GPIO for the device. While a non-existent <name> is considered valid
> for compatibility reasons (resolving to the "gpios" property), it is not allowed
> for new bindings. Also, GPIO properties named "[<name>-]gpio" are valid and old
> bindings use it, but are only supported for compatibility reasons and should not
> be used for newer bindings since it has been deprecated."
>
> So it should be "reset-gpios" for new bindings.
>
You are the third person to comment this. I wish people would have been
this observant when the original patch adding the completely backwards
"gpio-reset" was posted... :)
> Also, sound/soc/codecs/tlv320aic3x.c still uses "gpio-reset", and I do
> not see any patch in your series changing this:
> ret = of_get_named_gpio(np, "gpio-reset", 0);
>
Check patch #9
> Same remarks for your "[PATCH 6/9] ARM: dts: imx: Fix the audio
> CODEC's reset pin". I don't know if there are other DTS files using
> this, but they should all be updated accordingly. This would anyway
> break all of the out-of-tree DTS files using this, which I think is
> not allowed, so the driver should be changed in a way still allowing
> the legacy naming besides the new one.
>
I have updated every in-tree use. I know we should work to avoid
breaking out-of-tree DTS files, but I'm asking for a small exception
here for reasons I've described in the cover-letter and other patches.
I just can't see this breaking anything, if there actually exists a real
out-of-tree user of this with a DTB that is not being updated with the
kernel version and this patch causes any functionality change for them I
will literally eat a hat.
Thanks,
Andrew
> Best regards,
> Benoît
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
