Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > On Fri, Oct 26, 2007 at 11:11:22AM +0800, Dave Young wrote: > > On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > > > > Anyway the sysfs_dirent_exist is useful for extern use, How about add > > > > and export this function? Greg, If you agree, I would send it as > > > > another patch. > > > > > > What would need that function? > > I think the function is needed sometimes except for files related to > > devices like usb and others that could be removed suddenly. > > Almost all types of devices can now be removed "suddenly" from a system. > Hm, I almost don't know of a type that can _not_ be removed anymore... > > But if you have a patch that needs this function, I'd be glad to > reconsider. Not yet, anyway thank you very much :) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Fri, Oct 26, 2007 at 11:11:22AM +0800, Dave Young wrote: > On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > > > Anyway the sysfs_dirent_exist is useful for extern use, How about add > > > and export this function? Greg, If you agree, I would send it as > > > another patch. > > > > What would need that function? > I think the function is needed sometimes except for files related to > devices like usb and others that could be removed suddenly. Almost all types of devices can now be removed "suddenly" from a system. Hm, I almost don't know of a type that can _not_ be removed anymore... But if you have a patch that needs this function, I'd be glad to reconsider. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > On Fri, Oct 26, 2007 at 10:01:49AM +0800, Dave Young wrote: > > On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > > > On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote: > > > > On 10/19/07, Greg KH <[EMAIL PROTECTED]> wrote: > > > > >On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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. > > > > >> > > > > >> 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... > > > > > > > > > >I used to have the 'remove a non-existant file' warning, but that just > > > > >triggered _way_ too many responses :) > > > > > > > > > > > > > > >> 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); > > > > >> } > > > > > > > > > >If this fixes the problem, care to resend it with a signed-off-by:? > > > > > > > > > >Yeah, it's not the nicest solution, but I can't think of any other one > > > > >either right now :( > > > > Hi, greg > > > > > > > > How about this patch (based on 2.6.24-rc1): > > > > > > > > diff -upr linux/drivers/usb/core/message.c > > > > linux.new/drivers/usb/core/message.c > > > > --- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 > > > > +0800 > > > > +++ linux.new/drivers/usb/core/message.c 2007-10-25 > > > > 16:39:38.0 +0800 > > > > @@ -1641,7 +1641,8 @@ free_interfaces: > > > > intf->dev.bus_id, ret); > > > > continue; > > > > } > > > > - usb_create_sysfs_intf_files (intf); > > > > + if(!usb_sysfs_intf_exist(intf)) > > > > +
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Fri, Oct 26, 2007 at 10:01:49AM +0800, Dave Young wrote: > On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote: > > On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote: > > > On 10/19/07, Greg KH <[EMAIL PROTECTED]> wrote: > > > >On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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. > > > >> > > > >> 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... > > > > > > > >I used to have the 'remove a non-existant file' warning, but that just > > > >triggered _way_ too many responses :) > > > > > > > > > > > >> 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); > > > >> } > > > > > > > >If this fixes the problem, care to resend it with a signed-off-by:? > > > > > > > >Yeah, it's not the nicest solution, but I can't think of any other one > > > >either right now :( > > > Hi, greg > > > > > > How about this patch (based on 2.6.24-rc1): > > > > > > diff -upr linux/drivers/usb/core/message.c > > > linux.new/drivers/usb/core/message.c > > > --- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800 > > > +++ linux.new/drivers/usb/core/message.c 2007-10-25 > > > 16:39:38.0 +0800 > > > @@ -1641,7 +1641,8 @@ free_interfaces: > > > intf->dev.bus_id, ret); > > > continue; > > > } > > > - usb_create_sysfs_intf_files (intf); > > > + if(!usb_sysfs_intf_exist(intf)) > > > + usb_create_sysfs_intf_files (intf); > > > } > > > > > > usb_autosuspend_device(dev); > > > diff -upr linux/drivers/usb/core/sysfs.c > > > linux.new/drivers/usb/core/sysfs.c > > > --- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800 > > > +++
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH <[EMAIL PROTECTED]> wrote:
> On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
> > On 10/19/07, Greg KH <[EMAIL PROTECTED]> wrote:
> > >On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
> > >>
> > >> 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...
> > >
> > >I used to have the 'remove a non-existant file' warning, but that just
> > >triggered _way_ too many responses :)
> > >
> > >
> > >> 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);
> > >> }
> > >
> > >If this fixes the problem, care to resend it with a signed-off-by:?
> > >
> > >Yeah, it's not the nicest solution, but I can't think of any other one
> > >either right now :(
> > Hi, greg
> >
> > How about this patch (based on 2.6.24-rc1):
> >
> > diff -upr linux/drivers/usb/core/message.c
> > linux.new/drivers/usb/core/message.c
> > --- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800
> > +++ linux.new/drivers/usb/core/message.c 2007-10-25 16:39:38.0
> > +0800
> > @@ -1641,7 +1641,8 @@ free_interfaces:
> > intf->dev.bus_id, ret);
> > continue;
> > }
> > - usb_create_sysfs_intf_files (intf);
> > + if(!usb_sysfs_intf_exist(intf))
> > + usb_create_sysfs_intf_files (intf);
> > }
> >
> > usb_autosuspend_device(dev);
> > diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
> > --- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800
> > +++ linux.new/drivers/usb/core/sysfs.c2007-10-25 16:39:32.0
> > +0800
> > @@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
> > usb_remove_ep_files(_desc->endpoint[i]);
> > }
> >
> > +int usb_sysfs_intf_exist(struct usb_interface *intf)
> > +{
> > + struct
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
> On 10/19/07, Greg KH <[EMAIL PROTECTED]> wrote:
> >On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
> >>
> >> 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...
> >
> >I used to have the 'remove a non-existant file' warning, but that just
> >triggered _way_ too many responses :)
> >
> >
> >> 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);
> >> }
> >
> >If this fixes the problem, care to resend it with a signed-off-by:?
> >
> >Yeah, it's not the nicest solution, but I can't think of any other one
> >either right now :(
> Hi, greg
>
> How about this patch (based on 2.6.24-rc1):
>
> diff -upr linux/drivers/usb/core/message.c
> linux.new/drivers/usb/core/message.c
> --- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800
> +++ linux.new/drivers/usb/core/message.c 2007-10-25 16:39:38.0
> +0800
> @@ -1641,7 +1641,8 @@ free_interfaces:
> intf->dev.bus_id, ret);
> continue;
> }
> - usb_create_sysfs_intf_files (intf);
> + if(!usb_sysfs_intf_exist(intf))
> + usb_create_sysfs_intf_files (intf);
> }
>
> usb_autosuspend_device(dev);
> diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
> --- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800
> +++ linux.new/drivers/usb/core/sysfs.c2007-10-25 16:39:32.0
> +0800
> @@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
> usb_remove_ep_files(_desc->endpoint[i]);
> }
>
> +int usb_sysfs_intf_exist(struct usb_interface *intf)
> +{
> + struct device *dev = >dev;
> +
> + return sysfs_dirent_exist(>kobj, intf_attrs[0]->name);
The issue is that you can't just test for the first file. If you look
at the logic in the usb_create_sysfs_intf_file() code, we do create
different files based on the current interface. So this
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/19/07, Greg KH <[EMAIL PROTECTED]> wrote:
>On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
>>
>> 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...
>
>I used to have the 'remove a non-existant file' warning, but that just
>triggered _way_ too many responses :)
>
>
>> 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);
>> }
>
>If this fixes the problem, care to resend it with a signed-off-by:?
>
>Yeah, it's not the nicest solution, but I can't think of any other one
>either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c2007-10-25 16:41:32.0 +0800
+++ linux.new/drivers/usb/core/message.c2007-10-25 16:39:38.0
+0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf->dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c 2007-10-25 16:40:16.0 +0800
+++ linux.new/drivers/usb/core/sysfs.c 2007-10-25 16:39:32.0 +0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(_desc->endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = >dev;
+
+ return sysfs_dirent_exist(>kobj, intf_attrs[0]->name);
+}
+
int usb_create_sysfs_intf_files(struct usb_interface *intf)
{
struct device *dev = >dev;
diff -upr linux/drivers/usb/core/usb.h linux.new/drivers/usb/core/usb.h
--- linux/drivers/usb/core/usb.h2007-10-25 16:41:02.0 +0800
+++ linux.new/drivers/usb/core/usb.h2007-10-25 16:39:19.0 +0800
@@ -1,5 +1,6 @@
/* Functions local to drivers/usb/core/ */
+extern int usb_sysfs_intf_exist(struct usb_interface *intf);
extern int
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/19/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
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...
I used to have the 'remove a non-existant file' warning, but that just
triggered _way_ too many responses :)
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);
}
If this fixes the problem, care to resend it with a signed-off-by:?
Yeah, it's not the nicest solution, but I can't think of any other one
either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c2007-10-25 16:41:32.0 +0800
+++ linux.new/drivers/usb/core/message.c2007-10-25 16:39:38.0
+0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf-dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c 2007-10-25 16:40:16.0 +0800
+++ linux.new/drivers/usb/core/sysfs.c 2007-10-25 16:39:32.0 +0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(iface_desc-endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = intf-dev;
+
+ return sysfs_dirent_exist(dev-kobj, intf_attrs[0]-name);
+}
+
int usb_create_sysfs_intf_files(struct usb_interface *intf)
{
struct device *dev = intf-dev;
diff -upr linux/drivers/usb/core/usb.h linux.new/drivers/usb/core/usb.h
--- linux/drivers/usb/core/usb.h2007-10-25 16:41:02.0 +0800
+++ linux.new/drivers/usb/core/usb.h2007-10-25 16:39:19.0 +0800
@@ -1,5 +1,6 @@
/* Functions local to drivers/usb/core/ */
+extern int usb_sysfs_intf_exist(struct usb_interface *intf);
extern int usb_create_sysfs_dev_files (struct usb_device *dev);
extern void usb_remove_sysfs_dev_files (struct usb_device *dev);
extern int usb_create_sysfs_intf_files (struct usb_interface *intf);
diff -upr linux/fs/sysfs/dir.c linux.new/fs/sysfs/dir.c
---
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
On 10/19/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
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...
I used to have the 'remove a non-existant file' warning, but that just
triggered _way_ too many responses :)
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);
}
If this fixes the problem, care to resend it with a signed-off-by:?
Yeah, it's not the nicest solution, but I can't think of any other one
either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c
linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800
+++ linux.new/drivers/usb/core/message.c 2007-10-25 16:39:38.0
+0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf-dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800
+++ linux.new/drivers/usb/core/sysfs.c2007-10-25 16:39:32.0
+0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(iface_desc-endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = intf-dev;
+
+ return sysfs_dirent_exist(dev-kobj, intf_attrs[0]-name);
The issue is that you can't just test for the first file. If you look
at the logic in the usb_create_sysfs_intf_file() code, we do create
different files based on the current interface. So this might not
always end up with the proper files in userspace, from what I can tell.
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH [EMAIL PROTECTED] wrote:
On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
On 10/19/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
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...
I used to have the 'remove a non-existant file' warning, but that just
triggered _way_ too many responses :)
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);
}
If this fixes the problem, care to resend it with a signed-off-by:?
Yeah, it's not the nicest solution, but I can't think of any other one
either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c
linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800
+++ linux.new/drivers/usb/core/message.c 2007-10-25 16:39:38.0
+0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf-dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800
+++ linux.new/drivers/usb/core/sysfs.c2007-10-25 16:39:32.0
+0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(iface_desc-endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = intf-dev;
+
+ return sysfs_dirent_exist(dev-kobj, intf_attrs[0]-name);
The issue is that you can't just test for the first file. If you look
at the logic in the usb_create_sysfs_intf_file() code, we do create
different files based on the current interface. So this might not
always end up with the proper files in userspace, from what I can tell.
Yes, I know this is not good, it just fixed the bug for me. It's hard
to test all files simply.
The duplicate file
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Fri, Oct 26, 2007 at 10:01:49AM +0800, Dave Young wrote:
On 10/26/07, Greg KH [EMAIL PROTECTED] wrote:
On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
On 10/19/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
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...
I used to have the 'remove a non-existant file' warning, but that just
triggered _way_ too many responses :)
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);
}
If this fixes the problem, care to resend it with a signed-off-by:?
Yeah, it's not the nicest solution, but I can't think of any other one
either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c
linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0 +0800
+++ linux.new/drivers/usb/core/message.c 2007-10-25
16:39:38.0 +0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf-dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c
linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0 +0800
+++ linux.new/drivers/usb/core/sysfs.c2007-10-25
16:39:32.0 +0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(iface_desc-endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = intf-dev;
+
+ return sysfs_dirent_exist(dev-kobj, intf_attrs[0]-name);
The issue is that you can't just test for the first file. If you look
at the logic in the usb_create_sysfs_intf_file() code, we do create
different files based on the current interface. So this might not
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH [EMAIL PROTECTED] wrote:
On Fri, Oct 26, 2007 at 10:01:49AM +0800, Dave Young wrote:
On 10/26/07, Greg KH [EMAIL PROTECTED] wrote:
On Thu, Oct 25, 2007 at 05:06:59PM +0800, Dave Young wrote:
On 10/19/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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.
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...
I used to have the 'remove a non-existant file' warning, but that just
triggered _way_ too many responses :)
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);
}
If this fixes the problem, care to resend it with a signed-off-by:?
Yeah, it's not the nicest solution, but I can't think of any other one
either right now :(
Hi, greg
How about this patch (based on 2.6.24-rc1):
diff -upr linux/drivers/usb/core/message.c
linux.new/drivers/usb/core/message.c
--- linux/drivers/usb/core/message.c 2007-10-25 16:41:32.0
+0800
+++ linux.new/drivers/usb/core/message.c 2007-10-25
16:39:38.0 +0800
@@ -1641,7 +1641,8 @@ free_interfaces:
intf-dev.bus_id, ret);
continue;
}
- usb_create_sysfs_intf_files (intf);
+ if(!usb_sysfs_intf_exist(intf))
+ usb_create_sysfs_intf_files (intf);
}
usb_autosuspend_device(dev);
diff -upr linux/drivers/usb/core/sysfs.c
linux.new/drivers/usb/core/sysfs.c
--- linux/drivers/usb/core/sysfs.c2007-10-25 16:40:16.0
+0800
+++ linux.new/drivers/usb/core/sysfs.c2007-10-25
16:39:32.0 +0800
@@ -728,6 +728,13 @@ static inline void usb_remove_intf_ep_fi
usb_remove_ep_files(iface_desc-endpoint[i]);
}
+int usb_sysfs_intf_exist(struct usb_interface *intf)
+{
+ struct device *dev = intf-dev;
+
+ return sysfs_dirent_exist(dev-kobj, intf_attrs[0]-name);
The issue is
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Fri, Oct 26, 2007 at 11:11:22AM +0800, Dave Young wrote: On 10/26/07, Greg KH [EMAIL PROTECTED] wrote: Anyway the sysfs_dirent_exist is useful for extern use, How about add and export this function? Greg, If you agree, I would send it as another patch. What would need that function? I think the function is needed sometimes except for files related to devices like usb and others that could be removed suddenly. Almost all types of devices can now be removed suddenly from a system. Hm, I almost don't know of a type that can _not_ be removed anymore... But if you have a patch that needs this function, I'd be glad to reconsider. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/26/07, Greg KH [EMAIL PROTECTED] wrote: On Fri, Oct 26, 2007 at 11:11:22AM +0800, Dave Young wrote: On 10/26/07, Greg KH [EMAIL PROTECTED] wrote: Anyway the sysfs_dirent_exist is useful for extern use, How about add and export this function? Greg, If you agree, I would send it as another patch. What would need that function? I think the function is needed sometimes except for files related to devices like usb and others that could be removed suddenly. Almost all types of devices can now be removed suddenly from a system. Hm, I almost don't know of a type that can _not_ be removed anymore... But if you have a patch that needs this function, I'd be glad to reconsider. Not yet, anyway thank you very much :) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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. > > 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... I used to have the 'remove a non-existant file' warning, but that just triggered _way_ too many responses :) > 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); > } If this fixes the problem, care to resend it with a signed-off-by:? Yeah, it's not the nicest solution, but I can't think of any other one either right now :( thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Wed, Oct 17, 2007 at 10:48:52AM -0400, Alan Stern 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. 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... I used to have the 'remove a non-existant file' warning, but that just triggered _way_ too many responses :) 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); } If this fixes the problem, care to resend it with a signed-off-by:? Yeah, it's not the nicest solution, but I can't think of any other one either right now :( thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/17/07, Alan Stern <[EMAIL PROTECTED]> 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); > > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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. 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); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/17/07, Alan Stern [EMAIL PROTECTED] 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); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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. 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); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
>On 10/17/07, Matthew Dharm <[EMAIL PROTECTED]> 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? > > > 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. > > Matt > > -- > Matthew Dharm Home: [EMAIL PROTECTED] > Maintainer, Linux USB Mass Storage Driver > > C: Why are you upgrading to NT? > AJ: It must be the sick, sadistic streak that runs through me. > -- Chief and A.J. > User Friendly, 5/12/1998 > Hi, I prefer "remove then create". But IMHO the sysfs or driver core layer should have such functions to set some bit for the sysfs files creating. Regards dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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? > 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. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver C: Why are you upgrading to NT? AJ: It must be the sick, sadistic streak that runs through me. -- Chief and A.J. User Friendly, 5/12/1998 pgpyl02DsudZG.pgp Description: PGP signature
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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. > Heck, why do both call usb_create_sysfs_intf_file()? I would guess if > you're *changing* the active configuration you would need to do that, but > why in usb_set_interface() at all? For a couple of reasons. The "interface" attribute file contains the iInterface string descriptor, and that file is present only if such a descriptor exists. Since different altsettings might not agree on whether or not iInterface exists, the attribute has to be created anew for each altsetting. (Yes, we could let the file always be present and just be blank if there is no descriptor.) 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. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Tue, Oct 16, 2007 at 10:55:54AM -0400, Alan Stern wrote: > On Tue, 16 Oct 2007, Dave Young wrote: > > > > I finally duplicated this on one of my machines here at boot time, with > > > USB built into the kernel. I'll work tomorrow on tracking this down > > > further... > > Hi, > > I add some printk messages, dump_stack and some others, here is the > > dmesg dump with debug info(lines begin with "hidave"): > > Okay, good, the extra printk messages show exactly where the problem > lies. > > In usb_set_configuration(), each new interfaces is registered and then > usb_create_sysfs_intf_files() gets called for that interface. This > makes sense, because obviously we can't create sysfs files for an > interface before it is registered. > > The problem is that during registration drivers get probed, and drivers > sometimes call usb_set_interface() from their probe method. This > routine also calls usb_create_sysfs_intf_files(), and the result is > that the sysfs files get created twice: > > First by usb_set_interface, from the driver probe; > > Then by usb_set_configuration, when registration is > finished. > > I can think of two possible ways around the problem. One is to add a > bit to the usb_interface structure, recording whether the sysfs files > have been created. The other is always to remove the files just before > trying to create them. 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? Heck, why do both call usb_create_sysfs_intf_file()? I would guess if you're *changing* the active configuration you would need to do that, but why in usb_set_interface() at all? Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver I say, what are all those naked people doing? -- Big client to Stef User Friendly, 12/14/1997 pgpUQw629YoXR.pgp Description: PGP signature
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Tue, 16 Oct 2007, Dave Young wrote: > > I finally duplicated this on one of my machines here at boot time, with > > USB built into the kernel. I'll work tomorrow on tracking this down > > further... > Hi, > I add some printk messages, dump_stack and some others, here is the > dmesg dump with debug info(lines begin with "hidave"): Okay, good, the extra printk messages show exactly where the problem lies. In usb_set_configuration(), each new interfaces is registered and then usb_create_sysfs_intf_files() gets called for that interface. This makes sense, because obviously we can't create sysfs files for an interface before it is registered. The problem is that during registration drivers get probed, and drivers sometimes call usb_set_interface() from their probe method. This routine also calls usb_create_sysfs_intf_files(), and the result is that the sysfs files get created twice: First by usb_set_interface, from the driver probe; Then by usb_set_configuration, when registration is finished. I can think of two possible ways around the problem. One is to add a bit to the usb_interface structure, recording whether the sysfs files have been created. The other is always to remove the files just before trying to create them. The first seems more workable, although it is slightly awkward. Greg, what do you think? Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Tue, 16 Oct 2007, Dave Young wrote: I finally duplicated this on one of my machines here at boot time, with USB built into the kernel. I'll work tomorrow on tracking this down further... Hi, I add some printk messages, dump_stack and some others, here is the dmesg dump with debug info(lines begin with hidave): Okay, good, the extra printk messages show exactly where the problem lies. In usb_set_configuration(), each new interfaces is registered and then usb_create_sysfs_intf_files() gets called for that interface. This makes sense, because obviously we can't create sysfs files for an interface before it is registered. The problem is that during registration drivers get probed, and drivers sometimes call usb_set_interface() from their probe method. This routine also calls usb_create_sysfs_intf_files(), and the result is that the sysfs files get created twice: First by usb_set_interface, from the driver probe; Then by usb_set_configuration, when registration is finished. I can think of two possible ways around the problem. One is to add a bit to the usb_interface structure, recording whether the sysfs files have been created. The other is always to remove the files just before trying to create them. The first seems more workable, although it is slightly awkward. Greg, what do you think? Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Tue, Oct 16, 2007 at 10:55:54AM -0400, Alan Stern wrote: On Tue, 16 Oct 2007, Dave Young wrote: I finally duplicated this on one of my machines here at boot time, with USB built into the kernel. I'll work tomorrow on tracking this down further... Hi, I add some printk messages, dump_stack and some others, here is the dmesg dump with debug info(lines begin with hidave): Okay, good, the extra printk messages show exactly where the problem lies. In usb_set_configuration(), each new interfaces is registered and then usb_create_sysfs_intf_files() gets called for that interface. This makes sense, because obviously we can't create sysfs files for an interface before it is registered. The problem is that during registration drivers get probed, and drivers sometimes call usb_set_interface() from their probe method. This routine also calls usb_create_sysfs_intf_files(), and the result is that the sysfs files get created twice: First by usb_set_interface, from the driver probe; Then by usb_set_configuration, when registration is finished. I can think of two possible ways around the problem. One is to add a bit to the usb_interface structure, recording whether the sysfs files have been created. The other is always to remove the files just before trying to create them. 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? Heck, why do both call usb_create_sysfs_intf_file()? I would guess if you're *changing* the active configuration you would need to do that, but why in usb_set_interface() at all? Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver I say, what are all those naked people doing? -- Big client to Stef User Friendly, 12/14/1997 pgpUQw629YoXR.pgp Description: PGP signature
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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. Heck, why do both call usb_create_sysfs_intf_file()? I would guess if you're *changing* the active configuration you would need to do that, but why in usb_set_interface() at all? For a couple of reasons. The interface attribute file contains the iInterface string descriptor, and that file is present only if such a descriptor exists. Since different altsettings might not agree on whether or not iInterface exists, the attribute has to be created anew for each altsetting. (Yes, we could let the file always be present and just be blank if there is no descriptor.) 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. Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
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? 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. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver C: Why are you upgrading to NT? AJ: It must be the sick, sadistic streak that runs through me. -- Chief and A.J. User Friendly, 5/12/1998 pgpyl02DsudZG.pgp Description: PGP signature
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/17/07, Matthew Dharm [EMAIL PROTECTED] 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? 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. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver C: Why are you upgrading to NT? AJ: It must be the sick, sadistic streak that runs through me. -- Chief and A.J. User Friendly, 5/12/1998 Hi, I prefer remove then create. But IMHO the sysfs or driver core layer should have such functions to set some bit for the sysfs files creating. Regards dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
>On 10/16/07, Greg KH <[EMAIL PROTECTED]> wrote: > On Mon, Oct 15, 2007 at 02:38:25PM -0400, Alan Stern wrote: > > On Mon, 15 Oct 2007, Dave Young wrote: > > > > > On 10/14/07, Borislav Petkov <[EMAIL PROTECTED]> wrote: > > > > Hi, > > > > > > > > i get the following warning on yesterday's git tree > > > > (v2.6.23-2840-g752097c): > > > > > > > > Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename > > > > 'bInterfaceNumber' can not be created > > > > Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at > > > > fs/sysfs/dir.c:425 sysfs_add_one() > > > > Oct 14 09:07:15 zmei kernel: [ 49.368134] [] > > > > show_trace_log_lvl+0x1a/0x2f > > > > Oct 14 09:07:15 zmei kernel: [ 49.368220] [] > > > > show_trace+0x12/0x14 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368300] [] > > > > dump_stack+0x16/0x18 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368379] [] > > > > sysfs_add_one+0x57/0xbc > > > > Oct 14 09:07:15 zmei kernel: [ 49.368461] [] > > > > sysfs_add_file+0x49/0x71 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368541] [] > > > > sysfs_create_group+0x86/0xe8 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368621] [] > > > > usb_create_sysfs_intf_files+0x27/0x9b > > > > Oct 14 09:07:15 zmei kernel: [ 49.368704] [] > > > > usb_set_configuration+0x454/0x466 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368787] [] > > > > generic_probe+0x53/0x94 > > > > Oct 14 09:07:15 zmei kernel: [ 49.368867] [] > > > > usb_probe_device+0x35/0x3b > > > > Oct 14 09:07:15 zmei kernel: [ 49.368947] [] > > > > driver_probe_device+0xcb/0x14f > > > > Oct 14 09:07:15 zmei kernel: [ 49.369039] [] > > > > __device_attach+0x8/0xa > > > > Oct 14 09:07:15 zmei kernel: [ 49.369119] [] > > > > bus_for_each_drv+0x3b/0x63 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369199] [] > > > > device_attach+0x70/0x85 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369279] [] > > > > bus_attach_device+0x29/0x77 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369359] [] > > > > device_add+0x28c/0x445 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369439] [] > > > > usb_new_device+0x44/0x82 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369519] [] > > > > hub_thread+0x666/0x9c2 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369598] [] > > > > kthread+0x3b/0x62 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369679] [] > > > > kernel_thread_helper+0x7/0x10 > > > > Oct 14 09:07:15 zmei kernel: [ 49.369759] === > > > > > > > > The usb hub in question is named 4-1:1.0 and it has an extension > > > > connected to it > > > > which is used to activate the 2 usb connectors at the side of the pc's > > > > monitor. > > > > Correct me if i'm wrong but from what i've understood so far from > > > > reading the code, > > > > i think, it adds the bInterfaceNumber-file after calling > > > > usb_create_sysfs_intf_files(intf). > > > > However, the currently active usbhost interface alternate setting is > > > > the only one active > > > > so the bInterfaceNumber exists already and therefore the warning, but > > > > this is > > > > just a guess since i'm not that fluent in the usb internals. > > > Hi, > > > I have encountered the same problem which was reported in > > > http://lkml.org/lkml/2007/9/29/45 > > > > > > For the first one "usbcore duplicated sysfs filename" , I have submit > > > a patch to fix it. > > > > > > For the "bInterfaceNumber" one, I have no idea, the same problem still > > > exist in the latest 23-mm1 tree. > > > > I have tried several times to duplicate this, most recently under > > 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. > > > > You may have to do your own debugging. Try adding printk statements to > > usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you > > can tell when they get called. > > I finally duplicated this on one of my machines here at boot time, with > USB built into the kernel. I'll work tomorrow on tracking this down > further... Hi, I add some printk messages, dump_stack and some others, here is the dmesg dump with debug info(lines begin with "hidave"): Linux version 2.6.23-mm1 ([EMAIL PROTECTED]) (gcc version 3.4.6) #4 SMP PREEMPT Tue Oct 16 11:14:10 CST 2007 BIOS-provided physical RAM map: BIOS-e820: - 000a (usable) BIOS-e820: 000f - 0010 (reserved) BIOS-e820: 0010 - 3fe88c00 (usable) BIOS-e820: 3fe88c00 - 3fe8ac00 (ACPI NVS) BIOS-e820: 3fe8ac00 - 3fe8cc00 (ACPI data) BIOS-e820: 3fe8cc00 - 4000 (reserved) BIOS-e820: f000 - f400 (reserved) BIOS-e820: fec0 - fed00400 (reserved) BIOS-e820: fed2 - feda (reserved) BIOS-e820: fee0 - fef0 (reserved) BIOS-e820: ffb0 - 0001 (reserved) 126MB HIGHMEM available. 896MB LOWMEM available. found SMP MP-table at 000fe710
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Mon, Oct 15, 2007 at 02:38:25PM -0400, Alan Stern wrote: > On Mon, 15 Oct 2007, Dave Young wrote: > > > On 10/14/07, Borislav Petkov <[EMAIL PROTECTED]> wrote: > > > Hi, > > > > > > i get the following warning on yesterday's git tree > > > (v2.6.23-2840-g752097c): > > > > > > Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename > > > 'bInterfaceNumber' can not be created > > > Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at > > > fs/sysfs/dir.c:425 sysfs_add_one() > > > Oct 14 09:07:15 zmei kernel: [ 49.368134] [] > > > show_trace_log_lvl+0x1a/0x2f > > > Oct 14 09:07:15 zmei kernel: [ 49.368220] [] > > > show_trace+0x12/0x14 > > > Oct 14 09:07:15 zmei kernel: [ 49.368300] [] > > > dump_stack+0x16/0x18 > > > Oct 14 09:07:15 zmei kernel: [ 49.368379] [] > > > sysfs_add_one+0x57/0xbc > > > Oct 14 09:07:15 zmei kernel: [ 49.368461] [] > > > sysfs_add_file+0x49/0x71 > > > Oct 14 09:07:15 zmei kernel: [ 49.368541] [] > > > sysfs_create_group+0x86/0xe8 > > > Oct 14 09:07:15 zmei kernel: [ 49.368621] [] > > > usb_create_sysfs_intf_files+0x27/0x9b > > > Oct 14 09:07:15 zmei kernel: [ 49.368704] [] > > > usb_set_configuration+0x454/0x466 > > > Oct 14 09:07:15 zmei kernel: [ 49.368787] [] > > > generic_probe+0x53/0x94 > > > Oct 14 09:07:15 zmei kernel: [ 49.368867] [] > > > usb_probe_device+0x35/0x3b > > > Oct 14 09:07:15 zmei kernel: [ 49.368947] [] > > > driver_probe_device+0xcb/0x14f > > > Oct 14 09:07:15 zmei kernel: [ 49.369039] [] > > > __device_attach+0x8/0xa > > > Oct 14 09:07:15 zmei kernel: [ 49.369119] [] > > > bus_for_each_drv+0x3b/0x63 > > > Oct 14 09:07:15 zmei kernel: [ 49.369199] [] > > > device_attach+0x70/0x85 > > > Oct 14 09:07:15 zmei kernel: [ 49.369279] [] > > > bus_attach_device+0x29/0x77 > > > Oct 14 09:07:15 zmei kernel: [ 49.369359] [] > > > device_add+0x28c/0x445 > > > Oct 14 09:07:15 zmei kernel: [ 49.369439] [] > > > usb_new_device+0x44/0x82 > > > Oct 14 09:07:15 zmei kernel: [ 49.369519] [] > > > hub_thread+0x666/0x9c2 > > > Oct 14 09:07:15 zmei kernel: [ 49.369598] [] > > > kthread+0x3b/0x62 > > > Oct 14 09:07:15 zmei kernel: [ 49.369679] [] > > > kernel_thread_helper+0x7/0x10 > > > Oct 14 09:07:15 zmei kernel: [ 49.369759] === > > > > > > The usb hub in question is named 4-1:1.0 and it has an extension > > > connected to it > > > which is used to activate the 2 usb connectors at the side of the pc's > > > monitor. > > > Correct me if i'm wrong but from what i've understood so far from reading > > > the code, > > > i think, it adds the bInterfaceNumber-file after calling > > > usb_create_sysfs_intf_files(intf). > > > However, the currently active usbhost interface alternate setting is the > > > only one active > > > so the bInterfaceNumber exists already and therefore the warning, but > > > this is > > > just a guess since i'm not that fluent in the usb internals. > > Hi, > > I have encountered the same problem which was reported in > > http://lkml.org/lkml/2007/9/29/45 > > > > For the first one "usbcore duplicated sysfs filename" , I have submit > > a patch to fix it. > > > > For the "bInterfaceNumber" one, I have no idea, the same problem still > > exist in the latest 23-mm1 tree. > > I have tried several times to duplicate this, most recently under > 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. > > You may have to do your own debugging. Try adding printk statements to > usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you > can tell when they get called. I finally duplicated this on one of my machines here at boot time, with USB built into the kernel. I'll work tomorrow on tracking this down further... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Mon, 15 Oct 2007, Dave Young wrote: > On 10/14/07, Borislav Petkov <[EMAIL PROTECTED]> wrote: > > Hi, > > > > i get the following warning on yesterday's git tree (v2.6.23-2840-g752097c): > > > > Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename > > 'bInterfaceNumber' can not be created > > Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at fs/sysfs/dir.c:425 > > sysfs_add_one() > > Oct 14 09:07:15 zmei kernel: [ 49.368134] [] > > show_trace_log_lvl+0x1a/0x2f > > Oct 14 09:07:15 zmei kernel: [ 49.368220] [] > > show_trace+0x12/0x14 > > Oct 14 09:07:15 zmei kernel: [ 49.368300] [] > > dump_stack+0x16/0x18 > > Oct 14 09:07:15 zmei kernel: [ 49.368379] [] > > sysfs_add_one+0x57/0xbc > > Oct 14 09:07:15 zmei kernel: [ 49.368461] [] > > sysfs_add_file+0x49/0x71 > > Oct 14 09:07:15 zmei kernel: [ 49.368541] [] > > sysfs_create_group+0x86/0xe8 > > Oct 14 09:07:15 zmei kernel: [ 49.368621] [] > > usb_create_sysfs_intf_files+0x27/0x9b > > Oct 14 09:07:15 zmei kernel: [ 49.368704] [] > > usb_set_configuration+0x454/0x466 > > Oct 14 09:07:15 zmei kernel: [ 49.368787] [] > > generic_probe+0x53/0x94 > > Oct 14 09:07:15 zmei kernel: [ 49.368867] [] > > usb_probe_device+0x35/0x3b > > Oct 14 09:07:15 zmei kernel: [ 49.368947] [] > > driver_probe_device+0xcb/0x14f > > Oct 14 09:07:15 zmei kernel: [ 49.369039] [] > > __device_attach+0x8/0xa > > Oct 14 09:07:15 zmei kernel: [ 49.369119] [] > > bus_for_each_drv+0x3b/0x63 > > Oct 14 09:07:15 zmei kernel: [ 49.369199] [] > > device_attach+0x70/0x85 > > Oct 14 09:07:15 zmei kernel: [ 49.369279] [] > > bus_attach_device+0x29/0x77 > > Oct 14 09:07:15 zmei kernel: [ 49.369359] [] > > device_add+0x28c/0x445 > > Oct 14 09:07:15 zmei kernel: [ 49.369439] [] > > usb_new_device+0x44/0x82 > > Oct 14 09:07:15 zmei kernel: [ 49.369519] [] > > hub_thread+0x666/0x9c2 > > Oct 14 09:07:15 zmei kernel: [ 49.369598] [] kthread+0x3b/0x62 > > Oct 14 09:07:15 zmei kernel: [ 49.369679] [] > > kernel_thread_helper+0x7/0x10 > > Oct 14 09:07:15 zmei kernel: [ 49.369759] === > > > > The usb hub in question is named 4-1:1.0 and it has an extension connected > > to it > > which is used to activate the 2 usb connectors at the side of the pc's > > monitor. > > Correct me if i'm wrong but from what i've understood so far from reading > > the code, > > i think, it adds the bInterfaceNumber-file after calling > > usb_create_sysfs_intf_files(intf). > > However, the currently active usbhost interface alternate setting is the > > only one active > > so the bInterfaceNumber exists already and therefore the warning, but this > > is > > just a guess since i'm not that fluent in the usb internals. > Hi, > I have encountered the same problem which was reported in > http://lkml.org/lkml/2007/9/29/45 > > For the first one "usbcore duplicated sysfs filename" , I have submit > a patch to fix it. > > For the "bInterfaceNumber" one, I have no idea, the same problem still > exist in the latest 23-mm1 tree. I have tried several times to duplicate this, most recently under 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. You may have to do your own debugging. Try adding printk statements to usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you can tell when they get called. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Mon, 15 Oct 2007, Dave Young wrote: On 10/14/07, Borislav Petkov [EMAIL PROTECTED] wrote: Hi, i get the following warning on yesterday's git tree (v2.6.23-2840-g752097c): Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename 'bInterfaceNumber' can not be created Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at fs/sysfs/dir.c:425 sysfs_add_one() Oct 14 09:07:15 zmei kernel: [ 49.368134] [c010527c] show_trace_log_lvl+0x1a/0x2f Oct 14 09:07:15 zmei kernel: [ 49.368220] [c0105da0] show_trace+0x12/0x14 Oct 14 09:07:15 zmei kernel: [ 49.368300] [c0105db8] dump_stack+0x16/0x18 Oct 14 09:07:15 zmei kernel: [ 49.368379] [c019f2ee] sysfs_add_one+0x57/0xbc Oct 14 09:07:15 zmei kernel: [ 49.368461] [c019edeb] sysfs_add_file+0x49/0x71 Oct 14 09:07:15 zmei kernel: [ 49.368541] [c01a05c2] sysfs_create_group+0x86/0xe8 Oct 14 09:07:15 zmei kernel: [ 49.368621] [c024f5da] usb_create_sysfs_intf_files+0x27/0x9b Oct 14 09:07:15 zmei kernel: [ 49.368704] [c024c28b] usb_set_configuration+0x454/0x466 Oct 14 09:07:15 zmei kernel: [ 49.368787] [c0252b97] generic_probe+0x53/0x94 Oct 14 09:07:15 zmei kernel: [ 49.368867] [c024d4f7] usb_probe_device+0x35/0x3b Oct 14 09:07:15 zmei kernel: [ 49.368947] [c022a46c] driver_probe_device+0xcb/0x14f Oct 14 09:07:15 zmei kernel: [ 49.369039] [c022a4f8] __device_attach+0x8/0xa Oct 14 09:07:15 zmei kernel: [ 49.369119] [c02298d5] bus_for_each_drv+0x3b/0x63 Oct 14 09:07:15 zmei kernel: [ 49.369199] [c022a589] device_attach+0x70/0x85 Oct 14 09:07:15 zmei kernel: [ 49.369279] [c022984c] bus_attach_device+0x29/0x77 Oct 14 09:07:15 zmei kernel: [ 49.369359] [c0228abc] device_add+0x28c/0x445 Oct 14 09:07:15 zmei kernel: [ 49.369439] [c0247c7a] usb_new_device+0x44/0x82 Oct 14 09:07:15 zmei kernel: [ 49.369519] [c02486ec] hub_thread+0x666/0x9c2 Oct 14 09:07:15 zmei kernel: [ 49.369598] [c01370e9] kthread+0x3b/0x62 Oct 14 09:07:15 zmei kernel: [ 49.369679] [c0104eaf] kernel_thread_helper+0x7/0x10 Oct 14 09:07:15 zmei kernel: [ 49.369759] === The usb hub in question is named 4-1:1.0 and it has an extension connected to it which is used to activate the 2 usb connectors at the side of the pc's monitor. Correct me if i'm wrong but from what i've understood so far from reading the code, i think, it adds the bInterfaceNumber-file after calling usb_create_sysfs_intf_files(intf). However, the currently active usbhost interface alternate setting is the only one active so the bInterfaceNumber exists already and therefore the warning, but this is just a guess since i'm not that fluent in the usb internals. Hi, I have encountered the same problem which was reported in http://lkml.org/lkml/2007/9/29/45 For the first one usbcore duplicated sysfs filename , I have submit a patch to fix it. For the bInterfaceNumber one, I have no idea, the same problem still exist in the latest 23-mm1 tree. I have tried several times to duplicate this, most recently under 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. You may have to do your own debugging. Try adding printk statements to usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you can tell when they get called. Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On Mon, Oct 15, 2007 at 02:38:25PM -0400, Alan Stern wrote: On Mon, 15 Oct 2007, Dave Young wrote: On 10/14/07, Borislav Petkov [EMAIL PROTECTED] wrote: Hi, i get the following warning on yesterday's git tree (v2.6.23-2840-g752097c): Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename 'bInterfaceNumber' can not be created Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at fs/sysfs/dir.c:425 sysfs_add_one() Oct 14 09:07:15 zmei kernel: [ 49.368134] [c010527c] show_trace_log_lvl+0x1a/0x2f Oct 14 09:07:15 zmei kernel: [ 49.368220] [c0105da0] show_trace+0x12/0x14 Oct 14 09:07:15 zmei kernel: [ 49.368300] [c0105db8] dump_stack+0x16/0x18 Oct 14 09:07:15 zmei kernel: [ 49.368379] [c019f2ee] sysfs_add_one+0x57/0xbc Oct 14 09:07:15 zmei kernel: [ 49.368461] [c019edeb] sysfs_add_file+0x49/0x71 Oct 14 09:07:15 zmei kernel: [ 49.368541] [c01a05c2] sysfs_create_group+0x86/0xe8 Oct 14 09:07:15 zmei kernel: [ 49.368621] [c024f5da] usb_create_sysfs_intf_files+0x27/0x9b Oct 14 09:07:15 zmei kernel: [ 49.368704] [c024c28b] usb_set_configuration+0x454/0x466 Oct 14 09:07:15 zmei kernel: [ 49.368787] [c0252b97] generic_probe+0x53/0x94 Oct 14 09:07:15 zmei kernel: [ 49.368867] [c024d4f7] usb_probe_device+0x35/0x3b Oct 14 09:07:15 zmei kernel: [ 49.368947] [c022a46c] driver_probe_device+0xcb/0x14f Oct 14 09:07:15 zmei kernel: [ 49.369039] [c022a4f8] __device_attach+0x8/0xa Oct 14 09:07:15 zmei kernel: [ 49.369119] [c02298d5] bus_for_each_drv+0x3b/0x63 Oct 14 09:07:15 zmei kernel: [ 49.369199] [c022a589] device_attach+0x70/0x85 Oct 14 09:07:15 zmei kernel: [ 49.369279] [c022984c] bus_attach_device+0x29/0x77 Oct 14 09:07:15 zmei kernel: [ 49.369359] [c0228abc] device_add+0x28c/0x445 Oct 14 09:07:15 zmei kernel: [ 49.369439] [c0247c7a] usb_new_device+0x44/0x82 Oct 14 09:07:15 zmei kernel: [ 49.369519] [c02486ec] hub_thread+0x666/0x9c2 Oct 14 09:07:15 zmei kernel: [ 49.369598] [c01370e9] kthread+0x3b/0x62 Oct 14 09:07:15 zmei kernel: [ 49.369679] [c0104eaf] kernel_thread_helper+0x7/0x10 Oct 14 09:07:15 zmei kernel: [ 49.369759] === The usb hub in question is named 4-1:1.0 and it has an extension connected to it which is used to activate the 2 usb connectors at the side of the pc's monitor. Correct me if i'm wrong but from what i've understood so far from reading the code, i think, it adds the bInterfaceNumber-file after calling usb_create_sysfs_intf_files(intf). However, the currently active usbhost interface alternate setting is the only one active so the bInterfaceNumber exists already and therefore the warning, but this is just a guess since i'm not that fluent in the usb internals. Hi, I have encountered the same problem which was reported in http://lkml.org/lkml/2007/9/29/45 For the first one usbcore duplicated sysfs filename , I have submit a patch to fix it. For the bInterfaceNumber one, I have no idea, the same problem still exist in the latest 23-mm1 tree. I have tried several times to duplicate this, most recently under 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. You may have to do your own debugging. Try adding printk statements to usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you can tell when they get called. I finally duplicated this on one of my machines here at boot time, with USB built into the kernel. I'll work tomorrow on tracking this down further... thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'
On 10/16/07, Greg KH [EMAIL PROTECTED] wrote: On Mon, Oct 15, 2007 at 02:38:25PM -0400, Alan Stern wrote: On Mon, 15 Oct 2007, Dave Young wrote: On 10/14/07, Borislav Petkov [EMAIL PROTECTED] wrote: Hi, i get the following warning on yesterday's git tree (v2.6.23-2840-g752097c): Oct 14 09:07:15 zmei kernel: [ 49.368030] sysfs: duplicate filename 'bInterfaceNumber' can not be created Oct 14 09:07:15 zmei kernel: [ 49.368086] WARNING: at fs/sysfs/dir.c:425 sysfs_add_one() Oct 14 09:07:15 zmei kernel: [ 49.368134] [c010527c] show_trace_log_lvl+0x1a/0x2f Oct 14 09:07:15 zmei kernel: [ 49.368220] [c0105da0] show_trace+0x12/0x14 Oct 14 09:07:15 zmei kernel: [ 49.368300] [c0105db8] dump_stack+0x16/0x18 Oct 14 09:07:15 zmei kernel: [ 49.368379] [c019f2ee] sysfs_add_one+0x57/0xbc Oct 14 09:07:15 zmei kernel: [ 49.368461] [c019edeb] sysfs_add_file+0x49/0x71 Oct 14 09:07:15 zmei kernel: [ 49.368541] [c01a05c2] sysfs_create_group+0x86/0xe8 Oct 14 09:07:15 zmei kernel: [ 49.368621] [c024f5da] usb_create_sysfs_intf_files+0x27/0x9b Oct 14 09:07:15 zmei kernel: [ 49.368704] [c024c28b] usb_set_configuration+0x454/0x466 Oct 14 09:07:15 zmei kernel: [ 49.368787] [c0252b97] generic_probe+0x53/0x94 Oct 14 09:07:15 zmei kernel: [ 49.368867] [c024d4f7] usb_probe_device+0x35/0x3b Oct 14 09:07:15 zmei kernel: [ 49.368947] [c022a46c] driver_probe_device+0xcb/0x14f Oct 14 09:07:15 zmei kernel: [ 49.369039] [c022a4f8] __device_attach+0x8/0xa Oct 14 09:07:15 zmei kernel: [ 49.369119] [c02298d5] bus_for_each_drv+0x3b/0x63 Oct 14 09:07:15 zmei kernel: [ 49.369199] [c022a589] device_attach+0x70/0x85 Oct 14 09:07:15 zmei kernel: [ 49.369279] [c022984c] bus_attach_device+0x29/0x77 Oct 14 09:07:15 zmei kernel: [ 49.369359] [c0228abc] device_add+0x28c/0x445 Oct 14 09:07:15 zmei kernel: [ 49.369439] [c0247c7a] usb_new_device+0x44/0x82 Oct 14 09:07:15 zmei kernel: [ 49.369519] [c02486ec] hub_thread+0x666/0x9c2 Oct 14 09:07:15 zmei kernel: [ 49.369598] [c01370e9] kthread+0x3b/0x62 Oct 14 09:07:15 zmei kernel: [ 49.369679] [c0104eaf] kernel_thread_helper+0x7/0x10 Oct 14 09:07:15 zmei kernel: [ 49.369759] === The usb hub in question is named 4-1:1.0 and it has an extension connected to it which is used to activate the 2 usb connectors at the side of the pc's monitor. Correct me if i'm wrong but from what i've understood so far from reading the code, i think, it adds the bInterfaceNumber-file after calling usb_create_sysfs_intf_files(intf). However, the currently active usbhost interface alternate setting is the only one active so the bInterfaceNumber exists already and therefore the warning, but this is just a guess since i'm not that fluent in the usb internals. Hi, I have encountered the same problem which was reported in http://lkml.org/lkml/2007/9/29/45 For the first one usbcore duplicated sysfs filename , I have submit a patch to fix it. For the bInterfaceNumber one, I have no idea, the same problem still exist in the latest 23-mm1 tree. I have tried several times to duplicate this, most recently under 2.6.23-mm1. But nothing goes wrong; the error messages don't appear. You may have to do your own debugging. Try adding printk statements to usb_create_sysfs_intf_files() and usb_remove_sysfs_intf_files() so you can tell when they get called. I finally duplicated this on one of my machines here at boot time, with USB built into the kernel. I'll work tomorrow on tracking this down further... Hi, I add some printk messages, dump_stack and some others, here is the dmesg dump with debug info(lines begin with hidave): Linux version 2.6.23-mm1 ([EMAIL PROTECTED]) (gcc version 3.4.6) #4 SMP PREEMPT Tue Oct 16 11:14:10 CST 2007 BIOS-provided physical RAM map: BIOS-e820: - 000a (usable) BIOS-e820: 000f - 0010 (reserved) BIOS-e820: 0010 - 3fe88c00 (usable) BIOS-e820: 3fe88c00 - 3fe8ac00 (ACPI NVS) BIOS-e820: 3fe8ac00 - 3fe8cc00 (ACPI data) BIOS-e820: 3fe8cc00 - 4000 (reserved) BIOS-e820: f000 - f400 (reserved) BIOS-e820: fec0 - fed00400 (reserved) BIOS-e820: fed2 - feda (reserved) BIOS-e820: fee0 - fef0 (reserved) BIOS-e820: ffb0 - 0001 (reserved) 126MB HIGHMEM available. 896MB LOWMEM available. found SMP MP-table at 000fe710 Entering add_active_range(0, 0, 261768) 0 entries of 256 used sizeof(struct page) = 32 Zone PFN ranges: DMA 0 - 4096 Normal 4096 -
