Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200629225932.5036-1-daniel.gutson@eclypsium.com>
 <CAK8P3a2zzXHNB7CX8efpKeQF2gJkF2J4FwafU58wT2RGvjjTxw@mail.gmail.com>
 <CAFmMkTHrQ4LZk4+-3kdJ+dc47MXR1Jd76AXbO-ceT2zsfDRFGQ@mail.gmail.com>
 <CAK8P3a1bbpmD0wJkhkjqW9YttBpMmdn8Z5oTLm0cr-0gjyU2zA@mail.gmail.com> <CAFmMkTE3z6OZQ_v3jx-4MzMr8v+4qcF2uLn0ASGydj5oqDnfjg@mail.gmail.com>
In-Reply-To: <CAFmMkTE3z6OZQ_v3jx-4MzMr8v+4qcF2uLn0ASGydj5oqDnfjg@mail.gmail.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 6 Jul 2020 11:20:26 +0200
Message-ID: <CAK8P3a2G_UzKv5HmQk1gyaHXY+YzVedknwof+9fmCjQuF+hu2A@mail.gmail.com>
Subject: Re: [PATCH] SPI LPC information kernel module
To:     Daniel Gutson <daniel@eclypsium.com>
Cc:     Derek Kiernan <derek.kiernan@xilinx.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Richard Hughes <hughsient@gmail.com>,
        Alex Bazhaniuk <alex@eclypsium.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jul 3, 2020 at 10:43 PM Daniel Gutson <daniel@eclypsium.com> wrote:
> On Tue, Jun 30, 2020 at 5:58 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> After analyzing the intel-spi driver, I came up to these observations that led
> me conclude that it is not what I need to use:
>
> * Some SPI Controllers have 0xFFFF as their VID DID bytes in the PCI
> config space. This causes that the PCI devices enumeration doesn't include them,
> and thus a PCI device driver won't be probed even despite listing the DID
> that the datasheet specifies. I effectively confirmed this by doing a PCI device
> driver and trying the intel-spi in systems with this characteristic. In short, the
> intel-spi driver doesn't work with these systems.

I don't understand what you mean here. How do *you* find the device if the
device-id is 0xffff?

My impression was that the spi-nor host is not a PCI device at all
but instead is a function on the mfd device, and probed as a
platform_driver with the "intel-spi" name.

> * Additionally, there is a difference of functionality: the intel-spi driver is a MTD,
> and -despite it effectively contains some of the DIDs my proposed driver
> supports- it doesn't offer an API to read the BIOS related registers (which is not the
> function read_register, since it reads chip registers). In fact, note that it reads
> HSFS from the PCI config space using MMIO, by just reading the mapped
> memory offset. The read register callback reads a different type of registers
> with a different mechanism (eg it does a writel before a readl). By being a MTD,
> it doesn't need to read for example the Bios Control register (which I need), or
> other registers that I need but are irrelevant for MTD operations.
>
> I should add a new interface to the driver just to read these registers or an
> interface to read an arbitrary offset from the mapped memory.
> * Finally, some of the registers I need are present in the PCI Config Space

Config space of *which* device in particular? Is this the Atom MFD device,
the SPI-NOR host, or something else?

> (others are in other places), but the offsets (and/or the offsets of the fields)
> vary depending on the architecture of the CPU or the PCH. That's why I
> first detect the architecture based on the VID/DID and then use the appropriate
> definition. Moreover, there are cases where the same registers  is obtained
> by reading the PCI CFG in some architectures, whereas it is obtained by
> reading MMIO in other architectures (eg Atom). All this information is provided
> in the Intel's datasheets.

Is this MMIO space part of a PCI device BAR, or how do you find it?
If it belongs to a PCI device, which one is this, and why can't there
be a driver for it?

> Because of these reasons, I'm proposing a misc (not-device) driver that supports
> many Intel architectures (and families) to expose the information.
> I understand your proposal to first enhance existing _device_ drivers, but I
> couldn't find suitable options.

Maybe try adding an interface to one of the drivers at first, and then extend
it to the other hardware after an initial code review. Do not bypass the driver
model or try to do everything at once with a single module that knows
details of multiple unrelated hardware implementations.

     Arnd