Re: [PATCH 3/3] spi: rockchip: add debug interface support

[Shawn Lin]

Hi Mark,

On 2016/2/16 1:26, Mark Brown wrote:

On Mon, Feb 15, 2016 at 04:28:22PM +0800, Shawn Lin wrote:


+#ifdef CONFIG_DEBUG_FS
+#include 
+#endif


Just include the header.  Only add ifdefs if they do something.


+static int rockchip_spi_debugfs_init(struct rockchip_spi *rs)
+{
+   rs->debugfs = debugfs_create_dir("rockchip_spi", NULL);
+   if (!rs->debugfs)
+   return -ENOMEM;
+
+   debugfs_create_file("regs", S_IFREG | S_IRUGO,
+   rs->debugfs, (void *)rs, &rockchip_spi_dbg_ops);
+   return 0;
+}


This is completely separate to the core debugfs support for SPI, if
we're adding new debugfs stuff that's per device we should be extending
the core debugfs stuff so everything is in one place.


yes, I will check the spi core debugfs to see how we can extend it
for per device to dump specific msg.

Thanks.



Your register dump stuff looks like you might want to consider using
regmap, it's got this and other register access diagnostics already.




--
Best Regards
Shawn Lin

Re: [PATCH 3/3] spi: rockchip: add debug interface support

[Mark Brown]

On Mon, Feb 15, 2016 at 04:28:22PM +0800, Shawn Lin wrote:

> +#ifdef CONFIG_DEBUG_FS
> +#include 
> +#endif

Just include the header.  Only add ifdefs if they do something.

> +static int rockchip_spi_debugfs_init(struct rockchip_spi *rs)
> +{
> + rs->debugfs = debugfs_create_dir("rockchip_spi", NULL);
> + if (!rs->debugfs)
> + return -ENOMEM;
> +
> + debugfs_create_file("regs", S_IFREG | S_IRUGO,
> + rs->debugfs, (void *)rs, &rockchip_spi_dbg_ops);
> + return 0;
> +}

This is completely separate to the core debugfs support for SPI, if
we're adding new debugfs stuff that's per device we should be extending
the core debugfs stuff so everything is in one place.

Your register dump stuff looks like you might want to consider using
regmap, it's got this and other register access diagnostics already.


signature.asc
Description: PGP signature

[PATCH 3/3] spi: rockchip: add debug interface support

[Shawn Lin]

Add debugfs support for user to debug rockchip spi
problems.

Signed-off-by: Shawn Lin 
---

 drivers/spi/spi-rockchip.c | 79 ++
 1 file changed, 79 insertions(+)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 7cb1b2d..e858393 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -28,6 +28,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_DEBUG_FS
+#include 
+#endif
+
 #define DRIVER_NAME "rockchip-spi"
 
 /* SPI register offsets */
@@ -199,8 +203,81 @@ struct rockchip_spi {
struct sg_table rx_sg;
struct rockchip_spi_dma_data dma_rx;
struct rockchip_spi_dma_data dma_tx;
+
+   #ifdef CONFIG_DEBUG_FS
+   struct dentry *debugfs;
+   #endif
+};
+
+#ifdef CONFIG_DEBUG_FS
+static int rockchip_spi_dbg_show(struct seq_file *s, void *v)
+{
+   struct rockchip_spi *rs = s->private;
+
+   seq_printf(s, "CTRLR0:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_CTRLR0));
+   seq_printf(s, "CTRLR1:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_CTRLR1));
+   seq_printf(s, "BAUDR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_BAUDR));
+   seq_printf(s, "TXFTLR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_TXFTLR));
+   seq_printf(s, "RXFTLR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_RXFTLR));
+   seq_printf(s, "SR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_SR));
+   seq_printf(s, "ISR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_ISR));
+   seq_printf(s, "DMACR:\t0x%08x\n",
+  readl_relaxed(rs->regs + ROCKCHIP_SPI_DMACR));
+   seq_printf(s, "rs->state:\t0x%08x\n", rs->state);
+   seq_printf(s, "rs->speed:\t0x%08x\n", rs->speed);
+   seq_printf(s, "rs->tmode:\t0x%08x\n", rs->tmode);
+   seq_printf(s, "rs->mode:\t0x%08x\n", rs->mode);
+
+   return 0;
+}
+
+static int rockchip_spi_dbg_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, rockchip_spi_dbg_show, inode->i_private);
+}
+
+static const struct file_operations rockchip_spi_dbg_ops = {
+   .owner  = THIS_MODULE,
+   .open   = rockchip_spi_dbg_open,
+   .read   = seq_read,
+   .llseek = seq_lseek,
+   .release= single_release,
 };
 
+static int rockchip_spi_debugfs_init(struct rockchip_spi *rs)
+{
+   rs->debugfs = debugfs_create_dir("rockchip_spi", NULL);
+   if (!rs->debugfs)
+   return -ENOMEM;
+
+   debugfs_create_file("regs", S_IFREG | S_IRUGO,
+   rs->debugfs, (void *)rs, &rockchip_spi_dbg_ops);
+   return 0;
+}
+
+static void rockchip_spi_debugfs_remove(struct rockchip_spi *rs)
+{
+   debugfs_remove_recursive(rs->debugfs);
+}
+#else
+static inline int rockchip_spi_debugfs_init(struct rockchip_spi *rs)
+{
+   return 0;
+}
+
+static inline void rockchip_spi_debugfs_remove(struct rockchip_spi *rs)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
+
 static inline void spi_enable_chip(struct rockchip_spi *rs, int enable)
 {
writel_relaxed((enable ? 1 : 0), rs->regs + ROCKCHIP_SPI_SSIENR);
@@ -746,6 +823,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
goto err_register_master;
}
 
+   rockchip_spi_debugfs_init(rs);
return 0;
 
 err_register_master:
@@ -770,6 +848,7 @@ static int rockchip_spi_remove(struct platform_device *pdev)
struct rockchip_spi *rs = spi_master_get_devdata(master);
 
pm_runtime_disable(&pdev->dev);
+   rockchip_spi_debugfs_remove(rs);
 
clk_disable_unprepare(rs->spiclk);
clk_disable_unprepare(rs->apb_pclk);
-- 
2.3.7