diff mbox

[v4,19/23] drivers/fsi: Add GPIO based FSI master

Message ID 20170329174340.89109-20-cbostic@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christopher Bostic March 29, 2017, 5:43 p.m. UTC
From: Chris Bostic <cbostic@linux.vnet.ibm.com>

Implement a FSI master using GPIO.  Will generate FSI protocol for
read and write commands to particular addresses.  Sends master command
and waits for and decodes a slave response.

Includes changes from Edward A. James <eajames@us.ibm.com> and Jeremy
Kerr <jk@ozlabs.org>.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts |  10 +
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts  |  12 +
 drivers/fsi/Kconfig                           |  11 +
 drivers/fsi/Makefile                          |   1 +
 drivers/fsi/fsi-master-gpio.c                 | 614 ++++++++++++++++++++++++++
 5 files changed, 648 insertions(+)
 create mode 100644 drivers/fsi/fsi-master-gpio.c

Comments

Joel Stanley March 30, 2017, 5:48 a.m. UTC | #1
On Thu, Mar 30, 2017 at 4:13 AM, Christopher Bostic
<cbostic@linux.vnet.ibm.com> wrote:
> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
>
> Implement a FSI master using GPIO.  Will generate FSI protocol for
> read and write commands to particular addresses.  Sends master command
> and waits for and decodes a slave response.
>
> Includes changes from Edward A. James <eajames@us.ibm.com> and Jeremy
> Kerr <jk@ozlabs.org>.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts |  10 +
>  arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts  |  12 +
>  drivers/fsi/Kconfig                           |  11 +
>  drivers/fsi/Makefile                          |   1 +
>  drivers/fsi/fsi-master-gpio.c                 | 614 ++++++++++++++++++++++++++
>  5 files changed, 648 insertions(+)
>  create mode 100644 drivers/fsi/fsi-master-gpio.c
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> index 1d2fc1e..cf7d7d7 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> @@ -29,6 +29,16 @@
>                         reg = <0x5f000000 0x01000000>; /* 16M */
>                 };
>         };
> +
> +       gpio-fsi {
> +               compatible = "fsi-master-gpio", "fsi-master";
> +
> +               clock-gpios = <&gpio ASPEED_GPIO(A, 4) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(A, 5) GPIO_ACTIVE_HIGH>;
> +               mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
> +               enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> +               trans-gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
> +       };
>  };
>
>  &uart5 {
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 7a3b2b5..2fd7db7 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -29,6 +29,18 @@
>                         reg = <0xbf000000 0x01000000>; /* 16M */
>                 };
>         };
> +
> +       gpio-fsi {
> +               compatible = "fsi-master-gpio", "fsi-master";
> +
> +               status = "okay";
> +
> +               clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> +               mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
> +               enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> +               trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
> +       };
>  };

I'm not sure what happened here. The changes to the device trees
should not be in this patch.

Cheers,

Joel

