linux-kernel - Re: [UNVERIFIED SENDER] Re: [PATCH v4 2/2] soc: amazon: al-pos-edac: Introduce Amazon's Annapurna Labs POS EDAC driver

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] [day] [month] [year] [list]

Message-ID: <6815f9e9-f250-bd62-33b3-e3ab30aa47e2@amazon.com>
Date:   Mon, 7 Oct 2019 14:34:03 +0300
From:   "Shenhar, Talel" <talel@...zon.com>
To:     Marc Zyngier <maz@...nel.org>
CC:     <robh+dt@...nel.org>, <mark.rutland@....com>, <arnd@...db.de>,
        <bp@...en8.de>, <mchehab@...nel.org>, <james.morse@....com>,
        <davem@...emloft.net>, <gregkh@...uxfoundation.org>,
        <paulmck@...ux.ibm.com>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-edac@...r.kernel.org>,
        <dwmw@...zon.co.uk>, <benh@...nel.crashing.org>,
        <hhhawa@...zon.com>, <ronenk@...zon.com>, <jonnyc@...zon.com>,
        <hanochu@...zon.com>, <amirkl@...zon.com>, <barakw@...zon.com>
Subject: Re: [UNVERIFIED SENDER] Re: [PATCH v4 2/2] soc: amazon: al-pos-edac:
 Introduce Amazon's Annapurna Labs POS EDAC driver

thanks for the review

On 10/7/2019 2:26 PM, Marc Zyngier wrote:
> On Thu, 03 Oct 2019 12:32:41 +0100,
> Talel Shenhar <talel@...zon.com> wrote:
>> +	log1 = readl(al_pos->mmio_base + AL_POS_ERROR_LOG_1);
> I already commented on the misuse of strict accesses. Unless you can
> explain and document *why* you need the extra ordering, please use
> relaxed accesses.
agreeing on relaxed, shall be part of v5
>
>> +
>> +	if (al_pos->irq > 0) {
>> +		ret = devm_request_irq(&pdev->dev,
>> +				       al_pos->irq,
>> +				       al_pos_irq_handler,
>> +				       0,
>> +				       pdev->name,
>> +				       pdev);
>> +		if (ret != 0) {
>> +			dev_err(&pdev->dev,
>> +				"failed to register to irq %d (%d)\n",
>> +				al_pos->irq, ret);
>> +			goto err_remove_edac;
> Would it be worth continuing without interrupts? After all, the
> interrupt seems to be an optional part of the device...

indeed interrupts are optional, however, this is optional for some of 
the systems.

in some cases (and some systems), this error event is critical and 
should cause fast handling. for those, we define the interrupts.

so bottom line, i would like to keep this error in case of error in 
interrupt.

>
> Thanks,
>
> 	M.
>