Disable bus's drivers_autoprobe before rootfs has mounted

[Peter Chen]

Hi Greg,

Currently, we can't disable auto probe function during booting
if both device and device driver register code are built in due
to .drivers_autoprobe is a private value for bus core and this
value can only be changed by sys entry.

It causes we can't implement feature that the user can choose
manual binding and auto binding through module parameters.
Eg, the default binding is automatic, but the user can override
it by module parameter.

Let's take USB peripheral as an example, there is a device for
udc, and a device driver for usb gadget driver, at default, we want
the device to be bound to driver automatically, this is what
we have done now. But if there are more than one udcs and gadget
drivers (eg one B port for mass storage, another B port for usb ethernet),
the user may want to have specific binding (eg, udc-0 -> mass storage,
udc-1 -> usb ethernet), so the binding will be established
after rootfs has mounted. (This feature is implementing)

From what I read code, we can't implement above feature, but I may
be wrong, if you have some solutions, give me some hints please.
If there is no solution for above feature, do we agree with exporting
.drivers_autoprobe for bus driver or something similar?

--

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Greg KH]

On Wed, Jun 11, 2014 at 10:14:40AM +0800, Peter Chen wrote:
> Hi Greg,
>
> Currently, we can't disable auto probe function during booting
> if both device and device driver register code are built in due
> to .drivers_autoprobe is a private value for bus core and this
> value can only be changed by sys entry.

Then don't build them into the kernel :)

> It causes we can't implement feature that the user can choose
> manual binding and auto binding through module parameters.

Wait, you just asked about building the stuff into the kernel, not a
module.

> Eg, the default binding is automatic, but the user can override
> it by module parameter.

Do we do that for any other "bus" anywhere?

> Let's take USB peripheral as an example, there is a device for
> udc, and a device driver for usb gadget driver, at default, we want
> the device to be bound to driver automatically, this is what
> we have done now. But if there are more than one udcs and gadget
> drivers (eg one B port for mass storage, another B port for usb ethernet),
> the user may want to have specific binding (eg, udc-0 -> mass storage,
> udc-1 -> usb ethernet), so the binding will be established
> after rootfs has mounted. (This feature is implementing)

Then there better be a way to describe this on the kernel command line
(i.e. module paramaters), right? Which is a total mess, why not just
not bind anything in this case and let the user pick what they want?

> From what I read code, we can't implement above feature, but I may
> be wrong, if you have some solutions, give me some hints please.
> If there is no solution for above feature, do we agree with exporting
> .drivers_autoprobe for bus driver or something similar?

I don't understand what you mean by this, care to show me with code?

thanks,

greg k-h

[Felipe Balbi]

Hi,

On Tue, Jun 10, 2014 at 09:10:00PM -0700, Greg KH wrote:
> > Let's take USB peripheral as an example, there is a device for
> > udc, and a device driver for usb gadget driver, at default, we want
> > the device to be bound to driver automatically, this is what
> > we have done now. But if there are more than one udcs and gadget
> > drivers (eg one B port for mass storage, another B port for usb ethernet),
> > the user may want to have specific binding (eg, udc-0 -> mass storage,
> > udc-1 -> usb ethernet), so the binding will be established
> > after rootfs has mounted. (This feature is implementing)
>
> Then there better be a way to describe this on the kernel command line
> (i.e. module paramaters), right? Which is a total mess, why not just
> not bind anything in this case and let the user pick what they want?

you can also blacklist all gadget drivers and manually probe them or -
get this - you can refrain from using gadget drivers and use libusbg to
build the gadget drivers out of raw usb functions, then bind them to the
UDC of your liking.

--
balbi

[Peter Chen]

On Tue, Jun 10, 2014 at 09:10:00PM -0700, Greg KH wrote:

