On Fri, Aug 26, 2022 at 01:57:21PM +0300, Ivan Bornyakov wrote: > On Thu, Aug 25, 2022 at 02:24:32PM +0300, Ivan Bornyakov wrote: > > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2 > > FPGAs over slave SPI sysCONFIG interface. > > > > Signed-off-by: Ivan Bornyakov <i.bornyakov@xxxxxxxxxxx> > > --- > > > > ... snip ... > > > > + > > +struct sysconfig_priv { > > + struct gpio_desc *program; > > + struct gpio_desc *init; > > + struct gpio_desc *done; > > + struct spi_device *spi; > > + u8 isc_enable_operand; > > + u8 isc_erase_operand; > > +}; > > As Johannes Zink working on adding I2C to MachXO2, I am thinking how > about we add to struct sysconfig_priv a callback > > int (*sysconfig_transfer)(struct sysconfig_priv, > const void *tx_buf, size_t tx_len, > void *rx_buf, size_t rx_len) > > For SPI it would be defined like this: > > static int sysconfig_spi_transfer(struct sysconfig_priv *data, > const void *tx_buf, size_t tx_len, > void *rx_buf, size_t rx_len) > { > if (!rx_buf) > return spi_write(data->spi, tx_buf, tx_size); > > return spi_write_then_read(data->spi, tx_buf, tx_size, rx_buf, rx_size); > } > > And later in sysconfig_ commands we would call this callback. > > Would it be a good enough starting base for Johannes to add I2C interface > for MachXO2? > > > > > ... snip ... > > > > +static int sysconfig_isc_enable(struct sysconfig_priv *data) > > +{ > > + u8 isc_enable[] = SYSCONFIG_ISC_ENABLE; > > + u32 status; > > + int ret; > > + > > + isc_enable[1] = data->isc_enable_operand; > > + > > + ret = spi_write(data->spi, isc_enable, sizeof(isc_enable)); > > + if (ret) > > + return ret; > > + > > + ret = sysconfig_poll_status(data, &status); > > + if (ret || (status & SYSCONFIG_STATUS_FAIL)) > > + return ret ? : -EFAULT; > > + > > + return 0; > > +} > > Quirks for I2C enable and refresh commands are 3-bytes instead of > 4-bytes for SPI can be added here. Just check which of data->spi or > data->i2c is not NULL. > Actually, here is a patch for your tinkering. I guess, what you need to do for MachXO2 I2C support is: 1) implement sysconfig_transfer callback for I2C. Something like this: static int sysconfig_i2c_transfer(struct sysconfig_priv *data, const void *tx_buf, size_t tx_len, void *rx_buf, size_t rx_len) { struct i2c_client *i2c = data->i2c; struct i2c_msg msg[] = { { .addr = i2c->addr, .flags = 0, .buf = tx_buf, .len = tx_len, }, { .addr = i2c->addr, .flags = I2C_M_RD, .buf = rx_buf, .len = rx_len, } }; if (!rx_buf) return i2c_master_send(i2c, tx_buf, tx_len); return i2c_transfer(i2c->adapter, msg, ARRAY_SIZE(msg)); } 2) add quirks for 3-byte transfer to sysconfig_refresh() and sysconfig_isc_enable() 3) add probe routine 4) add separate fpga_manager_ops->write() for I2C state(), write_init() and write_complete() can be reused --- drivers/fpga/lattice-sysconfig-spi.c | 46 ++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c index 145b5b27b88d..c2caf613d2f0 100644 --- a/drivers/fpga/lattice-sysconfig-spi.c +++ b/drivers/fpga/lattice-sysconfig-spi.c @@ -41,8 +41,21 @@ struct sysconfig_priv { struct spi_device *spi; u8 isc_enable_operand; u8 isc_erase_operand; + int (*sysconfig_transfer)(struct sysconfig_priv *data, + const void *tx_buf, size_t tx_len, + void *rx_buf, size_t rx_len); }; +static int sysconfig_spi_transfer(struct sysconfig_priv *data, + const void *tx_buf, size_t tx_len, + void *rx_buf, size_t rx_len) +{ + if (!rx_buf) + return spi_write(data->spi, tx_buf, tx_len); + + return spi_write_then_read(data->spi, tx_buf, tx_len, rx_buf, rx_len); +} + static int sysconfig_poll_busy(struct sysconfig_priv *data) { const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY; @@ -50,9 +63,9 @@ static int sysconfig_poll_busy(struct sysconfig_priv *data) u8 busy; while (retries--) { - ret = spi_write_then_read(data->spi, - lsc_check_busy, sizeof(lsc_check_busy), - &busy, sizeof(busy)); + ret = data->sysconfig_transfer(data, lsc_check_busy, + sizeof(lsc_check_busy), + &busy, sizeof(busy)); if (ret) return ret; @@ -71,9 +84,9 @@ static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status) __be32 device_status; int ret; - ret = spi_write_then_read(data->spi, - lsc_read_status, sizeof(lsc_read_status), - &device_status, sizeof(device_status)); + ret = data->sysconfig_transfer(data, lsc_read_status, + sizeof(lsc_read_status), + &device_status, sizeof(device_status)); if (ret) return ret; @@ -116,7 +129,8 @@ static int sysconfig_refresh(struct sysconfig_priv *data) static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH; int ret; - ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh)); + ret = data->sysconfig_transfer(data, lsc_refresh, sizeof(lsc_refresh), + NULL, 0); if (ret) return ret; @@ -133,7 +147,8 @@ static int sysconfig_isc_enable(struct sysconfig_priv *data) isc_enable[1] = data->isc_enable_operand; - ret = spi_write(data->spi, isc_enable, sizeof(isc_enable)); + ret = data->sysconfig_transfer(data, isc_enable, sizeof(isc_enable), + NULL, 0); if (ret) return ret; @@ -152,7 +167,8 @@ static int sysconfig_isc_erase(struct sysconfig_priv *data) isc_erase[1] = data->isc_erase_operand; - ret = spi_write(data->spi, isc_erase, sizeof(isc_erase)); + ret = data->sysconfig_transfer(data, isc_erase, sizeof(isc_erase), + NULL, 0); if (ret) return ret; @@ -178,7 +194,8 @@ static int sysconfig_lsc_init_addr(struct sysconfig_priv *data) { const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR; - return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr)); + return data->sysconfig_transfer(data, lsc_init_addr, + sizeof(lsc_init_addr), NULL, 0); } static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data) @@ -202,7 +219,8 @@ static int sysconfig_isc_prog_done(struct sysconfig_priv *data) u32 status; int ret; - ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done)); + ret = data->sysconfig_transfer(data, isc_prog_done, + sizeof(isc_prog_done), NULL, 0); if (ret) return ret; @@ -220,7 +238,8 @@ static int sysconfig_isc_disable(struct sysconfig_priv *data) { const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE; - return spi_write(data->spi, isc_disable, sizeof(isc_disable)); + return data->sysconfig_transfer(data, isc_disable, sizeof(isc_disable), + NULL, 0); } static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr) @@ -336,7 +355,7 @@ static int ecp5_ops_write_complete(struct fpga_manager *mgr, spi_bus_unlock(spi->controller); /* Toggle CS and wait for bitstream write to finish */ - ret = spi_write(spi, NULL, 0); + ret = priv->sysconfig_transfer(priv, NULL, 0, NULL, 0); if (!ret) ret = sysconfig_poll_busy(priv); @@ -575,6 +594,7 @@ static int sysconfig_probe(struct spi_device *spi) return -ENOMEM; priv->spi = spi; + priv->sysconfig_transfer = sysconfig_spi_transfer; probe_func = of_device_get_match_data(&spi->dev); if (!probe_func) { -- 2.37.2