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 <[email protected]>
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
