On Wed, 13 May 2020 at 10:11, Mika Westerberg <mika.westerb...@linux.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