From: Rob Clark <[email protected]>
Add a debugfs file to show status registers.
Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f1a2493b86d9..a6f27648c015 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -5,6 +5,7 @@
*/
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/iopoll.h>
@@ -109,6 +110,7 @@ struct ti_sn_bridge {
struct drm_dp_aux aux;
struct drm_bridge bridge;
struct drm_connector connector;
+ struct dentry *debugfs;
struct device_node *host_node;
struct mipi_dsi_device *dsi;
struct clk *refclk;
@@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
};
+static int status_show(struct seq_file *s, void *data)
+{
+ struct ti_sn_bridge *pdata = s->private;
+ unsigned int reg, val;
+
+ seq_puts(s, "STATUS REGISTERS:\n");
+
+ pm_runtime_get_sync(pdata->dev);
+
+ /* IRQ Status Registers, see Table 31 in datasheet */
+ for (reg = 0xf0; reg <= 0xf8; reg++) {
+ regmap_read(pdata->regmap, reg, &val);
+ seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
+ }
+
+ pm_runtime_put(pdata->dev);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(status);
+
+static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
+{
+ pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
+
+ debugfs_create_file("status", 0600, pdata->debugfs, pdata,
+ &status_fops);
+}
+
+static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
+{
+ debugfs_remove_recursive(pdata->debugfs);
+ pdata->debugfs = NULL;
+}
+
/* Connector funcs */
static struct ti_sn_bridge *
connector_to_ti_sn_bridge(struct drm_connector *connector)
@@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
drm_bridge_add(&pdata->bridge);
+ ti_sn_debugfs_init(pdata);
+
return 0;
}
@@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
if (!pdata)
return -EINVAL;
+ ti_sn_debugfs_remove(pdata);
+
of_node_put(pdata->host_node);
pm_runtime_disable(pdata->dev);
--
2.20.1
On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <[email protected]> wrote:
>
> From: Rob Clark <[email protected]>
>
> Add a debugfs file to show status registers.
>
> Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Jeffrey Hugo <[email protected]>
On 02.07.2019 17:44, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> Add a debugfs file to show status registers.
>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f1a2493b86d9..a6f27648c015 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -5,6 +5,7 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/debugfs.h>
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/iopoll.h>
> @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> struct drm_dp_aux aux;
> struct drm_bridge bridge;
> struct drm_connector connector;
> + struct dentry *debugfs;
> struct device_node *host_node;
> struct mipi_dsi_device *dsi;
> struct clk *refclk;
> @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> };
>
> +static int status_show(struct seq_file *s, void *data)
> +{
> + struct ti_sn_bridge *pdata = s->private;
> + unsigned int reg, val;
> +
> + seq_puts(s, "STATUS REGISTERS:\n");
> +
> + pm_runtime_get_sync(pdata->dev);
> +
> + /* IRQ Status Registers, see Table 31 in datasheet */
> + for (reg = 0xf0; reg <= 0xf8; reg++) {
> + regmap_read(pdata->regmap, reg, &val);
> + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> + }
> +
> + pm_runtime_put(pdata->dev);
> +
> + return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(status);
> +
> +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> +{
> + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
If some day we will have board with two such bridges there will be a
problem.
Anyway:
Reviewed-by: Andrzej Hajda <[email protected]>
--
Regards
Andrzej
> +
> + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> + &status_fops);
> +}
> +
> +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> +{
> + debugfs_remove_recursive(pdata->debugfs);
> + pdata->debugfs = NULL;
> +}
> +
> /* Connector funcs */
> static struct ti_sn_bridge *
> connector_to_ti_sn_bridge(struct drm_connector *connector)
> @@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
>
> drm_bridge_add(&pdata->bridge);
>
> + ti_sn_debugfs_init(pdata);
> +
> return 0;
> }
>
> @@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
> if (!pdata)
> return -EINVAL;
>
> + ti_sn_debugfs_remove(pdata);
> +
> of_node_put(pdata->host_node);
>
> pm_runtime_disable(pdata->dev);
Hello,
On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> On 02.07.2019 17:44, Rob Clark wrote:
> > From: Rob Clark <[email protected]>
> >
> > Add a debugfs file to show status registers.
> >
> > Signed-off-by: Rob Clark <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index f1a2493b86d9..a6f27648c015 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -5,6 +5,7 @@
> > */
> >
> > #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > #include <linux/iopoll.h>
> > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > struct drm_dp_aux aux;
> > struct drm_bridge bridge;
> > struct drm_connector connector;
> > + struct dentry *debugfs;
> > struct device_node *host_node;
> > struct mipi_dsi_device *dsi;
> > struct clk *refclk;
> > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > };
> >
> > +static int status_show(struct seq_file *s, void *data)
> > +{
> > + struct ti_sn_bridge *pdata = s->private;
> > + unsigned int reg, val;
> > +
> > + seq_puts(s, "STATUS REGISTERS:\n");
NO NEED TO SHOUT :-)
> > +
> > + pm_runtime_get_sync(pdata->dev);
> > +
> > + /* IRQ Status Registers, see Table 31 in datasheet */
> > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > + regmap_read(pdata->regmap, reg, &val);
> > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > + }
> > +
> > + pm_runtime_put(pdata->dev);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(status);
> > +
> > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > +{
> > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
>
> If some day we will have board with two such bridges there will be a
> problem.
Could we use the platform device name for this ?
> Anyway:
>
> Reviewed-by: Andrzej Hajda <[email protected]>
>
> > +
> > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > + &status_fops);
> > +}
> > +
> > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > +{
> > + debugfs_remove_recursive(pdata->debugfs);
> > + pdata->debugfs = NULL;
> > +}
> > +
You need to conditionally-compile this based on CONFIG_DEBUG_FS.
> > /* Connector funcs */
> > static struct ti_sn_bridge *
> > connector_to_ti_sn_bridge(struct drm_connector *connector)
> > @@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> >
> > drm_bridge_add(&pdata->bridge);
> >
> > + ti_sn_debugfs_init(pdata);
> > +
> > return 0;
> > }
> >
> > @@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
> > if (!pdata)
> > return -EINVAL;
> >
> > + ti_sn_debugfs_remove(pdata);
> > +
> > of_node_put(pdata->host_node);
> >
> > pm_runtime_disable(pdata->dev);
--
Regards,
Laurent Pinchart
On Thu, Jul 4, 2019 at 5:35 AM Laurent Pinchart
<[email protected]> wrote:
>
> Hello,
>
> On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> > On 02.07.2019 17:44, Rob Clark wrote:
> > > From: Rob Clark <[email protected]>
> > >
> > > Add a debugfs file to show status registers.
> > >
> > > Signed-off-by: Rob Clark <[email protected]>
> > > ---
> > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > > 1 file changed, 42 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > index f1a2493b86d9..a6f27648c015 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > @@ -5,6 +5,7 @@
> > > */
> > >
> > > #include <linux/clk.h>
> > > +#include <linux/debugfs.h>
> > > #include <linux/gpio/consumer.h>
> > > #include <linux/i2c.h>
> > > #include <linux/iopoll.h>
> > > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > > struct drm_dp_aux aux;
> > > struct drm_bridge bridge;
> > > struct drm_connector connector;
> > > + struct dentry *debugfs;
> > > struct device_node *host_node;
> > > struct mipi_dsi_device *dsi;
> > > struct clk *refclk;
> > > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > > };
> > >
> > > +static int status_show(struct seq_file *s, void *data)
> > > +{
> > > + struct ti_sn_bridge *pdata = s->private;
> > > + unsigned int reg, val;
> > > +
> > > + seq_puts(s, "STATUS REGISTERS:\n");
>
> NO NEED TO SHOUT :-)
>
> > > +
> > > + pm_runtime_get_sync(pdata->dev);
> > > +
> > > + /* IRQ Status Registers, see Table 31 in datasheet */
> > > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > > + regmap_read(pdata->regmap, reg, &val);
> > > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > > + }
> > > +
> > > + pm_runtime_put(pdata->dev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +DEFINE_SHOW_ATTRIBUTE(status);
> > > +
> > > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > > +{
> > > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
> >
> > If some day we will have board with two such bridges there will be a
> > problem.
>
> Could we use the platform device name for this ?
hmm, yeah, that would solve the 2x bridges issue
> > Anyway:
> >
> > Reviewed-by: Andrzej Hajda <[email protected]>
> >
> > > +
> > > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > > + &status_fops);
> > > +}
> > > +
> > > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > > +{
> > > + debugfs_remove_recursive(pdata->debugfs);
> > > + pdata->debugfs = NULL;
> > > +}
> > > +
>
> You need to conditionally-compile this based on CONFIG_DEBUG_FS.
Hmm, is that really true? Debugfs appears to be sufficently stub'd w/
inline no-ops in the !CONFIG_DEBUG_FS case
BR,
-R
Hi Rob,
On Thu, Jul 04, 2019 at 06:56:56AM -0700, Rob Clark wrote:
> On Thu, Jul 4, 2019 at 5:35 AM Laurent Pinchartwrote:
> > On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> > > On 02.07.2019 17:44, Rob Clark wrote:
> > > > From: Rob Clark <[email protected]>
> > > >
> > > > Add a debugfs file to show status registers.
> > > >
> > > > Signed-off-by: Rob Clark <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > > > 1 file changed, 42 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > index f1a2493b86d9..a6f27648c015 100644
> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > @@ -5,6 +5,7 @@
> > > > */
> > > >
> > > > #include <linux/clk.h>
> > > > +#include <linux/debugfs.h>
> > > > #include <linux/gpio/consumer.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/iopoll.h>
> > > > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > > > struct drm_dp_aux aux;
> > > > struct drm_bridge bridge;
> > > > struct drm_connector connector;
> > > > + struct dentry *debugfs;
> > > > struct device_node *host_node;
> > > > struct mipi_dsi_device *dsi;
> > > > struct clk *refclk;
> > > > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > > > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > > > };
> > > >
> > > > +static int status_show(struct seq_file *s, void *data)
> > > > +{
> > > > + struct ti_sn_bridge *pdata = s->private;
> > > > + unsigned int reg, val;
> > > > +
> > > > + seq_puts(s, "STATUS REGISTERS:\n");
> >
> > NO NEED TO SHOUT :-)
> >
> > > > +
> > > > + pm_runtime_get_sync(pdata->dev);
> > > > +
> > > > + /* IRQ Status Registers, see Table 31 in datasheet */
> > > > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > > > + regmap_read(pdata->regmap, reg, &val);
> > > > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > > > + }
> > > > +
> > > > + pm_runtime_put(pdata->dev);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +DEFINE_SHOW_ATTRIBUTE(status);
> > > > +
> > > > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > > > +{
> > > > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
> > >
> > > If some day we will have board with two such bridges there will be a
> > > problem.
> >
> > Could we use the platform device name for this ?
>
> hmm, yeah, that would solve the 2x bridges issue
>
> > > Anyway:
> > >
> > > Reviewed-by: Andrzej Hajda <[email protected]>
> > >
> > > > +
> > > > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > > > + &status_fops);
> > > > +}
> > > > +
> > > > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > > > +{
> > > > + debugfs_remove_recursive(pdata->debugfs);
> > > > + pdata->debugfs = NULL;
> > > > +}
> > > > +
> >
> > You need to conditionally-compile this based on CONFIG_DEBUG_FS.
>
> Hmm, is that really true? Debugfs appears to be sufficently stub'd w/
> inline no-ops in the !CONFIG_DEBUG_FS case
You're right, my bad. I wonder if the compiler will optimise the above
two functions out. It might warrant a CONFIG_DEBUG_FS check here for
that reason, but that's really bikeshedding. So with the 2x bridges
issue addressed, I think the patch will be good.
--
Regards,
Laurent Pinchart