Re: [PATCH v4 1/3] spi-nor: Add support for Intel SPI serial flash controller
On Wed, Nov 09, 2016 at 02:51:20PM +0100, Cyrille Pitchen wrote:
> > +/* Reads max 64 bytes from the device fifo */
> > +static int intel_spi_read_block(struct intel_spi *ispi, void *buf, size_t
> > size)
> > +{
> > + size_t bytes;
> > + int i = 0;
> > +
> > + if (size > 64)
>
> This is not a blocking point but just a recommendation: you should define
> and use a macro for this 64 byte FIFO size instead of using this hardcoded
> 64 value here and in intel_spi_write_block(), intel_spi_write(),
> intel_spi_read().
Good point. I'll change that to use a macro.
> > + return -EINVAL;
> > +
> > + while (size > 0) {
> > + bytes = min_t(size_t, size, 4);
> > + memcpy_fromio(buf, ispi->base + FDATA(i++), bytes);
> Here again another general recommendation: be careful about using
> operators like ++ on macro parameters. In the case of this FDATA() macro
> it will work as expected but unwanted side effect might occur depending on
> the actual macro definition:
>
> int i = 1;
>
> #define DOUBLE(n) ((n) + (n))
> DOUBLE(i++); /* here i is incremented twice, not just once. */
Indeed, even though with the current macro it does not happen I'm going
change it to update i separately like:
memcpy_fromio(buf, ispi->base + FDATA(i), bytes);
i++
> > + size -= bytes;
> > + buf += bytes;
> > + }
> > +
> > + return 0;
> > +}
[snip]
> > +static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
> > + u_char *read_buf)
> > +{
> > + struct intel_spi *ispi = nor->priv;
> > + size_t block_size, retlen = 0;
> > + u32 val, status;
> > + ssize_t ret;
> > +
>
> As I understand some Intel SPI controllers can only use op codes in a fixed
> instruction set, so here you should check the nor->read_opcode.
> Indeed when the support of SFDP tables will be integrated, the
> nor->read_opcode might change between calls of the nor->read() handler.
OK, I did not know that it can change.
> spi_nor_read_sfdp() make use of this nor->read() handler but set
> nor->read_opcode to SPINOR_OP_RDSFDP (5Ah) before calling the handler.
>
> Then, if intel_spi_read() is called with an unsupported nor->read_opcode,
> it should fail returning -EINVAL. The spi-nor framework will handle this
> failure correctly.
>
> You can find the SFDP patch here:
> http://patchwork.ozlabs.org/patch/685984/
Thanks for the pointer. I'll update the driver to take read_opcode into
account in intel_spi_read().
Thanks for the comments.
Re: [PATCH v4 1/3] spi-nor: Add support for Intel SPI serial flash controller
Hi Mika, Le 17/10/2016 à 16:58, Mika Westerberg a écrit : > Add support for the SPI serial flash host controller found on many Intel > CPUs including Baytrail and Braswell. The SPI serial flash controller is > used to access BIOS and other platform specific information. By default the > driver exposes a single read-only MTD device but with a module parameter > "writeable=1" the MTD device can be made read-write which makes it possible > to upgrade BIOS directly from Linux. > > Signed-off-by: Mika Westerberg > --- > Documentation/mtd/intel-spi.txt | 88 > drivers/mtd/spi-nor/Kconfig | 20 + > drivers/mtd/spi-nor/Makefile | 2 + > drivers/mtd/spi-nor/intel-spi-platform.c | 57 +++ > drivers/mtd/spi-nor/intel-spi.c | 780 > +++ > drivers/mtd/spi-nor/intel-spi.h | 24 + > include/linux/platform_data/intel-spi.h | 31 ++ > 7 files changed, 1002 insertions(+) > create mode 100644 Documentation/mtd/intel-spi.txt > create mode 100644 drivers/mtd/spi-nor/intel-spi-platform.c > create mode 100644 drivers/mtd/spi-nor/intel-spi.c > create mode 100644 drivers/mtd/spi-nor/intel-spi.h > create mode 100644 include/linux/platform_data/intel-spi.h > > diff --git a/Documentation/mtd/intel-spi.txt b/Documentation/mtd/intel-spi.txt > new file mode 100644 > index ..bc357729c2cb > --- /dev/null > +++ b/Documentation/mtd/intel-spi.txt > @@ -0,0 +1,88 @@ > +Upgrading BIOS using intel-spi > +-- > + > +Many Intel CPUs like Baytrail and Braswell include SPI serial flash host > +controller which is used to hold BIOS and other platform specific data. > +Since contents of the SPI serial flash is crucial for machine to function, > +it is typically protected by different hardware protection mechanisms to > +avoid accidental (or on purpose) overwrite of the content. > + > +Not all manufacturers protect the SPI serial flash, mainly because it > +allows upgrading the BIOS image directly from an OS. > + > +The intel-spi driver makes it possible to read and write the SPI serial > +flash, if certain protection bits are not set and locked. If it finds > +any of them set, the whole MTD device is made read-only to prevent > +partial overwrites. By default the driver exposes SPI serial flash > +contents as read-only but it can be changed from kernel command line, > +passing "intel-spi.writeable=1". > + > +Please keep in mind that overwriting the BIOS image on SPI serial flash > +might render the machine unbootable and requires special equipment like > +Dediprog to revive. You have been warned! > + > +Below are the steps how to upgrade MinnowBoard MAX BIOS directly from > +Linux. > + > + 1) Download and extract the latest Minnowboard MAX BIOS SPI image > +[1]. At the time writing this the latest image is v92. > + > + 2) Install mtd-utils package [2]. We need this in order to erase the SPI > +serial flash. Distros like Debian and Fedora have this prepackaged with > +name "mtd-utils". > + > + 3) Add "intel-spi.writeable=1" to the kernel command line and reboot > +the board (you can also reload the driver passing "writeable=1" as > +module parameter to modprobe). > + > + 4) Once the board is up and running again, find the right MTD partition > +(it is named as "BIOS"): > + > +# cat /proc/mtd > +dev:size erasesize name > +mtd0: 0080 1000 "BIOS" > + > +So here it will be /dev/mtd0 but it may vary. > + > + 5) Make backup of the existing image first: > + > +# dd if=/dev/mtd0ro of=bios.bak > +16384+0 records in > +16384+0 records out > +8388608 bytes (8.4 MB) copied, 10.0269 s, 837 kB/s > + > + 6) Verify the backup > + > +# sha1sum /dev/mtd0ro bios.bak > +fdbb011920572ca6c991377c4b418a0502668b73 /dev/mtd0ro > +fdbb011920572ca6c991377c4b418a0502668b73 bios.bak > + > +The SHA1 sums must match. Otherwise do not continue any further! > + > + 7) Erase the SPI serial flash. After this step, do not reboot the > +board! Otherwise it will not start anymore. > + > +# flash_erase /dev/mtd0 0 0 > +Erasing 4 Kibyte @ 7ff000 -- 100 % complete > + > + 8) Once completed without errors you can write the new BIOS image: > + > +# dd if=MNW2MAX1.X64.0092.R01.1605221712.bin of=/dev/mtd0 > + > + 9) Verify that the new content of the SPI serial flash matches the new > +BIOS image: > + > +# sha1sum /dev/mtd0ro MNW2MAX1.X64.0092.R01.1605221712.bin > +9b4df9e4be2057fceec3a5529ec3d950836c87a2 /dev/mtd0ro > +9b4df9e4be2057fceec3a5529ec3d950836c87a2 > MNW2MAX1.X64.0092.R01.1605221712.bin > + > +The SHA1 sums should match. > + > + 10) Now you can reboot your board and observe the new BIOS starting up > + properly. > + > +References > +-- > + > +[1] > https://firmware.intel.com/sites/default/files/MinnowBoard.MAX_.X64.92.R01.zip > +[2] http://www.linux-mtd.infradead.org/ > diff --git a/drivers/mtd/spi-nor/Kco