> On Wed, Jun 11, 2014 at 10:14:40AM +0800, Peter Chen wrote:
> > Hi Greg,
> >
> > Currently, we can't disable auto probe function during booting
> > if both device and device driver register code are built in due
> > to .drivers_autoprobe is a private value for bus core and this
> > value can only be changed by sys entry.
>
> Then don't build them into the kernel :)
>
> > It causes we can't implement feature that the user can choose
> > manual binding and auto binding through module parameters.
>
> Wait, you just asked about building the stuff into the kernel, not a
> module.

Yes, build the code into the kernel.

>
> > Eg, the default binding is automatic, but the user can override
> > it by module parameter.
>
> Do we do that for any other "bus" anywhere?

I don't know.

>
> > Let's take USB peripheral as an example, there is a device for
> > udc, and a device driver for usb gadget driver, at default, we want
> > the device to be bound to driver automatically, this is what
> > we have done now. But if there are more than one udcs and gadget
> > drivers (eg one B port for mass storage, another B port for usb ethernet),
> > the user may want to have specific binding (eg, udc-0 -> mass storage,
> > udc-1 -> usb ethernet), so the binding will be established
> > after rootfs has mounted. (This feature is implementing)
>
> Then there better be a way to describe this on the kernel command line
> (i.e. module paramaters), right? Which is a total mess, why not just
> not bind anything in this case and let the user pick what they want?

If the user is used to do nothing at rootfs for current or earlier kernel,
Is it ok we change the driver's behaviour and a sys entry is mandatory
for user?

>
> > From what I read code, we can't implement above feature, but I may
> > be wrong, if you have some solutions, give me some hints please.
> > If there is no solution for above feature, do we agree with exporting
> > .drivers_autoprobe for bus driver or something similar?
>
> I don't understand what you mean by this, care to show me with code?

I mean the individual bus driver can't change bus->p->drivers_autoprobe?
bus->p->drivers_autoprobe is handled at drivers/base/bus.c.

If the individual bus driver can change bus->p->drivers_autoprobe, we
can disable autoprobe (auto-binding) during booting.

--

Best Regards,
Peter Chen

[Peter Chen]

I am just worried if we change the behaviour of using gadget driver,
can it be accepted by user? If you think it can be accepted if we can
have some docs, we can implement manually binding for gadget driver
from now on.

--

Best Regards,
Peter Chen

[Greg KH]

We can't break existing systems, so I don't understand the issue here.
Just don't configure your kernel for a system you don't have / want,
right?

> > > From what I read code, we can't implement above feature, but I may
> > > be wrong, if you have some solutions, give me some hints please.
> > > If there is no solution for above feature, do we agree with exporting
> > > .drivers_autoprobe for bus driver or something similar?
> >
> > I don't understand what you mean by this, care to show me with code?
>
> I mean the individual bus driver can't change bus->p->drivers_autoprobe?
> bus->p->drivers_autoprobe is handled at drivers/base/bus.c.
>
> If the individual bus driver can change bus->p->drivers_autoprobe, we
> can disable autoprobe (auto-binding) during booting.

No, that's a core only thing.

greg k-h

[Alan Stern]

On Wed, 11 Jun 2014, Greg KH wrote:

> We can't break existing systems, so I don't understand the issue here.
> Just don't configure your kernel for a system you don't have / want,
> right?
>
> > > > From what I read code, we can't implement above feature, but I may
> > > > be wrong, if you have some solutions, give me some hints please.
> > > > If there is no solution for above feature, do we agree with exporting
> > > > .drivers_autoprobe for bus driver or something similar?
> > >
> > > I don't understand what you mean by this, care to show me with code?
> >
> > I mean the individual bus driver can't change bus->p->drivers_autoprobe?
> > bus->p->drivers_autoprobe is handled at drivers/base/bus.c.
> >
> > If the individual bus driver can change bus->p->drivers_autoprobe, we
> > can disable autoprobe (auto-binding) during booting.
>
> No, that's a core only thing.