>
>  &uart5 {
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 04c1a0e..9cf8345 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -9,4 +9,15 @@ config FSI
>         ---help---
>           FSI - the FRU Support Interface - is a simple bus for low-level
>           access to POWER-based hardware.
> +
> +if FSI
> +
> +config FSI_MASTER_GPIO
> +       tristate "GPIO-based FSI master"
> +       depends on FSI && GPIOLIB
> +       ---help---
> +       This option enables a FSI master driver using GPIO lines.
> +
> +endif
> +
>  endmenu
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> index db0e5e7..ed28ac0 100644
> --- a/drivers/fsi/Makefile
> +++ b/drivers/fsi/Makefile
> @@ -1,2 +1,3 @@
>
>  obj-$(CONFIG_FSI) += fsi-core.o
> +obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> new file mode 100644
> index 0000000..0bf5caa
> --- /dev/null
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -0,0 +1,614 @@
> +/*
> + * A FSI master controller, using a simple GPIO bit-banging interface
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fsi.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#include "fsi-master.h"
> +
> +#define        FSI_GPIO_STD_DLY        1       /* Standard pin delay in nS */
> +#define        FSI_ECHO_DELAY_CLOCKS   16      /* Number clocks for echo delay */
> +#define        FSI_PRE_BREAK_CLOCKS    50      /* Number clocks to prep for break */
> +#define        FSI_BREAK_CLOCKS        256     /* Number of clocks to issue break */
> +#define        FSI_POST_BREAK_CLOCKS   16000   /* Number clocks to set up cfam */
> +#define        FSI_INIT_CLOCKS         5000    /* Clock out any old data */
> +#define        FSI_GPIO_STD_DELAY      10      /* Standard GPIO delay in nS */
> +                                       /* todo: adjust down as low as */
> +                                       /* possible or eliminate */
> +#define        FSI_GPIO_CMD_DPOLL      0x2
> +#define        FSI_GPIO_CMD_TERM       0x3f
> +#define FSI_GPIO_CMD_ABS_AR    0x4
> +
> +#define        FSI_GPIO_DPOLL_CLOCKS   100      /* < 21 will cause slave to hang */
> +
> +/* Bus errors */
> +#define        FSI_GPIO_ERR_BUSY       1       /* Slave stuck in busy state */
> +#define        FSI_GPIO_RESP_ERRA      2       /* Any (misc) Error */
> +#define        FSI_GPIO_RESP_ERRC      3       /* Slave reports master CRC error */
> +#define        FSI_GPIO_MTOE           4       /* Master time out error */
> +#define        FSI_GPIO_CRC_INVAL      5       /* Master reports slave CRC error */
> +
> +/* Normal slave responses */
> +#define        FSI_GPIO_RESP_BUSY      1
> +#define        FSI_GPIO_RESP_ACK       0
> +#define        FSI_GPIO_RESP_ACKD      4
> +
> +#define        FSI_GPIO_MAX_BUSY       100
> +#define        FSI_GPIO_MTOE_COUNT     1000
> +#define        FSI_GPIO_DRAIN_BITS     20
> +#define        FSI_GPIO_CRC_SIZE       4
> +#define        FSI_GPIO_MSG_ID_SIZE            2
> +#define        FSI_GPIO_MSG_RESPID_SIZE        2
> +#define        FSI_GPIO_PRIME_SLAVE_CLOCKS     100
> +
> +static DEFINE_SPINLOCK(fsi_gpio_cmd_lock);     /* lock around fsi commands */
> +
> +struct fsi_master_gpio {
> +       struct fsi_master       master;
> +       struct device           *dev;
> +       struct gpio_desc        *gpio_clk;
> +       struct gpio_desc        *gpio_data;
> +       struct gpio_desc        *gpio_trans;    /* Voltage translator */
> +       struct gpio_desc        *gpio_enable;   /* FSI enable */
> +       struct gpio_desc        *gpio_mux;      /* Mux control */
> +};
> +
> +#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
> +
> +struct fsi_gpio_msg {
> +       uint64_t        msg;
> +       uint8_t         bits;
> +};
> +
> +static void clock_toggle(struct fsi_master_gpio *master, int count)
> +{
> +       int i;
> +
> +       for (i = 0; i < count; i++) {
> +               ndelay(FSI_GPIO_STD_DLY);
> +               gpiod_set_value(master->gpio_clk, 0);
> +               ndelay(FSI_GPIO_STD_DLY);
> +               gpiod_set_value(master->gpio_clk, 1);
> +       }
> +}
> +
> +static int sda_in(struct fsi_master_gpio *master)
> +{
> +       int in;
> +
> +       ndelay(FSI_GPIO_STD_DLY);
> +       in = gpiod_get_value(master->gpio_data);
> +       return in ? 1 : 0;
> +}
> +
> +static void sda_out(struct fsi_master_gpio *master, int value)
> +{
> +       gpiod_set_value(master->gpio_data, value);
> +}
> +
> +static void set_sda_input(struct fsi_master_gpio *master)
> +{
> +       gpiod_direction_input(master->gpio_data);
> +       if (master->gpio_trans)
> +               gpiod_set_value(master->gpio_trans, 0);
> +}
> +
> +static void set_sda_output(struct fsi_master_gpio *master, int value)
> +{
> +       if (master->gpio_trans)
> +               gpiod_set_value(master->gpio_trans, 1);
> +       gpiod_direction_output(master->gpio_data, value);
> +}
> +
> +static void clock_zeros(struct fsi_master_gpio *master, int count)
> +{
> +       set_sda_output(master, 1);
> +       clock_toggle(master, count);
> +}
> +
> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
> +                       uint8_t num_bits)
> +{
> +       uint8_t bit, in_bit;
> +
> +       set_sda_input(master);
> +
> +       for (bit = 0; bit < num_bits; bit++) {
> +               clock_toggle(master, 1);
> +               in_bit = sda_in(master);
> +               msg->msg <<= 1;
> +               msg->msg |= ~in_bit & 0x1;      /* Data is negative active */
> +       }
> +       msg->bits += num_bits;
> +}
> +
> +static void serial_out(struct fsi_master_gpio *master,
> +                       const struct fsi_gpio_msg *cmd)
> +{
> +       uint8_t bit;
> +       uint64_t msg = ~cmd->msg;       /* Data is negative active */
> +       uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
> +       uint64_t last_bit = ~0;
> +       int next_bit;
> +
> +       if (!cmd->bits) {
> +               dev_warn(master->dev, "trying to output 0 bits\n");
> +               return;
> +       }
> +       set_sda_output(master, 0);
> +
> +       /* Send the start bit */
> +       sda_out(master, 0);
> +       clock_toggle(master, 1);
> +
> +       /* Send the message */
> +       for (bit = 0; bit < cmd->bits; bit++) {
> +               next_bit = (msg & sda_mask) >> (cmd->bits - 1);
> +               if (last_bit ^ next_bit) {
> +                       sda_out(master, next_bit);
> +                       last_bit = next_bit;
> +               }
> +               clock_toggle(master, 1);
> +               msg <<= 1;
> +       }
> +}
> +
> +static void msg_push_bits(struct fsi_gpio_msg *msg, uint64_t data, int bits)
> +{
> +       msg->msg <<= bits;
> +       msg->msg |= data & ((1ull << bits) - 1);
> +       msg->bits += bits;
> +}
> +
> +static void msg_push_crc(struct fsi_gpio_msg *msg)
> +{
> +       uint8_t crc;
> +       int top;
> +
> +       top = msg->bits & 0x3;
> +
> +       /* start bit, and any non-aligned top bits */
> +       crc = fsi_crc4(0,
> +                       1 << top | msg->msg >> (msg->bits - top),
> +                       top + 1);
> +
> +       /* aligned bits */
> +       crc = fsi_crc4(crc, msg->msg, msg->bits - top);
> +
> +       msg_push_bits(msg, crc, 4);
> +}
> +
> +static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
> +               uint8_t id, uint32_t addr, size_t size, const void *data)
> +{
> +       bool write = !!data;
> +       uint8_t ds;
> +       int i;
> +
> +       cmd->bits = 0;
> +       cmd->msg = 0;
> +
> +       msg_push_bits(cmd, id, 2);
> +       msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
> +       msg_push_bits(cmd, write ? 0 : 1, 1);
> +
> +       /*
> +        * The read/write size is encoded in the lower bits of the address
> +        * (as it must be naturally-aligned), and the following ds bit.
> +        *
> +        *      size    addr:1  addr:0  ds
> +        *      1       x       x       0
> +        *      2       x       0       1
> +        *      4       0       1       1
> +        *
> +        */
> +       ds = size > 1 ? 1 : 0;
> +       addr &= ~(size - 1);
> +       if (size == 4)
> +               addr |= 1;
> +
> +       msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
> +       msg_push_bits(cmd, ds, 1);
> +       for (i = 0; write && i < size; i++)
> +               msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
> +
> +       msg_push_crc(cmd);
> +}
> +
> +static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> +{
> +       cmd->bits = 0;
> +       cmd->msg = 0;
> +
> +       msg_push_bits(cmd, slave_id, 2);
> +       msg_push_bits(cmd, FSI_GPIO_CMD_DPOLL, 3);
> +       msg_push_crc(cmd);
> +}
> +
> +static void echo_delay(struct fsi_master_gpio *master)
> +{
> +       set_sda_output(master, 1);
> +       clock_toggle(master, FSI_ECHO_DELAY_CLOCKS);
> +}
> +
> +static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> +{
> +       cmd->bits = 0;
> +       cmd->msg = 0;
> +
> +       msg_push_bits(cmd, slave_id, 2);
> +       msg_push_bits(cmd, FSI_GPIO_CMD_TERM, 6);
> +       msg_push_crc(cmd);
> +}
> +
> +/*
> + * Store information on master errors so handler can detect and clean
> + * up the bus
> + */
> +static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
> +{
> +
> +}
> +
> +static int read_one_response(struct fsi_master_gpio *master,
> +               uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
> +{
> +       struct fsi_gpio_msg msg;
> +       uint8_t id, tag;
> +       uint32_t crc;
> +       int i;
> +
> +       /* wait for the start bit */
> +       for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
> +               msg.bits = 0;
> +               msg.msg = 0;
> +               serial_in(master, &msg, 1);
> +               if (msg.msg)
> +                       break;
> +       }
> +       if (i >= FSI_GPIO_MTOE_COUNT) {
> +               dev_dbg(master->dev,
> +                       "Master time out waiting for response\n");
> +               fsi_master_gpio_error(master, FSI_GPIO_MTOE);
> +               return -EIO;
> +       }
> +
> +       msg.bits = 0;
> +       msg.msg = 0;
> +
> +       /* Read slave ID & response tag */
> +       serial_in(master, &msg, 4);
> +
> +       id = (msg.msg >> FSI_GPIO_MSG_RESPID_SIZE) & 0x3;
> +       tag = msg.msg & 0x3;
> +
> +       /* if we have an ACK, and we're expecting data, clock the
> +        * data in too
> +        */
> +       if (tag == FSI_GPIO_RESP_ACK && data_size)
> +               serial_in(master, &msg, data_size * 8);
> +
> +       /* read CRC */
> +       serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
> +
> +       /* we have a whole message now; check CRC */
> +       crc = fsi_crc4(0, 1, 1);
> +       crc = fsi_crc4(crc, msg.msg, msg.bits);
> +       if (crc) {
> +               dev_dbg(master->dev, "ERR response CRC\n");
> +               fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
> +               return -EIO;
> +       }
> +
> +       if (msgp)
> +               *msgp = msg;
> +       if (tagp)
> +               *tagp = tag;
> +
> +       return 0;
> +}
> +
> +static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
> +{
> +       struct fsi_gpio_msg cmd;
> +       uint8_t tag;
> +       int rc;
> +
> +       build_term_command(&cmd, slave);
> +       serial_out(master, &cmd);
> +       echo_delay(master);
> +
> +       rc = read_one_response(master, 0, NULL, &tag);
> +       if (rc) {
> +               dev_err(master->dev,
> +                               "TERM failed; lost communication with slave\n");
> +               return -EIO;
> +       } else if (tag != FSI_GPIO_RESP_ACK) {
> +               dev_err(master->dev, "TERM failed; response %d\n", tag);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +static int poll_for_response(struct fsi_master_gpio *master,
> +               uint8_t slave, uint8_t size, void *data)
> +{
> +       struct fsi_gpio_msg response, cmd;
> +       int busy_count = 0, rc, i;
> +       uint8_t tag;
> +
> +retry:
> +       rc = read_one_response(master, size, &response, &tag);
> +       if (rc)
> +               return rc;
> +
> +       switch (tag) {
> +       case FSI_GPIO_RESP_ACK:
> +               if (size && data) {
> +                       uint64_t val = response.msg;
> +                       /* clear crc & mask */
> +                       val >>= 4;
> +                       val &= (1ull << (size * 8)) - 1;
> +
> +                       for (i = 0; i < size; i++) {
> +                               ((uint8_t *)data)[size-i-1] =
> +                                       val & 0xff;
> +                               val >>= 8;
> +                       }
> +               }
> +               break;
> +       case FSI_GPIO_RESP_BUSY:
> +               /*
> +                * Its necessary to clock slave before issuing
> +                * d-poll, not indicated in the hardware protocol
> +                * spec. < 20 clocks causes slave to hang, 21 ok.
> +                */
> +               clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
> +               if (busy_count++ < FSI_GPIO_MAX_BUSY) {
> +                       build_dpoll_command(&cmd, slave);
> +                       serial_out(master, &cmd);
> +                       echo_delay(master);
> +                       goto retry;
> +               }
> +               dev_warn(master->dev,
> +                       "ERR slave is stuck in busy state, issuing TERM\n");
> +               issue_term(master, slave);
> +               rc = -EIO;
> +               break;
> +
> +       case FSI_GPIO_RESP_ERRA:
> +       case FSI_GPIO_RESP_ERRC:
> +               dev_dbg(master->dev, "ERR%c received: 0x%x\n",
> +                       tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
> +                       (int)response.msg);
> +               fsi_master_gpio_error(master, response.msg);
> +               rc = -EIO;
> +               break;
> +       }
> +
> +       /* Clock the slave enough to be ready for next operation */
> +       clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
> +       return rc;
> +}
> +
> +static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
> +               struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
> +{
> +       unsigned long flags;
> +       int rc;
> +
> +       spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
> +       serial_out(master, cmd);
> +       echo_delay(master);
> +       rc = poll_for_response(master, slave, resp_len, resp);
> +       spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
> +
> +       return rc;
> +}
> +
> +static int fsi_master_gpio_read(struct fsi_master *_master, int link,
> +               uint8_t id, uint32_t addr, void *val, size_t size)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +       struct fsi_gpio_msg cmd;
> +
> +       if (link != 0)
> +               return -ENODEV;
> +
> +       build_abs_ar_command(&cmd, id, addr, size, NULL);
> +       return fsi_master_gpio_xfer(master, id, &cmd, size, val);
> +}
> +
> +static int fsi_master_gpio_write(struct fsi_master *_master, int link,
> +               uint8_t id, uint32_t addr, const void *val, size_t size)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +       struct fsi_gpio_msg cmd;
> +
> +       if (link != 0)
> +               return -ENODEV;
> +
> +       build_abs_ar_command(&cmd, id, addr, size, val);
> +       return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +}
> +
> +static int fsi_master_gpio_term(struct fsi_master *_master,
> +               int link, uint8_t id)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +       struct fsi_gpio_msg cmd;
> +
> +       if (link != 0)
> +               return -ENODEV;
> +
> +       build_term_command(&cmd, id);
> +       return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +}
> +
> +/*
> + * Issue a break command on link
> + */
> +static int fsi_master_gpio_break(struct fsi_master *_master, int link)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +
> +       if (link != 0)
> +               return -ENODEV;
> +
> +       set_sda_output(master, 1);
> +       sda_out(master, 1);
> +       clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> +       sda_out(master, 0);
> +       clock_toggle(master, FSI_BREAK_CLOCKS);
> +       echo_delay(master);
> +       sda_out(master, 1);
> +       clock_toggle(master, FSI_POST_BREAK_CLOCKS);
> +
> +       /* Wait for logic reset to take effect */
> +       udelay(200);
> +
> +       return 0;
> +}
> +
> +static void fsi_master_gpio_init(struct fsi_master_gpio *master)
> +{
> +       if (master->gpio_mux)
> +               gpiod_direction_output(master->gpio_mux, 1);
> +       if (master->gpio_trans)
> +               gpiod_direction_output(master->gpio_trans, 1);
> +       if (master->gpio_enable)
> +               gpiod_direction_output(master->gpio_enable, 1);
> +       gpiod_direction_output(master->gpio_clk, 1);
> +       gpiod_direction_output(master->gpio_data, 1);
> +
> +       /* todo: evaluate if clocks can be reduced */
> +       clock_zeros(master, FSI_INIT_CLOCKS);
> +}
> +
> +static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +
> +       if (link != 0)
> +               return -ENODEV;
> +       if (master->gpio_enable)
> +               gpiod_set_value(master->gpio_enable, 1);
> +
> +       return 0;
> +}
> +
> +static void fsi_master_gpio_release(struct device *dev)
> +{
> +       struct fsi_master_gpio *master = to_fsi_master_gpio(
> +                                               dev_to_fsi_master(dev));
> +
> +       kfree(master);
> +}
> +
> +static int fsi_master_gpio_probe(struct platform_device *pdev)
> +{
> +       struct fsi_master_gpio *master;
> +       struct gpio_desc *gpio;
> +
> +       master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
> +       if (!master)
> +               return -ENOMEM;
> +
> +       master->dev = &pdev->dev;
> +       master->master.dev.parent = master->dev;
> +       master->master.dev.release = fsi_master_gpio_release;
> +
> +       gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
> +       if (IS_ERR(gpio)) {
> +               dev_err(&pdev->dev, "failed to get clock gpio\n");
> +               return PTR_ERR(gpio);
> +       }
> +       master->gpio_clk = gpio;
> +
> +       gpio = devm_gpiod_get(&pdev->dev, "data", 0);
> +       if (IS_ERR(gpio)) {
> +               dev_err(&pdev->dev, "failed to get data gpio\n");
> +               return PTR_ERR(gpio);
> +       }
> +       master->gpio_data = gpio;
> +
> +       /* Optional GPIOs */
> +       gpio = devm_gpiod_get_optional(&pdev->dev, "trans", 0);
> +       if (IS_ERR(gpio)) {
> +               dev_err(&pdev->dev, "failed to get trans gpio\n");
> +               return PTR_ERR(gpio);
> +       }
> +       master->gpio_trans = gpio;
> +
> +       gpio = devm_gpiod_get_optional(&pdev->dev, "enable", 0);
> +       if (IS_ERR(gpio)) {
> +               dev_err(&pdev->dev, "failed to get enable gpio\n");
> +               return PTR_ERR(gpio);
> +       }
> +       master->gpio_enable = gpio;
> +
> +       gpio = devm_gpiod_get_optional(&pdev->dev, "mux", 0);
> +       if (IS_ERR(gpio)) {
> +               dev_err(&pdev->dev, "failed to get mux gpio\n");
> +               return PTR_ERR(gpio);
> +       }
> +       master->gpio_mux = gpio;
> +
> +       master->master.n_links = 1;
> +       master->master.read = fsi_master_gpio_read;
> +       master->master.write = fsi_master_gpio_write;
> +       master->master.term = fsi_master_gpio_term;
> +       master->master.send_break = fsi_master_gpio_break;
> +       master->master.link_enable = fsi_master_gpio_link_enable;
> +       platform_set_drvdata(pdev, master);
> +
> +       fsi_master_gpio_init(master);
> +
> +       fsi_master_register(&master->master);
> +
> +       return 0;
> +}
> +
> +
> +static int fsi_master_gpio_remove(struct platform_device *pdev)
> +{
> +       struct fsi_master_gpio *master = platform_get_drvdata(pdev);
> +
> +       devm_gpiod_put(&pdev->dev, master->gpio_clk);
> +       devm_gpiod_put(&pdev->dev, master->gpio_data);
> +       if (master->gpio_trans)
> +               devm_gpiod_put(&pdev->dev, master->gpio_trans);
> +       if (master->gpio_enable)
> +               devm_gpiod_put(&pdev->dev, master->gpio_enable);
> +       if (master->gpio_mux)
> +               devm_gpiod_put(&pdev->dev, master->gpio_mux);
> +       fsi_master_unregister(&master->master);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id fsi_master_gpio_match[] = {
> +       { .compatible = "fsi-master-gpio" },
> +       { },
> +};
> +
> +static struct platform_driver fsi_master_gpio_driver = {
> +       .driver = {
> +               .name           = "fsi-master-gpio",
> +               .of_match_table = fsi_master_gpio_match,
> +       },
> +       .probe  = fsi_master_gpio_probe,
> +       .remove = fsi_master_gpio_remove,
> +};
> +
> +module_platform_driver(fsi_master_gpio_driver);
> +MODULE_LICENSE("GPL");
> --
> 1.8.2.2
>
Christopher Bostic March 30, 2017, 6:15 p.m. UTC | #2
On 3/30/17 12:48 AM, Joel Stanley wrote:
> On Thu, Mar 30, 2017 at 4:13 AM, Christopher Bostic
> <cbostic@linux.vnet.ibm.com> wrote:
>> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
>>
>> Implement a FSI master using GPIO.  Will generate FSI protocol for
>> read and write commands to particular addresses.  Sends master command
>> and waits for and decodes a slave response.
>>
>> Includes changes from Edward A. James <eajames@us.ibm.com> and Jeremy
>> Kerr <jk@ozlabs.org>.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>   arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts |  10 +
>>   arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts  |  12 +
>>   drivers/fsi/Kconfig                           |  11 +
>>   drivers/fsi/Makefile                          |   1 +
>>   drivers/fsi/fsi-master-gpio.c                 | 614 ++++++++++++++++++++++++++
>>   5 files changed, 648 insertions(+)
>>   create mode 100644 drivers/fsi/fsi-master-gpio.c
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> index 1d2fc1e..cf7d7d7 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
>> @@ -29,6 +29,16 @@
>>                          reg = <0x5f000000 0x01000000>; /* 16M */
>>                  };
>>          };
>> +
>> +       gpio-fsi {
>> +               compatible = "fsi-master-gpio", "fsi-master";
>> +
>> +               clock-gpios = <&gpio ASPEED_GPIO(A, 4) GPIO_ACTIVE_HIGH>;
>> +               data-gpios = <&gpio ASPEED_GPIO(A, 5) GPIO_ACTIVE_HIGH>;
>> +               mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
>> +               enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
>> +               trans-gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
>> +       };
>>   };
>>
>>   &uart5 {
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> index 7a3b2b5..2fd7db7 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
>> @@ -29,6 +29,18 @@
>>                          reg = <0xbf000000 0x01000000>; /* 16M */
>>                  };
>>          };
>> +
>> +       gpio-fsi {
>> +               compatible = "fsi-master-gpio", "fsi-master";
>> +
>> +               status = "okay";
>> +
>> +               clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
>> +               data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
>> +               mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
>> +               enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
>> +               trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
>> +       };
>>   };
> I'm not sure what happened here. The changes to the device trees
> should not be in this patch.

