| Message ID | 20240930-wip-bhahn-tlv320aic3x_fix_reset-v1-1-c040d86a4b93@phytec.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | sound: soc: codecs: tlv320aic3x: Fix codec gpio-reset | expand |
On Mon, Sep 30, 2024 at 09:16:46AM +0200, Benjamin Hahn wrote: > The TLV320AIC3007 requires a hardware reset after power-up for proper > operation. After all power supplies are at their specified values, > the RESET pin must be driven low for at least 10 ns. Even though the > datasheet sais min 10ns, I needed more than 10ns when testing this out. > 15ns worked for me. Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone. > - if (aic3x->gpio_reset) > + if (aic3x->gpio_reset) { > gpiod_set_value(aic3x->gpio_reset, 1); > + ndelay(15); > + gpiod_set_value(aic3x->gpio_reset, 0); > + } This seems obviously buggy, it leaves the GPIO with the opposite state to that it would have prior to the patch. It's also not joined up with your changelog, that talks about actions taken after powering up the device but this is a callback run after power has been removed from the device so nothing in your changelog motivates leaving reset deasserted. > regcache_mark_dirty(aic3x->regmap); > } > > @@ -1813,6 +1816,10 @@ int aic3x_probe(struct device *dev, struct regmap *regmap, kernel_ulong_t driver > > gpiod_set_consumer_name(aic3x->gpio_reset, "tlv320aic3x reset"); > > + /* CODEC datasheet says minimum of 10ns */ > + ndelay(15); > + gpiod_set_value(aic3x->gpio_reset, 0); > + This seems more relevant to your changelog, but I don't understand why aic3x_set_power() is not also (instead?) updated?
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 56e795a00e22..d002fc0b99b5 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -1367,8 +1367,11 @@ static int aic3x_regulator_event(struct notifier_block *nb, * Put codec to reset and require cache sync as at least one * of the supplies was disabled */ - if (aic3x->gpio_reset) + if (aic3x->gpio_reset) { gpiod_set_value(aic3x->gpio_reset, 1); + ndelay(15); + gpiod_set_value(aic3x->gpio_reset, 0); + } regcache_mark_dirty(aic3x->regmap); } @@ -1813,6 +1816,10 @@ int aic3x_probe(struct device *dev, struct regmap *regmap, kernel_ulong_t driver gpiod_set_consumer_name(aic3x->gpio_reset, "tlv320aic3x reset"); + /* CODEC datasheet says minimum of 10ns */ + ndelay(15); + gpiod_set_value(aic3x->gpio_reset, 0); + for (i = 0; i < ARRAY_SIZE(aic3x->supplies); i++) aic3x->supplies[i].supply = aic3x_supply_names[i];
The TLV320AIC3007 requires a hardware reset after power-up for proper operation. After all power supplies are at their specified values, the RESET pin must be driven low for at least 10 ns. Even though the datasheet sais min 10ns, I needed more than 10ns when testing this out. 15ns worked for me. Signed-off-by: Benjamin Hahn <B.Hahn@phytec.de> --- sound/soc/codecs/tlv320aic3x.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) --- base-commit: ad46e8f95e931e113cb98253daf6d443ac244cde change-id: 20240930-wip-bhahn-tlv320aic3x_fix_reset-cca918a4889d Best regards,