linux-kernel - RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI controller

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: <CY4PR07MB27577F9A6DBF3F4985B014ACC1CF9@CY4PR07MB2757.namprd07.prod.outlook.com>
Date:   Fri, 3 Sep 2021 08:10:38 +0000
From:   Parshuram Raju Thombare <pthombar@...ence.com>
To:     Mark Brown <broonie@...nel.org>
CC:     "lukas@...ner.de" <lukas@...ner.de>,
        "p.yadav@...com" <p.yadav@...com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jayshri Dajiram Pawar <jpawar@...ence.com>,
        Milind Parab <mparab@...ence.com>,
        Konrad Kociolek <konrad@...ence.com>
Subject: RE: [PATCH v3 2/2] spi: cadence: add support for Cadence XSPI
 controller

Hi Mark,

Thanks for your reply.

>Please make the entire comment a C++ so things look more intentional.

Sure

>The core already validates this, are you seeing it happen?  If so we
>should fix the core and either way just remove setup() entirely.

Oh, right. spi_add_device() seems to be already doing that check.
I will remove it from the driver.

>We will just silently ignore any unknown interrupts here.  It would be
>better to either only ack known interrupts (so genirq can notice if
>there's a problem with other interrupts) or at least log that we're
>seeing unexpected interrupts.  The current code will cause trouble if
>this is deployed in a system with the interrupt line shared (which the
>driver claims to support), or if something goes wrong and the IP starts
>asserting some interrupt that isn't expected.

Ok, I will modify this to return IRQ_HANDLED only for enabled interrupts.

>> +	master->mode_bits = SPI_3WIRE | SPI_TX_DUAL  | SPI_TX_QUAD  |
>> +		SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL
>|
>> +		SPI_MODE_0  | SPI_MODE_3;
>
>I don't see any handling of these in the code?

This is just to declare controller's capability, so that spi_setup() can modify
spi->mode according to the capability of attached device.

Regards,
Parshuram Thombare