Where would you recommend they be placed?  I assume we want them 
somewhere in the  patch set.

Thanks,
Chris
> Cheers,
>
> Joel
>
>>   &uart5 {
>> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
>> index 04c1a0e..9cf8345 100644
>> --- a/drivers/fsi/Kconfig
>> +++ b/drivers/fsi/Kconfig
>> @@ -9,4 +9,15 @@ config FSI
>>          ---help---
>>            FSI - the FRU Support Interface - is a simple bus for low-level
>>            access to POWER-based hardware.
>> +
>> +if FSI
>> +
>> +config FSI_MASTER_GPIO
>> +       tristate "GPIO-based FSI master"
>> +       depends on FSI && GPIOLIB
>> +       ---help---
>> +       This option enables a FSI master driver using GPIO lines.
>> +
>> +endif
>> +
>>   endmenu
>> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
>> index db0e5e7..ed28ac0 100644
>> --- a/drivers/fsi/Makefile
>> +++ b/drivers/fsi/Makefile
>> @@ -1,2 +1,3 @@
>>
>>   obj-$(CONFIG_FSI) += fsi-core.o
>> +obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
>> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
>> new file mode 100644
>> index 0000000..0bf5caa
>> --- /dev/null
>> +++ b/drivers/fsi/fsi-master-gpio.c
>> @@ -0,0 +1,614 @@
>> +/*
>> + * A FSI master controller, using a simple GPIO bit-banging interface
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/fsi.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +
>> +#include "fsi-master.h"
>> +
>> +#define        FSI_GPIO_STD_DLY        1       /* Standard pin delay in nS */
>> +#define        FSI_ECHO_DELAY_CLOCKS   16      /* Number clocks for echo delay */
>> +#define        FSI_PRE_BREAK_CLOCKS    50      /* Number clocks to prep for break */
>> +#define        FSI_BREAK_CLOCKS        256     /* Number of clocks to issue break */
>> +#define        FSI_POST_BREAK_CLOCKS   16000   /* Number clocks to set up cfam */
>> +#define        FSI_INIT_CLOCKS         5000    /* Clock out any old data */
>> +#define        FSI_GPIO_STD_DELAY      10      /* Standard GPIO delay in nS */
>> +                                       /* todo: adjust down as low as */
>> +                                       /* possible or eliminate */
>> +#define        FSI_GPIO_CMD_DPOLL      0x2
>> +#define        FSI_GPIO_CMD_TERM       0x3f
>> +#define FSI_GPIO_CMD_ABS_AR    0x4
>> +
>> +#define        FSI_GPIO_DPOLL_CLOCKS   100      /* < 21 will cause slave to hang */
>> +
>> +/* Bus errors */
>> +#define        FSI_GPIO_ERR_BUSY       1       /* Slave stuck in busy state */
>> +#define        FSI_GPIO_RESP_ERRA      2       /* Any (misc) Error */
>> +#define        FSI_GPIO_RESP_ERRC      3       /* Slave reports master CRC error */
>> +#define        FSI_GPIO_MTOE           4       /* Master time out error */
>> +#define        FSI_GPIO_CRC_INVAL      5       /* Master reports slave CRC error */
>> +
>> +/* Normal slave responses */
>> +#define        FSI_GPIO_RESP_BUSY      1
>> +#define        FSI_GPIO_RESP_ACK       0
>> +#define        FSI_GPIO_RESP_ACKD      4
>> +
>> +#define        FSI_GPIO_MAX_BUSY       100
>> +#define        FSI_GPIO_MTOE_COUNT     1000
>> +#define        FSI_GPIO_DRAIN_BITS     20
>> +#define        FSI_GPIO_CRC_SIZE       4
>> +#define        FSI_GPIO_MSG_ID_SIZE            2
>> +#define        FSI_GPIO_MSG_RESPID_SIZE        2
>> +#define        FSI_GPIO_PRIME_SLAVE_CLOCKS     100
>> +
>> +static DEFINE_SPINLOCK(fsi_gpio_cmd_lock);     /* lock around fsi commands */
>> +
>> +struct fsi_master_gpio {
>> +       struct fsi_master       master;
>> +       struct device           *dev;
>> +       struct gpio_desc        *gpio_clk;
>> +       struct gpio_desc        *gpio_data;
>> +       struct gpio_desc        *gpio_trans;    /* Voltage translator */
>> +       struct gpio_desc        *gpio_enable;   /* FSI enable */
>> +       struct gpio_desc        *gpio_mux;      /* Mux control */
>> +};
>> +
>> +#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
>> +
>> +struct fsi_gpio_msg {
>> +       uint64_t        msg;
>> +       uint8_t         bits;
>> +};
>> +
>> +static void clock_toggle(struct fsi_master_gpio *master, int count)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < count; i++) {
>> +               ndelay(FSI_GPIO_STD_DLY);
>> +               gpiod_set_value(master->gpio_clk, 0);
>> +               ndelay(FSI_GPIO_STD_DLY);
>> +               gpiod_set_value(master->gpio_clk, 1);
>> +       }
>> +}
>> +
>> +static int sda_in(struct fsi_master_gpio *master)
>> +{
>> +       int in;
>> +
>> +       ndelay(FSI_GPIO_STD_DLY);
>> +       in = gpiod_get_value(master->gpio_data);
>> +       return in ? 1 : 0;
>> +}
>> +
>> +static void sda_out(struct fsi_master_gpio *master, int value)
>> +{
>> +       gpiod_set_value(master->gpio_data, value);
>> +}
>> +
>> +static void set_sda_input(struct fsi_master_gpio *master)
>> +{
>> +       gpiod_direction_input(master->gpio_data);
>> +       if (master->gpio_trans)
>> +               gpiod_set_value(master->gpio_trans, 0);
>> +}
>> +
>> +static void set_sda_output(struct fsi_master_gpio *master, int value)
>> +{
>> +       if (master->gpio_trans)
>> +               gpiod_set_value(master->gpio_trans, 1);
>> +       gpiod_direction_output(master->gpio_data, value);
>> +}
>> +
>> +static void clock_zeros(struct fsi_master_gpio *master, int count)
>> +{
>> +       set_sda_output(master, 1);
>> +       clock_toggle(master, count);
>> +}
>> +
>> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
>> +                       uint8_t num_bits)
>> +{
>> +       uint8_t bit, in_bit;
>> +
>> +       set_sda_input(master);
>> +
>> +       for (bit = 0; bit < num_bits; bit++) {
>> +               clock_toggle(master, 1);
>> +               in_bit = sda_in(master);
>> +               msg->msg <<= 1;
>> +               msg->msg |= ~in_bit & 0x1;      /* Data is negative active */
>> +       }
>> +       msg->bits += num_bits;
>> +}
>> +
>> +static void serial_out(struct fsi_master_gpio *master,
>> +                       const struct fsi_gpio_msg *cmd)
>> +{
>> +       uint8_t bit;
>> +       uint64_t msg = ~cmd->msg;       /* Data is negative active */
>> +       uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
>> +       uint64_t last_bit = ~0;
>> +       int next_bit;
>> +
>> +       if (!cmd->bits) {
>> +               dev_warn(master->dev, "trying to output 0 bits\n");
>> +               return;
>> +       }
>> +       set_sda_output(master, 0);
>> +
>> +       /* Send the start bit */
>> +       sda_out(master, 0);
>> +       clock_toggle(master, 1);
>> +
>> +       /* Send the message */
>> +       for (bit = 0; bit < cmd->bits; bit++) {
>> +               next_bit = (msg & sda_mask) >> (cmd->bits - 1);
>> +               if (last_bit ^ next_bit) {
>> +                       sda_out(master, next_bit);
>> +                       last_bit = next_bit;
>> +               }
>> +               clock_toggle(master, 1);
>> +               msg <<= 1;
>> +       }
>> +}
>> +
>> +static void msg_push_bits(struct fsi_gpio_msg *msg, uint64_t data, int bits)
>> +{
>> +       msg->msg <<= bits;
>> +       msg->msg |= data & ((1ull << bits) - 1);
>> +       msg->bits += bits;
>> +}
>> +
>> +static void msg_push_crc(struct fsi_gpio_msg *msg)
>> +{
>> +       uint8_t crc;
>> +       int top;
>> +
>> +       top = msg->bits & 0x3;
>> +
>> +       /* start bit, and any non-aligned top bits */
>> +       crc = fsi_crc4(0,
>> +                       1 << top | msg->msg >> (msg->bits - top),
>> +                       top + 1);
>> +
>> +       /* aligned bits */
>> +       crc = fsi_crc4(crc, msg->msg, msg->bits - top);
>> +
>> +       msg_push_bits(msg, crc, 4);
>> +}
>> +
>> +static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
>> +               uint8_t id, uint32_t addr, size_t size, const void *data)
>> +{
>> +       bool write = !!data;
>> +       uint8_t ds;
>> +       int i;
>> +
>> +       cmd->bits = 0;
>> +       cmd->msg = 0;
>> +
>> +       msg_push_bits(cmd, id, 2);
>> +       msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
>> +       msg_push_bits(cmd, write ? 0 : 1, 1);
>> +
>> +       /*
>> +        * The read/write size is encoded in the lower bits of the address
>> +        * (as it must be naturally-aligned), and the following ds bit.
>> +        *
>> +        *      size    addr:1  addr:0  ds
>> +        *      1       x       x       0
>> +        *      2       x       0       1
>> +        *      4       0       1       1
>> +        *
>> +        */
>> +       ds = size > 1 ? 1 : 0;
>> +       addr &= ~(size - 1);
>> +       if (size == 4)
>> +               addr |= 1;
>> +
>> +       msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
>> +       msg_push_bits(cmd, ds, 1);
>> +       for (i = 0; write && i < size; i++)
>> +               msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
>> +
>> +       msg_push_crc(cmd);
>> +}
>> +
>> +static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
>> +{
>> +       cmd->bits = 0;
>> +       cmd->msg = 0;
>> +
>> +       msg_push_bits(cmd, slave_id, 2);
>> +       msg_push_bits(cmd, FSI_GPIO_CMD_DPOLL, 3);
>> +       msg_push_crc(cmd);
>> +}
>> +
>> +static void echo_delay(struct fsi_master_gpio *master)
>> +{
>> +       set_sda_output(master, 1);
>> +       clock_toggle(master, FSI_ECHO_DELAY_CLOCKS);
>> +}
>> +
>> +static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
>> +{
>> +       cmd->bits = 0;
>> +       cmd->msg = 0;
>> +
>> +       msg_push_bits(cmd, slave_id, 2);
>> +       msg_push_bits(cmd, FSI_GPIO_CMD_TERM, 6);
>> +       msg_push_crc(cmd);
>> +}
>> +
>> +/*
>> + * Store information on master errors so handler can detect and clean
>> + * up the bus
>> + */
>> +static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
>> +{
>> +
>> +}
>> +
>> +static int read_one_response(struct fsi_master_gpio *master,
>> +               uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
>> +{
>> +       struct fsi_gpio_msg msg;
>> +       uint8_t id, tag;
>> +       uint32_t crc;
>> +       int i;
>> +
>> +       /* wait for the start bit */
>> +       for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
>> +               msg.bits = 0;
>> +               msg.msg = 0;
>> +               serial_in(master, &msg, 1);
>> +               if (msg.msg)
>> +                       break;
>> +       }
>> +       if (i >= FSI_GPIO_MTOE_COUNT) {
>> +               dev_dbg(master->dev,
>> +                       "Master time out waiting for response\n");
>> +               fsi_master_gpio_error(master, FSI_GPIO_MTOE);
>> +               return -EIO;
>> +       }
>> +
>> +       msg.bits = 0;
>> +       msg.msg = 0;
>> +
>> +       /* Read slave ID & response tag */
>> +       serial_in(master, &msg, 4);
>> +
>> +       id = (msg.msg >> FSI_GPIO_MSG_RESPID_SIZE) & 0x3;
>> +       tag = msg.msg & 0x3;
>> +
>> +       /* if we have an ACK, and we're expecting data, clock the
>> +        * data in too
>> +        */
>> +       if (tag == FSI_GPIO_RESP_ACK && data_size)
>> +               serial_in(master, &msg, data_size * 8);
>> +
>> +       /* read CRC */
>> +       serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
>> +
>> +       /* we have a whole message now; check CRC */
>> +       crc = fsi_crc4(0, 1, 1);
>> +       crc = fsi_crc4(crc, msg.msg, msg.bits);
>> +       if (crc) {
>> +               dev_dbg(master->dev, "ERR response CRC\n");
>> +               fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
>> +               return -EIO;
>> +       }
>> +
>> +       if (msgp)
>> +               *msgp = msg;
>> +       if (tagp)
>> +               *tagp = tag;
>> +
>> +       return 0;
>> +}
>> +
>> +static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
>> +{
>> +       struct fsi_gpio_msg cmd;
>> +       uint8_t tag;
>> +       int rc;
>> +
>> +       build_term_command(&cmd, slave);
>> +       serial_out(master, &cmd);
>> +       echo_delay(master);
>> +
>> +       rc = read_one_response(master, 0, NULL, &tag);
>> +       if (rc) {
>> +               dev_err(master->dev,
>> +                               "TERM failed; lost communication with slave\n");
>> +               return -EIO;
>> +       } else if (tag != FSI_GPIO_RESP_ACK) {
>> +               dev_err(master->dev, "TERM failed; response %d\n", tag);
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int poll_for_response(struct fsi_master_gpio *master,
>> +               uint8_t slave, uint8_t size, void *data)
>> +{
>> +       struct fsi_gpio_msg response, cmd;
>> +       int busy_count = 0, rc, i;
>> +       uint8_t tag;
>> +
>> +retry:
>> +       rc = read_one_response(master, size, &response, &tag);
>> +       if (rc)
>> +               return rc;
>> +
>> +       switch (tag) {
>> +       case FSI_GPIO_RESP_ACK:
>> +               if (size && data) {
>> +                       uint64_t val = response.msg;
>> +                       /* clear crc & mask */
>> +                       val >>= 4;
>> +                       val &= (1ull << (size * 8)) - 1;
>> +
>> +                       for (i = 0; i < size; i++) {
>> +                               ((uint8_t *)data)[size-i-1] =
>> +                                       val & 0xff;
>> +                               val >>= 8;
>> +                       }
>> +               }
>> +               break;
>> +       case FSI_GPIO_RESP_BUSY:
>> +               /*
>> +                * Its necessary to clock slave before issuing
>> +                * d-poll, not indicated in the hardware protocol
>> +                * spec. < 20 clocks causes slave to hang, 21 ok.
>> +                */
>> +               clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
>> +               if (busy_count++ < FSI_GPIO_MAX_BUSY) {
>> +                       build_dpoll_command(&cmd, slave);
>> +                       serial_out(master, &cmd);
>> +                       echo_delay(master);
>> +                       goto retry;
>> +               }
>> +               dev_warn(master->dev,
>> +                       "ERR slave is stuck in busy state, issuing TERM\n");
>> +               issue_term(master, slave);
>> +               rc = -EIO;
>> +               break;
>> +
>> +       case FSI_GPIO_RESP_ERRA:
>> +       case FSI_GPIO_RESP_ERRC:
>> +               dev_dbg(master->dev, "ERR%c received: 0x%x\n",
>> +                       tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
>> +                       (int)response.msg);
>> +               fsi_master_gpio_error(master, response.msg);
>> +               rc = -EIO;
>> +               break;
>> +       }
>> +
>> +       /* Clock the slave enough to be ready for next operation */
>> +       clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
>> +       return rc;
>> +}
>> +
>> +static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
>> +               struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
>> +{
>> +       unsigned long flags;
>> +       int rc;
>> +
>> +       spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
>> +       serial_out(master, cmd);
>> +       echo_delay(master);
>> +       rc = poll_for_response(master, slave, resp_len, resp);
>> +       spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
>> +
>> +       return rc;
>> +}
>> +
>> +static int fsi_master_gpio_read(struct fsi_master *_master, int link,
>> +               uint8_t id, uint32_t addr, void *val, size_t size)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>> +       struct fsi_gpio_msg cmd;
>> +
>> +       if (link != 0)
>> +               return -ENODEV;
>> +
>> +       build_abs_ar_command(&cmd, id, addr, size, NULL);
>> +       return fsi_master_gpio_xfer(master, id, &cmd, size, val);
>> +}
>> +
>> +static int fsi_master_gpio_write(struct fsi_master *_master, int link,
>> +               uint8_t id, uint32_t addr, const void *val, size_t size)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>> +       struct fsi_gpio_msg cmd;
>> +
>> +       if (link != 0)
>> +               return -ENODEV;
>> +
>> +       build_abs_ar_command(&cmd, id, addr, size, val);
>> +       return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
>> +}
>> +
>> +static int fsi_master_gpio_term(struct fsi_master *_master,
>> +               int link, uint8_t id)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>> +       struct fsi_gpio_msg cmd;
>> +
>> +       if (link != 0)
>> +               return -ENODEV;
>> +
>> +       build_term_command(&cmd, id);
>> +       return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
>> +}
>> +
>> +/*
>> + * Issue a break command on link
>> + */
>> +static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>> +
>> +       if (link != 0)
>> +               return -ENODEV;
>> +
>> +       set_sda_output(master, 1);
>> +       sda_out(master, 1);
>> +       clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
>> +       sda_out(master, 0);
>> +       clock_toggle(master, FSI_BREAK_CLOCKS);
>> +       echo_delay(master);
>> +       sda_out(master, 1);
>> +       clock_toggle(master, FSI_POST_BREAK_CLOCKS);
>> +
>> +       /* Wait for logic reset to take effect */
>> +       udelay(200);
>> +
>> +       return 0;
>> +}
>> +
>> +static void fsi_master_gpio_init(struct fsi_master_gpio *master)
>> +{
>> +       if (master->gpio_mux)
>> +               gpiod_direction_output(master->gpio_mux, 1);
>> +       if (master->gpio_trans)
>> +               gpiod_direction_output(master->gpio_trans, 1);
>> +       if (master->gpio_enable)
>> +               gpiod_direction_output(master->gpio_enable, 1);
>> +       gpiod_direction_output(master->gpio_clk, 1);
>> +       gpiod_direction_output(master->gpio_data, 1);
>> +
>> +       /* todo: evaluate if clocks can be reduced */
>> +       clock_zeros(master, FSI_INIT_CLOCKS);
>> +}
>> +
>> +static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>> +
>> +       if (link != 0)
>> +               return -ENODEV;
>> +       if (master->gpio_enable)
>> +               gpiod_set_value(master->gpio_enable, 1);
>> +
>> +       return 0;
>> +}
>> +
>> +static void fsi_master_gpio_release(struct device *dev)
>> +{
>> +       struct fsi_master_gpio *master = to_fsi_master_gpio(
>> +                                               dev_to_fsi_master(dev));
>> +
>> +       kfree(master);
>> +}
>> +
>> +static int fsi_master_gpio_probe(struct platform_device *pdev)
>> +{
>> +       struct fsi_master_gpio *master;
>> +       struct gpio_desc *gpio;
>> +
>> +       master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
>> +       if (!master)
>> +               return -ENOMEM;
>> +
>> +       master->dev = &pdev->dev;
>> +       master->master.dev.parent = master->dev;
>> +       master->master.dev.release = fsi_master_gpio_release;
>> +
>> +       gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
>> +       if (IS_ERR(gpio)) {
>> +               dev_err(&pdev->dev, "failed to get clock gpio\n");
>> +               return PTR_ERR(gpio);
>> +       }
>> +       master->gpio_clk = gpio;
>> +
>> +       gpio = devm_gpiod_get(&pdev->dev, "data", 0);
>> +       if (IS_ERR(gpio)) {
>> +               dev_err(&pdev->dev, "failed to get data gpio\n");
>> +               return PTR_ERR(gpio);
>> +       }
>> +       master->gpio_data = gpio;
>> +
>> +       /* Optional GPIOs */
>> +       gpio = devm_gpiod_get_optional(&pdev->dev, "trans", 0);
>> +       if (IS_ERR(gpio)) {
>> +               dev_err(&pdev->dev, "failed to get trans gpio\n");
>> +               return PTR_ERR(gpio);
>> +       }
>> +       master->gpio_trans = gpio;
>> +
>> +       gpio = devm_gpiod_get_optional(&pdev->dev, "enable", 0);
>> +       if (IS_ERR(gpio)) {
>> +               dev_err(&pdev->dev, "failed to get enable gpio\n");
>> +               return PTR_ERR(gpio);
>> +       }
>> +       master->gpio_enable = gpio;
>> +
>> +       gpio = devm_gpiod_get_optional(&pdev->dev, "mux", 0);
>> +       if (IS_ERR(gpio)) {
>> +               dev_err(&pdev->dev, "failed to get mux gpio\n");
>> +               return PTR_ERR(gpio);
>> +       }
>> +       master->gpio_mux = gpio;
>> +
>> +       master->master.n_links = 1;
>> +       master->master.read = fsi_master_gpio_read;
>> +       master->master.write = fsi_master_gpio_write;
>> +       master->master.term = fsi_master_gpio_term;
>> +       master->master.send_break = fsi_master_gpio_break;
>> +       master->master.link_enable = fsi_master_gpio_link_enable;
>> +       platform_set_drvdata(pdev, master);
>> +
>> +       fsi_master_gpio_init(master);
>> +
>> +       fsi_master_register(&master->master);
>> +
>> +       return 0;
>> +}
>> +
>> +
>> +static int fsi_master_gpio_remove(struct platform_device *pdev)
>> +{
>> +       struct fsi_master_gpio *master = platform_get_drvdata(pdev);
>> +
>> +       devm_gpiod_put(&pdev->dev, master->gpio_clk);
>> +       devm_gpiod_put(&pdev->dev, master->gpio_data);
>> +       if (master->gpio_trans)
>> +               devm_gpiod_put(&pdev->dev, master->gpio_trans);
>> +       if (master->gpio_enable)
>> +               devm_gpiod_put(&pdev->dev, master->gpio_enable);
>> +       if (master->gpio_mux)
>> +               devm_gpiod_put(&pdev->dev, master->gpio_mux);
>> +       fsi_master_unregister(&master->master);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id fsi_master_gpio_match[] = {
>> +       { .compatible = "fsi-master-gpio" },
>> +       { },
>> +};
>> +
>> +static struct platform_driver fsi_master_gpio_driver = {
>> +       .driver = {
>> +               .name           = "fsi-master-gpio",
>> +               .of_match_table = fsi_master_gpio_match,
>> +       },
>> +       .probe  = fsi_master_gpio_probe,
>> +       .remove = fsi_master_gpio_remove,
>> +};
>> +
>> +module_platform_driver(fsi_master_gpio_driver);
>> +MODULE_LICENSE("GPL");
>> --
>> 1.8.2.2
>>
Benjamin Herrenschmidt March 30, 2017, 8:50 p.m. UTC | #3
On Thu, 2017-03-30 at 13:15 -0500, Christopher Bostic wrote:
> > > +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
> > > +                       uint8_t num_bits)
> > > +{
> > > +       uint8_t bit, in_bit;
> > > +
> > > +       set_sda_input(master);
> > > +
> > > +       for (bit = 0; bit < num_bits; bit++) {
> > > +               clock_toggle(master, 1);
> > > +               in_bit = sda_in(master);
> > > +               msg->msg <<= 1;
> > > +               msg->msg |= ~in_bit & 0x1;      /* Data is negative active */
> > > +       }
> > > +       msg->bits += num_bi	ts;
> > > +}
> > > +
> > > +static void serial_out(struct fsi_master_gpio *master,
> > > +                       const struct fsi_gpio_msg *cmd)
> > > +{
> > > +       uint8_t bit;
> > > +       uint64_t msg = ~cmd->msg;       /* Data is negative active */
> > > +       uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
> > > +       uint64_t last_bit = ~0;
> > > +       int next_bit;
> > > +
> > > +       if (!cmd->bits) {
> > > +               dev_warn(master->dev, "trying to output 0 bits\n");
> > > +               return;
> > > +       }
> > > +       set_sda_output(master, 0);
> > > +
> > > +       /* Send the start bit */
> > > +       sda_out(master, 0);
> > > +       clock_toggle(master, 1);
> > > +
> > > +       /* Send the message */
> > > +       for (bit = 0; bit < cmd->bits; bit++) {
> > > +               next_bit = (msg & sda_mask) >> (cmd->bits - 1);
> > > +               if (last_bit ^ next_bit) {
> > > +                       sda_out(master, next_bit);
> > > +                       last_bit = next_bit;
> > > +               }
> > > +               clock_toggle(master, 1);
> > > +               msg <<= 1;
> > > +       }
> > > +}

