mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Dave Young" <hidave.darkstar@gmail.com>
To: "Alan Stern" <stern@rowland.harvard.edu>
Cc: "Matthew Dharm" <mdharm-kernel@one-eyed-alien.net>,
	"Greg KH" <greg@kroah.com>,
	bbpetkov@yahoo.de,
	"Kernel development list" <linux-kernel@vger.kernel.org>,
	"USB development list" <linux-usb-devel@lists.sourceforge.net>
Subject: Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
Date: Thu, 18 Oct 2007 09:52:03 +0800	[thread overview]
Message-ID: <a8e1da0710171852s679b57bdp817c3a1d636c1b9d@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0710171040440.4468-100000@iolanthe.rowland.org>

On 10/17/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Tue, 16 Oct 2007, Matthew Dharm wrote:
>
> > On Tue, Oct 16, 2007 at 02:04:43PM -0400, Alan Stern wrote:
> > > On Tue, 16 Oct 2007, Matthew Dharm wrote:
> > >
> > > > I haven't looked at this code at all, but neither approach feels right to
> > > > me.
> > > >
> > > > How does this work at all?  Even if you load a driver later, wouldn't it
> > > > call usb_set_interface(), which would call usb_create_sysfs_intf_files()
> > > > and hit the same issue?
> > >
> > > usb_set_interface() is smart enough to remove the old interface files
> > > before creating new ones, since it expects them to exist already.
> > > Hence there's no problem in that scenario.
> > >
> > > But usb_set_configuration doesn't expect there to be any pre-existing
> > > interface files, because there isn't even an interface until the
> > > registration is performed.
> >
> > And I'm guessing that you can't call usb_create_sysfs_intf_files() until
> > registration is performed, right?
>
> Right.
>
> > > The most important reason has to do with the endpoint pseudo-devices.
> > > Different altsettings can have different endpoints, so those have to be
> > > removed and re-created whenever the altsetting changes.
> >
> > Right, altsettings.  I forgot about those.  I only ever think in terms of
> > multiple configurations.
> >
> > *grumble*
> >
> > If usb_set_interface() has to be smart enough to remove existing files
> > first already, then I guess it's reasonably symmetric to have
> > usb_set_configuration() have the same smarts.  Maybe they can share some
> > common code, even.
>
> It's not a big deal to remove the files first.  In fact, here's a patch
> to do it.  Dave, see if this doesn't fix your problem.  I don't like it
> much because it does an unnecessary remove/create cycle, but that's
> better than doing something wrong.

Although it's not the best fix, the problem is fixed, Thanks.

>
> It's slightly odd that the sysfs core logs an error when you try to
> create the same file twice but it doesn't when you try to remove a
> non-existent file (or try to remove an existing file twice).  Oh
> well...
>
> Alan Stern
>
>
>
> Index: usb-2.6/drivers/usb/core/message.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/core/message.c
> +++ usb-2.6/drivers/usb/core/message.c
> @@ -1643,7 +1643,13 @@ free_interfaces:
>                                 intf->dev.bus_id, ret);
>                         continue;
>                 }
> -               usb_create_sysfs_intf_files (intf);
> +
> +               /* The driver's probe method can call usb_set_interface(),
> +                * which would mean the interface's sysfs files are already
> +                * created.  Just in case, we'll remove them first.
> +                */
> +               usb_remove_sysfs_intf_files(intf);
> +               usb_create_sysfs_intf_files(intf);
>         }
>
>         usb_autosuspend_device(dev);
>
>

  reply	other threads:[~2007-10-18  1:52 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-14 15:15 Borislav Petkov
2007-10-15  5:57 ` Dave Young
2007-10-15 18:38   ` [linux-usb-devel] " Alan Stern
2007-10-16  4:48     ` Greg KH
2007-10-16  5:22       ` Dave Young
2007-10-16 14:55         ` Alan Stern
2007-10-16 16:33           ` Matthew Dharm
2007-10-16 18:04             ` Alan Stern
2007-10-16 19:13               ` Matthew Dharm
2007-10-17  1:31                 ` Dave Young
2007-10-17 14:48                 ` Alan Stern
2007-10-18  1:52                   ` Dave Young [this message]
2007-10-18 22:48                   ` Greg KH
2007-10-25  9:06 Dave Young
2007-10-25 18:11 ` Greg KH
2007-10-26  2:01   ` Dave Young
2007-10-26  2:42     ` Greg KH
2007-10-26  3:11       ` Dave Young
2007-10-26  3:28         ` Greg KH
2007-10-26  4:43           ` Dave Young

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a8e1da0710171852s679b57bdp817c3a1d636c1b9d@mail.gmail.com \
    --to=hidave.darkstar@gmail.com \
    --cc=bbpetkov@yahoo.de \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=mdharm-kernel@one-eyed-alien.net \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®