lkml.org 
[lkml]   [2022]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v24 3/4] usb: misc: Add onboard_usb_hub driver
Hi,

On Thu, Jun 30, 2022 at 12:35 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> The main issue this driver addresses is that a USB hub needs to be
> powered before it can be discovered. For discrete onboard hubs (an
> example for such a hub is the Realtek RTS5411) this is often solved
> by supplying the hub with an 'always-on' regulator, which is kind
> of a hack. Some onboard hubs may require further initialization
> steps, like changing the state of a GPIO or enabling a clock, which
> requires even more hacks. This driver creates a platform device
> representing the hub which performs the necessary initialization.
> Currently it only supports switching on a single regulator, support
> for multiple regulators or other actions can be added as needed.
> Different initialization sequences can be supported based on the
> compatible string.
>
> Besides performing the initialization the driver can be configured
> to power the hub off during system suspend. This can help to extend
> battery life on battery powered devices which have no requirements
> to keep the hub powered during suspend. The driver can also be
> configured to leave the hub powered when a wakeup capable USB device
> is connected when suspending, and power it off otherwise.
>
> Technically the driver consists of two drivers, the platform driver
> described above and a very thin USB driver that subclasses the
> generic driver. The purpose of this driver is to provide the platform
> driver with the USB devices corresponding to the hub(s) (a hub
> controller may provide multiple 'logical' hubs, e.g. one to support
> USB 2.0 and another for USB 3.x).
>
> Co-developed-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
> Changes in v24:
> - changed USB_ONBOARD_HUB from bool to tristate and fixed Makefile
> entry
> - renamed 'companion-hub' property to 'peer-hub'
> - added comment to onboard_hub_create_pdevs() doc that explains the
> somewhat convoluted logic of the function
> - only cancel the driver attach work in onboard_hub_remove() if the
> function wasn't called by the work itself
>
> Changes in v23:
> - refactored onboard_hub_create_pdevs()
> - bail out before the loop for root hubs connected to
> the secondary HCD
> - check for exisiting pdev in the companion hub branch
> instead of doing it afterwards
> - added note about the role of the platform devices to
> documentation of onboard_hub_create_pdevs()
> - got rid of CONFIG_USB_ONBOARD_HUB_ACTUAL, it's not needed anymore
> now that onboard_hub_create/destroy_pdevs() are linked into the USB
> core
> - added some more details to comment about the need to re-attach the
> USB driver
>
> Changes in v22:
> - moved onboard_hub_create_pdevs(), onboard_hub_destroy_pdevs() and
> of_is_onboard_usb_hub() into a separate file to allow them to be
> linked into the USB core module (if CONFIG_USB=m)
> - Added extra checks to ensure only one platform device is created
> for each physical hub. This is necessary because
> onboard_hub_create_pdevs() is now called from hub_probe() instead
> of usb_add_hcd(). As a result the function is called twice for
> typical USB3 hub, once for the USB3 part of the hub, and once for
> USB <= 2.x. Generally the parent hub downstream of the primary HCD
> creates the onboard hub platform device.
> - use kzalloc() instead of devm_kzalloc() to allocate list entries. Using
> managed memory can cause issues since the managed memory is allocated
> before the device is probed.
> - use a workqueue to (re-)attach the USB driver during/after _probe().
> This is necessary to avoid self-deadlocks on systems with nested
> onboard hubs.
> - don't initialize list_head in onboard_hub_create_pdevs(), this needs
> to be done by the owner, to ensure that onboard_hub_destroy_pdevs()
> is called with a struct that was properly initialized even when
> onboard_hub_create_pdevs() is not called.
> - moved 'onboard_hub_match' into an include, so it can be used
> by both the the onboard hub driver and of_is_onboard_usb_hub()
> - removed note from the commit message which said that currently only
> onboard hubs connected to a root hub are supported. This isn't the
> case anymore.
> - updated kernel version and date in sysfs documentation
> - removed tags from Doug and Alan since this version changes quite
> a few things
>
> Changes in v21:
> - refactored _find_onboard_hub()
> - check presence of drvdata instead of using device_is_bound()
> - return -ENODEV if the platform device can't be found
> - always check drvdata, not only when looking for the companion
> hub
> - added comment explaining probe deferral
> - removed 'onboard_hub_dev' symlinks from USB devices
> - updated documentation of 'always_powered_in_suspend' sysfs attribute
>
> Changes in v20:
> - s/nontrivial/non-trivial/ in Kconfig doc
> - added include of 'export.h'
> - initialize variable 'power_off' in onboard_hub_suspend() at
> declaration time
> - renamed set_udev_link_name() to get_udev_link_name()
> - use kzalloc to allocate struct usbdev_node instead of devm_kzalloc()
> - release the lock in onboard_hub_add_usbdev() before calling
> get_udev_link_name()
> - free struct usbdev_node in onboard_hub_remove_usbdev(), now that
> it doesn't used managed memory
> - make struct onboard_hub const in always_powered_in_suspend_show()
> - call put_device() if pdev is not bound in _find_onboard_hub()
> - get driver data before calling put_device() in _find_onboard_hub()
> - log platform device name when sysfs link creation fails in
> onboard_hub_usbdev_probe()
> - added kernel doc for onboard_hub_create_pdevs() and
> onboard_hub_destroy_pdevs()
> - added 'Reviewed-by' tag from Doug
>
> Changes in v19:
> - added VID:PID pairs and compatible strings for RTS5414 hub
> - updated comments with RTS5411 USB versions to reflect those
> reported/supported by the hub
>
> Changes in v18:
> - introduced hidden Kconfig option to align module vs. builtin
> choice with CONFIG_USB (thanks Doug!)
> - refactored onboard_hub_create_pdevs()
>
> Changes in v17:
> - updated date and kernel version in ABI documentation for
> 'always_powered_in_suspend' attribute
> - removed obsolete .yaml entry from MAINTAINERS file
> - added entry for ABI documentation to MAINTAINERS file
> - renamed struct 'udev_node' to 'usbdev_node'
> - changed return logic in onboard_hub_suspend/resume() to
> get rid of 'rc' variable
> - added helper set_udev_link_name() to set link names for
> onboard hub USB devices
> - use of_parse_phandle() instead of of_property_read_u32() +
> of_find_node_by_phandle() combo
> - defer probing in _find_onboard_hub() if the platform device
> isn't bound yet
> - initialize list head passed as parameter to
> onboard_hub_create_pdevs() instead of relying on the caller
> to do so
> - don't require the 'companion-hub' property to be specified.
> This is needed to support hubs without companion hub
> - use devm_kzalloc() to allocate platform device list entries
> and stop freeing them explicitly
> - remove unnecessary INIT_LIST_HEAD() of platform device list
> entries
> - use '%pOF' to print DT node name
> - delete platform device list entries from the list of devices
> in onboard_hub_destroy_pdevs(). It shouldn't be strictly
> necessary, but better be on the safe side.
>
> Changes in v16:
> - none
>
> Changes in v15:
> - none
>
> Changes in v14:
> - none
>
> Changes in v13:
> - none
>
> Changes in v12:
> - use IS_ENABLED(CONFIG_USB_ONBOARD_HUB_MODULE) in onboard_hub.h to
> also check for the driver built as module
> - include onboard_hub.h again from the driver to make sure there are
> prototype declarations for the public functions
> - remove indentation from label in onboard_hub_create_pdevs()
>
> Changes in v11:
> - added onboard_hub_create/destroy_pdevs() helpers, to support multiple onboard
> hubs connected to the same parent hub
> - don't include ‘onboard_hub.h’ from the onboard hub driver
> - updated commit message
> - added ‘Acked-by' from Alan
>
> Changes in v10:
> - always use of_is_onboard_usb_hub() stub unless ONBOARD_USB_HUB=y/m
>
> Changes in v9:
> - none
>
> Changes in v8:
> - none
>
> Changes in v7:
> - don't declare stub for of_is_onboard_usb_hub() when
> CONFIG_COMPILE_TEST is defined
>
> Changes in v6:
> - use 'companion-hub' to locate the platform device, instead of
> scanning through the nodes of the parent
> - added ABI documentation for 'always_powered_in_suspend'
> - sysfs_emit() instead of sprintf() in always_powered_in_suspend_show()
> - register sysfs attribute through driver.dev_groups
> - evaluate return value of driver_attach() in _probe()
> - use dev_warn() instead of WARN_ON() in _probe()
> - include 'onboard_hub.h'
>
> Changes in v5:
> - the platform device is now instantiated from the same DT node
> as the 'primary' USB hub device
> - use the USB compatible strings for the platform device
> - refactored _find_onboard_hub() to search the parents child
> nodes for a platform device with a matching compatible string
> - added exported function of_is_onboard_usb_hub() to allow other
> drivers (like xhci_plat) to check if one of their child DT nodes
> is a supported hub
> - use late suspend to make sure info about wakeup enabled descendants
> is updated
> - call driver_attach() for the USB driver in onboard_hub_probe() to
> make sure the driver is re-attached after the device_release_driver()
> calls in onboard_hub_remove()
> - renamed sysfs attribute 'power_off_in_suspend' to
> 'always_powered_in_suspend'
> - added sysfs symlinks between platform device and USB devices
> - marked 'onboard_hub_pm_ops' as __maybe_unused
> - removed 'realtek' compatible string which is not needed at this
> point
> - fix log for regulator_disable() failure
>
> Changes in v4:
> - updated Kconfig documentation
> - changed the loop in onboard_hub_remove() to release the hub lock
> before unbinding the USB device and make self deadlock prevention
> less clunky
> - fixed return value in onboard_hub_usbdev_probe()
> - added entry to MAINTAINERS file
>
> Changes in v3:
> - updated the commit message
> - updated description in Kconfig
> - remove include of 'core/usb.h'
> - use 'is_powered_on' flag instead of 'has_wakeup_capable_descendants'
> - added 'going_away' flag to struct onboard_hub
> - don't allow adding new USB devices when the platform device is going away
> - don't bother with deleting the list item in onboard_hub_remove_usbdev()
> when the platform device is going away
> - don't assume in onboard_hub_suspend() that all USB hub devices are
> connected to the same controller
> - removed unnecessary devm_kfree() from onboard_hub_remove_usbdev()
> - fixed error handling in onboard_hub_remove_usbdev()
> - use kstrtobool() instead of strtobool() in power_off_in_suspend_store()
> - unbind USB devices in onboard_hub_remove() to avoid dangling references
> to the platform device
> - moved put_device() for platform device to _find_onboard_hub()
> - changed return value of onboard_hub_remove_usbdev() to void
> - evaluate return value of onboard_hub_add_usbdev()
> - register 'power_off_in_suspend' as managed device attribute
> - use USB_DEVICE macro instead manual initialization
> - add unwinding to onboard_hub_init()
> - updated MODULE_DESCRIPTION
> - use module_init() instead of device_initcall()
>
> Changes in v2:
> - check wakeup enabled state of the USB controller instead of
> using 'wakeup-source' property
> - use sysfs attribute instead of DT property to determine if
> the hub should be powered off at all during system suspend
> - added missing brace in onboard_hub_suspend()
> - updated commit message
> - use pm_ptr for pm_ops as suggested by Alan
>
> Changes in v1:
> - renamed the driver to 'onboard_usb_hub'
> - single file for platform and USB driver
> - USB hub devices register with the platform device
> - the DT includes a phandle of the platform device
> - the platform device now controls when power is turned off
> - the USB driver became a very thin subclass of the generic USB
> driver
> - enabled autosuspend support
>
> .../sysfs-bus-platform-onboard-usb-hub | 8 +
> MAINTAINERS | 7 +
> drivers/usb/core/Makefile | 4 +
> drivers/usb/misc/Kconfig | 16 +
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/onboard_usb_hub.c | 428 ++++++++++++++++++
> drivers/usb/misc/onboard_usb_hub.h | 17 +
> drivers/usb/misc/onboard_usb_hub_pdevs.c | 142 ++++++
> include/linux/usb/onboard_hub.h | 18 +
> 9 files changed, 641 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-platform-onboard-usb-hub
> create mode 100644 drivers/usb/misc/onboard_usb_hub.c
> create mode 100644 drivers/usb/misc/onboard_usb_hub.h
> create mode 100644 drivers/usb/misc/onboard_usb_hub_pdevs.c
> create mode 100644 include/linux/usb/onboard_hub.h

Reviewed-by: Douglas Anderson <dianders@chromium.org>

\
 
 \ /
  Last update: 2022-07-01 01:21    [W:0.086 / U:1.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site