On Thu, Jul 18, 2013 at 03:31:26PM +0530, Sourav Poddar wrote:I have tried using it in my patch3 of this series..
QSPI is a kind of spi module that allows single,Have you seen the ongoing thread about SPI buses with extra data lines?
dual and quad read access to external spi devices. The module
has a memory mapped interface which provide direct interface
for accessing data form external spi devices.
How does this driver fit in with that?
Ok.--- a/drivers/spi/MakefilePlease use SPI_ like the other drivers.
+++ b/drivers/spi/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SPI_OCTEON) += spi-octeon.o
obj-$(CONFIG_SPI_OMAP_UWIRE) += spi-omap-uwire.o
obj-$(CONFIG_SPI_OMAP_100K) += spi-omap-100k.o
obj-$(CONFIG_SPI_OMAP24XX) += spi-omap2-mcspi.o
+obj-$(CONFIG_QSPI_DRA7xxx) += spi-ti-qspi.o
obj-$(CONFIG_SPI_ORION) += spi-orion.o
obj-$(CONFIG_SPI_PL022) += spi-pl022.o
obj-$(CONFIG_SPI_PPC4xx) += spi-ppc4xx.o
May be yes.+static int ti_qspi_prepare_xfer(struct spi_master *master)This is a very common pattern, it should probably be factored out into
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(master);
+ int ret;
+
+ ret = pm_runtime_get_sync(qspi->dev);
+ if (ret< 0) {
+ dev_err(qspi->dev, "pm_runtime_get_sync() failed\n");
+ return ret;
+ }
+
+ return 0;
+}
the core, probably not even as ops but rather as an actual feature.
We are checking if there is any transfer left, if no we are signalling the+ list_for_each_entry(t,&m->transfers, transfer_list) {The use of list_is_last() here is *realy* strange - what's going on
+ qspi->cmd |= QSPI_WLEN(t->bits_per_word);
+ qspi->cmd |= QSPI_WC_CMD_INT_EN;
+
+ ret = qspi_transfer_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi->dev, "transfer message failed\n");
+ return -EINVAL;
+ }
+
+ m->actual_length += t->len;
+
+ if (list_is_last(&t->transfer_list,&m->transfers))
+ goto out;
+ }
there?
Yes, there is WC interrupt enable bit which enables the interrupt. This interrupt+static irqreturn_t ti_qspi_isr(int irq, void *dev_id)According to the above code we might interrupt for masked events...
+{
+ struct ti_qspi *qspi = dev_id;
+ u16 mask, stat;
+
+ irqreturn_t ret = IRQ_HANDLED;
+
+ spin_lock(&qspi->lock);
+
+ stat = ti_qspi_readl(qspi, QSPI_SPI_STATUS_REG);
+ mask = ti_qspi_readl(qspi, QSPI_INTR_ENABLE_SET_REG);
+
+ if (stat&& mask)
+ ret = IRQ_WAKE_THREAD;
+
+ spin_unlock(&qspi->lock);
+
+ return ret;
that's a bit worrying isn't it?
I am not sure about the exact flow. If we see the api description, it says about irq getting+ ret = devm_request_threaded_irq(&pdev->dev, irq, ti_qspi_isr,Standard question about devm_request_threaded_irq() - how can we be
+ ti_qspi_threaded_isr, IRQF_NO_SUSPEND | IRQF_ONESHOT,
+ dev_name(&pdev->dev), qspi);
+ if (ret< 0) {
+ dev_err(&pdev->dev, "Failed to register ISR for IRQ %d\n",
+ irq);
+ goto free_master;
+ }
certain it's safe to use during removal?