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?

By the way, I'd like to complain a bit about the debugfs patch :-)

First, was it really necessary to error out in case the debugfs files failed
to be created? Maybe we can simply omit the error check, unless there's some
reason and we *need* to know if is succeeded.

Also, should we add a compile time option for that so that the debugfs can
be disabled? 

Thanks!
-- 
Ezequiel GarcĂ­a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to