Re: [PATCH v4 1/5] gadget: Introduce the notifier functions

From: Felipe Balbi
Date: Thu Oct 01 2015 - 13:30:26 EST


On Thu, Sep 24, 2015 at 10:39:23AM -0700, Baolin Wang wrote:
> The usb charger framework is based on usb gadget. The usb charger
> need to be notified the state changing of usb gadget to confirm the
> usb charger state.
>
> Thus this patch adds a notifier mechanism for usb gadget to report a
> event to usb charger when the usb gadget state is changed.
>
> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
> ---
> drivers/usb/gadget/udc/udc-core.c | 32 ++++++++++++++++++++++++++++++++
> include/linux/usb/gadget.h | 18 ++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index f660afb..4238fc3 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -129,6 +129,32 @@ void usb_gadget_giveback_request(struct usb_ep *ep,
> }
> EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
>
> +int usb_gadget_register_notify(struct usb_gadget *gadget,
> + struct notifier_block *nb)
> +{
> + int ret;
> +
> + mutex_lock(&gadget->lock);
> + ret = raw_notifier_chain_register(&gadget->nh, nb);
> + mutex_unlock(&gadget->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_register_notify);
> +
> +int usb_gadget_unregister_notify(struct usb_gadget *gadget,
> + struct notifier_block *nb)
> +{
> + int ret;
> +
> + mutex_lock(&gadget->lock);
> + ret = raw_notifier_chain_unregister(&gadget->nh, nb);

Greg, this is the kind of thing I wanted to avoid adding more of.

I was wondering if you would accept subsystems using kdbus for
this sort of notification. I'm okay waiting for kdbus for another
couple merge windows (if we have to) before that's merged, but
if we take this raw notifier approach now, we will end up having
to support it forever.

Also, because soon enough we will have to support USB Power Delivery
with Type C connector, this is bound to change in the coming months.

Frankly, I wanted all of this to be decided in userland with the
kernel just providing notification and basic safety checks (we don't
want to allow a bogus userspace daemon frying anybody's devices).

How would you feel about that ?

--
balbi

Attachment: signature.asc
Description: PGP signature