在 2023/6/8 下午6:15, Andy Shevchenko 写道:
On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> wrote:
This bus driver supports the Loongson SPI hardware controller in the
Loongson platforms and supports to use DTS and PCI framework to
the use
okay, I got it.
register SPI device resources.
Thank you for an update. I have a few nit-picks below, but in general
this version is good (esp. if you address them)
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
You're welcome, I will add the reviewed-by in v13.
...
+static void loongson_spi_set_cs(struct spi_device *spi, bool val)
+{
+ int cs;
+ struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
+
+ cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG)
+ & ~(0x11 << spi_get_chipselect(spi, 0));
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG,
+ (val ? (0x11 << spi_get_chipselect(spi, 0)) :
+ (0x1 << spi_get_chipselect(spi, 0))) | cs);
Can be done as
static void loongson_spi_set_cs(struct spi_device *spi, bool en)
unsigned char mask = (BIT(4) | BIT(0)) << spi_get_chipselect(spi, 0);
unsigned char val = en ? mask : (BIT(0) << spi_get_chipselect(spi, 0));
cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG) & ~mask;
loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, val | cs);
okay, I will do it.
(Renamed variables to be consistent with the other uses in the driver below)
sorry, I don't got it. Are you referring to the following changes ?
loongson_spi_set_cs(spi, 1) => loongson_spi_set_cs(spi, true)
+}
+
+static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz)
+{
+ unsigned char val;
+ unsigned int div, div_tmp;
+ static const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
+
+ div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096);
+ div_tmp = rdiv[fls(div - 1)];
+ loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0;
+ loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2;
+ val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
val &= GENMASK(1, 0);
This seems to be "val &= ~GENMASK(1, 0);"
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) |
+ loongson_spi->spcr);
loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val
| loongson_spi->spcr);
okay, I got it.
+ val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);
val &= GENMASK(1, 0);
This seems to be "val &= ~GENMASK(1, 0);"
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) |
+ loongson_spi->sper);
loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, val
| loongson_spi->sper);
okay, I got it.
+ loongson_spi->hz = hz;
+}
+
+static void loongson_spi_set_mode(struct loongson_spi *loongson_spi,
+ struct spi_device *spi)
+{
+ unsigned char val;
+
+ val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
+ val &= ~(LOONGSON_SPI_SPCR_CPOL | LOONGSON_SPI_SPCR_CPHA);
+ if (spi->mode & SPI_CPOL)
+ val |= LOONGSON_SPI_SPCR_CPOL;
+ if (spi->mode & SPI_CPHA)
+ val |= LOONGSON_SPI_SPCR_CPHA;
+
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val);
+ loongson_spi->mode |= spi->mode;
+}
+
+static int loongson_spi_update_state(struct loongson_spi *loongson_spi,
+ struct spi_device *spi, struct spi_transfer *t)
+{
+ if (t && loongson_spi->hz != t->speed_hz)
+ loongson_spi_set_clk(loongson_spi, t->speed_hz);
+
+ if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK)
+ loongson_spi_set_mode(loongson_spi, spi);
+
+ return 0;
+}
+
+static int loongson_spi_setup(struct spi_device *spi)
+{
+ struct loongson_spi *loongson_spi;
+
+ loongson_spi = spi_controller_get_devdata(spi->controller);
+ if (spi->bits_per_word % 8)
+ return -EINVAL;
+
+ if (spi_get_chipselect(spi, 0) >= spi->controller->num_chipselect)
+ return -EINVAL;
+
+ loongson_spi->hz = 0;
+ loongson_spi_set_cs(spi, 1);
+
+ return 0;
+}
+
+static int loongson_spi_write_read_8bit(struct spi_device *spi, const u8 **tx_buf,
+ u8 **rx_buf, unsigned int num)
+{
+ int ret;
+ struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
+
+ if (tx_buf && *tx_buf)
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, *((*tx_buf)++));
+ else
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, 0);
+ Blank line
okay, I will add the blank line.
+ ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
+ (loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);
(loongson_spi->spsr &
LOONGSON_SPI_SPSR_RFEMPTY) != LOONGSON_SPI_SPSR_RFEMPTY,
1, MSEC_PER_SEC);
okay, I got it.
+ if (rx_buf && *rx_buf)
+ *(*rx_buf)++ = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
+ else
+ loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
+
+ return ret;
+}
+
+static int loongson_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
+{
+ int ret;
+ unsigned int count;
+ const u8 *tx = xfer->tx_buf;
+ u8 *rx = xfer->rx_buf;
+
+ count = xfer->len;
+
Unneeded blank line.
okay, I will remove the blank line.
+ do {
+ ret = loongson_spi_write_read_8bit(spi, &tx, &rx, count);
+ if (ret)
+ break;
+ } while (--count);
+
+ return ret;
+}
+
+static int loongson_spi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
+{
+ struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctlr);
+ loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);
+ loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para & ~1);
BIT(0) ?
LOONGSON_SPI_PARA_MEM_EN ?
I will use LOONGSON_SPI_PARA_MEM_EN.
+ return 0;
+}
+
...
+int loongson_spi_init_controller(struct device *dev, void __iomem *regs)
+{
+ struct spi_controller *controller;
+ struct loongson_spi *spi;
+ struct clk *clk;
+
+ controller = devm_spi_alloc_host(dev, sizeof(struct loongson_spi));
+ if (controller == NULL)
+ return -ENOMEM;
+
+ controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
... = SPI_MODE_X_MASK | SPI_CS_HIGH;
okay, I got it.
Thanks,
Yinbo