As I mentioned privately, I don't think this is right, unless your
clock signal is inverted or my protocol spec is wrong...

Your clock toggle is written so you call it right after the rising
edge. It does delay, 0, delay, 1.

But according to the FSI timing diagram I have, you need to establish
the data around the falling edge, it gets sampled by the slave on the
rising edge. So as it is, your code risks violating the slave hold
time.

On input, you need to sample on the falling edge, right before it. You
are sampling after the rising edge, so you aren't leaving enough time
for the slave to establish the data.

You could probably just flip clock_toggle() around. Make it: 0, delay,
1, delay.

That way you can do for sends: sda_out + toggle, and for receive
toggle + sda_in. That will make you establish your output data and
sample right before the falling edge, which should be ok provided the
diagram I have is right.

Cheers,
Ben.
Joel Stanley March 30, 2017, 10:57 p.m. UTC | #4
On Fri, Mar 31, 2017 at 4:45 AM, Christopher Bostic
<cbostic@linux.vnet.ibm.com> wrote:
> Where would you recommend they be placed?  I assume we want them somewhere
> in the  patch set.

Send them as a separate patch set to the Aspeed maintainer (me) and
the ARM list.

Cheers,

Joel
Christopher Bostic April 4, 2017, 5:32 p.m. UTC | #5
On 3/30/17 3:50 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2017-03-30 at 13:15 -0500, Christopher Bostic wrote:
>>>> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
>>>> +                       uint8_t num_bits)
>>>> +{
>>>> +       uint8_t bit, in_bit;
>>>> +
>>>> +       set_sda_input(master);
>>>> +
>>>> +       for (bit = 0; bit < num_bits; bit++) {
>>>> +               clock_toggle(master, 1);
>>>> +               in_bit = sda_in(master);
>>>> +               msg->msg <<= 1;
>>>> +               msg->msg |= ~in_bit & 0x1;      /* Data is negative active */
>>>> +       }
>>>> +       msg->bits += num_bi	ts;
>>>> +}
>>>> +
>>>> +static void serial_out(struct fsi_master_gpio *master,
>>>> +                       const struct fsi_gpio_msg *cmd)
>>>> +{
>>>> +       uint8_t bit;
>>>> +       uint64_t msg = ~cmd->msg;       /* Data is negative active */
>>>> +       uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
>>>> +       uint64_t last_bit = ~0;
>>>> +       int next_bit;
>>>> +
>>>> +       if (!cmd->bits) {
>>>> +               dev_warn(master->dev, "trying to output 0 bits\n");
>>>> +               return;
>>>> +       }
>>>> +       set_sda_output(master, 0);
>>>> +
>>>> +       /* Send the start bit */
>>>> +       sda_out(master, 0);
>>>> +       clock_toggle(master, 1);
>>>> +
>>>> +       /* Send the message */
>>>> +       for (bit = 0; bit < cmd->bits; bit++) {
>>>> +               next_bit = (msg & sda_mask) >> (cmd->bits - 1);
>>>> +               if (last_bit ^ next_bit) {
>>>> +                       sda_out(master, next_bit);
>>>> +                       last_bit = next_bit;
>>>> +               }
>>>> +               clock_toggle(master, 1);
>>>> +               msg <<= 1;
>>>> +       }
>>>> +}
> As I mentioned privately, I don't think this is right, unless your
> clock signal is inverted or my protocol spec is wrong...
>
> Your clock toggle is written so you call it right after the rising
> edge. It does delay, 0, delay, 1.
>
> But according to the FSI timing diagram I have, you need to establish
> the data around the falling edge, it gets sampled by the slave on the
> rising edge. So as it is, your code risks violating the slave hold
> time.
>
> On input, you need to sample on the falling edge, right before it. You
> are sampling after the rising edge, so you aren't leaving enough time
> for the slave to establish the data.
>
> You could probably just flip clock_toggle() around. Make it: 0, delay,
> 1, delay.
>
> That way you can do for sends: sda_out + toggle, and for receive
> toggle + sda_in. That will make you establish your output data and
> sample right before the falling edge, which should be ok provided the
> diagram I have is right.

