[PATCH 0/3] Implement role-switch notifications from dwc3-drd to dwc3-qcom

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a topic we have been discussing for some time, initially in the
context of gpio usb-c-connector role-switching.

https://lore.kernel.org/linux-usb/20200311191501.8165-1-bryan.odonoghue@xxxxxxxxxx

Hardware availability constraints limited scope to finish that off.

Thankfully Wesley Cheng made a new set of USB role-switch related patches
for dwc3-qcom, this time in conjunction with the qcom pm8150b type-c
silicon.

https://lore.kernel.org/linux-usb/20201009082843.28503-1-wcheng@xxxxxxxxxxxxxx

For the RB5 project we picked Wesley's changes and developed them further,
around a type-c port manager.

As a precursor to that TCPM I reposted Wesley's patches
https://lore.kernel.org/linux-usb/20210629144449.2550737-1-bryan.odonoghue@xxxxxxxxxx

Bjorn pointed out that having the role-switch triggered from dwc3-qcom to
dwc3-drd is not the right way around, indicating a preference for the
original notifier from dwc3-drd to dwc3-qcom.

There are two approaches I considred and prototyped to accomplish the
desired dwc3-drd -> dwc3-qcom messaging.

#1 Using a notifier in dwc3-drd to trigger dwc3-qcom

   This would be nice since it would accomplish the desired layering
   dwc3-drd -> dwc3-qcom.

   However:
   a) It would be a real mess as dwc3-qcom is the parent device of
      dwc3-core so, if the child-device dwc3-core deferred probing for
      whatever reason we would have to detect this and retry the parent's
      probe again. The point in time that dwc3-qcom could potentially parse
      such a deferral in the child device is late. It would also be weird
      and messy to try to roll back the parent's probe because of a child
      device deferral.

      I considered making some sort of worker in the parent to check for
      child device probe but, again this seemed like an atrocious hack so,
      I didn't even try to prototype that.

   b) One potential solution was using "__weak" linkage in a function
      provided by dwc3-drd that a wrapper such as dwc3-qcom could then
      over-ride.

      If a wrapper such as dwc3-qcom then implemented a function with
      regular linkage it would over-ride the __weak function and provide a
      method for the dwc3-drd code to call into dwc3-qcom when probing was
      complete, thus allowing registration of the notifier when the child
      was ready.

      This would work up until the point that you tried to compile two
      implementations of a dwc3 wrapper into the one kernel module or the
      one kernel image say dwc3-qcom and a similar implementation in
      dwc3-meson. At that point you would get linker breakage.

#2 Using USB role switching for the notification

   Wesley's implementation took the approach dwc3-qcom -> dwc3-drd, whereas
   the approach I'm proposing here is dwc3-drd -> dwc3-qcom, which is also
   what we discussed on the list.

   Having implemented it, I think USB role-switching in the direction
   dwc3-drd -> dwc3-qcom is also a much cleaner solution for several
   reasons.

   a) Handling probe deferral is built into Linux' USB role switching today
      so we don't have to re-invent that wheel, unlike with the original
      notifier model.

   b) There is no "wiring up" or traversing the graph tree for the wrapper
      layer to determine if the parent device has a compliant type-c
      connector associated with it, unlike in the dwc3-qcom -> dwc3-drd
      model.

      All that has to happen is "usb-role-switch" is declared in the parent
      dwc3-qcom node and the role-switch API takes care of the rest.

      That means its possible to use a usb-c-connector, qcom type-c pm8150b
      driver, a USCI, a tps659x, a fusb302 or something like ChromeOS
      cros_ec to notify dwc3-drd without dwc3-qcom having to have
      the slighest clue which type of device is sending the signal.

      All dwc3-qcom needs to do is waggle UTMI signals in a register when a
      role-switch happens.

   c) It "feels" like a layering violation to have the dwc3-qcom SoC
      wrapper receive the event and trigger the dwc3-drd core.

      The standard model of parent/child role switching or remote-endpoint
      traversal that USB role switching already has works just fine for
      dwc3-drd, we just need to trigger dwc3-qcom for the role-switch in a
      non-vendor and non-SoC specific way.

   d) Less code. It turns out there's less code implementing as a
      role-switch interface in the direction dwc3-drd -> dwc3-qcom.

   e) Portability. The mechanism used for dwc3-drd -> dwc3 qcom can be
      reused for any other similar wrapper which models the wrapper as a
      parent of the dwc3-drd.

For all of those reasons I've opted to use USB role-switch notification
from dwc3-drd to dwc3-qcom.

git add bod git://git.linaro.org/people/bryan.odonoghue/kernel.git
git fetch bod
git diff usb-next-5.13.rcx-rb5-tcpm..usb-next-5.13.rcx-rb5-tcpm-v2

Bryan O'Donoghue (2):
  usb: dwc3: Add role switch relay support
  usb: dwc3: dwc3-qcom: Make dwc3-qcom a role-switch signal recipient

Wesley Cheng (1):
  usb: dwc3: dwc3-qcom: Fix typo in the dwc3 vbus override API

 drivers/usb/dwc3/core.h      |  2 +
 drivers/usb/dwc3/drd.c       | 10 +++++
 drivers/usb/dwc3/dwc3-qcom.c | 77 ++++++++++++++++++++++++++++++++++--
 3 files changed, 85 insertions(+), 4 deletions(-)

-- 
2.30.1




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux