[PATCH v2] spi: QUP based bus driver for Qualcomm MSM chipsets

On Mon, Nov 14, 2011 at 02:58:27PM -0700, Harini Jayaraman wrote:

This bus driver supports the QUP SPI hardware controller in the Qualcomm
MSM SOCs. The Qualcomm Universal Peripheral Engine (QUP) is a general
purpose data path engine with input/output FIFOs and an embedded SPI
mini-core. The driver currently supports only FIFO mode.

Signed-off-by: Harini Jayaraman <redacted>

Wow, this driver is huge. This is a rough review only, mainly to see
what can go away. This will make further reviews easier.

I think this can be simplified. Example for this switch:

	if (mult > 3)
		return -EINVAL;

	*fifo_size = (words * 2) << mult;

Probably the whole function can be optimized away somehow?

+
+	*block_size = words * sizeof(u32); /* in bytes */
+	return 0;
+}

...

+#ifdef CONFIG_DEBUG_FS
+static int debugfs_iomem_x32_set(void *data, u64 val)
+{
+	iowrite32(val, data);
+	/* Ensure the previous write completed. */
+	wmb();
+	return 0;
+}
+
+static int debugfs_iomem_x32_get(void *data, u64 *val)
+{
+	*val = ioread32(data);
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_iomem_x32, debugfs_iomem_x32_get,
+			debugfs_iomem_x32_set, "0x%08llx\n");
+
+static void spi_debugfs_init(struct msm_spi *dd)
+{
+	dd->dent_spi = debugfs_create_dir(dev_name(dd->dev), NULL);
+	if (dd->dent_spi) {
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(debugfs_spi_regs); i++) {
+			dd->debugfs_spi_regs[i] =
+				debugfs_create_file(
+					debugfs_spi_regs[i].name,
+					debugfs_spi_regs[i].mode,
+					dd->dent_spi,
+					dd->base + debugfs_spi_regs[i].offset,
+					&fops_iomem_x32);
+		}
+	}
+}
+
+static void spi_debugfs_exit(struct msm_spi *dd)
+{
+	if (dd->dent_spi) {
+		int i;
+
+		debugfs_remove_recursive(dd->dent_spi);
+		dd->dent_spi = NULL;
+		for (i = 0; i < ARRAY_SIZE(debugfs_spi_regs); i++)
+			dd->debugfs_spi_regs[i] = NULL;
+	}
+}
+#else
+static void spi_debugfs_init(struct msm_spi *dd) {}
+static void spi_debugfs_exit(struct msm_spi *dd) {}
+#endif

That interface should go away. It might have been nice when developing
the driver, but we other mechanisms to read out register values.

+
+/* ===Device attributes begin=== */
+static ssize_t show_stats(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct msm_spi *dd =  spi_master_get_devdata(master);
+
+	return snprintf(buf, PAGE_SIZE,
+			"Device       %s\n"
+			"rx fifo_size = %d spi words\n"
+			"tx fifo_size = %d spi words\n"
+			"rx block size = %d bytes\n"
+			"tx block size = %d bytes\n"
+			"--statistics--\n"
+			"Rx isrs  = %d\n"
+			"Tx isrs  = %d\n"
+			"--debug--\n"
+			"NA yet\n",
+			dev_name(dev),
+			dd->input_fifo_size,
+			dd->output_fifo_size,
+			dd->input_block_size,
+			dd->output_block_size,
+			dd->stat_rx,
+			dd->stat_tx
+			);
+}
+
+/* Reset statistics on write */
+static ssize_t set_stats(struct device *dev, struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct msm_spi *dd = dev_get_drvdata(dev);
+
+	dd->stat_rx = 0;
+	dd->stat_tx = 0;
+	return count;
+}
+
+static DEVICE_ATTR(stats, S_IRUGO | S_IWUSR, show_stats, set_stats);
+
+static struct attribute *dev_attrs[] = {
+	&dev_attr_stats.attr,
+	NULL,
+};
+
+static struct attribute_group dev_attr_grp = {
+	.attrs = dev_attrs,
+};
+/* ===Device attributes end=== */

This should go as well. Not really needed.

+
+static int __init msm_spi_probe(struct platform_device *pdev)
+{
+	struct spi_master      *master;
+	struct msm_spi	       *dd;
+	struct resource	       *resource;
+	int                     rc = -ENXIO;
+	int                     locked = 0;
+	int                     clk_enabled = 0;
+	int                     pclk_enabled = 0;
+	struct msm_spi_platform_data *pdata = pdev->dev.platform_data;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(struct msm_spi));
+	if (!master) {
+		rc = -ENOMEM;
+		dev_err(&pdev->dev, "master allocation failed\n");
+		goto err_probe_exit;
+	}
+
+	master->bus_num        = pdev->id;
+	master->mode_bits      = SPI_SUPPORTED_MODES;
+	master->num_chipselect = SPI_NUM_CHIPSELECTS;
+	master->setup          = msm_spi_setup;
+	master->transfer       = msm_spi_transfer;
+	platform_set_drvdata(pdev, master);
+	dd = spi_master_get_devdata(master);
+
+	dd->pdata = pdata;
+	rc = msm_spi_get_irq_data(dd, pdev);
+	if (rc)
+		goto err_probe_res;
+
+	resource = platform_get_resource_byname(pdev,
+						IORESOURCE_MEM, "spi_base");
+	if (!resource) {
+		rc = -ENXIO;
+		goto err_probe_res;
+	}
+
+	dd->mem_phys_addr = resource->start;
+	dd->mem_size = resource_size(resource);
+
+	spin_lock_init(&dd->queue_lock);
+	mutex_init(&dd->core_lock);
+	INIT_LIST_HEAD(&dd->queue);
+	INIT_WORK(&dd->work_data, msm_spi_workq);
+	init_waitqueue_head(&dd->continue_suspend);
+	dd->workqueue = create_singlethread_workqueue(
+						dev_name(master->dev.parent));
+	if (!dd->workqueue)
+		goto err_probe_res;
+
+	if (!devm_request_mem_region(&pdev->dev, dd->mem_phys_addr,
+					dd->mem_size, SPI_DRV_NAME)) {
+		rc = -ENXIO;
+		goto err_probe_reqmem;
+	}
+
+	dd->base = devm_ioremap(&pdev->dev, dd->mem_phys_addr, dd->mem_size);
+	if (!dd->base) {
+		rc = -ENOMEM;
+		goto err_probe_reqmem;
+	}
+
+
+	mutex_lock(&dd->core_lock);
+	locked = 1;
+	dd->dev = &pdev->dev;
+	dd->clk = clk_get(&pdev->dev, "spi_clk");
+	if (IS_ERR(dd->clk)) {
+		dev_err(&pdev->dev, "%s: unable to get spi_clk\n", __func__);
+		rc = PTR_ERR(dd->clk);
+		goto err_probe_clk_get;
+	}
+
+	dd->pclk = clk_get(&pdev->dev, "spi_pclk");
+	if (IS_ERR(dd->pclk)) {
+		dev_err(&pdev->dev, "%s: unable to get spi_pclk\n", __func__);
+		rc = PTR_ERR(dd->pclk);
+		goto err_probe_pclk_get;
+	}
+
+	if (pdata && pdata->max_clock_speed)
+		msm_spi_clock_set(dd, dd->pdata->max_clock_speed);
+
+	rc = clk_enable(dd->clk);
+	if (rc) {
+		dev_err(&pdev->dev, "%s: unable to enable spi_clk\n",
+			__func__);
+		goto err_probe_clk_enable;
+	}
+
+	clk_enabled = 1;
+	rc = clk_enable(dd->pclk);
+	if (rc) {
+		dev_err(&pdev->dev, "%s: unable to enable spi_pclk\n",
+		__func__);
+		goto err_probe_pclk_enable;
+	}
+
+	pclk_enabled = 1;
+	rc = msm_spi_configure_gsbi(dd, pdev);
+	if (rc)
+		goto err_probe_gsbi;
+
+	msm_spi_calculate_fifo_size(dd);
+
+	/* Initialize registers */
+	writel(0x00000001, dd->base + SPI_SW_RESET);
+	msm_spi_set_state(dd, SPI_OP_STATE_RESET);
+	writel(0x00000000, dd->base + SPI_OPERATIONAL);
+	writel(0x00000000, dd->base + SPI_CONFIG);
+	writel(0x00000000, dd->base + SPI_IO_MODES);

0x0, or 0x1 will do. Save the leading 0s.

+	/*
+	 * The SPI core generates a bogus input overrun error on some targets,
+	 * when a transition from run to reset state occurs and if the FIFO has
+	 * an odd number of entries. Hence we disable the INPUT_OVER_RUN_ERR_EN
+	 * bit.
+	 */
+	msm_spi_enable_error_flags(dd);
+
+	writel(SPI_IO_C_NO_TRI_STATE, dd->base + SPI_IO_CONTROL);
+	rc = msm_spi_set_state(dd, SPI_OP_STATE_RESET);
+	if (rc)
+		goto err_probe_state;
+
+	clk_disable(dd->clk);
+	clk_disable(dd->pclk);
+	clk_enabled = 0;
+	pclk_enabled = 0;
+
+	dd->suspended = 0;
+	dd->transfer_pending = 0;
+	dd->multi_xfr = 0;
+	dd->mode = SPI_MODE_NONE;
+
+	rc = msm_spi_request_irq(dd, pdev->name, master);

There is also devm_request_irq

+	if (rc)
+		goto err_probe_irq;
+
+	msm_spi_disable_irqs(dd);
+	mutex_unlock(&dd->core_lock);
+	locked = 0;
+
+	rc = spi_register_master(master);
+	if (rc)
+		goto err_probe_reg_master;
+
+	rc = sysfs_create_group(&(dd->dev->kobj), &dev_attr_grp);
+	if (rc) {
+		dev_err(&pdev->dev, "failed to create dev. attrs : %d\n", rc);
+		goto err_attrs;
+	}
+
+	spi_debugfs_init(dd);
+
+	return 0;
+
+err_attrs:
+	spi_unregister_master(master);
+err_probe_reg_master:
+	msm_spi_free_irq(dd, master);
+err_probe_irq:
+err_probe_state:
+err_probe_gsbi:
+	if (pclk_enabled)
+		clk_disable(dd->pclk);
+err_probe_pclk_enable:
+	if (clk_enabled)
+		clk_disable(dd->clk);
+err_probe_clk_enable:
+	clk_put(dd->pclk);
+err_probe_pclk_get:
+	clk_put(dd->clk);
+err_probe_clk_get:
+	if (locked)
+		mutex_unlock(&dd->core_lock);
+err_probe_reqmem:
+	destroy_workqueue(dd->workqueue);
+err_probe_res:
+	spi_master_put(master);
+err_probe_exit:
+	return rc;
+}
+

...

+static struct platform_driver msm_spi_driver = {
+	.driver		= {
+		.name	= SPI_DRV_NAME,
+		.owner	= THIS_MODULE,
+	},
+	.suspend        = msm_spi_suspend,
+	.resume         = msm_spi_resume,
+	.remove         = __exit_p(msm_spi_remove),
+};

What about using module_platform_driver?