Hi Ben,

Agreed that there is room for improvement.   I intend to look further 
into your suggestions from here and our private conversation on the 
matter and make changes as appropriate.  I have an open issue to track 
this.  As it exists in this patch reads/writes from master to slave 
fundamentally work.   Given the pervasiveness and time to fully evaluate 
and test any protocol updates I intend address this in the near future 
with a separate follow on patch.

Thanks,
Chris
>
> Cheers,
> Ben.
>
Benjamin Herrenschmidt April 4, 2017, 10:19 p.m. UTC | #6
On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
> Agreed that there is room for improvement.   I intend to look further 
> into your suggestions from here and our private conversation on the 
> matter and make changes as appropriate.  I have an open issue to track 
> this.  As it exists in this patch reads/writes from master to slave 
> fundamentally work.  

My understanding is they "seem to work if you get lucky with the timing
and fall apart under load". Or did I hear wrong ?

>  Given the pervasiveness and time to fully evaluate 
> and test any protocol updates I intend address this in the near future 
> with a separate follow on patch.

Please try the simple change I proposed in my email. It's a 4 or 5
lines change max to your clock_toggle function and how it's called in
send and receive. It should be trivial to check if things still "seem
to work" to begin with.

Do you have some kind of test mechanism that hammers the FSI
continuously ? Such as doing a series of putmemproc/getmemproc &
checking the values ?

