| 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
| ||
|
Date: Fri, 25 Nov 2016 18:26:10 +0100
From: Marek Vasut <marek.vasut@...il.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>,
linux-mtd@...ts.infradead.org
Cc: Cyrille Pitchen <cyrille.pitchen@...el.com>,
Boris Brezillon <boris.brezillon@...e-electrons.com>,
Richard Weinberger <richard@....at>,
Brian Norris <computersforpeace@...il.com>,
David Woodhouse <dwmw2@...radead.org>,
Lee Jones <lee.jones@...aro.org>,
Peter Tyser <ptyser@...-inc.com>, key.seong.lim@...el.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/3] spi-nor: Add support for Intel SPI serial flash
controller
On 11/14/2016 11:24 AM, Mika Westerberg wrote:
> 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 <mika.westerberg@...ux.intel.com>
Only minor nits below. Sorry I took so long with the review.
[...]
> +static int intel_spi_wait_hw_busy(struct intel_spi *ispi)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(INTEL_SPI_TIMEOUT);
> + u32 hwstat;
Won't some include/linux/iopoll.h macro help here ?
> + while (!time_after(jiffies, timeout)) {
> + hwstat = readl(ispi->base + HSFSTS_CTL);
> + if (!(hwstat & HSFSTS_CTL_SCIP))
> + return 0;
> + cond_resched();
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(INTEL_SPI_TIMEOUT);
> + u32 swstat;
> +
> + while (!time_after(jiffies, timeout)) {
DTTO
> + swstat = readl(ispi->sregs + SSFSTS_CTL);
> + if (!(swstat & SSFSTS_CTL_SCIP))
> + return 0;
> + cond_resched();
> + }
> +
> + return -ETIMEDOUT;
> +}
[...]
> +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;
> +
> + switch (nor->read_opcode) {
> + case SPINOR_OP_READ:
> + case SPINOR_OP_READ_FAST:
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + while (len > 0) {
> + block_size = min_t(size_t, len, INTEL_SPI_FIFO_SZ);
> +
> + writel(from, ispi->base + FADDR);
> +
> + val = readl(ispi->base + HSFSTS_CTL);
> + val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
> + val &= ~HSFSTS_CTL_FDBC_MASK;
> + val |= (block_size - 1) << HSFSTS_CTL_FDBC_SHIFT;
> + val &= ~HSFSTS_CTL_FCYCLE_MASK;
> + val |= HSFSTS_CTL_FCYCLE_READ;
> + val |= HSFSTS_CTL_FGO;
Sort those bit clears and bit sets so you first clear and then set bits.
> + writel(val, ispi->base + HSFSTS_CTL);
> +
> + ret = intel_spi_wait_hw_busy(ispi);
> + if (ret)
> + return ret;
> +
> + status = readl(ispi->base + HSFSTS_CTL);
> + if (status & HSFSTS_CTL_FCERR)
> + ret = -EIO;
> + else if (status & HSFSTS_CTL_AEL)
> + ret = -EACCES;
> +
> + if (ret < 0) {
> + dev_err(ispi->dev, "read error: %llx: %#x\n", from,
> + status);
> + return ret;
> + }
> +
> + ret = intel_spi_read_block(ispi, read_buf, block_size);
> + if (ret)
> + return ret;
> +
> + len -= block_size;
> + from += block_size;
> + retlen += block_size;
> + read_buf += block_size;
> + }
> +
> + return retlen;
> +}
> +
> +static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
> + const u_char *write_buf)
> +{
> + struct intel_spi *ispi = nor->priv;
> + size_t block_size, retlen = 0;
> + u32 val, status;
> + ssize_t ret;
> +
> + while (len > 0) {
> + block_size = min_t(size_t, len, INTEL_SPI_FIFO_SZ);
> +
> + writel(to, ispi->base + FADDR);
> +
> + val = readl(ispi->base + HSFSTS_CTL);
> + val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
> + val &= ~HSFSTS_CTL_FDBC_MASK;
> + val |= (block_size - 1) << HSFSTS_CTL_FDBC_SHIFT;
> + val &= ~HSFSTS_CTL_FCYCLE_MASK;
> + val |= HSFSTS_CTL_FCYCLE_WRITE;
DTTO here.
> + /* Write enable */
> + if (ispi->preopcodes[1] == SPINOR_OP_WREN)
> + val |= SSFSTS_CTL_SPOP;
> + val |= SSFSTS_CTL_ACS;
> + writel(val, ispi->base + HSFSTS_CTL);
> +
> + ret = intel_spi_write_block(ispi, write_buf, block_size);
> + if (ret) {
> + dev_err(ispi->dev, "failed to write block\n");
> + return ret;
> + }
> +
> + /* Start the write now */
> + val = readl(ispi->base + HSFSTS_CTL);
> + writel(val | HSFSTS_CTL_FGO, ispi->base + HSFSTS_CTL);
> +
> + ret = intel_spi_wait_hw_busy(ispi);
> + if (ret) {
> + dev_err(ispi->dev, "timeout\n");
> + return ret;
> + }
> +
> + status = readl(ispi->base + HSFSTS_CTL);
> + if (status & HSFSTS_CTL_FCERR)
> + ret = -EIO;
> + else if (status & HSFSTS_CTL_AEL)
> + ret = -EACCES;
> +
> + if (ret < 0) {
> + dev_err(ispi->dev, "write error: %llx: %#x\n", to,
> + status);
> + return ret;
> + }
> +
> + len -= block_size;
> + to += block_size;
> + retlen += block_size;
> + write_buf += block_size;
> + }
> +
> + return retlen;
> +}
[...]
--
Best regards,
Marek Vasut
Powered by blists - more mailing lists