RE: [PATCH v4 1/2] SPI: Add SPI driver for Sunplus SP7021

[Lh Kuo 郭力豪 <lh.Kuo@xxxxxxxxxxx>]

Hi Mr Andy Shevchenko

> 
> > +config SPI_SUNPLUS_SP7021
> > +       tristate "Sunplus SP7021 SPI controller"
> > +       depends on SOC_SP7021
> 
> No COMPILE_TEST?
>

I will add it in next next submission




> ...
> 	
> > +config SPI_SUNPLUS_SP7021
> > +       tristate "Sunplus SP7021 SPI controller"
> > +       depends on SOC_SP7021
> 
> No COMPILE_TEST?
>

> 
> > +#define SP7021_FIFO_DATA_BITS     (16 * 8)
> 
> What does this multiplication mean?
> 
> ...
>

It will define the length of the FIFO
I will define it clearly as follows

#define SP7021_FIFO_DATA_LEN     16

> > +       void __iomem *mas_base;
> > +       void __iomem *sla_base;
> 
> These 3 letter abbreviations are not standard, maybe you can simply switch to one letter?
> 
> m_base;
> s_base;
> 
> and so on below?
>

I will add it in next next submission

> +static irqreturn_t sp7021_spi_sla_irq(int irq, void *dev) {
> > +       struct sp7021_spi_ctlr *pspim = dev;
> > +       unsigned int data_status;
> > +
> 
> > +       data_status = readl(pspim->sla_base + SP7021_DATA_RDY_REG);
> > +       writel(data_status | SP7021_SLA_CLR_INT, pspim->sla_base +
> > + SP7021_DATA_RDY_REG);
> 
> Can this IO be run in parallel with some other functions? If so, where is the synchronization?
>

No It can't run with otherfunctions.


> Useless assignment.
> 
> > +       mutex_lock(&pspim->buf_lock);
> > +       reinit_completion(&pspim->sla_isr);
> 
> > +       reg_temp = SP7021_SLA_DMA_EN | SP7021_SLA_DMA_RW |
> > + FIELD_PREP(SP7021_SLA_DMA_CMD, 3);
> 
> Do you really need to have this assignment under the lock?

No, And i will remove the lock for those setting.

> Ditto for the rest of the similar cases.
> 

I will modify those function as below

int sp7021_spi_sla_tx(struct spi_device *spi, struct spi_transfer *xfer)
{
	struct sp7021_spi_ctlr *pspim = spi_controller_get_devdata(spi->controller);

	reinit_completion(&pspim->sla_isr);
	writel(SP7021_SLA_DMA_EN | SP7021_SLA_DMA_RW | FIELD_PREP(SP7021_SLA_DMA_CMD, 3),
	       pspim->s_base + SP7021_SLV_DMA_CTRL_REG);
	writel(xfer->len, pspim->s_base + SP7021_SLV_DMA_LENGTH_REG);
	writel(xfer->tx_dma, pspim->s_base + SP7021_SLV_DMA_ADDR_REG);
	writel(readl(pspim->s_base + SP7021_DATA_RDY_REG) | SP7021_SLA_DATA_RDY,
	       pspim->s_base + SP7021_DATA_RDY_REG);
	if (wait_for_completion_interruptible(&pspim->isr_done)) {
		dev_err(&spi->dev, "%s() wait_for_completion err\n", __func__);
		return -EINTR;
	}
	return 0;
}

int sp7021_spi_sla_rx(struct spi_device *spi, struct spi_transfer *xfer)
{
	struct sp7021_spi_ctlr *pspim = spi_controller_get_devdata(spi->controller);
	int ret = 0;

	reinit_completion(&pspim->isr_done);
	writel(SP7021_SLA_DMA_EN | FIELD_PREP(SP7021_SLA_DMA_CMD, 3),
	       pspim->s_base + SP7021_SLV_DMA_CTRL_REG);
	writel(xfer->len, pspim->s_base + SP7021_SLV_DMA_LENGTH_REG);
	writel(xfer->rx_dma, pspim->s_base + SP7021_SLV_DMA_ADDR_REG);
	if (wait_for_completion_interruptible(&pspim->isr_done)) {
		dev_err(&spi->dev, "%s() wait_for_completion err\n", __func__);
		return -EINTR;
	}
	writel(SP7021_SLA_SW_RST, pspim->s_base + SP7021_SLV_DMA_CTRL_REG);
	return ret;
}




> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Sent: Saturday, December 18, 2021 2:05 AM
> To: Li-hao Kuo <lhjeff911@xxxxxxxxx>
> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>; linux-spi <linux-spi@xxxxxxxxxxxxxxx>; devicetree <devicetree@xxxxxxxxxxxxxxx>;
> Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>; Wells Lu 呂芳騰 <wells.lu@xxxxxxxxxxx>; Lh
> Kuo 郭力豪 <lh.Kuo@xxxxxxxxxxx>
> Subject: Re: [PATCH v4 1/2] SPI: Add SPI driver for Sunplus SP7021
> 
> On Fri, Dec 10, 2021 at 11:02 AM Li-hao Kuo <lhjeff911@xxxxxxxxx> wrote:
> >
> > From: "Li-hao Kuo" <lhjeff911@xxxxxxxxx>
> >
> > Add SPI driver for Sunplus SP7021.
> 
> ...
> 
> > +config SPI_SUNPLUS_SP7021
> > +       tristate "Sunplus SP7021 SPI controller"
> > +       depends on SOC_SP7021
> 
> No COMPILE_TEST?
> 
> > +       help
> > +         This enables Sunplus SP7021 SPI controller driver on the SP7021 SoCs.
> > +         This driver can also be built as a module. If so, the module will be
> > +         called as spi-sunplus-sp7021.
> > +
> > +         If you have a Sunplus SP7021 platform say Y here.
> > +         If unsure, say N.
> 
> ...
> 
> > +#define SP7021_FIFO_DATA_BITS     (16 * 8)
> 
> What does this multiplication mean?
> 
> ...
> 
> > +struct sp7021_spi_ctlr {
> > +       struct device *dev;
> > +       struct spi_controller *ctlr;
> 
> > +       void __iomem *mas_base;
> > +       void __iomem *sla_base;
> 
> These 3 letter abbreviations are not standard, maybe you can simply switch to one letter?
> 
> m_base;
> s_base;
> 
> and so on below?
> 
> Also consider to use terminology controller (ctrl) and peripheral (per).
> 
> > +       u32 xfer_conf;
> > +       int mode;
> > +       int mas_irq;
> > +       int sla_irq;
> > +       struct clk *spi_clk;
> > +       struct reset_control *rstc;
> > +       // irq spin lock
> > +       spinlock_t lock;
> > +       // data xfer lock
> > +       struct mutex buf_lock;
> > +       struct completion isr_done;
> > +       struct completion sla_isr;
> > +       unsigned int  rx_cur_len;
> > +       unsigned int  tx_cur_len;
> > +       unsigned int  data_unit;
> > +       const u8 *tx_buf;
> > +       u8 *rx_buf;
> > +};
> 
> ...
> 
> > +static irqreturn_t sp7021_spi_sla_irq(int irq, void *dev) {
> > +       struct sp7021_spi_ctlr *pspim = dev;
> > +       unsigned int data_status;
> > +
> 
> > +       data_status = readl(pspim->sla_base + SP7021_DATA_RDY_REG);
> > +       writel(data_status | SP7021_SLA_CLR_INT, pspim->sla_base +
> > + SP7021_DATA_RDY_REG);
> 
> Can this IO be run in parallel with some other functions? If so, where is the synchronization?
> 
> > +       complete(&pspim->sla_isr);
> > +       return IRQ_NONE;
> > +}
> 
> ...
> 
> > +int sp7021_spi_sla_tx(struct spi_device *spi, struct spi_transfer
> > +*xfer) {
> > +       struct sp7021_spi_ctlr *pspim =
> > +spi_controller_get_devdata(spi->controller);
> 
> > +       u32 reg_temp = 0;
> 
> Useless assignment.
> 
> > +       mutex_lock(&pspim->buf_lock);
> > +       reinit_completion(&pspim->sla_isr);
> 
> > +       reg_temp = SP7021_SLA_DMA_EN | SP7021_SLA_DMA_RW |
> > + FIELD_PREP(SP7021_SLA_DMA_CMD, 3);
> 
> Do you really need to have this assignment under the lock?
> Ditto for the rest of the similar cases.
> 
> > +       writel_relaxed(reg_temp, pspim->sla_base + SP7021_SLV_DMA_CTRL_REG);
> > +       writel_relaxed(xfer->len, pspim->sla_base + SP7021_SLV_DMA_LENGTH_REG);
> > +       writel_relaxed(xfer->tx_dma, pspim->sla_base + SP7021_SLV_DMA_ADDR_REG);
> > +       writel(readl(pspim->sla_base + SP7021_DATA_RDY_REG) | SP7021_SLA_DATA_RDY,
> > +              pspim->sla_base + SP7021_DATA_RDY_REG);
> > +       if (wait_for_completion_interruptible(&pspim->isr_done)) {
> > +               dev_err(&spi->dev, "%s() wait_for_completion err\n", __func__);
> > +               return -EINTR;
> > +       }
> > +       mutex_unlock(&pspim->buf_lock);
> > +       return 0;
> > +}
> > +void sp7021_spi_mas_rb(struct sp7021_spi_ctlr *pspim, unsigned int
> > +len) {
> > +       int i;
> > +
> > +       for (i = 0; i < len; i++) {
> > +               pspim->rx_buf[pspim->rx_cur_len] =
> > +                       readl(pspim->mas_base + SP7021_FIFO_REG);
> > +               pspim->rx_cur_len++;
> > +       }
> > +}
> > +
> > +void sp7021_spi_mas_wb(struct sp7021_spi_ctlr *pspim, unsigned int
> > +len) {
> > +       int i;
> > +
> > +       for (i = 0; i < len; i++) {
> > +               writel(pspim->tx_buf[pspim->tx_cur_len],
> > +                      pspim->mas_base + SP7021_FIFO_REG);
> > +               pspim->tx_cur_len++;
> > +       }
> > +}
> > +
> > +static irqreturn_t sp7021_spi_mas_irq(int irq, void *dev) {
> > +       struct sp7021_spi_ctlr *pspim = dev;
> > +       unsigned int tx_cnt, total_len;
> > +       unsigned int tx_len, rx_cnt;
> > +       unsigned int fd_status;
> > +       unsigned long flags;
> > +       bool isrdone = false;
> > +
> > +       fd_status = readl(pspim->mas_base + SP7021_SPI_STATUS_REG);
> > +       tx_cnt = FIELD_GET(SP7021_TX_CNT_MASK, fd_status);
> > +       tx_len = FIELD_GET(SP7021_TX_LEN_MASK, fd_status);
> > +       total_len = FIELD_GET(SP7021_GET_LEN_MASK, fd_status);
> > +
> > +       if ((fd_status & SP7021_TX_EMP_FLAG) && (fd_status & SP7021_RX_EMP_FLAG) &&
> total_len == 0)
> > +               return IRQ_NONE;
> > +
> > +       if (tx_len == 0 && total_len == 0)
> > +               return IRQ_NONE;
> > +
> > +       spin_lock_irqsave(&pspim->lock, flags);
> > +
> > +       rx_cnt = FIELD_GET(SP7021_RX_CNT_MASK, fd_status);
> > +       if (fd_status & SP7021_RX_FULL_FLAG)
> > +               rx_cnt = pspim->data_unit;
> > +
> > +       tx_cnt = min(tx_len - pspim->tx_cur_len, pspim->data_unit - tx_cnt);
> > +       dev_dbg(pspim->dev, "fd_st=0x%x rx_c:%d tx_c:%d tx_l:%d",
> > +               fd_status, rx_cnt, tx_cnt, tx_len);
> > +
> > +       if (rx_cnt > 0)
> > +               sp7021_spi_mas_rb(pspim, rx_cnt);
> > +       if (tx_cnt > 0)
> > +               sp7021_spi_mas_wb(pspim, tx_cnt);
> > +
> > +       fd_status = readl(pspim->mas_base + SP7021_SPI_STATUS_REG);
> > +       tx_len = FIELD_GET(SP7021_TX_LEN_MASK, fd_status);
> > +       total_len = FIELD_GET(SP7021_GET_LEN_MASK, fd_status);
> > +
> > +       if (fd_status & SP7021_FINISH_FLAG || tx_len == pspim->tx_cur_len) {
> > +               while (total_len != pspim->rx_cur_len) {
> > +                       fd_status = readl(pspim->mas_base + SP7021_SPI_STATUS_REG);
> > +                       total_len = FIELD_GET(SP7021_GET_LEN_MASK, fd_status);
> > +                       if (fd_status & SP7021_RX_FULL_FLAG)
> > +                               rx_cnt = pspim->data_unit;
> > +                       else
> > +                               rx_cnt = FIELD_GET(SP7021_RX_CNT_MASK,
> > + fd_status);
> > +
> > +                       if (rx_cnt > 0)
> > +                               sp7021_spi_mas_rb(pspim, rx_cnt);
> > +               }
> > +               writel(readl(pspim->mas_base + SP7021_INT_BUSY_REG)
> > +                       | SP7021_CLR_MAS_INT, pspim->mas_base + SP7021_INT_BUSY_REG);
> > +               writel(SP7021_FINISH_FLAG, pspim->mas_base +
> > + SP7021_SPI_STATUS_REG);
> > +
> > +               isrdone = true;
> > +       }
> > +
> > +       if (isrdone)
> > +               complete(&pspim->isr_done);
> > +       spin_unlock_irqrestore(&pspim->lock, flags);
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static void sp7021_prep_transfer(struct spi_controller *ctlr, struct
> > +spi_device *spi) {
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +
> > +       pspim->tx_cur_len = 0;
> > +       pspim->rx_cur_len = 0;
> > +       pspim->data_unit = SP7021_FIFO_DATA_BITS / spi->bits_per_word;
> > +}
> > +
> > +// preliminary set CS, CPOL, CPHA and LSB static int
> > +sp7021_spi_controller_prepare_message(struct spi_controller *ctlr,
> > +                                                struct spi_message
> > +*msg) {
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +       struct spi_device *s = msg->spi;
> > +       u32 rs = 0;
> > +
> > +       writel(readl(pspim->mas_base + SP7021_SPI_STATUS_REG) | SP7021_FD_SW_RST,
> > +              pspim->mas_base + SP7021_SPI_STATUS_REG);
> > +       rs |= SP7021_FD_SEL;
> > +       if (s->mode & SPI_CPOL)
> > +               rs |= SP7021_CPOL_FD;
> > +
> > +       if (s->mode & SPI_LSB_FIRST)
> > +               rs |= SP7021_LSB_SEL;
> > +
> > +       if (s->mode & SPI_CS_HIGH)
> > +               rs |= SP7021_CS_POR;
> > +
> > +       if (s->mode & SPI_CPHA)
> > +               rs |=  SP7021_CPHA_R;
> > +       else
> > +               rs |=  SP7021_CPHA_W;
> > +
> > +       rs |=  FIELD_PREP(SP7021_TX_UNIT, 0) | FIELD_PREP(SP7021_RX_UNIT, 0);
> > +       pspim->xfer_conf = rs;
> > +       if (pspim->xfer_conf & SP7021_CPOL_FD)
> > +               writel(pspim->xfer_conf, pspim->mas_base +
> > + SP7021_SPI_CONFIG_REG);
> > +
> > +       return 0;
> > +}
> > +
> > +static void sp7021_spi_setup_clk(struct spi_controller *ctlr, struct
> > +spi_transfer *xfer) {
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +       unsigned int clk_rate, clk_sel, div;
> > +
> > +       clk_rate = clk_get_rate(pspim->spi_clk);
> > +       div = clk_rate / xfer->speed_hz;
> > +       if (div < 2)
> > +               div = 2;
> > +       clk_sel = (div / 2) - 1;
> > +       pspim->xfer_conf |= ((clk_sel & 0xffff) << 16);
> > +       writel(pspim->xfer_conf, pspim->mas_base +
> > +SP7021_SPI_CONFIG_REG); }
> > +
> > +static int sp7021_spi_mas_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
> > +                                      struct spi_transfer *xfer) {
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +       unsigned long timeout = msecs_to_jiffies(1000);
> > +       unsigned int xfer_cnt, xfer_len, last_len;
> > +       unsigned int i, len_temp;
> > +       u32 reg_temp;
> > +       int ret;
> > +
> > +       xfer_cnt = xfer->len / SP7021_SPI_DATA_SIZE;
> > +       last_len = xfer->len % SP7021_SPI_DATA_SIZE;
> > +
> > +       for (i = 0; i <= xfer_cnt; i++) {
> > +               mutex_lock(&pspim->buf_lock);
> > +               sp7021_prep_transfer(ctlr, spi);
> > +               sp7021_spi_setup_clk(ctlr, xfer);
> > +               reinit_completion(&pspim->isr_done);
> > +
> > +               if (i == xfer_cnt)
> > +                       xfer_len = last_len;
> > +               else
> > +                       xfer_len = SP7021_SPI_DATA_SIZE;
> > +
> > +               pspim->tx_buf = xfer->tx_buf + i * SP7021_SPI_DATA_SIZE;
> > +               pspim->rx_buf = xfer->rx_buf + i *
> > + SP7021_SPI_DATA_SIZE;
> > +
> > +               if (pspim->tx_cur_len < xfer_len) {
> > +                       len_temp = min(pspim->data_unit, xfer_len);
> > +                       sp7021_spi_mas_wb(pspim, len_temp);
> > +               }
> > +               reg_temp = readl(pspim->mas_base + SP7021_SPI_CONFIG_REG);
> > +               reg_temp &= ~SP7021_CLEAN_RW_BYTE;
> > +               reg_temp &= ~SP7021_CLEAN_FLUG_MASK;
> > +               reg_temp |= SP7021_FD_SEL | SP7021_FINISH_FLAG_MASK |
> > +                           SP7021_TX_EMP_FLAG_MASK | SP7021_RX_FULL_FLAG_MASK |
> > +                           FIELD_PREP(SP7021_TX_UNIT, 0) | FIELD_PREP(SP7021_RX_UNIT,
> 0);
> > +               writel(reg_temp, pspim->mas_base +
> > + SP7021_SPI_CONFIG_REG);
> > +
> > +               reg_temp = FIELD_PREP(SP7021_SET_TX_LEN, xfer_len) |
> > +                                     FIELD_PREP(SP7021_SET_XFER_LEN, xfer_len) |
> > +                                     SP7021_SPI_START_FD;
> > +               writel(reg_temp, pspim->mas_base +
> > + SP7021_SPI_STATUS_REG);
> > +
> > +               if (!wait_for_completion_interruptible_timeout(&pspim->isr_done, timeout)) {
> > +                       dev_err(&spi->dev, "wait_for_completion err\n");
> > +                       return -ETIMEDOUT;
> > +               }
> > +
> > +               reg_temp = readl(pspim->mas_base + SP7021_SPI_STATUS_REG);
> > +               if (reg_temp & SP7021_FINISH_FLAG) {
> > +                       writel(SP7021_FINISH_FLAG, pspim->mas_base +
> SP7021_SPI_STATUS_REG);
> > +                       writel(readl(pspim->mas_base + SP7021_SPI_CONFIG_REG) &
> > +                               SP7021_CLEAN_FLUG_MASK, pspim->mas_base +
> SP7021_SPI_CONFIG_REG);
> > +               }
> > +
> > +               ret = 0;
> > +
> > +               if (pspim->xfer_conf & SP7021_CPOL_FD)
> > +                       writel(pspim->xfer_conf, pspim->mas_base +
> > + SP7021_SPI_CONFIG_REG);
> > +
> > +               mutex_unlock(&pspim->buf_lock);
> > +       }
> > +       return ret;
> > +}
> > +
> > +static int sp7021_spi_sla_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
> > +                                      struct spi_transfer *xfer) {
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +       struct device *dev = pspim->dev;
> > +       int mode, ret = 0;
> > +
> > +       mode = SP7021_SPI_IDLE;
> > +       if (spi_controller_is_slave(ctlr)) {
> > +               if (xfer->tx_buf && xfer->rx_buf) {
> > +                       dev_dbg(&ctlr->dev, "%s() wrong command\n", __func__);
> > +                       ret = -EINVAL;
> > +               } else if (xfer->tx_buf) {
> > +                       xfer->tx_dma = dma_map_single(dev, (void *)xfer->tx_buf,
> > +                                                     xfer->len, DMA_TO_DEVICE);
> > +                       if (dma_mapping_error(dev, xfer->tx_dma))
> > +                               return -ENOMEM;
> > +                       mode = SP7021_SLA_WRITE;
> > +               } else if (xfer->rx_buf) {
> > +                       xfer->rx_dma = dma_map_single(dev, xfer->rx_buf, xfer->len,
> > +                                                     DMA_FROM_DEVICE);
> > +                       if (dma_mapping_error(dev, xfer->rx_dma))
> > +                               return -ENOMEM;
> > +                       mode = SP7021_SLA_READ;
> > +               }
> > +
> > +               switch (mode) {
> > +               case SP7021_SLA_WRITE:
> > +                       sp7021_spi_sla_tx(spi, xfer);
> > +                       break;
> > +               case SP7021_SLA_READ:
> > +                       sp7021_spi_sla_rx(spi, xfer);
> > +                       break;
> > +               default:
> > +                       break;
> > +               }
> > +
> > +               if (xfer->tx_buf)
> > +                       dma_unmap_single(dev, xfer->tx_dma, xfer->len, DMA_TO_DEVICE);
> > +               if (xfer->rx_buf)
> > +                       dma_unmap_single(dev, xfer->rx_dma, xfer->len, DMA_FROM_DEVICE);
> > +       }
> > +
> > +       spi_finalize_current_transfer(ctlr);
> > +       return ret;
> > +}
> > +
> > +static void sp7021_spi_disable_unprepare(void *data) {
> > +       clk_disable_unprepare(data);
> > +}
> > +
> > +static void sp7021_spi_reset_control_assert(void *data) {
> > +       reset_control_assert(data);
> > +}
> > +
> > +static int sp7021_spi_controller_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct sp7021_spi_ctlr *pspim;
> > +       struct spi_controller *ctlr;
> > +       int mode, ret;
> > +
> > +       mode = SP7021_MASTER_MODE;
> > +       pdev->id = of_alias_get_id(pdev->dev.of_node, "sp_spi");
> > +
> > +       if (of_property_read_bool(pdev->dev.of_node, "spi-slave"))
> > +               mode = SP7021_SLAVE_MODE;
> > +
> > +       if (mode == SP7021_SLAVE_MODE)
> > +               ctlr = devm_spi_alloc_slave(dev, sizeof(*pspim));
> > +       else
> > +               ctlr = devm_spi_alloc_master(dev, sizeof(*pspim));
> > +       if (!ctlr)
> > +               return -ENOMEM;
> > +       device_set_node(&ctlr->dev, pdev->dev.fwnode);
> > +       ctlr->bus_num = pdev->id;
> > +       ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
> > +       ctlr->auto_runtime_pm = true;
> > +       ctlr->prepare_message = sp7021_spi_controller_prepare_message;
> > +       if (mode == SP7021_SLAVE_MODE) {
> > +               ctlr->transfer_one = sp7021_spi_sla_transfer_one;
> > +               ctlr->slave_abort = sp7021_spi_sla_abort;
> > +               ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
> > +       } else {
> > +               ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
> > +               ctlr->min_speed_hz = 40000;
> > +               ctlr->max_speed_hz = 25000000;
> > +               ctlr->use_gpio_descriptors = true;
> > +               ctlr->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
> > +               ctlr->transfer_one = sp7021_spi_mas_transfer_one;
> > +       }
> > +       platform_set_drvdata(pdev, ctlr);
> > +       pspim = spi_controller_get_devdata(ctlr);
> > +       pspim->ctlr = ctlr;
> > +       pspim->dev = dev;
> > +       spin_lock_init(&pspim->lock);
> > +       mutex_init(&pspim->buf_lock);
> > +       init_completion(&pspim->isr_done);
> > +       init_completion(&pspim->sla_isr);
> > +
> > +       pspim->mas_base = devm_platform_ioremap_resource_byname(pdev, "master");
> > +       if (IS_ERR(pspim->mas_base))
> > +               return dev_err_probe(dev, PTR_ERR(pspim->mas_base),
> > + "mas_base get fail\n");
> > +
> > +       pspim->sla_base = devm_platform_ioremap_resource_byname(pdev, "slave");
> > +       if (IS_ERR(pspim->sla_base))
> > +               return dev_err_probe(dev, PTR_ERR(pspim->sla_base),
> > + "sla_base get fail\n");
> > +
> > +       pspim->mas_irq = platform_get_irq_byname(pdev, "mas_risc");
> > +       if (pspim->mas_irq < 0)
> > +               return pspim->mas_irq;
> > +
> > +       pspim->sla_irq = platform_get_irq_byname(pdev, "slave_risc");
> > +       if (pspim->sla_irq < 0)
> > +               return pspim->sla_irq;
> > +
> > +       ret = devm_request_irq(dev, pspim->mas_irq, sp7021_spi_mas_irq,
> > +                              IRQF_TRIGGER_RISING, pdev->name, pspim);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_request_irq(dev, pspim->sla_irq, sp7021_spi_sla_irq,
> > +                              IRQF_TRIGGER_RISING, pdev->name, pspim);
> > +       if (ret)
> > +               return ret;
> > +
> > +       pspim->spi_clk = devm_clk_get(dev, NULL);
> > +       if (IS_ERR(pspim->spi_clk))
> > +               return dev_err_probe(dev, PTR_ERR(pspim->spi_clk),
> > + "clk get fail\n");
> > +
> > +       pspim->rstc = devm_reset_control_get_exclusive(dev, NULL);
> > +       if (IS_ERR(pspim->rstc))
> > +               return dev_err_probe(dev, PTR_ERR(pspim->rstc), "rst
> > + get fail\n");
> > +
> > +       ret = clk_prepare_enable(pspim->spi_clk);
> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "failed to enable
> > + clk\n");
> > +
> > +       ret = devm_add_action_or_reset(dev, sp7021_spi_disable_unprepare, pspim->spi_clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = reset_control_deassert(pspim->rstc);
> > +       if (ret)
> > +               return dev_err_probe(dev, ret, "failed to deassert
> > + reset\n");
> > +
> > +       ret = devm_add_action_or_reset(dev, sp7021_spi_reset_control_assert, pspim->rstc);
> > +       if (ret)
> > +               return ret;
> > +
> > +       pm_runtime_enable(dev);
> > +       ret = spi_register_controller(ctlr);
> > +       if (ret) {
> > +               pm_runtime_disable(dev);
> > +               return dev_err_probe(dev, ret, "spi_register_master fail\n");
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int sp7021_spi_controller_remove(struct platform_device *pdev)
> > +{
> > +       struct spi_controller *ctlr = dev_get_drvdata(&pdev->dev);
> > +
> > +       spi_unregister_controller(ctlr);
> > +       pm_runtime_disable(&pdev->dev);
> > +       pm_runtime_set_suspended(&pdev->dev);
> > +
> > +       return 0;
> > +}
> > +
> > +static int __maybe_unused sp7021_spi_controller_suspend(struct device
> > +*dev) {
> > +       struct spi_controller *ctlr = dev_get_drvdata(dev);
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +
> > +       return reset_control_assert(pspim->rstc);
> > +}
> > +
> > +static int __maybe_unused sp7021_spi_controller_resume(struct device
> > +*dev) {
> > +       struct spi_controller *ctlr = dev_get_drvdata(dev);
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +
> > +       reset_control_deassert(pspim->rstc);
> > +       return clk_prepare_enable(pspim->spi_clk);
> > +}
> > +
> > +static int sp7021_spi_runtime_suspend(struct device *dev) {
> > +       struct spi_controller *ctlr = dev_get_drvdata(dev);
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +
> > +       return reset_control_assert(pspim->rstc);
> > +}
> > +
> > +static int sp7021_spi_runtime_resume(struct device *dev) {
> > +       struct spi_controller *ctlr = dev_get_drvdata(dev);
> > +       struct sp7021_spi_ctlr *pspim = spi_master_get_devdata(ctlr);
> > +
> > +       return reset_control_deassert(pspim->rstc);
> > +}
> > +
> > +static const struct dev_pm_ops sp7021_spi_pm_ops = {
> > +       SET_RUNTIME_PM_OPS(sp7021_spi_runtime_suspend,
> > +                          sp7021_spi_runtime_resume, NULL)
> > +       SET_SYSTEM_SLEEP_PM_OPS(sp7021_spi_controller_suspend,
> > +                               sp7021_spi_controller_resume) };
> > +
> > +static const struct of_device_id sp7021_spi_controller_ids[] = {
> > +       { .compatible = "sunplus,sp7021-spi" },
> > +       {}
> > +};
> > +MODULE_DEVICE_TABLE(of, sp7021_spi_controller_ids);
> > +
> > +static struct platform_driver sp7021_spi_controller_driver = {
> > +       .probe = sp7021_spi_controller_probe,
> > +       .remove = sp7021_spi_controller_remove,
> > +       .driver = {
> > +               .name = "sunplus,sp7021-spi-controller",
> > +               .of_match_table = sp7021_spi_controller_ids,
> > +               .pm     = &sp7021_spi_pm_ops,
> > +       },
> > +};
> > +module_platform_driver(sp7021_spi_controller_driver);
> > +
> > +MODULE_AUTHOR("Li-hao Kuo <lhjeff911@xxxxxxxxx>");
> > +MODULE_DESCRIPTION("Sunplus SPI controller driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> >
> 
> 
> --
> With Best Regards,
> Andy Shevchenko