2010-04-23 10:44:33

by Eric Lescouet

[permalink] [raw]
Subject: USBIP (staging) driver's dependency on drivers/usb/core internal headers

Hi,

The USBIP modules (and some others) have a direct dependency on the following headers:
- drivers/usb/core/hdc.h (struct usb_hcd and various routines)
- drivers/usb/core/hub.h (couple of constants)
Because those headers are not part of include/, it makes it quite difficult
to compile them as standalone modules (e.g.: against pre-compiled kernel).

hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.

What do you think?

Thanks,
Eric.


2010-04-23 15:11:12

by Greg KH

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, Apr 23, 2010 at 12:44:11PM +0200, Eric Lescouet wrote:
> Hi,
>
> The USBIP modules (and some others) have a direct dependency on the following headers:
> - drivers/usb/core/hdc.h (struct usb_hcd and various routines)
> - drivers/usb/core/hub.h (couple of constants)
> Because those headers are not part of include/, it makes it quite difficult
> to compile them as standalone modules (e.g.: against pre-compiled kernel).

Then don't do that :)

> hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.
>
> What do you think?

I think the code needs to be cleaned up and merged to the main part of
the kernel tree, then this wouldn't be an issue, right?

Also, please cc: the [email protected] list for USB related
things in the future.

thanks,

greg k-h

2010-04-23 15:52:48

by Alan Stern

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, 23 Apr 2010, Greg KH wrote:

> On Fri, Apr 23, 2010 at 12:44:11PM +0200, Eric Lescouet wrote:
> > Hi,
> >
> > The USBIP modules (and some others) have a direct dependency on the following headers:
> > - drivers/usb/core/hdc.h (struct usb_hcd and various routines)
> > - drivers/usb/core/hub.h (couple of constants)
> > Because those headers are not part of include/, it makes it quite difficult
> > to compile them as standalone modules (e.g.: against pre-compiled kernel).
>
> Then don't do that :)
>
> > hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.
> >
> > What do you think?
>
> I think the code needs to be cleaned up and merged to the main part of
> the kernel tree, then this wouldn't be an issue, right?

I think moving hub.h into include/linux/usb would be a good thing.
However, if this is done then the file contents should be rearranged
slightly:

Everything from the "Hub request types" comment through the
various HUB_TTTT_* definitions (except the line declaring
struct usb_device) should be extracted into a separate file.
This new file could be called ch11.h (because it contains
constants and structures from Chapter 11 of the USB 2.0 spec,
just as ch9.h contains information from Chapter 9) or it
could keep the name hub.h. It could even be exported as
a userspace header file.

The remainder (not very much) should be merged into hcd.h.

Once that's done, moving hcd.h into include/linux/usb would be a
reasonable thing to do as well. It is shared between usbcore and the
HCDs, so it really does belong in a more public location.

Alan Stern

2010-04-23 16:19:08

by Greg KH

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, Apr 23, 2010 at 11:52:26AM -0400, Alan Stern wrote:
> On Fri, 23 Apr 2010, Greg KH wrote:
>
> > On Fri, Apr 23, 2010 at 12:44:11PM +0200, Eric Lescouet wrote:
> > > Hi,
> > >
> > > The USBIP modules (and some others) have a direct dependency on the following headers:
> > > - drivers/usb/core/hdc.h (struct usb_hcd and various routines)
> > > - drivers/usb/core/hub.h (couple of constants)
> > > Because those headers are not part of include/, it makes it quite difficult
> > > to compile them as standalone modules (e.g.: against pre-compiled kernel).
> >
> > Then don't do that :)
> >
> > > hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.
> > >
> > > What do you think?
> >
> > I think the code needs to be cleaned up and merged to the main part of
> > the kernel tree, then this wouldn't be an issue, right?
>
> I think moving hub.h into include/linux/usb would be a good thing.
> However, if this is done then the file contents should be rearranged
> slightly:
>
> Everything from the "Hub request types" comment through the
> various HUB_TTTT_* definitions (except the line declaring
> struct usb_device) should be extracted into a separate file.
> This new file could be called ch11.h (because it contains
> constants and structures from Chapter 11 of the USB 2.0 spec,
> just as ch9.h contains information from Chapter 9) or it
> could keep the name hub.h. It could even be exported as
> a userspace header file.
>
> The remainder (not very much) should be merged into hcd.h.
>
> Once that's done, moving hcd.h into include/linux/usb would be a
> reasonable thing to do as well. It is shared between usbcore and the
> HCDs, so it really does belong in a more public location.

Ok, fair enough. Patches are always welcome :)

thanks,

greg k-h

2010-04-23 16:31:58

by Eric Lescouet

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