Then you can run that while hammering the LPC bus and generally putting
the BMC under load and you'll quickly see if it's reliable or not.

Cheers,
Ben.
Christopher Bostic April 5, 2017, 1:24 a.m. UTC | #7
On 4/4/17 5:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
>> Agreed that there is room for improvement.   I intend to look further
>> into your suggestions from here and our private conversation on the
>> matter and make changes as appropriate.  I have an open issue to track
>> this.  As it exists in this patch reads/writes from master to slave
>> fundamentally work.
> My understanding is they "seem to work if you get lucky with the timing
> and fall apart under load". Or did I hear wrong ?
>
>>   Given the pervasiveness and time to fully evaluate
>> and test any protocol updates I intend address this in the near future
>> with a separate follow on patch.
> Please try the simple change I proposed in my email. It's a 4 or 5
> lines change max to your clock_toggle function and how it's called in
> send and receive. It should be trivial to check if things still "seem
> to work" to begin with.
>
> Do you have some kind of test mechanism that hammers the FSI
> continuously ? Such as doing a series of putmemproc/getmemproc &
> checking the values ?
>
> Then you can run that while hammering the LPC bus and generally putting
> the BMC under load and you'll quickly see if it's reliable or not.

Hi Ben,

I do now have a mechanism that puts the bus under heavy load.  I'll look 
into your changes tomorrow.

Thanks,
Chris
>
> Cheers,
> Ben.
>
Christopher Bostic April 9, 2017, 9:22 p.m. UTC | #8
On 4/4/17 5:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
>> Agreed that there is room for improvement.   I intend to look further
>> into your suggestions from here and our private conversation on the
>> matter and make changes as appropriate.  I have an open issue to track
>> this.  As it exists in this patch reads/writes from master to slave
>> fundamentally work.
> My understanding is they "seem to work if you get lucky with the timing
> and fall apart under load". Or did I hear wrong ?
>
>>   Given the pervasiveness and time to fully evaluate
>> and test any protocol updates I intend address this in the near future
>> with a separate follow on patch.
> Please try the simple change I proposed in my email. It's a 4 or 5
> lines change max to your clock_toggle function and how it's called in
> send and receive. It should be trivial to check if things still "seem
> to work" to begin with.
Hi Benjamin,

I did try reordering the clock delays from: delay, clock 0, delay clock 
1  to:  clock 0, delay, clock 1, delay.
This worked fine.  Making this change also removes the need for having a 
third delay I had in place prior to sampling
SDA when in slave response mode.

A 3 microsecond delay is required, however, to prevent occasional issues 
during heavy FSI bus load stress testing.
A 1 nanosecond delay using ndelay(1) had been specified prior to this 
but after looking more closely at real time performance it turned out to 
actually be roughly 1-2 microseconds.   This appears to be the minimum 
resolution using the delay() linux libraries on the AST2400/2500.   
Given this, increasing delay to 3 microseconds doesn't impact 
performance much considering I can now remove the sample input delay 
based on your recommendations to re-order the two clock delays.

Thanks for your input.
Chris

>
> Do you have some kind of test mechanism that hammers the FSI
> continuously ? Such as doing a series of putmemproc/getmemproc &
> checking the values ?
>
> Then you can run that while hammering the LPC bus and generally putting
> the BMC under load and you'll quickly see if it's reliable or not.
>
> Cheers,
> Ben.
>
Benjamin Herrenschmidt April 9, 2017, 9:41 p.m. UTC | #9
On Sun, 2017-04-09 at 16:22 -0500, Christopher Bostic wrote:
> A 3 microsecond delay is required, however, to prevent occasional issues 
> during heavy FSI bus load stress testing.
> A 1 nanosecond delay using ndelay(1) had been specified prior to this 
> but after looking more closely at real time performance it turned out to 
> actually be roughly 1-2 microseconds.   This appears to be the minimum 
> resolution using the delay() linux libraries on the AST2400/2500.   
> Given this, increasing delay to 3 microseconds doesn't impact 
> performance much considering I can now remove the sample input delay 
> based on your recommendations to re-order the two clock delays.

This is huge delays. We should consider a AST2xxx specific variant of
the backend that uses nops or similar lab-calibrated constructs
instead. Otherwise we are stuck in the kHz range, this is a >200Mhz bus
:)

I don't understand why 3us delay would thus be necessary.

Where about did you observe issues ? Could it be that you don't wait
long enough in the transitions from write to read ?

Cheers,
Ben.
Benjamin Herrenschmidt April 9, 2017, 9:53 p.m. UTC | #10
On Mon, 2017-04-10 at 07:41 +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2017-04-09 at 16:22 -0500, Christopher Bostic wrote:
> > A 3 microsecond delay is required, however, to prevent occasional
> > issues 
> > during heavy FSI bus load stress testing.
> > A 1 nanosecond delay using ndelay(1) had been specified prior to
> > this 
> > but after looking more closely at real time performance it turned
> > out to 
> > actually be roughly 1-2 microseconds.   This appears to be the
> > minimum 
> > resolution using the delay() linux libraries on the
> > AST2400/2500.   
> > Given this, increasing delay to 3 microseconds doesn't impact 
> > performance much considering I can now remove the sample input
> > delay 
> > based on your recommendations to re-order the two clock delays.
> 
> This is huge delays. We should consider a AST2xxx specific variant of
> the backend that uses nops or similar lab-calibrated constructs
> instead. Otherwise we are stuck in the kHz range, this is a >200Mhz
> bus
> :)
> 
> I don't understand why 3us delay would thus be necessary.
> 
> Where about did you observe issues ? Could it be that you don't wait
> long enough in the transitions from write to read ?

FYI. pdbg in userspace operates without any  delays in practice, the
overhead between the various load/store instructions seems sufficient.

The only delay that's needed is when going through the FSI2PIB (to do
SCOMs) where it seems like back-2-back accesses can be problematic.