+
+static int __init msm_spi_init(void)
+{
+	return platform_driver_probe(&msm_spi_driver, msm_spi_probe);
+}
+module_init(msm_spi_init);
+
+static void __exit msm_spi_exit(void)
+{
+	platform_driver_unregister(&msm_spi_driver);
+}
+module_exit(msm_spi_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("0.3");

MODULE_VERSION is not really needed these days.

...

+#ifdef CONFIG_DEBUG_FS
+static const struct {
+	const char *name;
+	mode_t mode;
+	int offset;
+} debugfs_spi_regs[] = {
+	{"config",                S_IRUGO | S_IWUSR, SPI_CONFIG},
+	{"io_control",            S_IRUGO | S_IWUSR, SPI_IO_CONTROL},
+	{"io_modes",              S_IRUGO | S_IWUSR, SPI_IO_MODES},
+	{"sw_reset",                        S_IWUSR, SPI_SW_RESET},
+	{"time_out",              S_IRUGO | S_IWUSR, SPI_TIME_OUT},
+	{"time_out_current",      S_IRUGO,           SPI_TIME_OUT_CURRENT},
+	{"mx_output_count",       S_IRUGO | S_IWUSR, SPI_MX_OUTPUT_COUNT},
+	{"mx_output_cnt_current", S_IRUGO,           SPI_MX_OUTPUT_CNT_CURRENT},
+	{"mx_input_count",        S_IRUGO | S_IWUSR, SPI_MX_INPUT_COUNT},
+	{"mx_input_cnt_current",  S_IRUGO,           SPI_MX_INPUT_CNT_CURRENT},
+	{"mx_read_count",         S_IRUGO | S_IWUSR, SPI_MX_READ_COUNT},
+	{"mx_read_cnt_current",   S_IRUGO,           SPI_MX_READ_CNT_CURRENT},
+	{"operational",           S_IRUGO | S_IWUSR, SPI_OPERATIONAL},
+	{"error_flags",           S_IRUGO | S_IWUSR, SPI_ERROR_FLAGS},
+	{"error_flags_en",        S_IRUGO | S_IWUSR, SPI_ERROR_FLAGS_EN},
+	{"deassert_wait",         S_IRUGO | S_IWUSR, SPI_DEASSERT_WAIT},
+	{"output_debug",          S_IRUGO,           SPI_OUTPUT_DEBUG},
+	{"input_debug",           S_IRUGO,           SPI_INPUT_DEBUG},
+	{"test_ctrl",             S_IRUGO | S_IWUSR, SPI_TEST_CTRL},
+	{"output_fifo",                     S_IWUSR, SPI_OUTPUT_FIFO},
+	{"input_fifo" ,           S_IRUSR,           SPI_INPUT_FIFO},
+	{"spi_state",             S_IRUGO | S_IWUSR, SPI_STATE},
+	{"qup_config",            S_IRUGO | S_IWUSR, QUP_CONFIG},
+	{"qup_error_flags",       S_IRUGO | S_IWUSR, QUP_ERROR_FLAGS},
+	{"qup_error_flags_en",    S_IRUGO | S_IWUSR, QUP_ERROR_FLAGS_EN},
+	{"mx_write_cnt",          S_IRUGO | S_IWUSR, QUP_MX_WRITE_COUNT},
+	{"mx_write_cnt_current",  S_IRUGO,           QUP_MX_WRITE_CNT_CURRENT},
+	{"output_fifo_word_cnt",  S_IRUGO,           SPI_OUTPUT_FIFO_WORD_CNT},
+	{"input_fifo_word_cnt",   S_IRUGO,           SPI_INPUT_FIFO_WORD_CNT},
+};
+#endif

Again, drop it.

+
+/**
+ * struct msm_spi
+ * @read_buf: rx_buf from the spi_transfer.
+ * @write_buf: tx_buf from the spi_transfer.
+ * @base: location of QUP controller I/O area in memory.
+ * @dev: parent platform device.
+ * @queue_lock: lock to protect queue.
+ * @core_lock: mutex used to protect this struct.
+ * @queue: to log SPI transfer requests.
+ * @workqueue: workqueue for the SPI transfer requests.
+ * @work_data: work.
+ * @cur_msg: the current spi_message being processed.
+ * @cur_transfer: the current spi_transfer being processed.
+ * @transfer_complete: completion function to signal the end of a spi_transfer.
+ * @clk: the SPI core clock
+ * @pclk: hardware core clock. Needs to be enabled to access the QUP register
+ * @mem_phys_addr: physical address of the QUP controller.
+ * @mem_size: size of the QUP controller block.
+ * @input_fifo_size: the input FIFO size (in bytes).
+ * @output_fifo_size: the output FIFO size (in bytes).
+ * @rx_bytes_remaining: the number of rx bytes remaining to be transferred.
+ * @tx_bytes_remaining: the number of tx bytes remaining to be transferred.
+ * @clock_speed: SPI clock speed.
+ * @irq_in: assigned interrupt line for QUP interrupts.
+ * @read_xfr_cnt: number of words read from the FIFO (per transfer).
+ * @write_xfr_cnt: number of words written to the FIFO (per transfer).
+ * @write_len: the total number of tx bytes to be transferred, per
+ *	spi_message. This is valid only for WR-WR and WR-RD transfers
+ *	in a single spi_message.
+ * @read_len: the total number of rx bytes to be transferred, per
+ *	spi_message. This is valid only for WR-WR and WR-RD transfers
+ *	in a single spi_message.
+ * @bytes_per_word: bytes per word
+ * @suspended: the suspend state.
+ * @transfer_pending: when set indicates a pending transfer.
+ * @continue_suspend: head of wait queue.
+ * @mode: mode for SPI operation.
+ * @input_block_size: the input block size (in bytes).
+ * @output_block_size: the output block size (in bytes).
+ * @stat_rx: count of input interrupts handled.
+ * @stat_tx: count of output interrupts handled.
+ * @dent_spi: used for debug purposes.
+ * @debugfs_spi_regs: used for debug purposes.
+ * @pdata: platform data
+ * @multi_xfr: when set indicates multiple spi_transfers in a single
+ *	spi_message.
+ * @done: flag used to signal completion.
+ * @cur_msg_len: combined length of all the transfers in a single
+ *	spi_message (in bytes).
+ * @cur_tx_transfer: the current tx transfer being processed. Used in
+ *	FIFO mode only.
+ * @cur_rx_transfer: the current rx transfer being processed. Used in
+ *	FIFO mode only.
+ *
+ * Early QUP controller used three separate interrupt lines for input, output,
+ * and error interrupts.  Later versions share a single interrupt line.
+ */
+struct msm_spi {
+	u8                      *read_buf;
+	const u8                *write_buf;
+	void __iomem            *base;
+	struct device           *dev;
+	spinlock_t               queue_lock;
+	struct mutex             core_lock;
+	struct list_head         queue;
+	struct workqueue_struct *workqueue;
+	struct work_struct       work_data;
+	struct spi_message      *cur_msg;
+	struct spi_transfer     *cur_transfer;
+	struct completion        transfer_complete;
+	struct clk              *clk;
+	struct clk              *pclk;
+	unsigned long            mem_phys_addr;
+	size_t                   mem_size;
+	int                      input_fifo_size;
+	int                      output_fifo_size;
+	u32                      rx_bytes_remaining;
+	u32                      tx_bytes_remaining;
+	u32                      clock_speed;
+	int                      irq_in;
+	int                      read_xfr_cnt;
+	int                      write_xfr_cnt;
+	int                      write_len;
+	int                      read_len;
+	int                      bytes_per_word;
+	bool                     suspended;
+	bool                     transfer_pending;
+	wait_queue_head_t        continue_suspend;
+	enum msm_spi_mode        mode;
+	int                      input_block_size;
+	int                      output_block_size;
+	int                      stat_rx;
+	int                      stat_tx;
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *dent_spi;
+	struct dentry *debugfs_spi_regs[ARRAY_SIZE(debugfs_spi_regs)];
+#endif
+	struct msm_spi_platform_data *pdata;
+	bool                     multi_xfr;
+	bool                     done;
+	u32                      cur_msg_len;
+	struct spi_transfer     *cur_tx_transfer;
+	struct spi_transfer     *cur_rx_transfer;
+};

This looks excessive. Please check if all of this is really needed?

+
+/* Forward declaration */
+static irqreturn_t msm_spi_input_irq(int irq, void *dev_id);
+static irqreturn_t msm_spi_output_irq(int irq, void *dev_id);
+static irqreturn_t msm_spi_error_irq(int irq, void *dev_id);
+static inline int msm_spi_set_state(struct msm_spi *dd,
+				enum msm_spi_state state);
+static void msm_spi_write_word_to_fifo(struct msm_spi *dd);
+static inline void msm_spi_write_rmn_to_fifo(struct msm_spi *dd);
+
+/* In QUP the same interrupt line is used for input, output and error */
+static inline int msm_spi_get_irq_data(struct msm_spi *dd,
+					struct platform_device *pdev)
+{
+	dd->irq_in  = platform_get_irq_byname(pdev, "spi_irq_in");
+	if (dd->irq_in < 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static inline int msm_spi_configure_gsbi(struct msm_spi *dd,
+					struct platform_device *pdev)
+{
+	struct resource *resource;
+	unsigned long   gsbi_mem_phys_addr;
+	size_t          gsbi_mem_size;
+	void __iomem    *gsbi_base;
+
+	resource  = platform_get_resource_byname(pdev,
+						 IORESOURCE_MEM, "gsbi_base");
+	if (!resource)
+		return -ENXIO;
+
+	gsbi_mem_phys_addr = resource->start;
+	gsbi_mem_size = resource_size(resource);
+	if (!devm_request_mem_region(&pdev->dev, gsbi_mem_phys_addr,
+					gsbi_mem_size, SPI_DRV_NAME))
+		return -ENXIO;
+
+	gsbi_base = devm_ioremap(&pdev->dev, gsbi_mem_phys_addr,
+					gsbi_mem_size);
+	if (!gsbi_base)
+		return -ENXIO;
+
+	/* Set GSBI to SPI mode */
+	writel(GSBI_SPI_CONFIG, gsbi_base + GSBI_CTRL_REG);
+
+	return 0;
+}
+
+/* Figure which irq occured and call the relevant functions */
+static irqreturn_t msm_spi_qup_irq(int irq, void *dev_id)
+{
+	u32 op, ret = IRQ_NONE;
+	struct msm_spi *dd = dev_id;
+
+	if (readl(dd->base + SPI_ERROR_FLAGS) ||
+		readl(dd->base + QUP_ERROR_FLAGS)) {
+		struct spi_master *master = dev_get_drvdata(dd->dev);
+		ret |= msm_spi_error_irq(irq, master);
+	}
+
+	op = readl(dd->base + SPI_OPERATIONAL);
+	if (op & SPI_OP_INPUT_SERVICE_FLAG) {
+		writel(SPI_OP_INPUT_SERVICE_FLAG, dd->base + SPI_OPERATIONAL);
+		ret |= msm_spi_input_irq(irq, dev_id);
+	}
+
+	if (op & SPI_OP_OUTPUT_SERVICE_FLAG) {
+		writel(SPI_OP_OUTPUT_SERVICE_FLAG, dd->base + SPI_OPERATIONAL);
+		ret |= msm_spi_output_irq(irq, dev_id);
+	}
+
+	if (dd->done) {
+		complete(&dd->transfer_complete);
+		dd->done = 0;
+	}
+	return ret;
+}

Too much code for a header file IMO.

+
+static inline int msm_spi_request_irq(struct msm_spi *dd,
+					const char *name,
+					struct spi_master *master)
+{
+	return request_irq(dd->irq_in, msm_spi_qup_irq, IRQF_TRIGGER_HIGH,
+			   name, dd);
+}
+
+static inline void msm_spi_free_irq(struct msm_spi *dd,
+					struct spi_master *master)
+{
+	free_irq(dd->irq_in, dd);
+}
+
+static inline void msm_spi_disable_irqs(struct msm_spi *dd)
+{
+	disable_irq(dd->irq_in);
+}
+
+static inline void msm_spi_enable_irqs(struct msm_spi *dd)
+{
+	enable_irq(dd->irq_in);
+}
+
+static inline void msm_spi_get_clk_err(struct msm_spi *dd, u32 *spi_err)
+{
+	*spi_err = readl(dd->base + QUP_ERROR_FLAGS);
+}
+
+static inline void msm_spi_ack_clk_err(struct msm_spi *dd)
+{
+	writel(QUP_ERR_MASK, dd->base + QUP_ERROR_FLAGS);
+}
+
+static inline void msm_spi_add_configs(struct msm_spi *dd, u32 *config, int n);
+
+/* QUP has no_input, no_output, and N bits at QUP_CONFIG */
+static inline void msm_spi_set_qup_config(struct msm_spi *dd, int bpw)
+{
+	u32 qup_config = readl(dd->base + QUP_CONFIG);
+
+	msm_spi_add_configs(dd, &qup_config, bpw-1);
+	writel(qup_config | QUP_CONFIG_SPI_MODE, dd->base + QUP_CONFIG);
+}
+
+static inline int msm_spi_prepare_for_write(struct msm_spi *dd)
+{
+	if (msm_spi_set_state(dd, SPI_OP_STATE_RUN))
+		return -EINVAL;
+
+	if (msm_spi_set_state(dd, SPI_OP_STATE_PAUSE))
+		return -EINVAL;
+
+	return 0;
+}
+
+static inline void msm_spi_start_write(struct msm_spi *dd, u32 read_count)
+{
+	if (read_count <= dd->input_fifo_size)
+		msm_spi_write_rmn_to_fifo(dd);
+	else
+		msm_spi_write_word_to_fifo(dd);
+}
+
+static inline void msm_spi_set_write_count(struct msm_spi *dd, int val)
+{
+	writel(val, dd->base + QUP_MX_WRITE_COUNT);
+}
+
+static inline void msm_spi_complete(struct msm_spi *dd)
+{
+	dd->done = 1;
+}
+
+static inline void msm_spi_enable_error_flags(struct msm_spi *dd)
+{
+	writel_relaxed(0x00000078, dd->base + SPI_ERROR_FLAGS_EN);
+}
+
+static inline void msm_spi_clear_error_flags(struct msm_spi *dd)
+{
+	writel_relaxed(0x0000007C, dd->base + SPI_ERROR_FLAGS);
+}

While most of these one liners are too few code for a header file IMO.
No need to encapsulate most of it, e.g use the *_irq-calls directly.
Also, please don't use magic values (0x7c) but combine the proper
defines, please.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111207/c17d418f/attachment-0001.sig>