Peter, correct me if this is wrong. It sounds like you want to have a
way for the user to control which gadget driver gets bound to which UDC
driver when everything is compiled into the kernel, nothing is built
as a separate module. Is that the basic idea?

This shouldn't be a problem if you use configfs or libusbg.

Alan Stern

[Felipe Balbi]

On Wed, Jun 11, 2014 at 11:29:57AM +0800, Peter Chen wrote:
> On Tue, Jun 10, 2014 at 11:35:07PM -0500, Felipe Balbi wrote:
> > Hi,
> >
> > On Tue, Jun 10, 2014 at 09:10:00PM -0700, Greg KH wrote:
> > > > Let's take USB peripheral as an example, there is a device for
> > > > udc, and a device driver for usb gadget driver, at default, we want
> > > > the device to be bound to driver automatically, this is what
> > > > we have done now. But if there are more than one udcs and gadget
> > > > drivers (eg one B port for mass storage, another B port for usb ethernet),
> > > > the user may want to have specific binding (eg, udc-0 -> mass storage,
> > > > udc-1 -> usb ethernet), so the binding will be established
> > > > after rootfs has mounted. (This feature is implementing)
> > >
> > > Then there better be a way to describe this on the kernel command line
> > > (i.e. module paramaters), right? Which is a total mess, why not just
> > > not bind anything in this case and let the user pick what they want?
> >
> > you can also blacklist all gadget drivers and manually probe them or -
> > get this - you can refrain from using gadget drivers and use libusbg to
> > build the gadget drivers out of raw usb functions, then bind them to the
> > UDC of your liking.
> >
>
> I am just worried if we change the behaviour of using gadget driver,
> can it be accepted by user? If you think it can be accepted if we can
> have some docs, we can implement manually binding for gadget driver
> from now on.

user shouldn't have to deal with direct module insertion/removal (unless
he's a developer and actually *wants* to do that). Docs are already in
tree. The entire configfs interface has been documented, it's based on
those documents that Matt started writing libusbg.

--
balbi

[Peter Chen]

On Wed, Jun 11, 2014 at 03:27:13PM -0400, Alan Stern wrote:
> On Wed, 11 Jun 2014, Greg KH wrote:
>
> > We can't break existing systems, so I don't understand the issue here.
> > Just don't configure your kernel for a system you don't have / want,
> > right?
> >
> > > > > From what I read code, we can't implement above feature, but I may
> > > > > be wrong, if you have some solutions, give me some hints please.
> > > > > If there is no solution for above feature, do we agree with exporting
> > > > > .drivers_autoprobe for bus driver or something similar?
> > > >
> > > > I don't understand what you mean by this, care to show me with code?
> > >
> > > I mean the individual bus driver can't change bus->p->drivers_autoprobe?
> > > bus->p->drivers_autoprobe is handled at drivers/base/bus.c.
> > >
> > > If the individual bus driver can change bus->p->drivers_autoprobe, we
> > > can disable autoprobe (auto-binding) during booting.
> >
> > No, that's a core only thing.
>
> Peter, correct me if this is wrong. It sounds like you want to have a
> way for the user to control which gadget driver gets bound to which UDC
> driver when everything is compiled into the kernel, nothing is built
> as a separate module. Is that the basic idea?

Yes, I know it can be done by gadget-configfs, but how about the user
chooses other gadgets, eg: g_webcam, g_audio?

