linux-kernel - Re: [PATCH v7 3/4] ASoC: rt5677: clear interrupts by polarity flip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOReqxhETC9xAojXyDWLMOJ3W22Zn4GNGry44=XC=t_k7SCHqA@mail.gmail.com>
Date:   Tue, 18 Jun 2019 11:12:58 -0700
From:   Curtis Malainey <cujomalainey@...gle.com>
To:     Fletcher Woodruff <fletcherw@...omium.org>
Cc:     Cezary Rojewski <cezary.rojewski@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Oder Chiou <oder_chiou@...ltek.com>,
        Takashi Iwai <tiwai@...e.com>,
        Curtis Malainey <cujomalainey@...omium.org>,
        Ross Zwisler <zwisler@...omium.org>,
        ALSA development <alsa-devel@...a-project.org>
Subject: Re: [PATCH v7 3/4] ASoC: rt5677: clear interrupts by polarity flip

On Tue, Jun 18, 2019 at 11:01 AM Fletcher Woodruff
<fletcherw@...omium.org> wrote:
>
> On Sun, Jun 16, 2019 at 10:56 AM Cezary Rojewski
> <cezary.rojewski@...el.com> wrote:
> > On 2019-06-14 21:48, Fletcher Woodruff wrote:
> > > +static irqreturn_t rt5677_irq(int unused, void *data)
> > > +{
> > > +     struct rt5677_priv *rt5677 = data;
> > > +     int ret = 0, i, reg_irq, virq;
> > > +     bool irq_fired = false;
> > > +
> > > +     mutex_lock(&rt5677->irq_lock);
> > > +     /* Read interrupt status */
> > > +     ret = regmap_read(rt5677->regmap, RT5677_IRQ_CTRL1, &reg_irq);
> > > +     if (ret) {
> > > +             pr_err("rt5677: failed reading IRQ status: %d\n", ret);
> >
> > The entire rt5677 makes use of dev_XXX with the exception of.. this very
> > function. Consider reusing "component" field which is already part of
> > struct rt5677_priv and removing pr_XXX.
>
> I was using dev_XXX, but I believe Curtis found that 'component' was
> sometimes uninitialized when this function was called, so I switched
> back to pr_XXX. I may be misremembering though, so I'll let Curtis
> comment as well.
>
The issue here is that the IRQ is setup in the i2c probe and the
component is setup in the codec probe. In theory if the hardware is in
a bad state you can get an interrupt between the probes and have the
interrupt called with the component being NULL. It might be worth
considering migrating the IRQ setup to the codec probe so this
condition cannot happen and then we can avoid using pr_XXX.
> > > +     if (ret) {
> > > +             dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
> > >               return ret;
> >
> > You may want to simplify this, similarly to how's it done in your
> > rt5677_i2c_probe - leave message alone and return "ret" explicitly at
> > the end.
>
> Good suggestion. I'll update that for the next patch.