From: John Stultz <john.stultz@linaro.org> To: Yu Chen <chenyu56@huawei.com> Cc: Linux USB List <linux-usb@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, Zhuangluan Su <suzhuangluan@hisilicon.com>, Kongfei <kongfei@hisilicon.com>, "Liuyu (R)" <liuyu712@hisilicon.com>, wanghu17@hisilicon.com, butao <butao@hisilicon.com>, chenyao11@huawei.com, fangshengzhou@hisilicon.com, Li Pengcheng <lipengcheng8@huawei.com>, songxiaowei@hisilicon.com, YiPing Xu <xuyiping@hisilicon.com>, xuyoujun4@huawei.com, Yudongbin <yudongbin@hisilicon.com>, Zang Leigang <zangleigang@hisilicon.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Hans de Goede <hdegoede@redhat.com>, Andy Shevchenko <andy.shevchenko@gmail.com> Subject: Re: [PATCH v5 09/13] usb: roles: Add usb role switch notifier. Date: Thu, 11 Apr 2019 18:12:30 -0700 [thread overview] Message-ID: <CALAqxLUQW03waz0LETcnYpc7qhm=RG6RfwbGVZ7MQz00jTZSWQ@mail.gmail.com> (raw) In-Reply-To: <20190329041409.70138-10-chenyu56@huawei.com> On Thu, Mar 28, 2019 at 9:14 PM Yu Chen <chenyu56@huawei.com> wrote: > > This patch adds notifier for drivers want to be informed of the usb role > switch. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: John Stultz <john.stultz@linaro.org> > Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Yu Chen <chenyu56@huawei.com> Hey Yu Chen! Thanks again for sending this patch out! As mentioned in my comments with the other patches, I've got one proposal I wanted to share to try to avoid state initialization races that I've seen with this patchset. > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c > index f45d8df5cfb8..e2caaa665d6e 100644 > --- a/drivers/usb/roles/class.c > +++ b/drivers/usb/roles/class.c > @@ -58,6 +61,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role) > } > EXPORT_SYMBOL_GPL(usb_role_switch_set_role); > > +int usb_role_switch_register_notifier(struct usb_role_switch *sw, > + struct notifier_block *nb) > +{ > + return blocking_notifier_chain_register(&sw->nh, nb); > +} > +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier); As noted earlier, one issue I've seen is that the hisi_hikey_usb driver's notifier may not get called early enough to receive notification of the initial usb state. It seems like on registration here, we should take the lock, read the role state and immediately call the notifier to properly initialize it? I suspect that should close the window for any state races around driver probe timings and Does that make sense? I have roughly prototyped this but need to do additional testing. thanks -john
WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org> To: Yu Chen <chenyu56@huawei.com> Cc: Linux USB List <linux-usb@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, Zhuangluan Su <suzhuangluan@hisilicon.com>, Kongfei <kongfei@hisilicon.com>, "Liuyu \(R\)" <liuyu712@hisilicon.com>, wanghu17@hisilicon.com, butao <butao@hisilicon.com>, chenyao11@huawei.com, fangshengzhou@hisilicon.com, Li Pengcheng <lipengcheng8@huawei.com>, songxiaowei@hisilicon.com, YiPing Xu <xuyiping@hisilicon.com>, xuyoujun4@huawei.com, Yudongbin <yudongbin@hisilicon.com>, Zang Leigang <zangleigang@hisilicon.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Hans de Goede <hdegoede@redhat.com>, Andy Shevchenko <andy.shevchenko@gmail.com> Subject: [v5,09/13] usb: roles: Add usb role switch notifier. Date: Thu, 11 Apr 2019 18:12:30 -0700 [thread overview] Message-ID: <CALAqxLUQW03waz0LETcnYpc7qhm=RG6RfwbGVZ7MQz00jTZSWQ@mail.gmail.com> (raw) On Thu, Mar 28, 2019 at 9:14 PM Yu Chen <chenyu56@huawei.com> wrote: > > This patch adds notifier for drivers want to be informed of the usb role > switch. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: John Stultz <john.stultz@linaro.org> > Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Yu Chen <chenyu56@huawei.com> Hey Yu Chen! Thanks again for sending this patch out! As mentioned in my comments with the other patches, I've got one proposal I wanted to share to try to avoid state initialization races that I've seen with this patchset. > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c > index f45d8df5cfb8..e2caaa665d6e 100644 > --- a/drivers/usb/roles/class.c > +++ b/drivers/usb/roles/class.c > @@ -58,6 +61,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role) > } > EXPORT_SYMBOL_GPL(usb_role_switch_set_role); > > +int usb_role_switch_register_notifier(struct usb_role_switch *sw, > + struct notifier_block *nb) > +{ > + return blocking_notifier_chain_register(&sw->nh, nb); > +} > +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier); As noted earlier, one issue I've seen is that the hisi_hikey_usb driver's notifier may not get called early enough to receive notification of the initial usb state. It seems like on registration here, we should take the lock, read the role state and immediately call the notifier to properly initialize it? I suspect that should close the window for any state races around driver probe timings and Does that make sense? I have roughly prototyped this but need to do additional testing. thanks -john
next prev parent reply other threads:[~2019-04-12 1:12 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-29 4:13 [PATCH v5 00/13] Add support for usb on Hikey960 Yu Chen 2019-03-29 4:13 ` Yu Chen 2019-03-29 4:13 ` [PATCH v5 01/13] dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY Yu Chen 2019-03-29 4:13 ` [v5,01/13] " Yu Chen 2019-03-29 4:13 ` [PATCH v5 01/13] " Yu Chen 2019-03-29 4:13 ` [PATCH v5 02/13] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 Yu Chen 2019-03-29 4:13 ` [v5,02/13] " Yu Chen 2019-03-29 4:13 ` [PATCH v5 02/13] " Yu Chen 2019-03-29 4:13 ` [PATCH v5 03/13] usb: dwc3: dwc3-of-simple: Add support for dwc3 of Hisilicon Soc Platform Yu Chen 2019-03-29 4:13 ` [v5,03/13] " Yu Chen 2019-03-29 4:13 ` [PATCH v5 03/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc Yu Chen 2019-03-29 4:14 ` [v5,04/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 04/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 05/13] usb: dwc3: Execute GCTL Core Soft Reset while switch mdoe " Yu Chen 2019-03-29 4:14 ` [v5,05/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 05/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 06/13] usb: dwc3: Increase timeout for CmdAct cleared by device controller Yu Chen 2019-03-29 4:14 ` [v5,06/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 06/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 07/13] phy: Add usb phy support for hi3660 Soc of Hisilicon Yu Chen 2019-03-29 4:14 ` [v5,07/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 07/13] " Yu Chen 2019-04-17 6:42 ` Kishon Vijay Abraham I 2019-04-17 6:42 ` [v5,07/13] " Kishon Vijay Abraham I 2019-04-17 6:42 ` [PATCH v5 07/13] " Kishon Vijay Abraham I 2019-04-17 7:19 ` Chen Yu 2019-04-17 7:19 ` [v5,07/13] " Yu Chen 2019-04-17 7:19 ` [PATCH v5 07/13] " Chen Yu 2019-03-29 4:14 ` [PATCH v5 08/13] usb: roles: Introduce stubs for the exiting functions in role.h Yu Chen 2019-03-29 4:14 ` [v5,08/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 08/13] " Yu Chen 2019-04-04 13:25 ` Heikki Krogerus 2019-04-04 13:25 ` [v5,08/13] " Heikki Krogerus 2019-03-29 4:14 ` [PATCH v5 09/13] usb: roles: Add usb role switch notifier Yu Chen 2019-03-29 4:14 ` [v5,09/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 09/13] " Yu Chen 2019-04-04 13:26 ` Heikki Krogerus 2019-04-04 13:26 ` [v5,09/13] " Heikki Krogerus 2019-04-12 1:12 ` John Stultz [this message] 2019-04-12 1:12 ` John Stultz 2019-04-12 1:12 ` [PATCH v5 09/13] " John Stultz 2019-04-12 3:59 ` John Stultz 2019-04-12 3:59 ` [v5,09/13] " John Stultz 2019-04-12 3:59 ` [PATCH v5 09/13] " John Stultz 2019-03-29 4:14 ` [PATCH v5 10/13] usb: dwc3: Registering a role switch in the DRD code Yu Chen 2019-03-29 4:14 ` [v5,10/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 10/13] " Yu Chen 2019-04-12 0:48 ` John Stultz 2019-04-12 0:48 ` [v5,10/13] " John Stultz 2019-04-12 0:48 ` [PATCH v5 10/13] " John Stultz 2019-04-12 1:23 ` Chen Yu 2019-04-12 1:23 ` [v5,10/13] " Yu Chen 2019-04-12 1:23 ` [PATCH v5 10/13] " Chen Yu 2019-03-29 4:14 ` [PATCH v5 11/13] hikey960: Support usb functionality of Hikey960 Yu Chen 2019-03-29 4:14 ` [v5,11/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 11/13] " Yu Chen 2019-04-12 0:55 ` John Stultz 2019-04-12 0:55 ` [v5,11/13] " John Stultz 2019-04-12 0:55 ` [PATCH v5 11/13] " John Stultz 2019-03-29 4:14 ` [PATCH v5 12/13] usb: gadget: Add configfs attribuite for controling match_existing_only Yu Chen 2019-03-29 4:14 ` [v5,12/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 12/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 13/13] dts: hi3660: Add support for usb on Hikey960 Yu Chen 2019-03-29 4:14 ` Yu Chen 2019-03-29 4:14 ` [v5,13/13] " Yu Chen 2019-03-29 4:14 ` [PATCH v5 13/13] " Yu Chen 2019-04-01 14:57 ` [PATCH v5 00/13] " Valentin Schneider
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CALAqxLUQW03waz0LETcnYpc7qhm=RG6RfwbGVZ7MQz00jTZSWQ@mail.gmail.com' \ --to=john.stultz@linaro.org \ --cc=andy.shevchenko@gmail.com \ --cc=butao@hisilicon.com \ --cc=chenyao11@huawei.com \ --cc=chenyu56@huawei.com \ --cc=devicetree@vger.kernel.org \ --cc=fangshengzhou@hisilicon.com \ --cc=gregkh@linuxfoundation.org \ --cc=hdegoede@redhat.com \ --cc=heikki.krogerus@linux.intel.com \ --cc=kongfei@hisilicon.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=lipengcheng8@huawei.com \ --cc=liuyu712@hisilicon.com \ --cc=songxiaowei@hisilicon.com \ --cc=suzhuangluan@hisilicon.com \ --cc=wanghu17@hisilicon.com \ --cc=xuyiping@hisilicon.com \ --cc=xuyoujun4@huawei.com \ --cc=yudongbin@hisilicon.com \ --cc=zangleigang@hisilicon.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.