Greg KH wrote:
> On Fri, Apr 23, 2010 at 11:52:26AM -0400, Alan Stern wrote:
>> On Fri, 23 Apr 2010, Greg KH wrote:
>>
>>> On Fri, Apr 23, 2010 at 12:44:11PM +0200, Eric Lescouet wrote:
>>>> Hi,
>>>>
>>>> The USBIP modules (and some others) have a direct dependency on the following headers:
>>>> - drivers/usb/core/hdc.h (struct usb_hcd and various routines)
>>>> - drivers/usb/core/hub.h (couple of constants)
>>>> Because those headers are not part of include/, it makes it quite difficult
>>>> to compile them as standalone modules (e.g.: against pre-compiled kernel).
>>> Then don't do that :)
>>>
>>>> hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.
>>>>
>>>> What do you think?
>>> I think the code needs to be cleaned up and merged to the main part of
>>> the kernel tree, then this wouldn't be an issue, right?
>> I think moving hub.h into include/linux/usb would be a good thing.
>> However, if this is done then the file contents should be rearranged
>> slightly:
>>
>> Everything from the "Hub request types" comment through the
>> various HUB_TTTT_* definitions (except the line declaring
>> struct usb_device) should be extracted into a separate file.
>> This new file could be called ch11.h (because it contains
>> constants and structures from Chapter 11 of the USB 2.0 spec,
>> just as ch9.h contains information from Chapter 9) or it
>> could keep the name hub.h. It could even be exported as
>> a userspace header file.
>>
>> The remainder (not very much) should be merged into hcd.h.
>>
>> Once that's done, moving hcd.h into include/linux/usb would be a
>> reasonable thing to do as well. It is shared between usbcore and the
>> HCDs, so it really does belong in a more public location.
>
> Ok, fair enough. Patches are always welcome :)
>
> thanks,
>
> greg k-h

If I understand correctly, splitting hub.h into hub.h + ch11.h is
independent of the move. At the end the 2 (or 3 files) would end up
into include/linux/usb/.
correct?

I've tried to move hub.h and hcd.h and to compile the kernel, already.
Changing the path of the #include directives in ~40 files did the trick.

Would you like 2 patches (move and then split) or only 1?

Thanks,
Eric.

2010-04-23 16:38:18

by Greg KH

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, Apr 23, 2010 at 06:31:28PM +0200, Eric Lescouet wrote:
> Greg KH wrote:
> >On Fri, Apr 23, 2010 at 11:52:26AM -0400, Alan Stern wrote:
> >>On Fri, 23 Apr 2010, Greg KH wrote:
> >>
> >>>On Fri, Apr 23, 2010 at 12:44:11PM +0200, Eric Lescouet wrote:
> >>>>Hi,
> >>>>
> >>>>The USBIP modules (and some others) have a direct dependency on the following headers:
> >>>>- drivers/usb/core/hdc.h (struct usb_hcd and various routines)
> >>>>- drivers/usb/core/hub.h (couple of constants)
> >>>>Because those headers are not part of include/, it makes it quite difficult
> >>>>to compile them as standalone modules (e.g.: against pre-compiled kernel).
> >>>Then don't do that :)
> >>>
> >>>>hdc.c and hub.c are relatively self-contained and could be moved to e.g.: include/linux/usb/.
> >>>>
> >>>>What do you think?
> >>>I think the code needs to be cleaned up and merged to the main part of
> >>>the kernel tree, then this wouldn't be an issue, right?
> >>I think moving hub.h into include/linux/usb would be a good
> >>thing. However, if this is done then the file contents should
> >>be rearranged slightly:
> >>
> >> Everything from the "Hub request types" comment through the
> >> various HUB_TTTT_* definitions (except the line declaring
> >> struct usb_device) should be extracted into a separate file.
> >> This new file could be called ch11.h (because it contains
> >> constants and structures from Chapter 11 of the USB 2.0 spec,
> >> just as ch9.h contains information from Chapter 9) or it
> >> could keep the name hub.h. It could even be exported as
> >> a userspace header file.
> >>
> >> The remainder (not very much) should be merged into hcd.h.
> >>
> >>Once that's done, moving hcd.h into include/linux/usb would be a
> >>reasonable thing to do as well. It is shared between usbcore
> >>and the HCDs, so it really does belong in a more public
> >>location.
> >
> >Ok, fair enough. Patches are always welcome :)
> >
> >thanks,
> >
> >greg k-h
>
> If I understand correctly, splitting hub.h into hub.h + ch11.h is
> independent of the move. At the end the 2 (or 3 files) would end up
> into include/linux/usb/.
> correct?
>
> I've tried to move hub.h and hcd.h and to compile the kernel, already.
> Changing the path of the #include directives in ~40 files did the trick.

40 files need to be changed? wow.

> Would you like 2 patches (move and then split) or only 1?

Move and then split is fine.

