Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-25 Thread Dave Young
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'

2007-10-25 Thread Greg KH
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'

2007-10-25 Thread Dave Young
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'

2007-10-25 Thread Greg KH
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
> > > +++ lin

Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-25 Thread Dave Young
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)
> > +{
> > + st

Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-25 Thread Greg KH
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 int

Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-25 Thread Dave Young
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 

Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-18 Thread Greg KH
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'

2007-10-17 Thread Dave Young
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'

2007-10-17 Thread Alan Stern
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'

2007-10-16 Thread Dave Young
>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'

2007-10-16 Thread Matthew Dharm
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'

2007-10-16 Thread Alan Stern
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'

2007-10-16 Thread Matthew Dharm
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'

2007-10-16 Thread Alan Stern
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'

2007-10-15 Thread Dave Young
>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
En

Re: [linux-usb-devel] usb+sysfs: duplicate filename 'bInterfaceNumber'

2007-10-15 Thread Greg KH
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'

2007-10-15 Thread Alan Stern
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/