diff mbox

[4/4] ASoC: ts3a227e: set irq flag base on dmi

Message ID 1430358238-74659-4-git-send-email-yang.a.fang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

yang.a.fang@intel.com April 30, 2015, 1:43 a.m. UTC
From: "Fang, Yang A" <yang.a.fang@intel.com>

Cyan platform need edge trigger interrupt

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/codecs/ts3a227e.c |   35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

Comments

Dylan Reid April 30, 2015, 5:45 p.m. UTC | #1
On Wed, Apr 29, 2015 at 6:43 PM,  <yang.a.fang@intel.com> wrote:
> From: "Fang, Yang A" <yang.a.fang@intel.com>
>
> Cyan platform need edge trigger interrupt
>
> Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
> ---
>  sound/soc/codecs/ts3a227e.c |   35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
> index 422f66f..75f2a43 100644
> --- a/sound/soc/codecs/ts3a227e.c
> +++ b/sound/soc/codecs/ts3a227e.c
> @@ -15,6 +15,7 @@
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/regmap.h>
> +#include <linux/dmi.h>
>
>  #include <sound/core.h>
>  #include <sound/jack.h>
> @@ -270,6 +271,33 @@ static int ts3a227e_parse_dt(struct ts3a227e *ts3a227e, struct device_node *np)
>         return 0;
>  }
>
> +static struct ts3a227e_platform_data *ts3a227e_pdata;
> +
> +static struct ts3a227e_platform_data cyan_platform_data = {
> +
> +       .irqflag = IRQF_TRIGGER_FALLING | IRQF_ONESHOT,

As we discussed internally, using an edge trigger here can lead to
missed interrupts if another event happens while an interrupt is being
processed.  I'd investigate getting the level interrupt to work on
this platform, and only if that is impossible try to make this work
with an edge interrupt.

> +
> +};
> +
> +static int cyan_quirk_cb(const struct dmi_system_id *id)
> +{
> +
> +       ts3a227e_pdata = &cyan_platform_data;
> +
> +       return 1;
> +}
> +
> +static struct dmi_system_id dmi_platform_intel_braswell[] = {
> +       {
> +               .ident = "Braswell Cyan",
> +               .callback = cyan_quirk_cb,
> +               .matches = {
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Cyan"),
> +               },
> +       },
> +       { }
> +};
> +
>  static int ts3a227e_i2c_probe(struct i2c_client *i2c,
>                               const struct i2c_device_id *id)
>  {
> @@ -287,9 +315,12 @@ static int ts3a227e_i2c_probe(struct i2c_client *i2c,
>
>         ts3a227e->pdata.irqflag = IRQF_TRIGGER_LOW | IRQF_ONESHOT;
>
> -       if (pdata)
> +       if (pdata) {
>                 ts3a227e->pdata = *pdata;
> -
> +       } else {
> +               if (dmi_check_system(dmi_platform_intel_braswell))
> +                       ts3a227e->pdata = *ts3a227e_pdata;
> +       }
>         ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
>         if (IS_ERR(ts3a227e->regmap))
>                 return PTR_ERR(ts3a227e->regmap);
> --
> 1.7.9.5
>
Mark Brown April 30, 2015, 8:08 p.m. UTC | #2
On Thu, Apr 30, 2015 at 10:45:04AM -0700, Dylan Reid wrote:
> On Wed, Apr 29, 2015 at 6:43 PM,  <yang.a.fang@intel.com> wrote:

> > +static struct ts3a227e_platform_data cyan_platform_data = {
> > +
> > +       .irqflag = IRQF_TRIGGER_FALLING | IRQF_ONESHOT,

> As we discussed internally, using an edge trigger here can lead to
> missed interrupts if another event happens while an interrupt is being
> processed.  I'd investigate getting the level interrupt to work on
> this platform, and only if that is impossible try to make this work
> with an edge interrupt.

TBH given the prevelance of devices out there which only support level
triggered interrupts it probably makes sense to handle this somewhere in
genirq, either generically in a driver that handles whatever bit of the
platform creates the restriction.  At the very least it should probably
be put into a library since it's a bit of a pattern.

There's some workarounds for systems with similar restrictions in a few
of the Wolfson drivers if you want to copy (they do need a GPIO mapping
though IIRC since they do the level trigger in software).
diff mbox

Patch

diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 422f66f..75f2a43 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -15,6 +15,7 @@ 
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/regmap.h>
+#include <linux/dmi.h>
 
 #include <sound/core.h>
 #include <sound/jack.h>
@@ -270,6 +271,33 @@  static int ts3a227e_parse_dt(struct ts3a227e *ts3a227e, struct device_node *np)
 	return 0;
 }
 
+static struct ts3a227e_platform_data *ts3a227e_pdata;
+
+static struct ts3a227e_platform_data cyan_platform_data = {
+
+	.irqflag = IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+
+};
+
+static int cyan_quirk_cb(const struct dmi_system_id *id)
+{
+
+	ts3a227e_pdata = &cyan_platform_data;
+
+	return 1;
+}
+
+static struct dmi_system_id dmi_platform_intel_braswell[] = {
+	{
+		.ident = "Braswell Cyan",
+		.callback = cyan_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "Cyan"),
+		},
+	},
+	{ }
+};
+
 static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 			      const struct i2c_device_id *id)
 {
@@ -287,9 +315,12 @@  static int ts3a227e_i2c_probe(struct i2c_client *i2c,
 
 	ts3a227e->pdata.irqflag = IRQF_TRIGGER_LOW | IRQF_ONESHOT;
 
-	if (pdata)
+	if (pdata) {
 		ts3a227e->pdata = *pdata;
-
+	} else {
+		if (dmi_check_system(dmi_platform_intel_braswell))
+			ts3a227e->pdata = *ts3a227e_pdata;
+	}
 	ts3a227e->regmap = devm_regmap_init_i2c(i2c, &ts3a227e_regmap_config);
 	if (IS_ERR(ts3a227e->regmap))
 		return PTR_ERR(ts3a227e->regmap);