thanks,

greg k-h

2010-04-23 16:50:25

by Alan Stern

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, 23 Apr 2010, Eric Lescouet wrote:

> If I understand correctly, splitting hub.h into hub.h + ch11.h is
> independent of the move. At the end the 2 (or 3 files) would end up
> into include/linux/usb/.
> correct?

Two files. The first would be ch11.h (or maybe keep the name hub.h;
I'm not sure which is best), and the second would be hcd.h (including
the leftover parts from the original hub.h).

> I've tried to move hub.h and hcd.h and to compile the kernel, already.
> Changing the path of the #include directives in ~40 files did the trick.
>
> Would you like 2 patches (move and then split) or only 1?

If it weren't for the rename, I'd say do it in two patches. But if the
name is changed to ch11.h then those 40 files would have to be altered
twice.

Alan Stern

2010-04-23 17:52:10

by Eric Lescouet

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

Alan Stern wrote:
> On Fri, 23 Apr 2010, Eric Lescouet wrote:
>
>> If I understand correctly, splitting hub.h into hub.h + ch11.h is
>> independent of the move. At the end the 2 (or 3 files) would end up
>> into include/linux/usb/.
>> correct?
>
> Two files. The first would be ch11.h (or maybe keep the name hub.h;
> I'm not sure which is best), and the second would be hcd.h (including
> the leftover parts from the original hub.h).
>

OK. understood now ;-)

>> I've tried to move hub.h and hcd.h and to compile the kernel, already.
>> Changing the path of the #include directives in ~40 files did the trick.
>>
>> Would you like 2 patches (move and then split) or only 1?
>
> If it weren't for the rename, I'd say do it in two patches. But if the
> name is changed to ch11.h then those 40 files would have to be altered
> twice.
>
> Alan Stern
>

Well, most of the files include hcd.h (which includes hub.h),
only ~4 of them directly use hub.h.
So what's your vote? rename hub.h into ch11.h?

Eric.

2010-04-23 18:04:23

by Alan Stern

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Fri, 23 Apr 2010, Eric Lescouet wrote:

> Well, most of the files include hcd.h (which includes hub.h),
> only ~4 of them directly use hub.h.

Oh. Okay, that makes life easier. Probably some of the files which
include hub.h directly also include hcd.h.

> So what's your vote? rename hub.h into ch11.h?

Yeah, go ahead. I like the idea of the kernel containing information
about Chapter 11. (Although I haven't heard of anyone yet going
bankrupt as a result of using Linux...)

Alan Stern

2010-04-24 01:24:26

by Eric Lescouet

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On 04/23/2010 08:04 PM, Alan Stern wrote:
> On Fri, 23 Apr 2010, Eric Lescouet wrote:
>
>> Well, most of the files include hcd.h (which includes hub.h),
>> only ~4 of them directly use hub.h.
>
> Oh. Okay, that makes life easier. Probably some of the files which
> include hub.h directly also include hcd.h.
>
>> So what's your vote? rename hub.h into ch11.h?
>
> Yeah, go ahead. I like the idea of the kernel containing information
> about Chapter 11. (Although I haven't heard of anyone yet going
> bankrupt as a result of using Linux...)
>
> Alan Stern
>

It looks like couple of files in drivers/usb/core and usb/host have been
modified already in latest snapshot (2.6.34-rc5-git5).
Is it still ok to provide the patches against the mainline (2.6.34-rc5)
or should I do it against the snapshot (2.6.34-rc5-git5)?

Thanks,
Eric.

2010-04-24 15:16:46

by Greg KH

[permalink] [raw]
Subject: Re: USBIP (staging) driver's dependency on drivers/usb/core internal headers

On Sat, Apr 24, 2010 at 03:24:17AM +0200, Eric Lescouet wrote:
> On 04/23/2010 08:04 PM, Alan Stern wrote:
> > On Fri, 23 Apr 2010, Eric Lescouet wrote:
> >
> >> Well, most of the files include hcd.h (which includes hub.h),
> >> only ~4 of them directly use hub.h.
> >
> > Oh. Okay, that makes life easier. Probably some of the files which
> > include hub.h directly also include hcd.h.
> >
> >> So what's your vote? rename hub.h into ch11.h?
> >
> > Yeah, go ahead. I like the idea of the kernel containing information
> > about Chapter 11. (Although I haven't heard of anyone yet going
> > bankrupt as a result of using Linux...)
> >
> > Alan Stern
> >
>
> It looks like couple of files in drivers/usb/core and usb/host have been
> modified already in latest snapshot (2.6.34-rc5-git5).
> Is it still ok to provide the patches against the mainline (2.6.34-rc5)
> or should I do it against the snapshot (2.6.34-rc5-git5)?

The latest snapshot is best.

thanks,

greg k-h