[U-Boot] [PATCH v3] x86: ich-spi: Convert driver to spi-mem ops
Bin Meng
bmeng.cn at gmail.com
Thu Aug 1 07:02:21 UTC 2019
On Mon, Jul 29, 2019 at 6:43 PM Bernhard Messerklinger
<bernhard.messerklinger at br-automation.com> wrote:
>
> With the introduction of the new spi-mem model operations changed
> slightly. The new spi-mem operations make things a bit easier to
> handle for ich-spi flash interface. This patch converts the ich-spi
> driver by using spi-mem operations.
> ---
>
> Changes in v2:
> - make 2 routines static; update commit message a little bit
> Changes in v3:
> - update erase op code handling to work with latest master
> spi-nor driver
The changelog should go after SoB, and below --.
>
> Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger at br-automation.com>
> ---
> drivers/spi/ich.c | 267 +++++++++++++++++-----------------------------
> drivers/spi/ich.h | 9 +-
> 2 files changed, 100 insertions(+), 176 deletions(-)
>
> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> index 03531a8c0c..179f801d42 100644
> --- a/drivers/spi/ich.c
> +++ b/drivers/spi/ich.c
> @@ -14,6 +14,8 @@
> #include <pci_ids.h>
> #include <spi.h>
> #include <asm/io.h>
> +#include <spi-mem.h>
> +#include <div64.h>
>
> #include "ich.h"
>
> @@ -171,18 +173,6 @@ static int ich_init_controller(struct udevice *dev,
> return 0;
> }
>
> -static inline void spi_use_out(struct spi_trans *trans, unsigned bytes)
> -{
> - trans->out += bytes;
> - trans->bytesout -= bytes;
> -}
> -
> -static inline void spi_use_in(struct spi_trans *trans, unsigned bytes)
> -{
> - trans->in += bytes;
> - trans->bytesin -= bytes;
> -}
> -
> static void spi_lock_down(struct ich_spi_platdata *plat, void *sbase)
> {
> if (plat->ich_version == ICHV_7) {
> @@ -213,47 +203,12 @@ static bool spi_lock_status(struct ich_spi_platdata *plat, void *sbase)
> return lock != 0;
> }
>
> -static void spi_setup_type(struct spi_trans *trans, int data_bytes)
> -{
> - trans->type = 0xFF;
> -
> - /* Try to guess spi type from read/write sizes */
> - if (trans->bytesin == 0) {
> - if (trans->bytesout + data_bytes > 4)
> - /*
> - * If bytesin = 0 and bytesout > 4, we presume this is
> - * a write data operation, which is accompanied by an
> - * address.
> - */
> - trans->type = SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS;
> - else
> - trans->type = SPI_OPCODE_TYPE_WRITE_NO_ADDRESS;
> - return;
> - }
> -
> - if (trans->bytesout == 1) { /* and bytesin is > 0 */
> - trans->type = SPI_OPCODE_TYPE_READ_NO_ADDRESS;
> - return;
> - }
> -
> - if (trans->bytesout == 4) /* and bytesin is > 0 */
> - trans->type = SPI_OPCODE_TYPE_READ_WITH_ADDRESS;
> -
> - /* Fast read command is called with 5 bytes instead of 4 */
> - if (trans->out[0] == SPI_OPCODE_FAST_READ && trans->bytesout == 5) {
> - trans->type = SPI_OPCODE_TYPE_READ_WITH_ADDRESS;
> - --trans->bytesout;
> - }
> -}
> -
> static int spi_setup_opcode(struct ich_spi_priv *ctlr, struct spi_trans *trans,
> bool lock)
> {
> uint16_t optypes;
> uint8_t opmenu[ctlr->menubytes];
>
> - trans->opcode = trans->out[0];
> - spi_use_out(trans, 1);
> if (!lock) {
> /* The lock is off, so just use index 0. */
> ich_writeb(ctlr, trans->opcode, ctlr->opmenu);
> @@ -285,12 +240,7 @@ static int spi_setup_opcode(struct ich_spi_priv *ctlr, struct spi_trans *trans,
>
> optypes = ich_readw(ctlr, ctlr->optype);
> optype = (optypes >> (opcode_index * 2)) & 0x3;
> - if (trans->type == SPI_OPCODE_TYPE_WRITE_NO_ADDRESS &&
> - optype == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS &&
> - trans->bytesout >= 3) {
> - /* We guessed wrong earlier. Fix it up. */
> - trans->type = optype;
> - }
> +
> if (optype != trans->type) {
> printf("ICH SPI: Transaction doesn't fit type %d\n",
> optype);
> @@ -300,26 +250,6 @@ static int spi_setup_opcode(struct ich_spi_priv *ctlr, struct spi_trans *trans,
> }
> }
>
> -static int spi_setup_offset(struct spi_trans *trans)
> -{
> - /* Separate the SPI address and data */
> - switch (trans->type) {
> - case SPI_OPCODE_TYPE_READ_NO_ADDRESS:
> - case SPI_OPCODE_TYPE_WRITE_NO_ADDRESS:
> - return 0;
> - case SPI_OPCODE_TYPE_READ_WITH_ADDRESS:
> - case SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS:
> - trans->offset = ((uint32_t)trans->out[0] << 16) |
> - ((uint32_t)trans->out[1] << 8) |
> - ((uint32_t)trans->out[2] << 0);
> - spi_use_out(trans, 3);
> - return 1;
> - default:
> - printf("Unrecognized SPI transaction type %#x\n", trans->type);
> - return -EPROTO;
> - }
> -}
> -
> /*
> * Wait for up to 6s til status register bit(s) turn 1 (in case wait_til_set
> * below is true) or 0. In case the wait was for the bit(s) to set - write
> @@ -350,7 +280,7 @@ static int ich_status_poll(struct ich_spi_priv *ctlr, u16 bitmask,
> return -ETIMEDOUT;
> }
>
> -void ich_spi_config_opcode(struct udevice *dev)
> +static void ich_spi_config_opcode(struct udevice *dev)
> {
> struct ich_spi_priv *ctlr = dev_get_priv(dev);
>
> @@ -365,72 +295,48 @@ void ich_spi_config_opcode(struct udevice *dev)
> ich_writel(ctlr, SPI_OPMENU_UPPER, ctlr->opmenu + sizeof(u32));
> }
>
> -static int ich_spi_xfer(struct udevice *dev, unsigned int bitlen,
> - const void *dout, void *din, unsigned long flags)
> +static int ich_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
> {
> - struct udevice *bus = dev_get_parent(dev);
> + struct udevice *bus = dev_get_parent(slave->dev);
> struct ich_spi_platdata *plat = dev_get_platdata(bus);
> struct ich_spi_priv *ctlr = dev_get_priv(bus);
> - uint16_t control;
> - int16_t opcode_index;
> - int with_address;
> - int status;
> - int bytes = bitlen / 8;
> struct spi_trans *trans = &ctlr->trans;
> - unsigned type = flags & (SPI_XFER_BEGIN | SPI_XFER_END);
> - int using_cmd = 0;
> + u16 control;
> + u16 opcode_index;
Why changing "uint16_t control" to "u16 control", and "int16_t
opcode_index" to "u16 opcode_index"?
> + int ret = 0;
No need to change here as "int ret" was declared below.
> + int status;
Nothing changes here as "int status" was there in the original codes.
> bool lock = spi_lock_status(plat, ctlr->base);
> - int ret;
> -
> - /* We don't support writing partial bytes */
> - if (bitlen % 8) {
> - debug("ICH SPI: Accessing partial bytes not supported\n");
> - return -EPROTONOSUPPORT;
> - }
> -
> - /* An empty end transaction can be ignored */
> - if (type == SPI_XFER_END && !dout && !din)
> - return 0;
> + bool with_address = 0;
Why? the original codes have "int with_address".
>
> - if (type & SPI_XFER_BEGIN)
> - memset(trans, '\0', sizeof(*trans));
> + trans->in = NULL;
> + trans->out = NULL;
> + trans->type = 0xFF;
>
> - /* Dp we need to come back later to finish it? */
> - if (dout && type == SPI_XFER_BEGIN) {
> - if (bytes > ICH_MAX_CMD_LEN) {
> - debug("ICH SPI: Command length limit exceeded\n");
> - return -ENOSPC;
> + if (op->data.nbytes) {
> + if (op->data.dir == SPI_MEM_DATA_IN) {
> + trans->in = op->data.buf.in;
> + trans->bytesin = op->data.nbytes;
> + } else {
> + trans->out = op->data.buf.out;
> + trans->bytesout = op->data.nbytes;
> }
> - memcpy(trans->cmd, dout, bytes);
> - trans->cmd_len = bytes;
> - debug_trace("ICH SPI: Saved %d bytes\n", bytes);
> - return 0;
> }
>
> - /*
> - * We process a 'middle' spi_xfer() call, which has no
> - * SPI_XFER_BEGIN/END, as an independent transaction as if it had
> - * an end. We therefore repeat the command. This is because ICH
> - * seems to have no support for this, or because interest (in digging
> - * out the details and creating a special case in the code) is low.
> - */
> - if (trans->cmd_len) {
> - trans->out = trans->cmd;
> - trans->bytesout = trans->cmd_len;
> - using_cmd = 1;
> - debug_trace("ICH SPI: Using %d bytes\n", trans->cmd_len);
> - } else {
> - trans->out = dout;
> - trans->bytesout = dout ? bytes : 0;
> - }
> + if (trans->opcode != op->cmd.opcode)
> + trans->opcode = op->cmd.opcode;
>
> - trans->in = din;
> - trans->bytesin = din ? bytes : 0;
> + if (lock && trans->opcode == SPI_OPCODE_WRDIS)
> + return 0;
>
> - /* There has to always at least be an opcode */
> - if (!trans->bytesout) {
> - debug("ICH SPI: No opcode for transfer\n");
> - return -EPROTO;
> + if (trans->opcode == SPI_OPCODE_WREN) {
> + /*
> + * Treat Write Enable as Atomic Pre-Op if possible
> + * in order to prevent the Management Engine from
> + * issuing a transaction between WREN and DATA.
> + */
> + if (!lock)
> + ich_writew(ctlr, trans->opcode, ctlr->preop);
> + return 0;
> }
[snip]
Regards,
Bin
More information about the U-Boot
mailing list