| Message ID | 1437154736-15320-1-git-send-email-anatol.pomozov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 17, 2015 at 10:38:56AM -0700, Anatol Pomozov wrote: > According to the datasheet RESET is active low pin, i.e. system goes to reset > state when pin is low. > > Handle logic state correctly - set reset_pin to logical 0 at boot time, and > set it to logical 1 when we need to reset the chip. I'm a bit confused here - how was the original code (written by you it seems...) tested? This looks like it's fixing the device being held in reset when it should be operation which seems like something that'd get noticed. Are there existing systems that the current code works for which we need to handle here?
Hi Returning back to this discussion that slipped away from my mailbox. On Fri, Aug 14, 2015 at 12:01 PM, Mark Brown <broonie@kernel.org> wrote: > On Fri, Jul 17, 2015 at 10:38:56AM -0700, Anatol Pomozov wrote: >> According to the datasheet RESET is active low pin, i.e. system goes to reset >> state when pin is low. >> >> Handle logic state correctly - set reset_pin to logical 0 at boot time, and >> set it to logical 1 when we need to reset the chip. > > I'm a bit confused here - how was the original code (written by you it > seems...) tested? This looks like it's fixing the device being held in > reset when it should be operation which seems like something that'd get > noticed. Are there existing systems that the current code works for > which we need to handle here? While working with the original patch you mentioned I missed the active low description at RESET pin. I configured gpio in the dts as ACTIVE_HIGH and set the logic state to 0 at suspend mode. This puts the chip into reset state correctly. Later I discovered correct polarity and given change modifies gpio properties accordingly. I also changed our dts gpio to ACTIVE_LOW. Chip reset state still works as expected. The change does not modify anything at the signalling level, it just adds double negative to the code. Compare: pin configured as ACTIVE_HIGH - to move codec out of reset we need to set gpio logic level to 1. Physical signal is 1. pin configured as ACTIVE_LOW - to move codec out of reset we need to set gpio logic level to 0. Gpio becomes non-active that is represented by physical signal level 1.
On Wed, Oct 28, 2015 at 12:55:14PM -0700, Anatol Pomozov wrote:
> Returning back to this discussion that slipped away from my mailbox.
I'm afraid I no longer remember what this is about, sorry - if the patch
is needed please resend it.
diff --git a/Documentation/devicetree/bindings/sound/rt5677.txt b/Documentation/devicetree/bindings/sound/rt5677.txt index f070789..1b3c13d 100644 --- a/Documentation/devicetree/bindings/sound/rt5677.txt +++ b/Documentation/devicetree/bindings/sound/rt5677.txt @@ -18,7 +18,7 @@ Required properties: Optional properties: - realtek,pow-ldo2-gpio : The GPIO that controls the CODEC's POW_LDO2 pin. -- realtek,reset-gpio : The GPIO that controls the CODEC's RESET pin. +- realtek,reset-gpio : The GPIO that controls the CODEC's RESET pin. Active low. - realtek,in1-differential - realtek,in2-differential diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c index 3b46b35..433652f 100644 --- a/sound/soc/codecs/rt5677.c +++ b/sound/soc/codecs/rt5677.c @@ -4766,7 +4766,7 @@ static int rt5677_remove(struct snd_soc_codec *codec) if (rt5677->pow_ldo2) gpiod_set_value_cansleep(rt5677->pow_ldo2, 0); if (rt5677->reset_pin) - gpiod_set_value_cansleep(rt5677->reset_pin, 0); + gpiod_set_value_cansleep(rt5677->reset_pin, 1); return 0; } @@ -4783,7 +4783,7 @@ static int rt5677_suspend(struct snd_soc_codec *codec) if (rt5677->pow_ldo2) gpiod_set_value_cansleep(rt5677->pow_ldo2, 0); if (rt5677->reset_pin) - gpiod_set_value_cansleep(rt5677->reset_pin, 0); + gpiod_set_value_cansleep(rt5677->reset_pin, 1); } return 0; @@ -4797,7 +4797,7 @@ static int rt5677_resume(struct snd_soc_codec *codec) if (rt5677->pow_ldo2) gpiod_set_value_cansleep(rt5677->pow_ldo2, 1); if (rt5677->reset_pin) - gpiod_set_value_cansleep(rt5677->reset_pin, 1); + gpiod_set_value_cansleep(rt5677->reset_pin, 0); if (rt5677->pow_ldo2 || rt5677->reset_pin) msleep(10); @@ -5142,7 +5142,7 @@ static int rt5677_i2c_probe(struct i2c_client *i2c, rt5677->pow_ldo2 = 0; } rt5677->reset_pin = devm_gpiod_get_optional(&i2c->dev, - "realtek,reset", GPIOD_OUT_HIGH); + "realtek,reset", GPIOD_OUT_LOW); if (IS_ERR(rt5677->reset_pin)) { ret = PTR_ERR(rt5677->reset_pin); dev_err(&i2c->dev, "Failed to request RESET: %d\n", ret);
According to the datasheet RESET is active low pin, i.e. system goes to reset state when pin is low. Handle logic state correctly - set reset_pin to logical 0 at boot time, and set it to logical 1 when we need to reset the chip. Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> --- Documentation/devicetree/bindings/sound/rt5677.txt | 2 +- sound/soc/codecs/rt5677.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)