diff mbox

[v4,02/11] usb: musb: kill global and static for multi instance

Message ID 1342698367-12978-3-git-send-email-ajay.gupta@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ajay Kumar Gupta July 19, 2012, 11:45 a.m. UTC
Moved global variable "musb_debugfs_root" and static variable
"old_state" to 'struct musb' to help support multi instance of
musb controller as present on AM335x platform.

Also removed the global variable "orig_dma_mask" and filled the
dev->dma_mask with parent device's dma_mask.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musb_core.c    |   16 +++-------------
 drivers/usb/musb/musb_core.h    |    4 ++++
 drivers/usb/musb/musb_debugfs.c |   14 ++++++++------
 3 files changed, 15 insertions(+), 19 deletions(-)

Comments

Felipe Balbi July 25, 2012, 7:50 a.m. UTC | #1
Hi,

On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote:
> Moved global variable "musb_debugfs_root" and static variable
> "old_state" to 'struct musb' to help support multi instance of
> musb controller as present on AM335x platform.
> 
> Also removed the global variable "orig_dma_mask" and filled the
> dev->dma_mask with parent device's dma_mask.
> 
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> ---
>  drivers/usb/musb/musb_core.c    |   16 +++-------------
>  drivers/usb/musb/musb_core.h    |    4 ++++
>  drivers/usb/musb/musb_debugfs.c |   14 ++++++++------
>  3 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 3e09984..a5db4dd 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1802,10 +1802,9 @@ static const struct attribute_group musb_attr_group = {
>  static void musb_irq_work(struct work_struct *data)
>  {
>  	struct musb *musb = container_of(data, struct musb, irq_work);
> -	static int old_state;
>  
> -	if (musb->xceiv->state != old_state) {
> -		old_state = musb->xceiv->state;
> +	if (musb->xceiv->state != musb->xceiv_old_state) {
> +		musb->xceiv_old_state = musb->xceiv->state;
>  		sysfs_notify(&musb->controller->kobj, NULL, "mode");
>  	}
>  }
> @@ -2115,11 +2114,6 @@ fail0:
>  /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just
>   * bridge to a platform device; this driver then suffices.
>   */
> -
> -#ifndef CONFIG_MUSB_PIO_ONLY
> -static u64	*orig_dma_mask;
> -#endif
> -
>  static int __devinit musb_probe(struct platform_device *pdev)
>  {
>  	struct device	*dev = &pdev->dev;
> @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> -#ifndef CONFIG_MUSB_PIO_ONLY
> -	/* clobbered by use_dma=n */
> -	orig_dma_mask = dev->dma_mask;
> -#endif
>  	status = musb_init_controller(dev, irq, base);
>  	if (status < 0)
>  		iounmap(base);
> @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct platform_device *pdev)
>  	iounmap(ctrl_base);
>  	device_init_wakeup(&pdev->dev, 0);
>  #ifndef CONFIG_MUSB_PIO_ONLY
> -	pdev->dev.dma_mask = orig_dma_mask;
> +	pdev->dev.dma_mask = (&dev->dev.parent)->dma_mask;

are you sure ?? This should be:

dev->parent.dma_mask;

In fact, while doing that you could use dma_set_mask() instead ;-)
Ajay Kumar Gupta July 25, 2012, 10:34 a.m. UTC | #2
Hi,
> 
> On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote:
> > Moved global variable "musb_debugfs_root" and static variable
> > "old_state" to 'struct musb' to help support multi instance of musb
> > controller as present on AM335x platform.
> >
> > Also removed the global variable "orig_dma_mask" and filled the
> > dev->dma_mask with parent device's dma_mask.
> >
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > ---
> >  drivers/usb/musb/musb_core.c    |   16 +++-------------
> >  drivers/usb/musb/musb_core.h    |    4 ++++
> >  drivers/usb/musb/musb_debugfs.c |   14 ++++++++------
> >  3 files changed, 15 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_core.c
> > b/drivers/usb/musb/musb_core.c index 3e09984..a5db4dd 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -1802,10 +1802,9 @@ static const struct attribute_group
> > musb_attr_group = {  static void musb_irq_work(struct work_struct
> > *data)  {
> >  	struct musb *musb = container_of(data, struct musb, irq_work);
> > -	static int old_state;
> >
> > -	if (musb->xceiv->state != old_state) {
> > -		old_state = musb->xceiv->state;
> > +	if (musb->xceiv->state != musb->xceiv_old_state) {
> > +		musb->xceiv_old_state = musb->xceiv->state;
> >  		sysfs_notify(&musb->controller->kobj, NULL, "mode");
> >  	}
> >  }
> > @@ -2115,11 +2114,6 @@ fail0:
> >  /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just
> >   * bridge to a platform device; this driver then suffices.
> >   */
> > -
> > -#ifndef CONFIG_MUSB_PIO_ONLY
> > -static u64	*orig_dma_mask;
> > -#endif
> > -
> >  static int __devinit musb_probe(struct platform_device *pdev)  {
> >  	struct device	*dev = &pdev->dev;
> > @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct
> platform_device *pdev)
> >  		return -ENOMEM;
> >  	}
> >
> > -#ifndef CONFIG_MUSB_PIO_ONLY
> > -	/* clobbered by use_dma=n */
> > -	orig_dma_mask = dev->dma_mask;
> > -#endif
> >  	status = musb_init_controller(dev, irq, base);
> >  	if (status < 0)
> >  		iounmap(base);
> > @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct
> platform_device *pdev)
> >  	iounmap(ctrl_base);
> >  	device_init_wakeup(&pdev->dev, 0);
> >  #ifndef CONFIG_MUSB_PIO_ONLY
> > -	pdev->dev.dma_mask = orig_dma_mask;
> > +	pdev->dev.dma_mask = (&dev->dev.parent)->dma_mask;
> 
> are you sure ?? This should be:
> 
> dev->parent.dma_mask;

Actually both are fine and I used parent based on your recommendation
in my earlier version of patch at
http://marc.info/?l=linux-usb&m=134121391908734&w=2

anyways I will change to :
pdev->dev.dma_mask = dma_set_mask((&pdev->dev, DMA_BIT_MASK(64));

Ajay
> 
> In fact, while doing that you could use dma_set_mask() instead ;-)
> 
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi July 25, 2012, 11:10 a.m. UTC | #3
On Wed, Jul 25, 2012 at 10:34:49AM +0000, Gupta, Ajay Kumar wrote:
> Hi,
> > 
> > On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote:
> > > Moved global variable "musb_debugfs_root" and static variable
> > > "old_state" to 'struct musb' to help support multi instance of musb
> > > controller as present on AM335x platform.
> > >
> > > Also removed the global variable "orig_dma_mask" and filled the
> > > dev->dma_mask with parent device's dma_mask.
> > >
> > > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > > ---
> > >  drivers/usb/musb/musb_core.c    |   16 +++-------------
> > >  drivers/usb/musb/musb_core.h    |    4 ++++
> > >  drivers/usb/musb/musb_debugfs.c |   14 ++++++++------
> > >  3 files changed, 15 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/usb/musb/musb_core.c
> > > b/drivers/usb/musb/musb_core.c index 3e09984..a5db4dd 100644
> > > --- a/drivers/usb/musb/musb_core.c
> > > +++ b/drivers/usb/musb/musb_core.c
> > > @@ -1802,10 +1802,9 @@ static const struct attribute_group
> > > musb_attr_group = {  static void musb_irq_work(struct work_struct
> > > *data)  {
> > >  	struct musb *musb = container_of(data, struct musb, irq_work);
> > > -	static int old_state;
> > >
> > > -	if (musb->xceiv->state != old_state) {
> > > -		old_state = musb->xceiv->state;
> > > +	if (musb->xceiv->state != musb->xceiv_old_state) {
> > > +		musb->xceiv_old_state = musb->xceiv->state;
> > >  		sysfs_notify(&musb->controller->kobj, NULL, "mode");
> > >  	}
> > >  }
> > > @@ -2115,11 +2114,6 @@ fail0:
> > >  /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just
> > >   * bridge to a platform device; this driver then suffices.
> > >   */
> > > -
> > > -#ifndef CONFIG_MUSB_PIO_ONLY
> > > -static u64	*orig_dma_mask;
> > > -#endif
> > > -
> > >  static int __devinit musb_probe(struct platform_device *pdev)  {
> > >  	struct device	*dev = &pdev->dev;
> > > @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct
> > platform_device *pdev)
> > >  		return -ENOMEM;
> > >  	}
> > >
> > > -#ifndef CONFIG_MUSB_PIO_ONLY
> > > -	/* clobbered by use_dma=n */
> > > -	orig_dma_mask = dev->dma_mask;
> > > -#endif
> > >  	status = musb_init_controller(dev, irq, base);
> > >  	if (status < 0)
> > >  		iounmap(base);
> > > @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct
> > platform_device *pdev)
> > >  	iounmap(ctrl_base);
> > >  	device_init_wakeup(&pdev->dev, 0);
> > >  #ifndef CONFIG_MUSB_PIO_ONLY
> > > -	pdev->dev.dma_mask = orig_dma_mask;
> > > +	pdev->dev.dma_mask = (&dev->dev.parent)->dma_mask;
> > 
> > are you sure ?? This should be:
> > 
> > dev->parent.dma_mask;
> 
> Actually both are fine and I used parent based on your recommendation
> in my earlier version of patch at
> http://marc.info/?l=linux-usb&m=134121391908734&w=2
> 
> anyways I will change to :
> pdev->dev.dma_mask = dma_set_mask((&pdev->dev, DMA_BIT_MASK(64));

no Ajay, you misunderstood... I don't see how this patch would compile.

the variable named "dev" is already a pointer to a struct device, and
struct device doesn't have a "dev" field.

what I asked to do while doing that is:

dev_set_mask(dev, *dev->parent->dma_mask);
Ajay Kumar Gupta July 25, 2012, 11:24 a.m. UTC | #4
Hi,
> On Wed, Jul 25, 2012 at 10:34:49AM +0000, Gupta, Ajay Kumar wrote:
> > Hi,
> > >
> > > On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote:
> > > > Moved global variable "musb_debugfs_root" and static variable
> > > > "old_state" to 'struct musb' to help support multi instance of
> > > > musb controller as present on AM335x platform.
> > > >
> > > > Also removed the global variable "orig_dma_mask" and filled the
> > > > dev->dma_mask with parent device's dma_mask.
> > > >
> > > > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> > > > ---
> > > >  drivers/usb/musb/musb_core.c    |   16 +++-------------
> > > >  drivers/usb/musb/musb_core.h    |    4 ++++
> > > >  drivers/usb/musb/musb_debugfs.c |   14 ++++++++------
> > > >  3 files changed, 15 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/musb/musb_core.c
> > > > b/drivers/usb/musb/musb_core.c index 3e09984..a5db4dd 100644
> > > > --- a/drivers/usb/musb/musb_core.c
> > > > +++ b/drivers/usb/musb/musb_core.c
> > > > @@ -1802,10 +1802,9 @@ static const struct attribute_group
> > > > musb_attr_group = {  static void musb_irq_work(struct work_struct
> > > > *data)  {
> > > >  	struct musb *musb = container_of(data, struct musb, irq_work);
> > > > -	static int old_state;
> > > >
> > > > -	if (musb->xceiv->state != old_state) {
> > > > -		old_state = musb->xceiv->state;
> > > > +	if (musb->xceiv->state != musb->xceiv_old_state) {
> > > > +		musb->xceiv_old_state = musb->xceiv->state;
> > > >  		sysfs_notify(&musb->controller->kobj, NULL, "mode");
> > > >  	}
> > > >  }
> > > > @@ -2115,11 +2114,6 @@ fail0:
> > > >  /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just
> > > >   * bridge to a platform device; this driver then suffices.
> > > >   */
> > > > -
> > > > -#ifndef CONFIG_MUSB_PIO_ONLY
> > > > -static u64	*orig_dma_mask;
> > > > -#endif
> > > > -
> > > >  static int __devinit musb_probe(struct platform_device *pdev)  {
> > > >  	struct device	*dev = &pdev->dev;
> > > > @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct
> > > platform_device *pdev)
> > > >  		return -ENOMEM;
> > > >  	}
> > > >
> > > > -#ifndef CONFIG_MUSB_PIO_ONLY
> > > > -	/* clobbered by use_dma=n */
> > > > -	orig_dma_mask = dev->dma_mask;
> > > > -#endif
> > > >  	status = musb_init_controller(dev, irq, base);
> > > >  	if (status < 0)
> > > >  		iounmap(base);
> > > > @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct
> > > platform_device *pdev)
> > > >  	iounmap(ctrl_base);
> > > >  	device_init_wakeup(&pdev->dev, 0);  #ifndef
> CONFIG_MUSB_PIO_ONLY
> > > > -	pdev->dev.dma_mask = orig_dma_mask;
> > > > +	pdev->dev.dma_mask = (&dev->dev.parent)->dma_mask;
> > >
> > > are you sure ?? This should be:
> > >
> > > dev->parent.dma_mask;
> >
> > Actually both are fine and I used parent based on your recommendation
> > in my earlier version of patch at
> > http://marc.info/?l=linux-usb&m=134121391908734&w=2
> >
> > anyways I will change to :
> > pdev->dev.dma_mask = dma_set_mask((&pdev->dev, DMA_BIT_MASK(64));
> 
> no Ajay, you misunderstood... I don't see how this patch would compile.
It was my bad. I am testing it with PIO mode only and so code was not compiled.
"dev" was actually a "pdev".

> the variable named "dev" is already a pointer to a struct device, and struct
> device doesn't have a "dev" field.
> 
> what I asked to do while doing that is:
> 
> dev_set_mask(dev, *dev->parent->dma_mask);
Sure.

Thanks.
> 
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 3e09984..a5db4dd 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1802,10 +1802,9 @@  static const struct attribute_group musb_attr_group = {
 static void musb_irq_work(struct work_struct *data)
 {
 	struct musb *musb = container_of(data, struct musb, irq_work);
-	static int old_state;
 
-	if (musb->xceiv->state != old_state) {
-		old_state = musb->xceiv->state;
+	if (musb->xceiv->state != musb->xceiv_old_state) {
+		musb->xceiv_old_state = musb->xceiv->state;
 		sysfs_notify(&musb->controller->kobj, NULL, "mode");
 	}
 }
@@ -2115,11 +2114,6 @@  fail0:
 /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just
  * bridge to a platform device; this driver then suffices.
  */
-
-#ifndef CONFIG_MUSB_PIO_ONLY
-static u64	*orig_dma_mask;
-#endif
-
 static int __devinit musb_probe(struct platform_device *pdev)
 {
 	struct device	*dev = &pdev->dev;
@@ -2138,10 +2132,6 @@  static int __devinit musb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-#ifndef CONFIG_MUSB_PIO_ONLY
-	/* clobbered by use_dma=n */
-	orig_dma_mask = dev->dma_mask;
-#endif
 	status = musb_init_controller(dev, irq, base);
 	if (status < 0)
 		iounmap(base);
@@ -2166,7 +2156,7 @@  static int __devexit musb_remove(struct platform_device *pdev)
 	iounmap(ctrl_base);
 	device_init_wakeup(&pdev->dev, 0);
 #ifndef CONFIG_MUSB_PIO_ONLY
-	pdev->dev.dma_mask = orig_dma_mask;
+	pdev->dev.dma_mask = (&dev->dev.parent)->dma_mask;
 #endif
 	return 0;
 }
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 69ed141..6b6cee9 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -452,6 +452,10 @@  struct musb {
 #endif
 	/* id for multiple musb instances */
 	u8			id;
+	int                     xceiv_old_state;
+#ifdef CONFIG_DEBUG_FS
+	struct dentry           *debugfs_root;
+#endif
 };
 
 static inline struct musb *gadget_to_musb(struct usb_gadget *g)
diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index 40a37c9..b1e8f21 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -103,8 +103,6 @@  static const struct musb_register_map musb_regmap[] = {
 	{  }	/* Terminating Entry */
 };
 
-static struct dentry *musb_debugfs_root;
-
 static int musb_regdump_show(struct seq_file *s, void *unused)
 {
 	struct musb		*musb = s->private;
@@ -240,20 +238,24 @@  int __devinit musb_init_debugfs(struct musb *musb)
 	struct dentry		*root;
 	struct dentry		*file;
 	int			ret;
+	char			name[10];
 
-	root = debugfs_create_dir("musb", NULL);
+	sprintf(name, "musb%d", musb->id);
+	root = debugfs_create_dir(name, NULL);
 	if (!root) {
 		ret = -ENOMEM;
 		goto err0;
 	}
 
-	file = debugfs_create_file("regdump", S_IRUGO, root, musb,
+	sprintf(name, "regdump%d", musb->id);
+	file = debugfs_create_file(name, S_IRUGO, root, musb,
 			&musb_regdump_fops);
 	if (!file) {
 		ret = -ENOMEM;
 		goto err1;
 	}
 
+	sprintf(name, "testmode%d", musb->id);
 	file = debugfs_create_file("testmode", S_IRUGO | S_IWUSR,
 			root, musb, &musb_test_mode_fops);
 	if (!file) {
@@ -261,7 +263,7 @@  int __devinit musb_init_debugfs(struct musb *musb)
 		goto err1;
 	}
 
-	musb_debugfs_root = root;
+	musb->debugfs_root = root;
 
 	return 0;
 
@@ -274,5 +276,5 @@  err0:
 
 void /* __init_or_exit */ musb_exit_debugfs(struct musb *musb)
 {
-	debugfs_remove_recursive(musb_debugfs_root);
+	debugfs_remove_recursive(musb->debugfs_root);
 }