Re: [PATCH] USB: musb: dsps: move debugfs_remove_recursive()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 22, 2014 at 10:26:13AM -0500, Felipe Balbi wrote:
> On Tue, Apr 22, 2014 at 12:19:16PM -0300, Ezequiel Garcia wrote:
> > On Apr 22, Felipe Balbi wrote:
> > > On Tue, Apr 22, 2014 at 11:01:28AM -0300, Ezequiel Garcia wrote:
> > > > On Apr 02, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Wed, Apr 02, 2014 at 06:16:23PM +0200, Daniel Mack wrote:
> > > > > > On 04/02/2014 06:05 PM, Felipe Balbi wrote:
> > > > > > > On Wed, Apr 02, 2014 at 11:46:51AM +0200, Daniel Mack wrote:
> > > > > > 
> > > > > > >> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > > > > > >> index 3372ded..e2fd263 100644
> > > > > > >> --- a/drivers/usb/musb/musb_dsps.c
> > > > > > >> +++ b/drivers/usb/musb/musb_dsps.c
> > > > > > >> @@ -470,8 +470,9 @@ static int dsps_musb_exit(struct musb *musb)
> > > > > > >>  	struct dsps_glue *glue = dev_get_drvdata(dev->parent);
> > > > > > >>  
> > > > > > >>  	del_timer_sync(&glue->timer);
> > > > > > >> -
> > > > > > >>  	usb_phy_shutdown(musb->xceiv);
> > > > > > >> +	debugfs_remove_recursive(glue->dbgfs_root);
> > > > > > >> +
> > > > > > >>  	return 0;
> > > > > > >>  }
> > > > > > >>  
> > > > > > >> @@ -708,8 +709,6 @@ static int dsps_remove(struct platform_device *pdev)
> > > > > > >>  	pm_runtime_put(&pdev->dev);
> > > > > > >>  	pm_runtime_disable(&pdev->dev);
> > > > > > >>  
> > > > > > >> -	debugfs_remove_recursive(glue->dbgfs_root);
> > > > > > > 
> > > > > > > I don't think this is the right fix. debugfs_remove_recursive is
> > > > > > > supposed to remove the directory as well. Why isn't dsps_musb_exit()
> > > > > > > called ?
> > > > > > 
> > > > > > dsps_musb_exit() is called, hence my fix works :) The question is rather
> > > > > > why dsps_remove() isn't called.
> > > > > > 
> > > > > > For me, the fix looked obvious, as resources that are claimed in
> > > > > > dsps_musb_init() should obviously be freed in its counterpart function,
> > > > > > dsps_musb_exit(). For reasons of readability if not for any other :)
> > > > > 
> > > > > you're correct, for whatever reason I read it as moving the lines the
> > > > > other way around (from _exit() to _remove()).
> > > > > 
> > > > > I'll queue this once -rc1 is tagged.
> > > > > 
> > > > 
> > > > Felipe:
> > > > 
> > > > This fix didn't land on -rc2, and it's not even in -next.
> > > > Am I missing something? Is it possible to queue this as urgently as
> > > > possible?
> > > 
> > > Grass hopper, please... :-)
> > > 
> > > http://marc.info/?l=linux-usb&m=139809466025568&w=2
> > > 
> > 
> > Oh, nice. Thanks for taking care of this!
> > 
> > Is there any strong reason for the fixes branch not being part of -next?
> > Or maybe you should push the patches to your next branch, instead?
> > 
> > I found functionality breaks *very* frequently on early -rc, and it's
> > usually quicker to take a peek at -next and see if somebody already fixed it.
> > 
> > That way silly grass-hoppers like me won't have to go through the
> > find-bisect-fix bug cycle, only to discover someone else already fixed
> > it but the patch is floating around :-)
> 
> yeah, maybe.. I don't want to merge my fixes into next though. Maybe
> Stephen could just merge both my branches on next. Greg, are you merging
> your usb-linus into linux-next ?

Yes, both of my branches get merged into linux-next.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux