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]

Date:   Tue, 18 Jun 2019 12:00:59 -0600
From:   Fletcher Woodruff <fletcherw@...omium.org>
To:     Cezary Rojewski <cezary.rojewski@...el.com>
Cc:     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-devel@...a-project.org
Subject: Re: [PATCH v7 3/4] ASoC: rt5677: clear interrupts by polarity flip

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.

> > +     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.