linux-kernel - Re: [PATCH] mfd: Export LPC attributes for the system SPI chip

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: <CAD2FfiGNErUhz=7DH6Z37X573hSkJkzbOEXbb++X+Ey5WLc9=Q@mail.gmail.com>
Date:   Wed, 13 May 2020 15:13:28 +0100
From:   Richard Hughes <hughsient@...il.com>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     ptyser@...-inc.com, Lee Jones <lee.jones@...aro.org>,
        tudor.ambarus@...rochip.com,
        Kate Stewart <kstewart@...uxfoundation.org>,
        allison@...utok.net, tglx@...utronix.de, jethro@...tanix.com,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mfd: Export LPC attributes for the system SPI chip

On Wed, 13 May 2020 at 10:11, Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> > I can fix up all those, but out of interest how did you "know" the
> > right three digit identifier to use?
> I work for Intel ;-)

Hah, okay, thanks :)

> > I'm really wondering if drivers/mfd/lpc_ich.c is the right place for
> > this kind of "just expose one byte of PCI config space" functionality.
> Ideally there is one driver per device.

My idea in https://github.com/hughsie/spi_lpc was to not actually
register a pci_driver.

> If this is touching the 00:1f.5 PCI device (SPI-NOR controller) then the
> right place is the intel-spi-pci.c as that's the driver for this
> controller.

So Cannon Lake, Cannon Point and Ice Lake would go into
drivers/mtd/spi-nor/controllers/intel-spi-pci.c and the systems like
Sunrise Point using an ISA bridge would use drivers/mfd/lpc_ich.c?

> We can put this there so that it does not enable the SPI-NOR
> functionality itself and the mark the SPI-NOR functionality only as
> being dangerous or something like that.

I think getting the distros to enable SPI_INTEL_SPI_PCI might be a
tough sell. Could we perhaps remove the DANGEROUS label as it's not
writeable without a module option?

> > > > +     char tmp[2];
> > > Wouldn't this need to account the '\0' as well?
> You sprint() there "%d\n", so that includes a number, '\n' and '\0' unless
> I'm missing something.

Doh, of course you're right. Will fix, thanks.

Richard