| Message ID | 1430358238-74659-3-git-send-email-yang.a.fang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 29, 2015 at 06:43:57PM -0700, yang.a.fang@intel.com wrote: > @@ -296,7 +303,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c, > } > > ret = devm_request_threaded_irq(dev, i2c->irq, NULL, ts3a227e_interrupt, > - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > + ts3a227e->pdata.irqflag, > "TS3A227E", ts3a227e); > if (ret) { > dev_err(dev, "Cannot request irq %d (%d)\n", i2c->irq, ret); This doesn't make much sense to me - no change is made to the configuration of the device, the driver just passes the flag through to the interrupt controller code. I can't see how that's going to work well, somewhere along the line something must be misconfigured.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Thursday, April 30, 2015 1:02 PM > To: Fang, Yang A > Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; > dgreid@chromium.org; Koul, Vinod; Diwakar, Praveen; > kevin.strasser@linux.intel.com; Jain, Praveen K; Iriawan, Denny > Subject: Re: [PATCH 3/4] ASoC: ts3a227e: add platform data for ts3a227e > driver > > On Wed, Apr 29, 2015 at 06:43:57PM -0700, yang.a.fang@intel.com wrote: > > > @@ -296,7 +303,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c, > > } > > > > ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > ts3a227e_interrupt, > > - IRQF_TRIGGER_LOW | > IRQF_ONESHOT, > > + ts3a227e->pdata.irqflag, > > "TS3A227E", ts3a227e); > > if (ret) { > > dev_err(dev, "Cannot request irq %d (%d)\n", i2c->irq, ret); > > This doesn't make much sense to me - no change is made to the > configuration of the device, the driver just passes the flag through to the > interrupt controller code. I can't see how that's going to work well, > somewhere along the line something must be misconfigured. Hi Mark, My intention is to make irqflag configurable. I made them two patches. Patch 3/4 to make sure existing code no impact. 4/4 is set different Irq value via DMI for specific platform. Maybe I should squash them? Thanks, Yang
On Thu, Apr 30, 2015 at 08:45:40PM +0000, Fang, Yang A wrote: > > ts3a227e_interrupt, > > > - IRQF_TRIGGER_LOW | > > IRQF_ONESHOT, > > > + ts3a227e->pdata.irqflag, > > > "TS3A227E", ts3a227e); > > > if (ret) { > > > dev_err(dev, "Cannot request irq %d (%d)\n", i2c->irq, ret); > > This doesn't make much sense to me - no change is made to the > > configuration of the device, the driver just passes the flag through to the > > interrupt controller code. I can't see how that's going to work well, > > somewhere along the line something must be misconfigured. > My intention is to make irqflag configurable. I made them two patches. > Patch 3/4 to make sure existing code no impact. 4/4 is set different > Irq value via DMI for specific platform. Maybe I should squash them? No, you're missing the point - if you're changing the flags for the interrupt without also reconfiguring the hardware there's a bug. This seems like it's what Dylan identified with missing interrupts, it sounds like the device is always level triggered.
diff --git a/include/sound/ts3a227e.h b/include/sound/ts3a227e.h new file mode 100644 index 0000000..c005084 --- /dev/null +++ b/include/sound/ts3a227e.h @@ -0,0 +1,19 @@ +/* + * Platform data for TS3A227E + * + * Copyright (C) 2015 Google, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __LINUX_SND_TS3A227E_H +#define __LINUX_SND_TS3A227E_H + +struct ts3a227e_platform_data { + + unsigned long irqflag; +}; + +#endif diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c index 9fd80ac..422f66f 100644 --- a/sound/soc/codecs/ts3a227e.c +++ b/sound/soc/codecs/ts3a227e.c @@ -25,6 +25,7 @@ struct ts3a227e { struct regmap *regmap; struct snd_soc_jack *jack; + struct ts3a227e_platform_data pdata; bool plugged; bool mic_present; unsigned int buttons_held; @@ -274,6 +275,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c, { struct ts3a227e *ts3a227e; struct device *dev = &i2c->dev; + struct ts3a227e_platform_data *pdata = dev_get_platdata(&i2c->dev); int ret; unsigned int acc_reg; @@ -283,6 +285,11 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c, i2c_set_clientdata(i2c, ts3a227e); + ts3a227e->pdata.irqflag = IRQF_TRIGGER_LOW | IRQF_ONESHOT; + + if (pdata) + ts3a227e->pdata = *pdata; + ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config); if (IS_ERR(ts3a227e->regmap)) return PTR_ERR(ts3a227e->regmap); @@ -296,7 +303,7 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c, } ret = devm_request_threaded_irq(dev, i2c->irq, NULL, ts3a227e_interrupt, - IRQF_TRIGGER_LOW | IRQF_ONESHOT, + ts3a227e->pdata.irqflag, "TS3A227E", ts3a227e); if (ret) { dev_err(dev, "Cannot request irq %d (%d)\n", i2c->irq, ret); diff --git a/sound/soc/codecs/ts3a227e.h b/sound/soc/codecs/ts3a227e.h index e2acf9c..a587a72 100644 --- a/sound/soc/codecs/ts3a227e.h +++ b/sound/soc/codecs/ts3a227e.h @@ -11,6 +11,8 @@ #ifndef _TS3A227E_H #define _TS3A227E_H +#include <sound/ts3a227e.h> + int ts3a227e_enable_jack_detect(struct snd_soc_component *component, struct snd_soc_jack *jack);