Cheers,
Ben
Benjamin Herrenschmidt April 9, 2017, 9:55 p.m. UTC | #11
On Mon, 2017-04-10 at 07:53 +1000, Benjamin Herrenschmidt wrote:
> FYI. pdbg in userspace operates without any  delays in practice, the
> overhead between the various load/store instructions seems
> sufficient.
> 
> The only delay that's needed is when going through the FSI2PIB (to do
> SCOMs) where it seems like back-2-back accesses can be problematic.

Note: This isn't an objection to merging the patches. We can look at
doing an improved backend later.

Cheers,
Ben.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index 1d2fc1e..cf7d7d7 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -29,6 +29,16 @@ 
 			reg = <0x5f000000 0x01000000>; /* 16M */
 		};
 	};
+
+	gpio-fsi {
+		compatible = "fsi-master-gpio", "fsi-master";
+
+		clock-gpios = <&gpio ASPEED_GPIO(A, 4) GPIO_ACTIVE_HIGH>;
+		data-gpios = <&gpio ASPEED_GPIO(A, 5) GPIO_ACTIVE_HIGH>;
+		mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
+		enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
+		trans-gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &uart5 {
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 7a3b2b5..2fd7db7 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -29,6 +29,18 @@ 
 			reg = <0xbf000000 0x01000000>; /* 16M */
 		};
 	};
