[PATCH v4 0/2] usb: musb dsps updates

[PATCH v4 0/2] usb: musb dsps updates

[Markus Pargmann]

Hi,

The two remaining patches from the previous series "usb: musb bugfixes". In v4
I used the device name for the debugfs root dir and added a commit message to
the second patch.

Regards,

Markus


Markus Pargmann (2):
  usb: musb: dsps, debugfs files
  usb: musb: dsps, use devm_kzalloc

 drivers/usb/musb/musb_dsps.c | 58 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 3 deletions(-)

-- 
1.8.5.2

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Markus Pargmann]

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)
+		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;
 }
 
-- 
1.8.5.2

[PATCH v4 2/2] usb: musb: dsps, use devm_kzalloc

[Markus Pargmann]

Replace kzalloc by devm_kzalloc and remove the kfree() calls.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/usb/musb/musb_dsps.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index d0a97d6..6cae0c8 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -668,7 +668,7 @@ static int dsps_probe(struct platform_device *pdev)
 	wrp = match->data;
 
 	/* allocate glue */
-	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
 	if (!glue) {
 		dev_err(&pdev->dev, "unable to allocate glue memory\n");
 		return -ENOMEM;
@@ -696,7 +696,6 @@ err3:
 	pm_runtime_put(&pdev->dev);
 err2:
 	pm_runtime_disable(&pdev->dev);
-	kfree(glue);
 	return ret;
 }
 
@@ -709,7 +708,6 @@ static int dsps_remove(struct platform_device *pdev)
 	/* disable usbss clocks */
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	kfree(glue);
 
 	debugfs_remove_recursive(glue->dbgfs_root);
 
-- 
1.8.5.2

[PATCH v4 0/2] usb: musb dsps updates

[Markus Pargmann]

Hi,

On Fri, Jan 17, 2014 at 10:22:34AM +0100, Markus Pargmann wrote:
> Hi,
> 
> The two remaining patches from the previous series "usb: musb bugfixes". In v4
> I used the device name for the debugfs root dir and added a commit message to
> the second patch.

Felipe, could you please have a look at this?

Thanks,

Markus

> 
> Regards,
> 
> Markus
> 
> 
> Markus Pargmann (2):
>   usb: musb: dsps, debugfs files
>   usb: musb: dsps, use devm_kzalloc
> 
>  drivers/usb/musb/musb_dsps.c | 58 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 3 deletions(-)
> 
> -- 
> 1.8.5.2
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

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()

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/2b850196/attachment.sig>

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

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.

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

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().

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/553260bb/attachment.sig>

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

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

[PATCH v4 1/2] usb: musb: dsps, debugfs files

[Felipe Balbi]

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

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/508bb275/attachment.sig>