| Message ID | 1389950556-6453-2-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote: > debugfs files to show the contents of important dsps registers. > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > --- > drivers/usb/musb/musb_dsps.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > index 593d3c9..d0a97d6 100644 > --- a/drivers/usb/musb/musb_dsps.c > +++ b/drivers/usb/musb/musb_dsps.c > @@ -46,6 +46,8 @@ > #include <linux/of_irq.h> > #include <linux/usb/of.h> > > +#include <linux/debugfs.h> > + > #include "musb_core.h" > > static const struct of_device_id musb_dsps_of_match[]; > @@ -137,6 +139,26 @@ struct dsps_glue { > unsigned long last_timer; /* last timer data for each instance */ > > struct dsps_context context; > + struct debugfs_regset32 regset; > + struct dentry *dbgfs_root; > +}; > + > +static const struct debugfs_reg32 dsps_musb_regs[] = { > + { "revision", 0x00 }, > + { "control", 0x14 }, > + { "status", 0x18 }, > + { "eoi", 0x24 }, > + { "intr0_stat", 0x30 }, > + { "intr1_stat", 0x34 }, > + { "intr0_set", 0x38 }, > + { "intr1_set", 0x3c }, > + { "txmode", 0x70 }, > + { "rxmode", 0x74 }, > + { "autoreq", 0xd0 }, > + { "srpfixtime", 0xd4 }, > + { "tdown", 0xd8 }, > + { "phy_utmi", 0xe0 }, > + { "mode", 0xe8 }, > }; > > static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout) > @@ -369,6 +391,30 @@ out: > return ret; > } > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) > +{ > + struct dentry *root; > + struct dentry *file; > + char buf[128]; > + > + sprintf(buf, "%s.dsps", dev_name(musb->controller)); > + root = debugfs_create_dir(buf, NULL); > + if (!root) wrong, you should be using IS_ERR()
On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote: > On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote: > > debugfs files to show the contents of important dsps registers. > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > --- > > drivers/usb/musb/musb_dsps.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > > index 593d3c9..d0a97d6 100644 > > --- a/drivers/usb/musb/musb_dsps.c > > +++ b/drivers/usb/musb/musb_dsps.c > > @@ -46,6 +46,8 @@ > > #include <linux/of_irq.h> > > #include <linux/usb/of.h> > > > > +#include <linux/debugfs.h> > > + > > #include "musb_core.h" > > > > static const struct of_device_id musb_dsps_of_match[]; > > @@ -137,6 +139,26 @@ struct dsps_glue { > > unsigned long last_timer; /* last timer data for each instance */ > > > > struct dsps_context context; > > + struct debugfs_regset32 regset; > > + struct dentry *dbgfs_root; > > +}; > > + > > +static const struct debugfs_reg32 dsps_musb_regs[] = { > > + { "revision", 0x00 }, > > + { "control", 0x14 }, > > + { "status", 0x18 }, > > + { "eoi", 0x24 }, > > + { "intr0_stat", 0x30 }, > > + { "intr1_stat", 0x34 }, > > + { "intr0_set", 0x38 }, > > + { "intr1_set", 0x3c }, > > + { "txmode", 0x70 }, > > + { "rxmode", 0x74 }, > > + { "autoreq", 0xd0 }, > > + { "srpfixtime", 0xd4 }, > > + { "tdown", 0xd8 }, > > + { "phy_utmi", 0xe0 }, > > + { "mode", 0xe8 }, > > }; > > > > static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout) > > @@ -369,6 +391,30 @@ out: > > return ret; > > } > > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) > > +{ > > + struct dentry *root; > > + struct dentry *file; > > + char buf[128]; > > + > > + sprintf(buf, "%s.dsps", dev_name(musb->controller)); > > + root = debugfs_create_dir(buf, NULL); > > + if (!root) > > wrong, you should be using IS_ERR() !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.
On Tue, Feb 18, 2014 at 08:59:11AM -0800, Greg KH wrote: > On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote: > > On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote: > > > debugfs files to show the contents of important dsps registers. > > > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > > --- > > > drivers/usb/musb/musb_dsps.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 54 insertions(+) > > > > > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > > > index 593d3c9..d0a97d6 100644 > > > --- a/drivers/usb/musb/musb_dsps.c > > > +++ b/drivers/usb/musb/musb_dsps.c > > > @@ -46,6 +46,8 @@ > > > #include <linux/of_irq.h> > > > #include <linux/usb/of.h> > > > > > > +#include <linux/debugfs.h> > > > + > > > #include "musb_core.h" > > > > > > static const struct of_device_id musb_dsps_of_match[]; > > > @@ -137,6 +139,26 @@ struct dsps_glue { > > > unsigned long last_timer; /* last timer data for each instance */ > > > > > > struct dsps_context context; > > > + struct debugfs_regset32 regset; > > > + struct dentry *dbgfs_root; > > > +}; > > > + > > > +static const struct debugfs_reg32 dsps_musb_regs[] = { > > > + { "revision", 0x00 }, > > > + { "control", 0x14 }, > > > + { "status", 0x18 }, > > > + { "eoi", 0x24 }, > > > + { "intr0_stat", 0x30 }, > > > + { "intr1_stat", 0x34 }, > > > + { "intr0_set", 0x38 }, > > > + { "intr1_set", 0x3c }, > > > + { "txmode", 0x70 }, > > > + { "rxmode", 0x74 }, > > > + { "autoreq", 0xd0 }, > > > + { "srpfixtime", 0xd4 }, > > > + { "tdown", 0xd8 }, > > > + { "phy_utmi", 0xe0 }, > > > + { "mode", 0xe8 }, > > > }; > > > > > > static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout) > > > @@ -369,6 +391,30 @@ out: > > > return ret; > > > } > > > > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) > > > +{ > > > + struct dentry *root; > > > + struct dentry *file; > > > + char buf[128]; > > > + > > > + sprintf(buf, "%s.dsps", dev_name(musb->controller)); > > > + root = debugfs_create_dir(buf, NULL); > > > + if (!root) > > > > wrong, you should be using IS_ERR() > > !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled. in that case, files will be created on parent directory right ? If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in __create_file().
On Tue, Feb 18, 2014 at 11:03:35AM -0600, Felipe Balbi wrote: > On Tue, Feb 18, 2014 at 08:59:11AM -0800, Greg KH wrote: > > On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote: > > > On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote: > > > > debugfs files to show the contents of important dsps registers. > > > > > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > > > --- > > > > drivers/usb/musb/musb_dsps.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 54 insertions(+) > > > > > > > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > > > > index 593d3c9..d0a97d6 100644 > > > > --- a/drivers/usb/musb/musb_dsps.c > > > > +++ b/drivers/usb/musb/musb_dsps.c > > > > @@ -46,6 +46,8 @@ > > > > #include <linux/of_irq.h> > > > > #include <linux/usb/of.h> > > > > > > > > +#include <linux/debugfs.h> > > > > + > > > > #include "musb_core.h" > > > > > > > > static const struct of_device_id musb_dsps_of_match[]; > > > > @@ -137,6 +139,26 @@ struct dsps_glue { > > > > unsigned long last_timer; /* last timer data for each instance */ > > > > > > > > struct dsps_context context; > > > > + struct debugfs_regset32 regset; > > > > + struct dentry *dbgfs_root; > > > > +}; > > > > + > > > > +static const struct debugfs_reg32 dsps_musb_regs[] = { > > > > + { "revision", 0x00 }, > > > > + { "control", 0x14 }, > > > > + { "status", 0x18 }, > > > > + { "eoi", 0x24 }, > > > > + { "intr0_stat", 0x30 }, > > > > + { "intr1_stat", 0x34 }, > > > > + { "intr0_set", 0x38 }, > > > > + { "intr1_set", 0x3c }, > > > > + { "txmode", 0x70 }, > > > > + { "rxmode", 0x74 }, > > > > + { "autoreq", 0xd0 }, > > > > + { "srpfixtime", 0xd4 }, > > > > + { "tdown", 0xd8 }, > > > > + { "phy_utmi", 0xe0 }, > > > > + { "mode", 0xe8 }, > > > > }; > > > > > > > > static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout) > > > > @@ -369,6 +391,30 @@ out: > > > > return ret; > > > > } > > > > > > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) > > > > +{ > > > > + struct dentry *root; > > > > + struct dentry *file; > > > > + char buf[128]; > > > > + > > > > + sprintf(buf, "%s.dsps", dev_name(musb->controller)); > > > > + root = debugfs_create_dir(buf, NULL); > > > > + if (!root) > > > > > > wrong, you should be using IS_ERR() > > > > !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled. > > in that case, files will be created on parent directory right ? If, for some reason, creating a directory fails and then creating a file would succeed, yes, that will happen. > If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in > __create_file(). No, because -ENODEV will only happen if debugfs is not enabled, so no dereference will ever happen. Don't worry about checking return values from debugfs, it shouldn't be needed. thanks, greg k-h
Hi, On Tue, Feb 18, 2014 at 09:30:21AM -0800, Greg KH wrote: > > > > > +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) > > > > > +{ > > > > > + struct dentry *root; > > > > > + struct dentry *file; > > > > > + char buf[128]; > > > > > + > > > > > + sprintf(buf, "%s.dsps", dev_name(musb->controller)); > > > > > + root = debugfs_create_dir(buf, NULL); > > > > > + if (!root) > > > > > > > > wrong, you should be using IS_ERR() > > > > > > !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled. > > > > in that case, files will be created on parent directory right ? > > If, for some reason, creating a directory fails and then creating a file > would succeed, yes, that will happen. > > > If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in > > __create_file(). > > No, because -ENODEV will only happen if debugfs is not enabled, so no > dereference will ever happen. > > Don't worry about checking return values from debugfs, it shouldn't be > needed. aha, now I see. I missed the: 348 if (error) { 349 dentry = NULL; 350 simple_release_fs(&debugfs_mount, &debugfs_mount_count); 351 } in fs/debugfs/inode.c::__create_file(). I'll apply this patch in a few hours (randconfig running). cheers
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 593d3c9..d0a97d6 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -46,6 +46,8 @@ #include <linux/of_irq.h> #include <linux/usb/of.h> +#include <linux/debugfs.h> + #include "musb_core.h" static const struct of_device_id musb_dsps_of_match[]; @@ -137,6 +139,26 @@ struct dsps_glue { unsigned long last_timer; /* last timer data for each instance */ struct dsps_context context; + struct debugfs_regset32 regset; + struct dentry *dbgfs_root; +}; + +static const struct debugfs_reg32 dsps_musb_regs[] = { + { "revision", 0x00 }, + { "control", 0x14 }, + { "status", 0x18 }, + { "eoi", 0x24 }, + { "intr0_stat", 0x30 }, + { "intr1_stat", 0x34 }, + { "intr0_set", 0x38 }, + { "intr1_set", 0x3c }, + { "txmode", 0x70 }, + { "rxmode", 0x74 }, + { "autoreq", 0xd0 }, + { "srpfixtime", 0xd4 }, + { "tdown", 0xd8 }, + { "phy_utmi", 0xe0 }, + { "mode", 0xe8 }, }; static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout) @@ -369,6 +391,30 @@ out: return ret; } +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue) +{ + struct dentry *root; + struct dentry *file; + char buf[128]; + + sprintf(buf, "%s.dsps", dev_name(musb->controller)); + root = debugfs_create_dir(buf, NULL); + if (!root) + return -ENOMEM; + glue->dbgfs_root = root; + + glue->regset.regs = dsps_musb_regs; + glue->regset.nregs = ARRAY_SIZE(dsps_musb_regs); + glue->regset.base = musb->ctrl_base; + + file = debugfs_create_regset32("regdump", S_IRUGO, root, &glue->regset); + if (!file) { + debugfs_remove_recursive(root); + return -ENOMEM; + } + return 0; +} + static int dsps_musb_init(struct musb *musb) { struct device *dev = musb->controller; @@ -378,6 +424,7 @@ static int dsps_musb_init(struct musb *musb) void __iomem *reg_base; struct resource *r; u32 rev, val; + int ret; r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control"); if (!r) @@ -411,6 +458,10 @@ static int dsps_musb_init(struct musb *musb) val &= ~(1 << wrp->otg_disable); dsps_writel(musb->ctrl_base, wrp->phy_utmi, val); + ret = dsps_musb_dbg_init(musb, glue); + if (ret) + return ret; + return 0; } @@ -659,6 +710,9 @@ static int dsps_remove(struct platform_device *pdev) pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); kfree(glue); + + debugfs_remove_recursive(glue->dbgfs_root); + return 0; }
debugfs files to show the contents of important dsps registers. Signed-off-by: Markus Pargmann <mpa@pengutronix.de> --- drivers/usb/musb/musb_dsps.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)