+
+	gpio-fsi {
+		compatible = "fsi-master-gpio", "fsi-master";
+
+		status = "okay";
+
+		clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
+		data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
+		mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
+		enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
+		trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &uart5 {
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 04c1a0e..9cf8345 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -9,4 +9,15 @@  config FSI
 	---help---
 	  FSI - the FRU Support Interface - is a simple bus for low-level
 	  access to POWER-based hardware.
+
+if FSI
+
+config FSI_MASTER_GPIO
+	tristate "GPIO-based FSI master"
+	depends on FSI && GPIOLIB
+	---help---
+	This option enables a FSI master driver using GPIO lines.
+
+endif
+
 endmenu
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index db0e5e7..ed28ac0 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -1,2 +1,3 @@ 
 
 obj-$(CONFIG_FSI) += fsi-core.o
+obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
new file mode 100644
index 0000000..0bf5caa
--- /dev/null
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -0,0 +1,614 @@ 
+/*
+ * A FSI master controller, using a simple GPIO bit-banging interface
+ */
+
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/fsi.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "fsi-master.h"
+
+#define	FSI_GPIO_STD_DLY	1	/* Standard pin delay in nS */
+#define	FSI_ECHO_DELAY_CLOCKS	16	/* Number clocks for echo delay */
+#define	FSI_PRE_BREAK_CLOCKS	50	/* Number clocks to prep for break */
+#define	FSI_BREAK_CLOCKS	256	/* Number of clocks to issue break */
+#define	FSI_POST_BREAK_CLOCKS	16000	/* Number clocks to set up cfam */
+#define	FSI_INIT_CLOCKS		5000	/* Clock out any old data */
+#define	FSI_GPIO_STD_DELAY	10	/* Standard GPIO delay in nS */
+					/* todo: adjust down as low as */
+					/* possible or eliminate */
+#define	FSI_GPIO_CMD_DPOLL      0x2
+#define	FSI_GPIO_CMD_TERM	0x3f
+#define FSI_GPIO_CMD_ABS_AR	0x4
+
+#define	FSI_GPIO_DPOLL_CLOCKS	100      /* < 21 will cause slave to hang */
+
+/* Bus errors */
+#define	FSI_GPIO_ERR_BUSY	1	/* Slave stuck in busy state */
+#define	FSI_GPIO_RESP_ERRA	2	/* Any (misc) Error */
+#define	FSI_GPIO_RESP_ERRC	3	/* Slave reports master CRC error */
+#define	FSI_GPIO_MTOE		4	/* Master time out error */
+#define	FSI_GPIO_CRC_INVAL	5	/* Master reports slave CRC error */
+
+/* Normal slave responses */
+#define	FSI_GPIO_RESP_BUSY	1
+#define	FSI_GPIO_RESP_ACK	0
+#define	FSI_GPIO_RESP_ACKD	4
+
+#define	FSI_GPIO_MAX_BUSY	100
+#define	FSI_GPIO_MTOE_COUNT	1000
+#define	FSI_GPIO_DRAIN_BITS	20
+#define	FSI_GPIO_CRC_SIZE	4
+#define	FSI_GPIO_MSG_ID_SIZE		2
+#define	FSI_GPIO_MSG_RESPID_SIZE	2
+#define	FSI_GPIO_PRIME_SLAVE_CLOCKS	100
+
+static DEFINE_SPINLOCK(fsi_gpio_cmd_lock);	/* lock around fsi commands */
+
+struct fsi_master_gpio {
+	struct fsi_master	master;
+	struct device		*dev;
+	struct gpio_desc	*gpio_clk;
+	struct gpio_desc	*gpio_data;
+	struct gpio_desc	*gpio_trans;	/* Voltage translator */
+	struct gpio_desc	*gpio_enable;	/* FSI enable */
+	struct gpio_desc	*gpio_mux;	/* Mux control */
+};
+
+#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
+
+struct fsi_gpio_msg {
+	uint64_t	msg;
+	uint8_t		bits;
+};
+
+static void clock_toggle(struct fsi_master_gpio *master, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		ndelay(FSI_GPIO_STD_DLY);
+		gpiod_set_value(master->gpio_clk, 0);
+		ndelay(FSI_GPIO_STD_DLY);
+		gpiod_set_value(master->gpio_clk, 1);
+	}
+}
+
+static int sda_in(struct fsi_master_gpio *master)
+{
+	int in;
+
+	ndelay(FSI_GPIO_STD_DLY);
+	in = gpiod_get_value(master->gpio_data);
+	return in ? 1 : 0;
+}
+
+static void sda_out(struct fsi_master_gpio *master, int value)
+{
+	gpiod_set_value(master->gpio_data, value);
+}
+
+static void set_sda_input(struct fsi_master_gpio *master)
+{
+	gpiod_direction_input(master->gpio_data);
+	if (master->gpio_trans)
+		gpiod_set_value(master->gpio_trans, 0);
+}
+
+static void set_sda_output(struct fsi_master_gpio *master, int value)
+{
+	if (master->gpio_trans)
+		gpiod_set_value(master->gpio_trans, 1);
+	gpiod_direction_output(master->gpio_data, value);
+}
+
+static void clock_zeros(struct fsi_master_gpio *master, int count)
+{
+	set_sda_output(master, 1);
+	clock_toggle(master, count);
+}
+
+static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
+			uint8_t num_bits)
+{
+	uint8_t bit, in_bit;
+
+	set_sda_input(master);
+
+	for (bit = 0; bit < num_bits; bit++) {
+		clock_toggle(master, 1);
+		in_bit = sda_in(master);
+		msg->msg <<= 1;
+		msg->msg |= ~in_bit & 0x1;	/* Data is negative active */
+	}
+	msg->bits += num_bits;
+}
+
+static void serial_out(struct fsi_master_gpio *master,
+			const struct fsi_gpio_msg *cmd)
+{
+	uint8_t bit;
+	uint64_t msg = ~cmd->msg;	/* Data is negative active */
+	uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
+	uint64_t last_bit = ~0;
+	int next_bit;
+
+	if (!cmd->bits) {
+		dev_warn(master->dev, "trying to output 0 bits\n");
+		return;
+	}
+	set_sda_output(master, 0);
+
+	/* Send the start bit */
+	sda_out(master, 0);
+	clock_toggle(master, 1);
+
+	/* Send the message */
+	for (bit = 0; bit < cmd->bits; bit++) {
+		next_bit = (msg & sda_mask) >> (cmd->bits - 1);
+		if (last_bit ^ next_bit) {
+			sda_out(master, next_bit);
+			last_bit = next_bit;
+		}
+		clock_toggle(master, 1);
+		msg <<= 1;
+	}
+}
+
+static void msg_push_bits(struct fsi_gpio_msg *msg, uint64_t data, int bits)
+{
+	msg->msg <<= bits;
+	msg->msg |= data & ((1ull << bits) - 1);
+	msg->bits += bits;
+}
+
+static void msg_push_crc(struct fsi_gpio_msg *msg)
+{
+	uint8_t crc;
+	int top;
+
+	top = msg->bits & 0x3;
+
+	/* start bit, and any non-aligned top bits */
+	crc = fsi_crc4(0,
+			1 << top | msg->msg >> (msg->bits - top),
+			top + 1);
+
+	/* aligned bits */
+	crc = fsi_crc4(crc, msg->msg, msg->bits - top);
+
+	msg_push_bits(msg, crc, 4);
+}
+
+static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
+		uint8_t id, uint32_t addr, size_t size, const void *data)
+{
+	bool write = !!data;
+	uint8_t ds;
+	int i;
+
+	cmd->bits = 0;
+	cmd->msg = 0;
+
+	msg_push_bits(cmd, id, 2);
+	msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
+	msg_push_bits(cmd, write ? 0 : 1, 1);
+
+	/*
+	 * The read/write size is encoded in the lower bits of the address
+	 * (as it must be naturally-aligned), and the following ds bit.
+	 *
+	 *	size	addr:1	addr:0	ds
+	 *	1	x	x	0
+	 *	2	x	0	1
+	 *	4	0	1	1
+	 *
+	 */
+	ds = size > 1 ? 1 : 0;
+	addr &= ~(size - 1);
+	if (size == 4)
+		addr |= 1;
+
+	msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
+	msg_push_bits(cmd, ds, 1);
+	for (i = 0; write && i < size; i++)
+		msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
+
+	msg_push_crc(cmd);
+}
+
+static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
+{
+	cmd->bits = 0;
+	cmd->msg = 0;
+
+	msg_push_bits(cmd, slave_id, 2);
+	msg_push_bits(cmd, FSI_GPIO_CMD_DPOLL, 3);
+	msg_push_crc(cmd);
+}
+
+static void echo_delay(struct fsi_master_gpio *master)
+{
+	set_sda_output(master, 1);
+	clock_toggle(master, FSI_ECHO_DELAY_CLOCKS);
+}
+
+static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
+{
+	cmd->bits = 0;
+	cmd->msg = 0;
+
+	msg_push_bits(cmd, slave_id, 2);
+	msg_push_bits(cmd, FSI_GPIO_CMD_TERM, 6);
+	msg_push_crc(cmd);
+}
+
+/*
+ * Store information on master errors so handler can detect and clean
+ * up the bus
+ */
+static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
+{
+
+}
+
+static int read_one_response(struct fsi_master_gpio *master,
+		uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
+{
+	struct fsi_gpio_msg msg;
+	uint8_t id, tag;
+	uint32_t crc;
+	int i;
+
+	/* wait for the start bit */
+	for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
+		msg.bits = 0;
+		msg.msg = 0;
+		serial_in(master, &msg, 1);
+		if (msg.msg)
+			break;
+	}
+	if (i >= FSI_GPIO_MTOE_COUNT) {
+		dev_dbg(master->dev,
+			"Master time out waiting for response\n");
+		fsi_master_gpio_error(master, FSI_GPIO_MTOE);
+		return -EIO;
+	}
+
+	msg.bits = 0;
+	msg.msg = 0;
+
+	/* Read slave ID & response tag */
+	serial_in(master, &msg, 4);
+
+	id = (msg.msg >> FSI_GPIO_MSG_RESPID_SIZE) & 0x3;
+	tag = msg.msg & 0x3;
+
+	/* if we have an ACK, and we're expecting data, clock the
+	 * data in too
+	 */
+	if (tag == FSI_GPIO_RESP_ACK && data_size)
+		serial_in(master, &msg, data_size * 8);
+
+	/* read CRC */
+	serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
+
+	/* we have a whole message now; check CRC */
+	crc = fsi_crc4(0, 1, 1);
+	crc = fsi_crc4(crc, msg.msg, msg.bits);
+	if (crc) {
+		dev_dbg(master->dev, "ERR response CRC\n");
+		fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
+		return -EIO;
+	}
+
+	if (msgp)
+		*msgp = msg;
+	if (tagp)
+		*tagp = tag;
+
+	return 0;
+}
+
+static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
+{
+	struct fsi_gpio_msg cmd;
+	uint8_t tag;
+	int rc;
+
+	build_term_command(&cmd, slave);
+	serial_out(master, &cmd);
+	echo_delay(master);
+
+	rc = read_one_response(master, 0, NULL, &tag);
+	if (rc) {
+		dev_err(master->dev,
+				"TERM failed; lost communication with slave\n");
+		return -EIO;
+	} else if (tag != FSI_GPIO_RESP_ACK) {
+		dev_err(master->dev, "TERM failed; response %d\n", tag);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int poll_for_response(struct fsi_master_gpio *master,
+		uint8_t slave, uint8_t size, void *data)
+{
+	struct fsi_gpio_msg response, cmd;
+	int busy_count = 0, rc, i;
+	uint8_t tag;
+
+retry:
+	rc = read_one_response(master, size, &response, &tag);
+	if (rc)
+		return rc;
+
+	switch (tag) {
+	case FSI_GPIO_RESP_ACK:
+		if (size && data) {
+			uint64_t val = response.msg;
+			/* clear crc & mask */
+			val >>= 4;
+			val &= (1ull << (size * 8)) - 1;
+
+			for (i = 0; i < size; i++) {
+				((uint8_t *)data)[size-i-1] =
+					val & 0xff;
+				val >>= 8;
+			}
+		}
+		break;
+	case FSI_GPIO_RESP_BUSY:
+		/*
+		 * Its necessary to clock slave before issuing
+		 * d-poll, not indicated in the hardware protocol
+		 * spec. < 20 clocks causes slave to hang, 21 ok.
+		 */
+		clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
+		if (busy_count++ < FSI_GPIO_MAX_BUSY) {
+			build_dpoll_command(&cmd, slave);
+			serial_out(master, &cmd);
+			echo_delay(master);
+			goto retry;
+		}
+		dev_warn(master->dev,
+			"ERR slave is stuck in busy state, issuing TERM\n");
+		issue_term(master, slave);
+		rc = -EIO;
+		break;
+
+	case FSI_GPIO_RESP_ERRA:
+	case FSI_GPIO_RESP_ERRC:
+		dev_dbg(master->dev, "ERR%c received: 0x%x\n",
+			tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
+			(int)response.msg);
+		fsi_master_gpio_error(master, response.msg);
+		rc = -EIO;
+		break;
+	}
+
+	/* Clock the slave enough to be ready for next operation */
+	clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
+	return rc;
+}
+
+static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
+		struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
+{
+	unsigned long flags;
+	int rc;
+
+	spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
+	serial_out(master, cmd);
+	echo_delay(master);
+	rc = poll_for_response(master, slave, resp_len, resp);
+	spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
+
+	return rc;
+}
+
+static int fsi_master_gpio_read(struct fsi_master *_master, int link,
+		uint8_t id, uint32_t addr, void *val, size_t size)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+	struct fsi_gpio_msg cmd;
+
+	if (link != 0)
+		return -ENODEV;
+
+	build_abs_ar_command(&cmd, id, addr, size, NULL);
+	return fsi_master_gpio_xfer(master, id, &cmd, size, val);
+}
+
+static int fsi_master_gpio_write(struct fsi_master *_master, int link,
+		uint8_t id, uint32_t addr, const void *val, size_t size)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+	struct fsi_gpio_msg cmd;
+
+	if (link != 0)
+		return -ENODEV;
+
+	build_abs_ar_command(&cmd, id, addr, size, val);
+	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+}
+
+static int fsi_master_gpio_term(struct fsi_master *_master,
+		int link, uint8_t id)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+	struct fsi_gpio_msg cmd;
+
+	if (link != 0)
+		return -ENODEV;
+
+	build_term_command(&cmd, id);
+	return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
+}
+
+/*
+ * Issue a break command on link
+ */
+static int fsi_master_gpio_break(struct fsi_master *_master, int link)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+
+	set_sda_output(master, 1);
+	sda_out(master, 1);
+	clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
+	sda_out(master, 0);
+	clock_toggle(master, FSI_BREAK_CLOCKS);
+	echo_delay(master);
+	sda_out(master, 1);
+	clock_toggle(master, FSI_POST_BREAK_CLOCKS);
+
+	/* Wait for logic reset to take effect */
+	udelay(200);
+
+	return 0;
+}
+
+static void fsi_master_gpio_init(struct fsi_master_gpio *master)
+{
+	if (master->gpio_mux)
+		gpiod_direction_output(master->gpio_mux, 1);
+	if (master->gpio_trans)
+		gpiod_direction_output(master->gpio_trans, 1);
+	if (master->gpio_enable)
+		gpiod_direction_output(master->gpio_enable, 1);
+	gpiod_direction_output(master->gpio_clk, 1);
+	gpiod_direction_output(master->gpio_data, 1);
+
+	/* todo: evaluate if clocks can be reduced */
+	clock_zeros(master, FSI_INIT_CLOCKS);
+}
+
+static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
+
+	if (link != 0)
+		return -ENODEV;
+	if (master->gpio_enable)
+		gpiod_set_value(master->gpio_enable, 1);
+
+	return 0;
+}
+
+static void fsi_master_gpio_release(struct device *dev)
+{
+	struct fsi_master_gpio *master = to_fsi_master_gpio(
+						dev_to_fsi_master(dev));
+
+	kfree(master);
+}
+
+static int fsi_master_gpio_probe(struct platform_device *pdev)
+{
+	struct fsi_master_gpio *master;
+	struct gpio_desc *gpio;
+
+	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return -ENOMEM;
+
+	master->dev = &pdev->dev;
+	master->master.dev.parent = master->dev;
+	master->master.dev.release = fsi_master_gpio_release;
+
+	gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
+	if (IS_ERR(gpio)) {
+		dev_err(&pdev->dev, "failed to get clock gpio\n");
+		return PTR_ERR(gpio);
+	}
+	master->gpio_clk = gpio;
+
+	gpio = devm_gpiod_get(&pdev->dev, "data", 0);
+	if (IS_ERR(gpio)) {
+		dev_err(&pdev->dev, "failed to get data gpio\n");
+		return PTR_ERR(gpio);
+	}
+	master->gpio_data = gpio;
+
+	/* Optional GPIOs */
+	gpio = devm_gpiod_get_optional(&pdev->dev, "trans", 0);
+	if (IS_ERR(gpio)) {
+		dev_err(&pdev->dev, "failed to get trans gpio\n");
+		return PTR_ERR(gpio);
+	}
+	master->gpio_trans = gpio;
+
+	gpio = devm_gpiod_get_optional(&pdev->dev, "enable", 0);
+	if (IS_ERR(gpio)) {
+		dev_err(&pdev->dev, "failed to get enable gpio\n");
+		return PTR_ERR(gpio);
+	}
+	master->gpio_enable = gpio;
+
+	gpio = devm_gpiod_get_optional(&pdev->dev, "mux", 0);
+	if (IS_ERR(gpio)) {
+		dev_err(&pdev->dev, "failed to get mux gpio\n");
+		return PTR_ERR(gpio);
+	}
+	master->gpio_mux = gpio;
+
+	master->master.n_links = 1;
+	master->master.read = fsi_master_gpio_read;
+	master->master.write = fsi_master_gpio_write;
+	master->master.term = fsi_master_gpio_term;
+	master->master.send_break = fsi_master_gpio_break;
+	master->master.link_enable = fsi_master_gpio_link_enable;
+	platform_set_drvdata(pdev, master);
+
+	fsi_master_gpio_init(master);
+
+	fsi_master_register(&master->master);
+
+	return 0;
+}
+
+
+static int fsi_master_gpio_remove(struct platform_device *pdev)
+{
+	struct fsi_master_gpio *master = platform_get_drvdata(pdev);
+
+	devm_gpiod_put(&pdev->dev, master->gpio_clk);
+	devm_gpiod_put(&pdev->dev, master->gpio_data);
+	if (master->gpio_trans)
+		devm_gpiod_put(&pdev->dev, master->gpio_trans);
+	if (master->gpio_enable)
+		devm_gpiod_put(&pdev->dev, master->gpio_enable);
+	if (master->gpio_mux)
+		devm_gpiod_put(&pdev->dev, master->gpio_mux);
+	fsi_master_unregister(&master->master);
+
+	return 0;
+}
+
+static const struct of_device_id fsi_master_gpio_match[] = {
+	{ .compatible = "fsi-master-gpio" },
+	{ },
+};
+
+static struct platform_driver fsi_master_gpio_driver = {
+	.driver = {
+		.name		= "fsi-master-gpio",
+		.of_match_table	= fsi_master_gpio_match,
+	},
+	.probe	= fsi_master_gpio_probe,
+	.remove = fsi_master_gpio_remove,
+};
+
+module_platform_driver(fsi_master_gpio_driver);
+MODULE_LICENSE("GPL");