linux-kernel - Re: [PATCH v2 1/4] ASoC: cs35l45: Checks index of cs35l45

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: <5B0EB2A2-2048-4A71-A4A9-D5167C7AB5EC@opensource.cirrus.com>
Date:   Thu, 31 Aug 2023 09:56:05 -0500
From:   "Rivera-Matos, Ricardo" <rriveram@...nsource.cirrus.com>
To:     Mark Brown <broonie@...nel.org>
CC:     Vlad Karpovich <vkarpovi@...nsource.cirrus.com>,
        James Schulman <james.schulman@...rus.com>,
        David Rhodes <david.rhodes@...rus.com>,
        "Richard Fitzgerald" <rf@...nsource.cirrus.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Rob Herring <robh+dt@...nel.org>,
        <alsa-devel@...a-project.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] ASoC: cs35l45: Checks index of cs35l45_irqs[]

Hello Mark,

> On Aug 30, 2023, at 3:59 PM, Mark Brown <broonie@...nel.org> wrote:
> 
> On Wed, Aug 30, 2023 at 02:55:33PM -0500, Vlad Karpovich wrote:
>> Checks the index computed by the virq offset before printing the
>> error condition in cs35l45_spk_safe_err() handler.
>> 
>> Signed-off-by: Ricardo Rivera-Matos <rriveram@...nsource.cirrus.com>
>> Signed-off-by: Vlad Karpovich <vkarpovi@...nsource.cirrus.com>
> 
> Who actually wrote this patch?

I am the original author, allow me to clarify how and why this is supposed to work.

How:

static const struct cs35l45_irq cs35l45_irqs[] = {
	CS35L45_IRQ(AMP_SHORT_ERR, "Amplifier short error", cs35l45_spk_safe_err),
	CS35L45_IRQ(UVLO_VDDBATT_ERR, "VDDBATT undervoltage error", cs35l45_spk_safe_err),
	CS35L45_IRQ(BST_SHORT_ERR, "Boost inductor error", cs35l45_spk_safe_err),
	CS35L45_IRQ(BST_UVP_ERR, "Boost undervoltage error", cs35l45_spk_safe_err),
	CS35L45_IRQ(TEMP_ERR, "Overtemperature error", cs35l45_spk_safe_err),
	CS35L45_IRQ(AMP_CAL_ERR, "Amplifier calibration error", cs35l45_spk_safe_err),
	CS35L45_IRQ(UVLO_VDDLV_ERR, "LV threshold detector error", cs35l45_spk_safe_err),
	CS35L45_IRQ(GLOBAL_ERROR, "Global error", cs35l45_global_err),
	CS35L45_IRQ(DSP_WDT_EXPIRE, "DSP Watchdog Timer", cs35l45_dsp_wdt_expire),
	CS35L45_IRQ(PLL_UNLOCK_FLAG_RISE, "PLL unlock flag rise", cs35l45_pll_unlock),
	CS35L45_IRQ(PLL_LOCK_FLAG, "PLL lock", cs35l45_pll_lock),
	CS35L45_IRQ(DSP_VIRT2_MBOX, "DSP virtual MBOX 2 write flag", cs35l45_dsp_virt2_mbox_cb),
};

static irqreturn_t cs35l45_spk_safe_err(int irq, void *data)
{
	struct cs35l45_private *cs35l45 = data;
	int i;

	i = irq - regmap_irq_get_virq(cs35l45->irq_data, 0);

	if (i < 0 || i > 6)
		dev_err(cs35l45->dev, "Unspecified global error condition (%d) detected!\n", irq);
	else
		dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name);

	return IRQ_HANDLED;
}

This snippet here is from the OoT CS35L45 driver. There are only seven root causes for a speaker safe error and when one of those root cause bits are set, we enter the common handler to print the root cause. Using the IRQ and the VIRQ number we do some math and print the name of the error as a dev_err.

Why:

Originally these root cause bits were treated as general bits and simply checked in the cs35l45_global_err() handler. A problem arose when the CS35L45 would come out of hibernation and the root cause bits would be masked by default. Treating the root cause bits as IRQs ensured that, like the other IRQ bits, the root cause bits would be unmasked before an IRQ could be serviced.

Further notes:

static const struct cs35l45_irq cs35l45_irqs[] = {
<snip>
	CS35L45_IRQ(GLOBAL_ERROR, "Global error", cs35l45_spk_safe_err),
	CS35L45_IRQ(DSP_WDT_EXPIRE, "DSP Watchdog Timer", cs35l45_spk_safe_err),
<snip>
};

This is from next-20230831. I am not sure how this happened, but these IRQs are not pointing to cs35l45_global_err and cs35l45_dsp_wdt_expire respectively. Maybe a bad cherry-pick. We will address this shortly.

I hope this addresses any confusion, please let me know if I can offer any further details.

Thanks,
Ricardo