I forget to introduce the background of this topic, I have this issue
when I implement gadget bus patch set.
(http://www.spinics.net/lists/linux-usb/msg107797.html)
The current behaviour for other gadgets is auto-binding, so I want
to keep auto-binding at my gadget bus implementation, but manual-binding
is also a feature we need to support for other gadgets, so I
want auto-binding is the default binding way, and the user can switch
the two binding ways, eg, module parameters or sys entry.

But if both udc driver and gadget driver are built in, the user can't
let manual-binding work during the boot since the device-model will
do probe, and do auto-binding.

My v1 patch set [3/4] do a tricky way to work around it, I would like to
know if it can be supported by device model framework?

>
> This shouldn't be a problem if you use configfs or libusbg.
>
> Alan Stern
>

--

Best Regards,
Peter Chen

[Peter Chen]

Yes, gadget-configfs is a good direction.

I would like to know your plan for other gadget drivers (g_mass_storage,
g_webcam, etc)

All functions will be supported by configfs in future, and current
driver will be deleted?
- If yes, how to cover the user who still use the old file system?
- If no, which binding way for udc and gadget driver will be used?
Current binding way for non-configfs gadgets is auto-binding, if we
don't want break user's current system, we need to keep auto-binding
as the default binding way. With more, we can support manual
binding with udc core.

I am implementing gadget bus[1], it may not be benefit for gadget-configfs,
but is benefit for other gadgets, eg, support loading gadget driver
before udc driver, manual-binding between udc and gadget drivers, etc.
So, I would like to know if you agree to keep auto-binding for
current non-configfs gadget driver as default way (in that way, we can keep
current system with no change), and manual-binding can be used
by user with module parameters and /sys entry.

[1] http://www.spinics.net/lists/linux-usb/msg109015.html

[Alan Stern]

On Thu, 12 Jun 2014, Peter Chen wrote:

> > Peter, correct me if this is wrong. It sounds like you want to have a
> > way for the user to control which gadget driver gets bound to which UDC
> > driver when everything is compiled into the kernel, nothing is built
> > as a separate module. Is that the basic idea?
>
> Yes, I know it can be done by gadget-configfs, but how about the user
> chooses other gadgets, eg: g_webcam, g_audio?
>
> I forget to introduce the background of this topic, I have this issue
> when I implement gadget bus patch set.
> (http://www.spinics.net/lists/linux-usb/msg107797.html)
> The current behaviour for other gadgets is auto-binding, so I want
> to keep auto-binding at my gadget bus implementation, but manual-binding
> is also a feature we need to support for other gadgets, so I
> want auto-binding is the default binding way, and the user can switch
> the two binding ways, eg, module parameters or sys entry.
>
> But if both udc driver and gadget driver are built in, the user can't
> let manual-binding work during the boot since the device-model will
> do probe, and do auto-binding.
>
> My v1 patch set [3/4] do a tricky way to work around it, I would like to
> know if it can be supported by device model framework?

I think we can keep everything a lot simpler. Let's agree that the old
non-composite gadget drivers (like g_ether, g_mass_storage, and so on)
should only be used one at a time. That is, there should never be more
than one of them compiled into the kernel or loaded as a module, and
they shouldn't be used if there is more than one unbound UDC.

If a user wants to work with more than one UDC or more than one gadget
driver then he should use function drivers with the configfs interface
(or functionfs or gadgetfs or whatever).

Does that seem reasonable? It doesn't solve your problem so much as
declare it an unsupported case, but I don't think there is any
reasonable way to solve the problem without using something like
configfs.

Alan Stern

[Felipe Balbi]

they can be built dynamically too. We only provided a version of
g_mass_storage in order to avoid regressions. We can't simply remove
that driver from the kernel.

> All functions will be supported by configfs in future, and current
> driver will be deleted?

I don't think we will be able to delete legacy drivers, but they're all
supported through configfs. I guess only webcam is missing.

> - If yes, how to cover the user who still use the old file system?
> - If no, which binding way for udc and gadget driver will be used?

going forward, we want to stick with configfs.

--
balbi

[Peter Chen]

> > > > I am just worried if we change the behaviour of using gadget
> > > > driver, can it be accepted by user? If you think it can be
> > > > accepted if we can have some docs, we can implement manually
> > > > binding for gadget driver from now on.
> > >
> > > user shouldn't have to deal with direct module insertion/removal
> > > (unless he's a developer and actually *wants* to do that). Docs are
> > > already in tree. The entire configfs interface has been documented,
> > > it's based on those documents that Matt started writing libusbg.
> > >
> > > --
> > > balbi
> >
> > Yes, gadget-configfs is a good direction.
> >
> > I would like to know your plan for other gadget drivers
> > (g_mass_storage, g_webcam, etc)
>
> they can be built dynamically too. We only provided a version of
> g_mass_storage in order to avoid regressions. We can't simply remove that
> driver from the kernel.
>
> > All functions will be supported by configfs in future, and current
> > driver will be deleted?
>
> I don't think we will be able to delete legacy drivers, but they're all
> supported through configfs. I guess only webcam is missing.
>
> > - If yes, how to cover the user who still use the old file system?
> > - If no, which binding way for udc and gadget driver will be used?
>
> going forward, we want to stick with configfs.
>

ok, then, what's the plan for current bug for legacy gadget driver (eg, we
can't load gadget driver before udc is ready)? I know this bug is existed
from 3.10, we may need to at least 1 year for switch to configfs, it
includes lots of user habit problems. The user will complaint this problem
for more than 2 years.

If you think we should fix this bug before we totally switch to configfs, I will
continue to enhancement my gadget bus implementation,
and let it support configfs and legacy gadget driver well.

Peter

[Peter Chen]

OK, we can keep our g_xxx gadget driver just support the basic feature. But
the bug that causes gadget driver load fail due to udc is probed deferral should
be fixed, do you think so, we can't wait until configfs has total been ready.

Peter

[Alan Stern]

On Fri, 13 Jun 2014, Peter Chen wrote:

> OK, we can keep our g_xxx gadget driver just support the basic feature. But
> the bug that causes gadget driver load fail due to udc is probed deferral should
> be fixed, do you think so, we can't wait until configfs has total been ready.

That problem has always existed. There never has been a time when a
gadget driver could be loaded before the UDC driver was ready. Does it
really need to be fixed now?

If you do want to fix the problem, there's a much easier way than what
you posted. See below.

Alan Stern



Index: usb-3.15/include/linux/usb/gadget.h
===================================================================
--- usb-3.15.orig/include/linux/usb/gadget.h
+++ usb-3.15/include/linux/usb/gadget.h
@@ -821,6 +821,7 @@ static inline int usb_gadget_disconnect(
* @suspend: Invoked on USB suspend. May be called in_interrupt.
* @resume: Invoked on USB resume. May be called in_interrupt.
* @driver: Driver model state for this driver.
+ * @probe_list: List of drivers waiting to be probed.
*
* Devices are disabled till a gadget driver successfully bind()s, which
* means the driver will handle setup() requests needed to enumerate (and
@@ -881,6 +882,7 @@ struct usb_gadget_driver {

/* FIXME support safe rmmod */
struct device_driver driver;
+ struct list_head probe_list;
};


Index: usb-3.15/drivers/usb/gadget/udc-core.c
===================================================================
--- usb-3.15.orig/drivers/usb/gadget/udc-core.c
+++ usb-3.15/drivers/usb/gadget/udc-core.c
@@ -47,8 +47,12 @@ struct usb_udc {

static struct class *udc_class;
static LIST_HEAD(udc_list);
+static LIST_HEAD(pending_drivers);
static DEFINE_MUTEX(udc_lock);

+static int udc_bind_to_driver(struct usb_udc *udc,
+ struct usb_gadget_driver *driver);
+
/* ------------------------------------------------------------------------- */

#ifdef CONFIG_HAS_DMA
@@ -242,6 +246,15 @@ int usb_add_gadget_udc_release(struct de

usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);

+ while (!list_empty(&pending_drivers)) {
+ struct usb_gadget_driver *driver;
+
+ driver = list_first_entry(&pending_drivers,
+ struct usb_gadget_driver, probe_list);
+ if (udc_bind_to_driver(udc, driver) == 0)
+ break;
+ }
+
mutex_unlock(&udc_lock);

return 0;
@@ -338,6 +351,7 @@ static int udc_bind_to_driver(struct usb

dev_dbg(&udc->dev, "registering UDC driver [%s]\n",
driver->function);
+ list_del_init(&driver->probe_list);

udc->driver = driver;
udc->dev.driver = &driver->driver;
@@ -400,6 +414,7 @@ int usb_gadget_probe_driver(struct usb_g
return -EINVAL;

mutex_lock(&udc_lock);
+ INIT_LIST_HEAD(&driver->probe_list);
list_for_each_entry(udc, &udc_list, list) {
/* For now we take the first one */
if (!udc->driver)
@@ -407,8 +422,9 @@ int usb_gadget_probe_driver(struct usb_g
}

pr_debug("couldn't find an available UDC\n");
+ list_add_tail(&driver->probe_list, &pending_drivers);
mutex_unlock(&udc_lock);
- return -ENODEV;
+ return 0;
found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(&udc_lock);
@@ -425,6 +441,7 @@ int usb_gadget_unregister_driver(struct
return -EINVAL;

mutex_lock(&udc_lock);
+ list_del(&driver->probe_list);
list_for_each_entry(udc, &udc_list, list)
if (udc->driver == driver) {
usb_gadget_remove_driver(udc);

[Peter Chen]

On Fri, Jun 13, 2014 at 10:19:36AM -0400, Alan Stern wrote:
> On Fri, 13 Jun 2014, Peter Chen wrote:
>
> > OK, we can keep our g_xxx gadget driver just support the basic feature. But
> > the bug that causes gadget driver load fail due to udc is probed deferral should
> > be fixed, do you think so, we can't wait until configfs has total been ready.
>
> That problem has always existed. There never has been a time when a
> gadget driver could be loaded before the UDC driver was ready. Does it
> really need to be fixed now?
>

Yes, otherwise, the device which the udc is probed deferral and the
gadget driver is build-in will never work. If we skip fixing it, this
problem will exist for more than 2 years, it is too long.

I have a support request from android team that usb peripheral function
never works from 3.10.x kernel at one device, I think it is a common problem,
no only I meet it.

Below are links which this problem reports:

http://marc.info/?l=linux-usb&m=139380872501745&w=2
http://marc.info/?l=linux-usb&m=137706435611447&w=2

> If you do want to fix the problem, there's a much easier way than what
> you posted. See below.
>

Robert Baldyga posts the similar solution, but seems Felipe doesn't
agree it.
http://www.spinics.net/lists/linux-usb/msg102795.html

Then, you and Felipe has a discussion for this problem:
http://www.spinics.net/lists/linux-usb/msg106760.html

Best Regards,
Peter Chen

[Alan Stern]

Okay, I get it. Felipe didn't want to do this because you might end up
loading a gadget driver that never gets bound. There might not even be
any UDC devices at all.

Felipe, I think the problem of having unbound gadget drivers isn't so
bad. It's just like having unbound drivers of any sort. For example,
the Fedora kernels have ohci-hcd and uhci-hcd built-in. Relatively few
systems have both OHCI and UHCI controllers (lots of current machines
have neither); everybody else ends up with at least one unused driver
and no way to remove it. If Fedora can do this, I think we can allow
users to have unnecessary gadget drivers.

In fact, I don't know of any other kind of driver module whose init
routine will fail if there's no hardware to bind to.

I suggest you go ahead and accept Robert Baldyga's original patch --
except for one little problem: The patch forgets to add driver->list to
the global driver_list if the driver gets bound immediately. The

list_add_tail(&driver->list, &driver_list);

call needs to be moved up near the start of usb_gadget_probe_driver().

Alan Stern

[Peter Chen]

Thanks, Alan.

The another improvement is: set driver->attached as true at udc_bind_to_driver.

Peter