Linux GPIO subsystem development
 help / color / mirror / Atom feed
* [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers
@ 2026-10-12 17:35 Adrian Ambrozewicz
  2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 17:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Donald Shannon, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Bartosz Golaszewski, Lee Jones, Pavel Machek,
	Andi Shyti, Mark Brown, Jiri Slaby, Corey Minyard,
	Rafael J. Wysocki, Len Brown, Jonathan Corbet, Ilpo Järvinen,
	Sakari Ailus, Hans de Goede, Lixu Zhang, Danilo Krummrich,
	Johan Hovold, Oliver Neukum, Quan Nguyen, Andrew Jeffery,
	linux-usb, devicetree, linux-gpio, linux-leds, linux-i2c,
	linux-spi, linux-serial, openipmi-developer, linux-acpi,
	linux-doc, driver-core, linux-kernel

This series adds drivers for the NVIDIA LSTP USB adapter: a USB core
driver and GPIO, LED, I2C, SPI, UART and BMC-side IPMI function drivers,
with their device tree bindings, an ACPI description document and the
sysfs ABI.

What LSTP is
------------

LSTP (Low-Speed Transport Protocol or Low-Speed Tunnelling Protocol) is
a vendor-specific USB interface (class 0xff, subclass 0x3f) that
multiplexes several low-speed "channels" over one pair of bulk
endpoints. It is implemented by NVIDIA's SMA, an MCU (0955:cf11), and
by other vendors' parts: two hub/bridge chips have it built in
(0424:4940, 174c:2214). Boards use it to
reach SPI flash, I2C sensors, GPIOs and LEDs, a UART and a host-to-BMC
IPMI path, on both BMCs and host systems.

The channel types are SPI host, I2C host, GPIO (with interrupts and LED
pins), UART, IPMI and MMIO. Channel 0 is the control channel: at probe
the driver reads the channel table from it, which gives each channel's
ID, type and configuration. The set of channels is decided by the
firmware, so the same VID:PID carries different channels on different
boards. No firmware description is needed for the channels to work.

Series layout
-------------

   1  dt-bindings: gpio: Add NVIDIA LSTP GPIO
   2  dt-bindings: i2c: Add NVIDIA LSTP I2C
   3  dt-bindings: spi: Add NVIDIA LSTP SPI
   4  dt-bindings: usb: Add NVIDIA LSTP USB adapter
   5  usb: misc: Add NVIDIA LSTP USB adapter driver
   6  gpio: Add NVIDIA LSTP GPIO driver
   7  leds: Add NVIDIA LSTP LED driver
   8  i2c: Add NVIDIA LSTP I2C driver
   9  spi: Add NVIDIA LSTP SPI driver
  10  tty: Add NVIDIA LSTP UART driver
  11  ipmi: Add NVIDIA LSTP IPMI BMC driver

Patch 4 $refs the schemas from patches 1-3, so it comes last among the
bindings; it also defines the serial@N and ipmi@N channel nodes inline.
The MAINTAINERS entry starts in patch 1 and gains each later patch's
files. Patch 5 carries:
- the core in drivers/usb/misc/;
- include/linux/usb/lstp.h for the function drivers;
- the sysfs ABI file;
- Documentation/driver-api/usb/lstp.rst;
- Documentation/firmware-guide/acpi/lstp.rst.

Patches 6 and 8-11 each depend only on patch 5; patch 7 (LEDs) also
needs patch 6, whose GPIO driver hands the LED pins to it.

Why the auxiliary bus, and why not MFD
--------------------------------------

The core binds the USB interface and owns everything shared:
- URBs and packet framing;
- request/response matching and the IRQ-style event path;
- channel 0 discovery.

Each channel the device reports is then created as an auxiliary device,
lstp.<type>.<id>, and the function drivers in their subsystems bind
those. They reach the device only through functions the core exports in
the "LSTP" symbol namespace, such as lstp_request() and lstp_send().

The closest model in tree is LJCA (drivers/usb/misc/usb-ljca.c), and
the series follows it. LJCA is a USB adapter that muxes GPIO, I2C and
SPI over its bulk endpoints. At probe it asks the device which
functions it has, creates one auxiliary device per reported function,
and exports a transfer and an event interface to the function drivers.
LSTP does the same with more channel types, with channel IDs assigned
by the device firmware, and with device tree as well as ACPI.

A channel has no registers, no MMIO and no interrupt lines of its own;
it is a logical function of one protocol stream, and its driver needs
nothing but the core's API. That is the use the auxiliary bus
documentation describes (Documentation/driver-api/auxiliary_bus.rst):
no dependency on a physical bus, device, register accesses or regmap.
It names MFD as the wrong fit for that case, because MFD relies on the
function devices being physical devices: an MFD cell is a platform
device, and its resources are ranges of the parent's memory and
interrupts (struct mfd_cell). Firmware matching does not decide between
the two, since MFD cells can also match DT nodes and ACPI _ADR children.

The same documentation says auxiliary devices "are not physical devices
that are controlled by DT/ACPI". An LSTP channel is not enumerated from
firmware either: it exists because the device reported it. Firmware
only describes it and what is attached to it. Before
auxiliary_device_add(), the core resolves each channel's node and sets
it with device_set_node():
- device tree: a child of the interface node the USB core attached;
- ACPI: a Device() under the interface's Device(), which is a child of
  the port companion (Documentation/firmware-guide/acpi/lstp.rst);
- neither: the channel has no firmware node and works as reported.
A channel matches on its address (reg, or _ADR on ACPI) plus
compatible. The I2C, SPI and GPIO cores then enumerate children and
resolve references as for any other controller.

The UART: a plain tty_driver (patch 10)
---------------------------------------

The firmware owns the UART line: baud rate, framing, parity and flow
control are provisioned on the device and only read at probe. The host
cannot read or set modem control lines; it can send a fixed-length
break, and a carrier loss reported by the device hangs the tty up unless
CLOCAL is set. Most of serial_core's uart_ops would be stubs, and
usb-serial does not fit because it binds the whole USB interface, where
the UART is one function among several. So patch 10 registers a plain
tty_driver (TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV) with one
tty_port per channel, the structure cdc-acm uses for a UART behind USB.

set_termios keeps the hardware fields at the firmware's values, so stty
reports the real line settings. The nodes are /dev/ttyLSTP<n>, parented
on the channel's auxiliary device. Jiri, Ilpo: if you would rather see
this on serial_core despite the empty uart_ops, please say so on
patch 10.

The IPMI channel: BMC side, with a message number (patch 11)
------------------------------------------------------------

On a BMC, the IPMI channel carries the host's IPMI requests to BMC
userspace and the responses back, like ssif_bmc does for SSIF; it is
not an SSIF transport and has no host-side (ipmi_si) part. Each channel
is a character device, /dev/ipmi-lstp<n>.

By default, each message on that node carries a one-byte message number
between the length and the payload, and a response must echo the number
of the request it answers. The number exists only on the node, not in
the LSTP protocol, and no other BMC-side IPMI driver has it. It names
the request a response is for: when the host has sent a newer request,
a late response to the older one is dropped, even if userspace has
already read the newer request. Without the number, a response answers
the request read last. With CONFIG_USB_LSTP_IPMI_MSG_NUM=n the node uses
ssif_bmc's struct ipmi_ssif_msg layout instead, so userspace written for
ssif_bmc works unchanged. Corey: is the message-number layout
acceptable as an LSTP-specific extension, or would you prefer it as an
option of the generic BMC message format (and ssif_bmc)?

Responses carry no tag
----------------------

The protocol has no request tag: the first response after a request is
its answer. Two places in the series depend on that:
- The core (patch 5) drops a response that arrives when no request is
  waiting, or after the waiting request already has its answer, and
  counts it in the channel's rx_late statistic. A surplus response (a
  reply to a request that timed out, one left from before a rebind, or
  a device answering twice) therefore cannot overwrite a response the
  waiter is still reading.
- The GPIO driver (patch 6) reads every pin's interrupt configuration
  at probe. A late reply can answer the next request and shift every
  later pin's state by one, so a single read is not retried on its
  own: after a timeout, all pins are read again, in alternating order,
  until two consecutive passes without a timeout agree. That is at most
  five passes, ending within 10 s of the first timeout, or the probe
  fails as it would without the re-read. The bound is there because
  the probe runs in the context that added the device, often the hub's
  worker.

The DT nodes for IPMI and UART channels
---------------------------------------

The USB binding (patch 4) defines serial@N and ipmi@N channel nodes with
only compatible, reg and status. They exist for status = "disabled",
which stops a channel the firmware reports from being created on a
board where it must not be exposed. Without a node, a channel is still
created. The binding shape, including one vendor property
(nvidia,retries on the I2C channel), is discussed in a separate mail to
the DT maintainers sent with this series:
https://lore.kernel.org/r/20261012-lstp-dt-rfc-v1-aambrozewicz@nvidia.com

What is not in this series
--------------------------

- The MMIO channel stays out of tree: raw register access, with no
  binding and no kernel consumer.
- No auto-created spidev. The out-of-tree driver has a module parameter
  that creates a spidev on every chip select of an undescribed SPI
  channel; it is not part of this series. With CONFIG_SPI_DYNAMIC,
  userspace does that through the controller's new_device attribute:

    echo "spidev <cs>" > /sys/class/spi_master/spi<B>/new_device

  (Documentation/ABI/testing/sysfs-class-spi-master)
- No separate IPMI POST-code node and no debug fault injection; both
  stay in the out-of-tree package.

Testing
-------

The drivers were tested on arm64 and x86_64 with four kinds of device:
- real endpoints, the SMA and other vendors' implementations, driven by
  the hardware test suite of the out-of-tree package on Raspberry Pi
  hosts;
- a real SMA passed through over USB to QEMU guests running
  v7.3-rc5 with this series;
- an emulated endpoint, a raw-gadget program on dummy_hcd, playing
  scripted protocol scenarios;
- the same emulator mutating its replies at random.

The test plan covers:
- every channel type end to end: SPI flash identification, write and
  read-back verification and throughput through spidev; I2C transfers;
  GPIO values, interrupts, line names and hogs; LED default states,
  brightness and hardware patterns; UART and IPMI traffic; concurrent
  traffic on all channels;
- firmware descriptions: ACPI SSDTs and device tree overlays in many
  variants, including incomplete, mismatched and outdated ones;
- lifetime: channel driver, USB interface and module unbind, rebind and
  removal, USB disconnect and reset, with nodes open and consumers
  attached, under load, and with nodes opened as soon as they appear;
- protocol faults: late, duplicate, missing and malformed replies, and
  a disconnect in the middle of a transfer;
- allocation failures injected across the interface bind and unbind,
  the function driver probes, open, read and module load.

The kernels used KASAN with kmemleak, KCSAN and KMSAN; lockdep with
PROVE_RCU, DEBUG_OBJECTS, DEBUG_LIST, DEBUG_KOBJECT_RELEASE,
DEBUG_ATOMIC_SLEEP and UBSAN; and failslab, fail_page_alloc and
fail-nth for fault injection. None of them reported a problem in these
drivers. The only reports involving them came from gpiolib when a GPIO
chip is removed with lines still requested; the fixes are posted
separately:
https://lore.kernel.org/r/20261012-gpiolib-chip-removal-v1-0-aambrozewicz@nvidia.com

Per patch, on the posted base: each patch builds (allmodconfig, W=1,
LSTP objects) on x86_64 with gcc and clang and on arm64 and arm with
gcc; sparse; kernel-doc; checkpatch --strict (clean apart from two
MACRO_ARG_REUSE checks in patch 5); dt_binding_check on each binding;
make htmldocs adds no warnings.

The out-of-tree package these drivers come from is at
https://github.com/NVIDIA/lstp_module; the version these patches match
is the public-next-v1 tag:
https://github.com/NVIDIA/lstp_module/tree/public-next-v1
It stays as the backport for kernels without this series.

Base: usb-next at bbf6b99a57b2 ("usb: typec: ucsi_ccg: Add retry
mechanism in ccg_read").

Adrian Ambrozewicz (11):
  dt-bindings: gpio: Add NVIDIA LSTP GPIO
  dt-bindings: i2c: Add NVIDIA LSTP I2C
  dt-bindings: spi: Add NVIDIA LSTP SPI
  dt-bindings: usb: Add NVIDIA LSTP USB adapter
  usb: misc: Add NVIDIA LSTP USB adapter driver
  gpio: Add NVIDIA LSTP GPIO driver
  leds: Add NVIDIA LSTP LED driver
  i2c: Add NVIDIA LSTP I2C driver
  spi: Add NVIDIA LSTP SPI driver
  tty: Add NVIDIA LSTP UART driver
  ipmi: Add NVIDIA LSTP IPMI BMC driver

 .../ABI/testing/sysfs-bus-usb-devices-lstp    |  331 ++
 .../bindings/gpio/nvidia,lstp-gpio.yaml       |  276 ++
 .../bindings/i2c/nvidia,lstp-i2c.yaml         |   71 +
 .../bindings/spi/nvidia,lstp-spi.yaml         |   68 +
 .../devicetree/bindings/usb/nvidia,lstp.yaml  |  280 ++
 Documentation/driver-api/usb/index.rst        |    1 +
 Documentation/driver-api/usb/lstp.rst         |  276 ++
 Documentation/firmware-guide/acpi/index.rst   |    1 +
 Documentation/firmware-guide/acpi/lstp.rst    |  419 +++
 MAINTAINERS                                   |   21 +
 drivers/char/ipmi/Kconfig                     |   32 +
 drivers/char/ipmi/Makefile                    |    1 +
 drivers/char/ipmi/ipmi-lstp.c                 |  464 +++
 drivers/gpio/Kconfig                          |   14 +
 drivers/gpio/Makefile                         |    1 +
 drivers/gpio/gpio-lstp.c                      | 1194 +++++++
 drivers/i2c/busses/Kconfig                    |   13 +
 drivers/i2c/busses/Makefile                   |    1 +
 drivers/i2c/busses/i2c-lstp.c                 |  573 ++++
 drivers/leds/Kconfig                          |   14 +
 drivers/leds/Makefile                         |    1 +
 drivers/leds/leds-lstp.c                      |  404 +++
 drivers/spi/Kconfig                           |   13 +
 drivers/spi/Makefile                          |    1 +
 drivers/spi/spi-lstp.c                        |  316 ++
 drivers/tty/Kconfig                           |   13 +
 drivers/tty/Makefile                          |    1 +
 drivers/tty/lstp_tty.c                        |  829 +++++
 drivers/usb/misc/Kconfig                      |   15 +
 drivers/usb/misc/Makefile                     |    1 +
 drivers/usb/misc/lstp.c                       | 2983 +++++++++++++++++
 include/linux/usb/lstp.h                      |  219 ++
 32 files changed, 8847 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-usb-devices-lstp
 create mode 100644 Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,lstp-i2c.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/nvidia,lstp-spi.yaml
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,lstp.yaml
 create mode 100644 Documentation/driver-api/usb/lstp.rst
 create mode 100644 Documentation/firmware-guide/acpi/lstp.rst
 create mode 100644 drivers/char/ipmi/ipmi-lstp.c
 create mode 100644 drivers/gpio/gpio-lstp.c
 create mode 100644 drivers/i2c/busses/i2c-lstp.c
 create mode 100644 drivers/leds/leds-lstp.c
 create mode 100644 drivers/spi/spi-lstp.c
 create mode 100644 drivers/tty/lstp_tty.c
 create mode 100644 drivers/usb/misc/lstp.c
 create mode 100644 include/linux/usb/lstp.h


base-commit: bbf6b99a57b2479bf51c3c8ad0946f4b8c33ed69
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO
  2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
@ 2026-10-12 17:35 ` Adrian Ambrozewicz
  2026-10-12 17:43   ` sashiko-bot
  2026-10-13  6:09   ` Krzysztof Kozlowski
  2026-10-12 17:36 ` [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver Adrian Ambrozewicz
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 17:35 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Donald Shannon, linux-usb, linux-gpio, devicetree, linux-kernel,
	Greg Kroah-Hartman

Add the binding for a GPIO channel of an NVIDIA LSTP USB adapter. The
node is a child of the adapter's USB device or interface node, matched
to a channel the device reports by reg (the channel ID) and compatible
(the channel type).

The device firmware reports the number of lines and their names and
directions; the node adds line names, hogs and interrupt consumers. Pins
the firmware reports as LEDs are LEDs, not GPIO lines, and an optional
leds child node describes them.

Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
 .../bindings/gpio/nvidia,lstp-gpio.yaml       | 276 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 283 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
new file mode 100644
index 000000000000..4ca5081e509d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
@@ -0,0 +1,276 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/nvidia,lstp-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA LSTP GPIO Channel
+
+maintainers:
+  - Adrian Ambrozewicz <aambrozewicz@nvidia.com>
+  - Donald Shannon <donalds@nvidia.com>
+
+description: |
+  GPIO controller channel on an NVIDIA LSTP USB device.
+
+  The number of GPIO lines and their default names and directions are
+  reported by the device firmware.
+
+  gpio-line-names overrides the firmware-reported pin names.  The
+  gpio-controller and #gpio-cells properties let consumers (e.g.
+  gpio-leds, gpio-keys) reference the lines by phandle.
+
+  The channel is also an interrupt controller.  A consumer may reference
+  a line through a *-gpios property and use it as an interrupt, or
+  reference it through interrupts-extended, or interrupt-parent and
+  interrupts, using the interrupt-controller and #interrupt-cells
+  properties.  An interrupt line must be an input; the firmware-reported
+  pin direction is authoritative and is not changed at runtime.
+
+  GPIO hog child nodes are supported for boot-time pin configuration.
+
+  Pins the device firmware reports as LEDs are not GPIO lines; each is
+  an LED instead, named after the firmware pin name.  An optional leds
+  child node describes some of them: their names, initial state, default
+  trigger, and a pattern the device plays on its own.
+
+  Because the controller sits behind USB, every line access is a USB
+  transaction that blocks until it completes.  Consumers that need
+  non-blocking line access cannot use this controller.
+
+properties:
+  compatible:
+    const: nvidia,lstp-gpio
+
+  reg:
+    description: Channel ID reported by the device firmware.
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description: |
+      Two cells: pin number and flags.
+      Cell 0: GPIO pin offset (0-based).
+      Cell 1: GPIO flags (e.g. GPIO_ACTIVE_LOW).
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+    description: |
+      Two cells: GPIO line offset and trigger flags.
+      Cell 0: GPIO line offset (0-based), same numbering as #gpio-cells.
+      Cell 1: Trigger flags (see <dt-bindings/interrupt-controller/irq.h>).
+
+  gpio-line-names:
+    description: |
+      Optional list of human-readable names for each GPIO line.
+      Overrides the firmware-reported pin names.  The number of entries
+      must not exceed the number of GPIO lines reported by firmware.
+    minItems: 1
+    maxItems: 255
+
+  leds:
+    type: object
+    description: |
+      LED pins of the channel.  A pin the device firmware reports as an
+      LED is an LED whether or not it has a node here, and a node for a
+      pin the firmware does not report as an LED is ignored.
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+    patternProperties:
+      "^led@[0-9a-f]+$":
+        type: object
+        $ref: /schemas/leds/common.yaml#
+        unevaluatedProperties: false
+        description: |
+          One LED pin.  The LED takes its name from label, or from color,
+          function and function-enumerator, instead of the firmware pin
+          name.
+
+        properties:
+          reg:
+            description: Pin number, in the channel's own pin numbering.
+            maxItems: 1
+
+          default-state:
+            description: |
+              Written to the device once, when the LED is set up, and "off"
+              when absent, as in leds/common.yaml.  "keep" leaves the LED as
+              the device firmware set it; the device cannot report its
+              current state.  Not written for a node with led-pattern.
+
+          led-pattern:
+            description: |
+              A pattern the device plays in a loop once the LED is set up.
+              Each entry is <brightness duration>: brightness from 0 to
+              255, held or faded towards for duration milliseconds,
+              depending on the pin's firmware-reported LED mode.  The
+              pattern is sent to the device once and played there without
+              further USB traffic.  It must fit in a single request, which
+              for a device with 64-byte USB endpoints is ten entries; a
+              longer pattern is not sent.
+            minItems: 1
+            maxItems: 255
+            items:
+              items:
+                - description: Brightness
+                  maximum: 255
+                - description: Duration in milliseconds
+
+          active-low: false
+          flash-max-microamp: false
+          flash-max-timeout-us: false
+          inactive-high-impedance: false
+          led-max-microamp: false
+          led-sources: false
+          max-brightness: false
+          panic-indicator: false
+          retain-state-shutdown: false
+          retain-state-suspended: false
+
+        required:
+          - reg
+
+        anyOf:
+          - required: [label]
+          - required: [function]
+
+        dependentSchemas:
+          led-pattern:
+            properties:
+              default-state: false
+              linux,default-trigger: false
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+
+    additionalProperties: false
+
+patternProperties:
+  "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
+    type: object
+    required:
+      - gpio-hog
+
+dependentRequired:
+  interrupt-controller: ["#interrupt-cells"]
+  "#interrupt-cells": [interrupt-controller]
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    /* Minimal: let firmware define pin count and names */
+    interface {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio@3 {
+            compatible = "nvidia,lstp-gpio";
+            reg = <3>;
+            gpio-controller;
+            #gpio-cells = <2>;
+        };
+    };
+  - |
+    /* Override pin names and add a hog */
+    interface {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio_ch: gpio@3 {
+            compatible = "nvidia,lstp-gpio";
+            reg = <3>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            gpio-line-names = "STATUS_LED", "ALERT_INT";
+
+            led-hog {
+                gpio-hog;
+                gpios = <0 0>;
+                output-high;
+                line-name = "status-led-hog";
+            };
+        };
+    };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    /* Act as an interrupt controller for a consumer via interrupts-extended */
+    interface {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio_irq_ch: gpio@3 {
+            compatible = "nvidia,lstp-gpio";
+            reg = <3>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
+
+    gpio-keys {
+        compatible = "gpio-keys";
+
+        key-alert {
+            label = "alert";
+            linux,code = <143>; /* KEY_WAKEUP */
+            /* Line 1 as a falling-edge interrupt source */
+            interrupts-extended = <&gpio_irq_ch 1 IRQ_TYPE_EDGE_FALLING>;
+        };
+    };
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    /* Name two firmware-reported LED pins; one plays a pattern on its own */
+    interface {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio@3 {
+            compatible = "nvidia,lstp-gpio";
+            reg = <3>;
+            gpio-controller;
+            #gpio-cells = <2>;
+
+            leds {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                led@4 {
+                    reg = <4>;
+                    color = <LED_COLOR_ID_GREEN>;
+                    function = LED_FUNCTION_POWER;
+                    default-state = "on";
+                };
+
+                led@5 {
+                    reg = <5>;
+                    color = <LED_COLOR_ID_AMBER>;
+                    function = LED_FUNCTION_STATUS;
+                    /* Two short flashes, then a pause */
+                    led-pattern = <255 100>, <0 100>, <255 100>, <0 700>;
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 65e8a4b5c90b..284000589694 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19565,6 +19565,13 @@ L:	linux-acpi@vger.kernel.org
 S:	Maintained
 F:	drivers/acpi/apei/ghes-nvidia.c
 
+NVIDIA LSTP USB ADAPTER DRIVERS
+M:	Adrian Ambrozewicz <aambrozewicz@nvidia.com>
+M:	Donald Shannon <donalds@nvidia.com>
+L:	linux-usb@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
+
 NVIDIA VRS RTC DRIVER
 M:	Shubhi Garg <shgarg@nvidia.com>
 L:	linux-tegra@vger.kernel.org
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver
  2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
  2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
@ 2026-10-12 17:36 ` Adrian Ambrozewicz
  2026-10-12 17:53   ` sashiko-bot
  2026-10-12 17:36 ` [PATCH 07/11] leds: Add NVIDIA LSTP LED driver Adrian Ambrozewicz
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 17:36 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Donald Shannon, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt, linux-kernel, linux-gpio, linux-usb,
	llvm, Greg Kroah-Hartman

Add a driver for the GPIO channels of NVIDIA LSTP USB adapters. Each
channel is registered as a GPIO chip with an interrupt controller for
its pins; every line access is a USB transaction, so the chip can
sleep. Pins the firmware reports as LEDs are not GPIO lines: the driver
hands them to the LED driver as an auxiliary device.

At probe the driver reads each pin's interrupt configuration from the
device. A reply that misses the response timeout can answer a later
request and shift every following pin's state by one, so a single read
cannot simply be retried. After a timeout the driver reads all pins
again, alternating ascending and descending order, until two
consecutive passes without a timeout agree: at most five passes, ending
within 10 s of the first timeout, after which the probe fails as
before. The probe runs in the context that added the device, often the
hub's worker, hence the bound.

Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
 MAINTAINERS              |    1 +
 drivers/gpio/Kconfig     |   14 +
 drivers/gpio/Makefile    |    1 +
 drivers/gpio/gpio-lstp.c | 1194 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 1210 insertions(+)
 create mode 100644 drivers/gpio/gpio-lstp.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e18d45bd7da5..639cd7b6b146 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19577,6 +19577,7 @@ F:	Documentation/devicetree/bindings/spi/nvidia,lstp-spi.yaml
 F:	Documentation/devicetree/bindings/usb/nvidia,lstp.yaml
 F:	Documentation/driver-api/usb/lstp.rst
 F:	Documentation/firmware-guide/acpi/lstp.rst
+F:	drivers/gpio/gpio-lstp.c
 F:	drivers/usb/misc/lstp.c
 F:	include/linux/usb/lstp.h
 
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a48586bb8edb..7ef6a9031475 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -2051,6 +2051,20 @@ config GPIO_USBIO
 	  This driver can also be built as a module. If so, the module
 	  will be called gpio_usbio.
 
+config GPIO_LSTP
+	tristate "NVIDIA LSTP GPIO support"
+	depends on USB_LSTP
+	default USB_LSTP
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y or M here to support the GPIO channels of NVIDIA LSTP USB
+	  adapters. Each channel is registered as a GPIO chip, together
+	  with an interrupt controller for its pins. Pins the firmware
+	  reports as LEDs are left to the LEDS_LSTP driver.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called gpio-lstp.
+
 endmenu
 
 menu "Virtual GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index dc9e6d643b5b..acd0cfae2914 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_GPIO_LP873X)		+= gpio-lp873x.o
 obj-$(CONFIG_GPIO_LP87565)		+= gpio-lp87565.o
 obj-$(CONFIG_GPIO_LPC18XX)		+= gpio-lpc18xx.o
 obj-$(CONFIG_GPIO_LPC32XX)		+= gpio-lpc32xx.o
+obj-$(CONFIG_GPIO_LSTP)			+= gpio-lstp.o
 obj-$(CONFIG_GPIO_LTC4283)		+= gpio-ltc4283.o
 obj-$(CONFIG_GPIO_MACSMC)		+= gpio-macsmc.o
 obj-$(CONFIG_GPIO_MADERA)		+= gpio-madera.o
diff --git a/drivers/gpio/gpio-lstp.c b/drivers/gpio/gpio-lstp.c
new file mode 100644
index 000000000000..cc988a09582b
--- /dev/null
+++ b/drivers/gpio/gpio-lstp.c
@@ -0,0 +1,1194 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * GPIO driver for LSTP USB interface.
+ *
+ * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.  All rights reserved.
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitmap.h>
+#include <linux/build_bug.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/irq.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/wait_bit.h>
+#include <linux/workqueue.h>
+#include <uapi/linux/gpio.h>
+
+#include <linux/usb/lstp.h>
+
+enum lstp_gpio_cmd {
+	LSTP_GPIO_CMD_GET_VALUE = 0x00,
+	LSTP_GPIO_CMD_SET_VALUE = 0x01,
+	LSTP_GPIO_CMD_GET_IRQ_CONFIG = 0x02,
+	LSTP_GPIO_CMD_SET_IRQ_CONFIG = 0x03,
+	LSTP_GPIO_CMD_IRQ_EVENT = 0x04,
+	/* 0x05, SET_LED_PATTERN, is sent by the LED driver in leds-lstp.c */
+} __packed;
+
+struct lstp_gpio_get_value_req {
+	__le16 pin;
+} __packed;
+
+struct lstp_gpio_set_value_req {
+	__le16 pin;
+	u8 value;
+} __packed;
+
+struct lstp_gpio_get_irq_config_req {
+	__le16 pin;
+} __packed;
+
+struct lstp_gpio_set_irq_config_req {
+	__le16 pin;
+	u8 irq_type;
+} __packed;
+
+struct lstp_gpio_irq_event_req {
+	__le16 pin;
+	u8 value;
+} __packed;
+
+struct lstp_gpio_get_value_resp {
+	u8 value;
+} __packed;
+
+struct lstp_gpio_get_irq_config_resp {
+	u8 irq_type;
+} __packed;
+
+enum lstp_gpio_irq_type {
+	LSTP_GPIO_IRQ_NONE = 0x00,
+	LSTP_GPIO_IRQ_RISING = 0x01,
+	LSTP_GPIO_IRQ_FALLING = 0x02,
+	LSTP_GPIO_IRQ_BOTH = 0x03,
+	LSTP_GPIO_IRQ_HIGH = 0x04,
+	LSTP_GPIO_IRQ_LOW = 0x05,
+	LSTP_GPIO_IRQ_MAX = LSTP_GPIO_IRQ_LOW,
+} __packed;
+
+struct lstp_gpio_pin_config {
+	u8 direction;
+	u8 default_output;
+	u8 output_drive_config;
+	u8 output_persist_state; /* bool */
+	u8 bias_pull_config;
+	__le16 bias_pull_strength; /* 16 ohm increments */
+	__le16 output_drive_strength; /* in mA */
+	__le16 slew_rate; /* in 100 ps (0.1 ns) increments */
+	__le16 input_debounce_time; /* in us */
+	u8 led_support;
+	u8 reserved_0xE;
+	u8 reserved_0xF;
+} __packed;
+
+struct lstp_gpio_pin {
+	char pin_name[LSTP_GPIO_PIN_NAME_LEN];
+	struct lstp_gpio_pin_config config;
+} __packed;
+
+struct lstp_gpio_config {
+	u8 npins;
+	struct lstp_gpio_pin pin_info[];
+} __packed;
+
+/* clang-format off */
+enum lstp_gpio_direction {
+	LSTP_GPIO_OUTPUT = 0x00,
+	LSTP_GPIO_INPUT = 0x01
+} __packed;
+enum lstp_gpio_pin_state {
+	LSTP_GPIO_LOW = 0x00,
+	LSTP_GPIO_HIGH = 0x01
+} __packed;
+enum lstp_gpio_drive_config {
+	LSTP_GPIO_PUSH_PULL = 0x00,
+	LSTP_GPIO_OPEN_DRAIN = 0x01,
+	LSTP_GPIO_OPEN_SOURCE = 0x02
+} __packed;
+enum lstp_gpio_bias_config {
+	LSTP_GPIO_NO_PULL = 0x00,
+	LSTP_GPIO_PULL_UP = 0x01,
+	LSTP_GPIO_PULL_DOWN = 0x02
+} __packed;
+enum lstp_gpio_led_support {
+	LSTP_GPIO_NOT_LED = 0x00,
+	LSTP_GPIO_LED_MODE_HOLD = 0x01, /* pattern brightness jumps between steps */
+	LSTP_GPIO_LED_MODE_INTERPOLATE = 0x02, /* pattern brightness interpolates between steps */
+} __packed; /* clang-format on */
+
+struct lstp_gpio_priv {
+	struct lstp_channel *ch;
+	struct gpio_chip gc;
+	struct workqueue_struct *wq; /* ordered: IRQ batches run in RX order */
+	struct mutex irq_lock; /* Serializes irq_chip callbacks (irq_bus_lock/sync_unlock) */
+	u8 *shadow_irq_type; /* Pending IRQ type per pin [npins] */
+	u8 *hw_irq_type; /* Last successfully committed IRQ type per pin [npins] */
+	const char **pin_names; /* Line names [npins] */
+	unsigned long *led_pins; /* [npins] bitmap: pins handed to the LED driver */
+	u16 npins; /* Pin count from device config */
+	u16 ngpio; /* Non-LED GPIO pin count */
+	u16 nleds; /* LED pin count */
+};
+
+/* One work item per unsolicited RX batch; raw payload parsed in process context */
+struct lstp_gpio_irq_event_work {
+	struct work_struct work;
+	struct lstp_gpio_priv *priv;
+	u16 payload_len;
+	u8 payload[];
+};
+
+/* Read @pin's direction byte out of the channel config descriptor. */
+static int lstp_gpio_read_direction(struct lstp_gpio_priv *priv, unsigned int pin, u8 *direction)
+{
+	if (pin >= priv->npins)
+		return -EINVAL;
+
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_ch0_read_resp *resp __free(kfree) = NULL;
+	u16 pin_offset = sizeof(struct lstp_gpio_config) + pin * sizeof(struct lstp_gpio_pin);
+	u16 offset = pin_offset + offsetof(struct lstp_gpio_pin, config) +
+		     offsetof(struct lstp_gpio_pin_config, direction);
+	size_t actual;
+	int ret = lstp_ch0_read_config(ch, offset, sizeof(*direction), &resp, &actual);
+
+	if (ret) {
+		if (ret != -ENODEV)
+			dev_err_ratelimited(ch->dev,
+					    "%s: ch_%d: Failed to read direction config for pin %u (%pe)\n",
+					    __func__, ch->ch_id, pin, ERR_PTR(ret));
+		return ret;
+	}
+
+	*direction = *(const u8 *)resp->ch_config;
+	return 0;
+}
+
+static int lstp_gpio_get_value(struct gpio_chip *gc, unsigned int pin)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_get_value_req req = { .pin = cpu_to_le16(pin) };
+	struct lstp_gpio_get_value_resp resp;
+
+	int ret = lstp_request(ch, LSTP_GPIO_CMD_GET_VALUE, &req, sizeof(req), &resp, sizeof(resp),
+			       NULL, NULL);
+	if (ret) {
+		if (ret != -ENODEV)
+			dev_err_ratelimited(ch->dev,
+					    "%s: ch_%d: Could not get value for pin %u (%pe)\n",
+					    __func__, ch->ch_id, pin, ERR_PTR(ret));
+		return ret;
+	}
+
+	dev_dbg(ch->dev, "%s: ch_%d: pin=%u, value=%d\n", __func__, ch->ch_id, pin, resp.value);
+	return !!resp.value;
+}
+
+static int lstp_gpio_set_value(struct gpio_chip *gc, unsigned int pin, int value)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_set_value_req req = {
+		.pin = cpu_to_le16(pin),
+		.value = !!value,
+	};
+
+	int ret = lstp_request(ch, LSTP_GPIO_CMD_SET_VALUE, &req, sizeof(req), NULL, 0, NULL, NULL);
+
+	if (ret && ret != -ENODEV)
+		dev_err_ratelimited(ch->dev, "%s: ch_%d: Could not set value %d for pin %u (%pe)\n",
+				    __func__, ch->ch_id, value, pin, ERR_PTR(ret));
+	else if (!ret)
+		dev_dbg(ch->dev, "%s: ch_%d: pin=%u, value=%d\n", __func__, ch->ch_id, pin, value);
+	return ret;
+}
+
+/*
+ * Chunked across packets when the pin count does not fit one request, one
+ * lstp_request() each, so a concurrent access may observe state from between
+ * two chunks.
+ */
+static int lstp_gpio_get_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_get_value_req *req = NULL;
+	struct lstp_gpio_get_value_resp *resp = NULL;
+	int max_pins_per_tx = ch->max_tx_payload / sizeof(*req);
+	int max_pins_per_rx = ch->max_rx_payload / sizeof(*resp);
+	int max_pins_per_pkt = min(max_pins_per_tx, max_pins_per_rx);
+	int total_pins = bitmap_weight(mask, gc->ngpio);
+	unsigned int bit_off = 0;
+	int pins_sent = 0;
+	int ret;
+
+	/* Probe enforces bulk_*_size >= LSTP_USB_EP_MIN_SIZE; anchor it here. */
+	BUILD_BUG_ON(sizeof(struct lstp_header) + sizeof(*req) > LSTP_USB_EP_MIN_SIZE);
+	BUILD_BUG_ON(sizeof(struct lstp_header) + sizeof(*resp) > LSTP_USB_EP_MIN_SIZE);
+
+	if (total_pins == 0)
+		return 0;
+
+	req = kmalloc_objs(*req, max_pins_per_pkt);
+	if (!req) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+	resp = kmalloc_objs(*resp, max_pins_per_pkt);
+	if (!resp) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	bitmap_zero(bits, gc->ngpio);
+
+	while (pins_sent < total_pins) {
+		int pins_in_pkt = min(max_pins_per_pkt, total_pins - pins_sent);
+
+		for (int i = 0; i < pins_in_pkt; i++) {
+			bit_off = find_next_bit(mask, gc->ngpio, bit_off);
+			req[i].pin = cpu_to_le16(bit_off);
+			bit_off++;
+		}
+
+		ret = lstp_request(ch, LSTP_GPIO_CMD_GET_VALUE, req, pins_in_pkt * sizeof(*req),
+				   resp, pins_in_pkt * sizeof(*resp), NULL, NULL);
+		if (ret) {
+			if (ret != -ENODEV)
+				dev_err_ratelimited(ch->dev,
+						    "%s: ch_%d: Could not get values for %d pins (%pe)\n",
+						    __func__, ch->ch_id, pins_in_pkt, ERR_PTR(ret));
+			goto out_free;
+		}
+
+		for (int i = 0; i < pins_in_pkt; i++) {
+			if (resp[i].value)
+				__set_bit(le16_to_cpu(req[i].pin), bits);
+		}
+		pins_sent += pins_in_pkt;
+	}
+
+	dev_dbg(ch->dev, "%s: ch_%d: Get %d pins, mask=%*pb, bits=%*pb\n", __func__, ch->ch_id,
+		total_pins, gc->ngpio, mask, gc->ngpio, bits);
+	ret = 0;
+
+out_free:
+	kfree(resp);
+	kfree(req);
+	return ret;
+}
+
+/* Chunked like lstp_gpio_get_multiple(); same cross-chunk visibility caveat. */
+static int lstp_gpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_set_value_req *req = NULL;
+	int max_pins_per_pkt = ch->max_tx_payload / sizeof(*req);
+	int total_pins = bitmap_weight(mask, gc->ngpio);
+	unsigned int bit_off = 0;
+	int pins_sent = 0;
+	int ret = 0;
+
+	/* Probe enforces bulk_tx_size >= LSTP_USB_EP_MIN_SIZE; anchor it here. */
+	BUILD_BUG_ON(sizeof(struct lstp_header) + sizeof(*req) > LSTP_USB_EP_MIN_SIZE);
+
+	if (total_pins == 0)
+		goto done;
+
+	req = kmalloc_objs(*req, max_pins_per_pkt);
+	if (!req) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
+	while (pins_sent < total_pins) {
+		int pins_in_pkt = min(max_pins_per_pkt, total_pins - pins_sent);
+
+		for (int i = 0; i < pins_in_pkt; i++) {
+			bit_off = find_next_bit(mask, gc->ngpio, bit_off);
+			req[i].pin = cpu_to_le16(bit_off);
+			req[i].value = test_bit(bit_off, bits) ? 1 : 0;
+			bit_off++;
+		}
+
+		ret = lstp_request(ch, LSTP_GPIO_CMD_SET_VALUE, req, pins_in_pkt * sizeof(*req),
+				   NULL, 0, NULL, NULL);
+		if (ret) {
+			if (ret != -ENODEV)
+				dev_err_ratelimited(ch->dev,
+						    "%s: ch_%d: Could not set values for pins (mask=%*pb) (%pe)\n",
+						    __func__, ch->ch_id, gc->ngpio, mask,
+						    ERR_PTR(ret));
+			goto out_free;
+		}
+		pins_sent += pins_in_pkt;
+	}
+
+	dev_dbg(ch->dev, "%s: ch_%d: Set %d pins, mask=%*pb, bits=%*pb\n", __func__, ch->ch_id,
+		total_pins, gc->ngpio, mask, gc->ngpio, bits);
+
+out_free:
+	kfree(req);
+done:
+	return ret;
+}
+
+/* Read @pin's IRQ type from the device; returns LSTP_GPIO_IRQ_* or -errno. */
+static int lstp_gpio_get_irq_config(struct lstp_gpio_priv *priv, u16 pin)
+{
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_get_irq_config_req req = { .pin = cpu_to_le16(pin) };
+	struct lstp_gpio_get_irq_config_resp resp;
+
+	int ret = lstp_request(ch, LSTP_GPIO_CMD_GET_IRQ_CONFIG, &req, sizeof(req), &resp,
+			       sizeof(resp), NULL, NULL);
+	if (ret) {
+		dev_err(ch->dev, "%s: ch_%d: GET_IRQ_CONFIG failed for pin %u (%pe)\n", __func__,
+			ch->ch_id, pin, ERR_PTR(ret));
+		return ret;
+	}
+
+	return resp.irq_type;
+}
+
+static int lstp_gpio_set_irq_config(struct lstp_gpio_priv *priv, u16 pin, u8 irq_type)
+{
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_gpio_set_irq_config_req req = {
+		.pin = cpu_to_le16(pin),
+		.irq_type = irq_type,
+	};
+
+	int ret = lstp_request(ch, LSTP_GPIO_CMD_SET_IRQ_CONFIG, &req, sizeof(req), NULL, 0, NULL,
+			       NULL);
+	if (ret && ret != -ENODEV)
+		dev_err_ratelimited(ch->dev, "%s: ch_%d: SET_IRQ_CONFIG failed for pin %u (%pe)\n",
+				    __func__, ch->ch_id, pin, ERR_PTR(ret));
+	return ret;
+}
+
+/*
+ * 0 if @pin's device-side direction matches @direction, else -EOPNOTSUPP: the
+ * device owns pin direction and the driver never changes it.
+ */
+static int lstp_gpio_require_direction(struct lstp_gpio_priv *priv, unsigned int pin, u8 direction)
+{
+	u8 cur_direction;
+
+	int ret = lstp_gpio_read_direction(priv, pin, &cur_direction);
+
+	if (ret)
+		return ret;
+
+	if (cur_direction == direction)
+		return 0;
+
+	return -EOPNOTSUPP;
+}
+
+static int lstp_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	u8 direction;
+
+	int ret = lstp_gpio_read_direction(priv, pin, &direction);
+
+	if (ret)
+		return ret;
+
+	if (direction == LSTP_GPIO_INPUT)
+		return GPIO_LINE_DIRECTION_IN;
+
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int lstp_gpio_direction_input(struct gpio_chip *gc, unsigned int pin)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+
+	return lstp_gpio_require_direction(priv, pin, LSTP_GPIO_INPUT);
+}
+
+static int lstp_gpio_direction_output(struct gpio_chip *gc, unsigned int pin, int value)
+{
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	int ret = lstp_gpio_require_direction(priv, pin, LSTP_GPIO_OUTPUT);
+
+	if (ret)
+		return ret;
+
+	return lstp_gpio_set_value(gc, pin, value);
+}
+
+static int lstp_gpio_linux_irq_to_lstp(unsigned int type, u8 *irq_type)
+{
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_NONE:
+		*irq_type = LSTP_GPIO_IRQ_NONE;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		*irq_type = LSTP_GPIO_IRQ_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		*irq_type = LSTP_GPIO_IRQ_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		*irq_type = LSTP_GPIO_IRQ_BOTH;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		*irq_type = LSTP_GPIO_IRQ_HIGH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		*irq_type = LSTP_GPIO_IRQ_LOW;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+/* Host-side filter against stale batches or firmware that skips filtering. */
+static bool lstp_gpio_irq_event_matches(u8 irq_type, u8 value)
+{
+	switch (irq_type) {
+	case LSTP_GPIO_IRQ_NONE:
+		return false;
+	case LSTP_GPIO_IRQ_RISING:
+		return value == LSTP_GPIO_HIGH;
+	case LSTP_GPIO_IRQ_FALLING:
+		return value == LSTP_GPIO_LOW;
+	case LSTP_GPIO_IRQ_BOTH:
+		return true;
+	case LSTP_GPIO_IRQ_HIGH:
+		return value == LSTP_GPIO_HIGH;
+	case LSTP_GPIO_IRQ_LOW:
+		return value == LSTP_GPIO_LOW;
+	default:
+		return false;
+	}
+}
+
+/*
+ * Read every GPIO pin's IRQ type into @types, in descending pin order if
+ * @reverse. With @send_by, return -ETIME instead of sending a request after it.
+ */
+static int lstp_gpio_read_irq_types(struct lstp_gpio_priv *priv, u8 *types, bool reverse,
+				    const unsigned long *send_by)
+{
+	for (unsigned int i = 0; i < priv->npins; i++) {
+		unsigned int pin = reverse ? priv->npins - 1 - i : i;
+
+		if (test_bit(pin, priv->led_pins))
+			continue;
+		if (send_by && time_after(jiffies, *send_by))
+			return -ETIME;
+
+		int ret = lstp_gpio_get_irq_config(priv, pin);
+
+		if (ret < 0)
+			return ret;
+		types[pin] = ret;
+	}
+
+	return 0;
+}
+
+#define LSTP_GPIO_IRQ_SYNC_PASSES 5
+#define LSTP_GPIO_IRQ_SYNC_BUDGET_MS 10000
+
+/*
+ * Seed hw_irq_type and shadow_irq_type from the device at probe, so the driver
+ * starts in sync even after a module reload with no device reset.
+ *
+ * A reply carries no tag; the first one after a request is its answer. The
+ * reply to a timed-out request can therefore answer a later request, whose
+ * own reply then answers the request after it, and every pin from there on
+ * reads the type of the pin read before it. So once a pass times out, all
+ * pins are read again until two consecutive passes without a timeout agree.
+ * Each pass follows a response timeout with no request waiting, in which an
+ * owed reply is dropped, and goes in the opposite pin order to the one before.
+ * A pass that agrees with an unshifted one is right, so two passes can agree
+ * on a wrong result only if both were shifted, in opposite directions.
+ *
+ * The probe runs in the context that added the device, often the hub's
+ * worker, so the re-reads and the final wait end within
+ * LSTP_GPIO_IRQ_SYNC_BUDGET_MS of the first timeout, or the probe fails.
+ */
+static int lstp_gpio_sync_hw_irq_state(struct lstp_gpio_priv *priv)
+{
+	struct lstp_channel *ch = priv->ch;
+	u8 *types[2] = { priv->hw_irq_type, priv->shadow_irq_type };
+	bool timed_out = false, have_prev = false;
+	unsigned long send_by = 0;
+	unsigned int active = 0;
+	int pass, ret;
+
+	for (pass = 0; pass < LSTP_GPIO_IRQ_SYNC_PASSES; pass++) {
+		if (timed_out)
+			msleep(LSTP_USB_RESPONSE_TIMEOUT_MS);
+
+		ret = lstp_gpio_read_irq_types(priv, types[pass & 1], pass & 1,
+					       timed_out ? &send_by : NULL);
+		if (ret == -ETIME) {
+			ret = -ETIMEDOUT;
+			break;
+		}
+		if (ret == -ETIMEDOUT) {
+			if (!timed_out) {
+				dev_warn(ch->dev,
+					 "%s: ch_%d: Reading the IRQ config of all pins again\n",
+					 __func__, ch->ch_id);
+				/* Leave room for the last request's timeout and the final wait. */
+				send_by = jiffies +
+					  msecs_to_jiffies(LSTP_GPIO_IRQ_SYNC_BUDGET_MS -
+							   2 * LSTP_USB_RESPONSE_TIMEOUT_MS);
+			}
+			timed_out = true;
+			have_prev = false;
+			continue;
+		}
+		if (ret)
+			return ret;
+		if (!timed_out || (have_prev && !memcmp(types[0], types[1], priv->npins)))
+			break;
+		have_prev = true;
+		ret = -EIO;
+	}
+	if (ret) {
+		dev_err(ch->dev, "%s: ch_%d: No consistent IRQ config (%pe)\n", __func__, ch->ch_id,
+			ERR_PTR(ret));
+		return ret;
+	}
+	/* A reply a shifted pass left owed must not answer the first request after probe. */
+	if (timed_out)
+		msleep(LSTP_USB_RESPONSE_TIMEOUT_MS);
+
+	if (pass & 1)
+		memcpy(priv->hw_irq_type, priv->shadow_irq_type, priv->npins);
+	else
+		memcpy(priv->shadow_irq_type, priv->hw_irq_type, priv->npins);
+	for (unsigned int pin = 0; pin < priv->npins; pin++) {
+		if (test_bit(pin, priv->led_pins))
+			continue;
+		if (priv->hw_irq_type[pin] > LSTP_GPIO_IRQ_MAX)
+			dev_warn(ch->dev, "%s: ch_%d: pin %u has unknown IRQ type %u\n", __func__,
+				 ch->ch_id, pin, priv->hw_irq_type[pin]);
+		if (priv->hw_irq_type[pin] != LSTP_GPIO_IRQ_NONE)
+			active++;
+	}
+
+	if (active)
+		dev_info(ch->dev,
+			 "%s: ch_%d: Synced IRQ state, %u/%u pins with active IRQ config\n",
+			 __func__, ch->ch_id, active, priv->ngpio);
+
+	return 0;
+}
+
+/* Shadow only; SET_IRQ_CONFIG is sent from irq_bus_sync_unlock(). */
+static void lstp_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	unsigned int pin = irqd_to_hwirq(d);
+
+	priv->shadow_irq_type[pin] = LSTP_GPIO_IRQ_NONE;
+	gpiochip_disable_irq(gc, pin);
+}
+
+static void lstp_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	unsigned int pin = irqd_to_hwirq(d);
+	unsigned int type = irqd_get_trigger_type(d);
+	u8 irq_type;
+
+	if (lstp_gpio_linux_irq_to_lstp(type, &irq_type)) {
+		dev_err_ratelimited(priv->ch->dev,
+				    "%s: ch_%d: Invalid trigger type %u for gpio %u\n", __func__,
+				    priv->ch->ch_id, type, pin);
+		return;
+	}
+
+	gpiochip_enable_irq(gc, pin);
+	priv->shadow_irq_type[pin] = irq_type;
+}
+
+/*
+ * Validates the trigger type and nothing else. The callback still has to
+ * exist: with no .irq_set_type, __irq_set_trigger() accepts whatever it was
+ * given without asking the driver. IRQCHIP_SET_TYPE_MASKED makes the core
+ * mask around this call and unmask after, and irq_unmask() is what puts the
+ * type in the shadow.
+ */
+static int lstp_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	u8 irq_type;
+
+	return lstp_gpio_linux_irq_to_lstp(type, &irq_type);
+}
+
+static void lstp_gpio_irq_bus_lock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+
+	mutex_lock(&priv->irq_lock);
+}
+
+/* Commit the shadow if it differs from hardware; revert the shadow on failure. */
+static void lstp_gpio_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct lstp_gpio_priv *priv = gpiochip_get_data(gc);
+	u16 pin = irqd_to_hwirq(d);
+
+	if (priv->shadow_irq_type[pin] != priv->hw_irq_type[pin]) {
+		if (lstp_gpio_set_irq_config(priv, pin, priv->shadow_irq_type[pin]))
+			priv->shadow_irq_type[pin] = priv->hw_irq_type[pin];
+		else
+			priv->hw_irq_type[pin] = priv->shadow_irq_type[pin];
+	}
+
+	mutex_unlock(&priv->irq_lock);
+}
+
+static void lstp_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+
+	seq_puts(p, gc->label ? gc->label : d->chip->name);
+}
+
+static const struct irq_chip lstp_gpio_irq_chip = {
+	.name = "lstp-gpio",
+	.irq_mask = lstp_gpio_irq_mask,
+	.irq_unmask = lstp_gpio_irq_unmask,
+	.irq_set_type = lstp_gpio_irq_set_type,
+	.irq_bus_lock = lstp_gpio_irq_bus_lock,
+	.irq_bus_sync_unlock = lstp_gpio_irq_bus_sync_unlock,
+	.irq_print_chip = lstp_gpio_irq_print_chip,
+	.flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+/*
+ * Deferred GPIO IRQ batch handler. handle_nested_irq() runs the line's
+ * threaded handler inline in this context, and the ordered workqueue
+ * serialises batches, so events reach handlers in USB receive order.
+ */
+static void lstp_gpio_irq_event_work(struct work_struct *work)
+{
+	struct lstp_gpio_irq_event_work *batch =
+		container_of(work, struct lstp_gpio_irq_event_work, work);
+	struct lstp_gpio_priv *priv = batch->priv;
+	struct lstp_channel *ch = priv->ch;
+	struct gpio_chip *gc = &priv->gc;
+	const struct lstp_gpio_irq_event_req *event;
+	const struct lstp_gpio_irq_event_req *end;
+
+	/* Probe enforces bulk_rx_size >= LSTP_USB_EP_MIN_SIZE; anchor base here. */
+	BUILD_BUG_ON(sizeof(struct lstp_header) + sizeof(*event) > LSTP_USB_EP_MIN_SIZE);
+
+	if (batch->payload_len < sizeof(*event)) {
+		dev_err_ratelimited(ch->dev,
+				    "%s: ch_%d: IRQ event packet too small (got %u, need >=%zu)\n",
+				    __func__, ch->ch_id, batch->payload_len, sizeof(*event));
+		goto out;
+	}
+
+	if (batch->payload_len % sizeof(*event)) {
+		dev_err_ratelimited(ch->dev,
+				    "%s: ch_%d: Malformed IRQ event payload (len %u, record size %zu)\n",
+				    __func__, ch->ch_id, batch->payload_len, sizeof(*event));
+		goto out;
+	}
+
+	end = (void *)(batch->payload + batch->payload_len);
+	for (event = (void *)batch->payload; event < end; event++) {
+		u16 pin = le16_to_cpu(event->pin);
+
+		if (pin >= priv->npins || test_bit(pin, priv->led_pins)) {
+			dev_warn_ratelimited(ch->dev, "%s: ch_%d: IRQ on non-GPIO pin %u\n",
+					     __func__, ch->ch_id, pin);
+			continue;
+		}
+
+		u8 value = event->value;
+
+		dev_dbg(ch->dev, "%s: ch=%d: IRQ event: pin=%u, value=%u\n", __func__, ch->ch_id,
+			pin, value);
+
+		if (!lstp_gpio_irq_event_matches(priv->hw_irq_type[pin], value)) {
+			dev_dbg(ch->dev, "%s: ch_%d: Skipping pin %u value %u (irq_type %u)\n",
+				__func__, ch->ch_id, pin, value, priv->hw_irq_type[pin]);
+			continue;
+		}
+
+		int irq = irq_find_mapping(gc->irq.domain, pin);
+
+		if (irq) {
+			handle_nested_irq(irq);
+			dev_dbg(ch->dev, "%s: ch_%d: Nested IRQ %d handled for pin %u\n", __func__,
+				ch->ch_id, irq, pin);
+		} else {
+			dev_warn_ratelimited(ch->dev,
+					     "%s: ch_%d: No IRQ mapping found for pin %u\n",
+					     __func__, ch->ch_id, pin);
+		}
+	}
+
+out:
+	kfree(batch);
+}
+
+/*
+ * ops->irq_packet: copy the payload out and queue it. IRQ_EVENT is never
+ * answered, so a batch dropped here is a lost edge, not a deferred one.
+ */
+static void lstp_gpio_irq_event(struct lstp_channel *ch, struct lstp_packet *pkt)
+{
+	struct lstp_gpio_priv *priv = ch->priv;
+
+	/* No GPIO lines: no irqchip and no workqueue, so nothing to deliver. */
+	if (!priv->ngpio) {
+		dev_warn_ratelimited(ch->dev,
+				     "%s: ch_%d: Dropping IRQ event on a channel without GPIO lines\n",
+				     __func__, ch->ch_id);
+		return;
+	}
+
+	u16 payload_len = le16_to_cpu(pkt->hdr.length);
+
+	if (!payload_len)
+		return;
+
+	if (payload_len > ch->max_rx_payload) {
+		dev_warn_ratelimited(ch->dev,
+				     "%s: ch_%d: Dropping IRQ payload too large (%u > %zu bytes)\n",
+				     __func__, ch->ch_id, payload_len, ch->max_rx_payload);
+		return;
+	}
+
+	struct lstp_gpio_irq_event_work *batch =
+		kzalloc_flex(*batch, payload, payload_len, GFP_ATOMIC);
+	if (!batch)
+		return;
+
+	batch->priv = priv;
+	batch->payload_len = payload_len;
+	memcpy(batch->payload, pkt->payload, payload_len);
+
+	INIT_WORK(&batch->work, lstp_gpio_irq_event_work);
+	if (!queue_work(priv->wq, &batch->work)) {
+		dev_warn_ratelimited(ch->dev,
+				     "%s: ch_%d: Failed to queue IRQ event work (%u bytes)\n",
+				     __func__, ch->ch_id, payload_len);
+		kfree(batch);
+	}
+}
+
+/*
+ * HOLD and INTERPOLATE differ only in device-side rendering; the host treats
+ * both as LED pins, which are excluded from the gpiochip.
+ */
+static bool lstp_gpio_pin_is_led(u8 led_support)
+{
+	return led_support == LSTP_GPIO_LED_MODE_HOLD ||
+	       led_support == LSTP_GPIO_LED_MODE_INTERPOLATE;
+}
+
+static void lstp_gpio_clear_led_valid_mask(struct lstp_gpio_priv *priv, unsigned long *valid_mask,
+					   unsigned int ngpios)
+{
+	bitmap_andnot(valid_mask, valid_mask, priv->led_pins, ngpios);
+}
+
+/* gpio_chip and gpio_irq_chip valid_mask callbacks: LED pins are not lines. */
+static int lstp_gpio_init_valid_mask(struct gpio_chip *gc, unsigned long *valid_mask,
+				     unsigned int ngpios)
+{
+	lstp_gpio_clear_led_valid_mask(gpiochip_get_data(gc), valid_mask, ngpios);
+	return 0;
+}
+
+static void lstp_gpio_irq_init_valid_mask(struct gpio_chip *gc, unsigned long *valid_mask,
+					  unsigned int ngpios)
+{
+	lstp_gpio_clear_led_valid_mask(gpiochip_get_data(gc), valid_mask, ngpios);
+}
+
+static void lstp_gpio_destroy_wq(void *wq)
+{
+	destroy_workqueue(wq);
+}
+
+static int lstp_gpio_create_gpiochip(struct lstp_gpio_priv *priv)
+{
+	struct lstp_channel *ch = priv->ch;
+	struct device *dev = ch->dev;
+	struct gpio_chip *gc = &priv->gc;
+
+	gc->label = ch->display_name;
+	gc->owner = THIS_MODULE;
+	gc->names = priv->pin_names;
+	gc->base = -1;
+	gc->ngpio = priv->npins;
+	gc->get = lstp_gpio_get_value;
+	gc->set = lstp_gpio_set_value;
+	gc->can_sleep = true;
+	gc->get_multiple = lstp_gpio_get_multiple;
+	gc->set_multiple = lstp_gpio_set_multiple;
+	gc->get_direction = lstp_gpio_get_direction;
+	gc->direction_input = lstp_gpio_direction_input;
+	gc->direction_output = lstp_gpio_direction_output;
+	gc->init_valid_mask = lstp_gpio_init_valid_mask;
+	gc->parent = dev;
+	gc->fwnode = ch->fwnode;
+
+	return 0;
+}
+
+static int lstp_gpio_create_irq_chip(struct lstp_gpio_priv *priv)
+{
+	struct lstp_channel *ch = priv->ch;
+	struct device *dev = ch->dev;
+	struct gpio_chip *gc = &priv->gc;
+	struct gpio_irq_chip *girq = &gc->irq;
+
+	if (!priv->ngpio)
+		return 0;
+
+	priv->shadow_irq_type =
+		devm_kcalloc(dev, priv->npins, sizeof(*priv->shadow_irq_type), GFP_KERNEL);
+	if (!priv->shadow_irq_type)
+		return -ENOMEM;
+
+	priv->hw_irq_type = devm_kcalloc(dev, priv->npins, sizeof(*priv->hw_irq_type), GFP_KERNEL);
+	if (!priv->hw_irq_type)
+		return -ENOMEM;
+
+	gpio_irq_chip_set_chip(girq, &lstp_gpio_irq_chip);
+	girq->init_valid_mask = lstp_gpio_irq_init_valid_mask;
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_simple_irq;
+	girq->threaded = true;
+
+	mutex_init(&priv->irq_lock);
+
+	priv->wq = alloc_ordered_workqueue("lstp_gpio_wq", 0);
+	if (!priv->wq) {
+		dev_err(dev, "%s: ch_%d: Failed to create ordered IRQ workqueue\n", __func__,
+			ch->ch_id);
+		return -ENOMEM;
+	}
+
+	int ret = devm_add_action_or_reset(dev, lstp_gpio_destroy_wq, priv->wq);
+
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * Claim @pin for the LED driver if it is an output whose firmware config
+ * reports HOLD or INTERPOLATE; otherwise -EINVAL, and the pin stays a GPIO.
+ */
+static int lstp_gpio_claim_led(struct lstp_gpio_priv *priv, u16 pin, u8 direction, u8 led_support)
+{
+	struct lstp_channel *ch = priv->ch;
+
+	if (!lstp_gpio_pin_is_led(led_support))
+		return -EINVAL;
+
+	if (direction != LSTP_GPIO_OUTPUT) {
+		dev_warn(ch->dev,
+			 "%s: ch_%d: led_support=0x%02x for pin %u but direction is input, treating as GPIO\n",
+			 __func__, ch->ch_id, led_support, pin);
+		return -EINVAL;
+	}
+
+	set_bit(pin, priv->led_pins);
+	return 0;
+}
+
+/*
+ * LED children not yet released. Their release runs this module's code and
+ * can come after the GPIO driver's unbind, so module exit, and an init that
+ * fails, wait until none remain.
+ */
+static atomic_t lstp_gpio_live_leds = ATOMIC_INIT(0);
+
+/*
+ * Deletion drops the child's parent reference, so the child pins the channel
+ * itself: this release then runs before the channel's, which the core's
+ * module exit waits for.
+ */
+static void lstp_gpio_leds_release(struct device *dev)
+{
+	struct lstp_leds *leds = container_of(to_auxiliary_dev(dev), struct lstp_leds, auxdev);
+	struct device *parent = leds->ch->dev;
+
+	kfree(leds);
+	put_device(parent);
+	if (atomic_dec_and_test(&lstp_gpio_live_leds))
+		wake_up_var(&lstp_gpio_live_leds);
+}
+
+static void lstp_gpio_leds_remove(void *data)
+{
+	struct auxiliary_device *auxdev = data;
+
+	auxiliary_device_delete(auxdev);
+	auxiliary_device_uninit(auxdev);
+}
+
+/*
+ * Hand the LED pins to the LED driver on an auxiliary child named after the
+ * channel's instance @id. Best-effort: on failure the GPIOs stay up. The child
+ * goes with this driver's devres, while the channel can still carry the LED
+ * driver's last requests.
+ */
+static void lstp_gpio_add_leds(struct lstp_gpio_priv *priv, u32 id)
+{
+	struct lstp_channel *ch = priv->ch;
+	struct lstp_leds *leds;
+	unsigned int pin;
+	u16 i = 0;
+	int ret;
+
+	if (!priv->nleds)
+		return;
+
+	leds = kzalloc_flex(*leds, pins, priv->nleds);
+	if (!leds) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	leds->npins = priv->nleds;
+	leds->ch = ch;
+	for_each_set_bit(pin, priv->led_pins, priv->npins) {
+		leds->pins[i].pin = pin;
+		strscpy(leds->pins[i].name, priv->pin_names[pin], sizeof(leds->pins[i].name));
+		i++;
+	}
+
+	leds->auxdev.name = "led";
+	leds->auxdev.id = id;
+	leds->auxdev.dev.parent = ch->dev;
+	leds->auxdev.dev.release = lstp_gpio_leds_release;
+
+	ret = auxiliary_device_init(&leds->auxdev);
+	if (ret) {
+		kfree(leds);
+		goto err;
+	}
+	atomic_inc(&lstp_gpio_live_leds);
+	get_device(ch->dev);
+	ret = __auxiliary_device_add(&leds->auxdev, LSTP_LEDS_AUXDEV_MODNAME);
+	if (ret) {
+		auxiliary_device_uninit(&leds->auxdev);
+		goto err;
+	}
+	ret = devm_add_action_or_reset(ch->dev, lstp_gpio_leds_remove, &leds->auxdev);
+	if (ret)
+		goto err;
+	return;
+
+err:
+	dev_warn(ch->dev, "%s: ch_%d: Could not add the device for %u LEDs (%pe)\n", __func__,
+		 ch->ch_id, priv->nleds, ERR_PTR(ret));
+}
+
+/* lstp_read_config_table() callback: classify one pin entry as GPIO or LED. */
+static int lstp_gpio_pin_setup_on_entry(struct lstp_channel *ch, unsigned int pin,
+					const void *entry, void *ctx)
+{
+	struct lstp_gpio_priv *priv = ctx;
+	const struct lstp_gpio_pin *pin_cfg = entry;
+	u8 led_support = pin_cfg->config.led_support;
+	char *name = devm_kzalloc(ch->dev, GPIO_MAX_NAME_SIZE, GFP_KERNEL);
+
+	if (!name)
+		return -ENOMEM;
+	snprintf(name, GPIO_MAX_NAME_SIZE, "%.*s", (int)LSTP_GPIO_PIN_NAME_LEN, pin_cfg->pin_name);
+	priv->pin_names[pin] = name;
+
+	if (lstp_gpio_pin_is_led(led_support)) {
+		if (!lstp_gpio_claim_led(priv, pin, pin_cfg->config.direction, led_support)) {
+			priv->nleds++;
+			return 0;
+		}
+		/* LED config rejected (claim_led logged why); fall back to GPIO. */
+	} else if (led_support != LSTP_GPIO_NOT_LED) {
+		dev_warn(ch->dev,
+			 "%s: ch_%d: Unknown led_support=0x%02x for pin %u, treating as GPIO\n",
+			 __func__, ch->ch_id, led_support, pin);
+	}
+
+	priv->ngpio++;
+	return 0;
+}
+
+static void lstp_gpio_flush_irq_work(void *wq)
+{
+	flush_workqueue(wq);
+}
+
+/* Classifies each configured pin as GPIO or LED, then registers both. */
+static int lstp_gpio_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
+{
+	struct lstp_channel *ch = lstp_channel_from_auxdev(auxdev);
+	const struct lstp_channel_init_info *info = dev_get_platdata(&auxdev->dev);
+	int ret;
+
+	if (!info)
+		return -ENODEV;
+
+	const struct lstp_gpio_config *config = info->config;
+
+	if (info->config_len < sizeof(*config)) {
+		dev_err(ch->dev, "%s: ch_%d: GPIO config too short: %zu < %zu\n", __func__,
+			ch->ch_id, info->config_len, sizeof(*config));
+		return -EINVAL;
+	}
+	if (config->npins == 0) {
+		dev_err(ch->dev, "%s: ch_%d: Invalid npins of 0\n", __func__, ch->ch_id);
+		return -EINVAL;
+	}
+
+	struct lstp_gpio_priv *priv = devm_kzalloc(ch->dev, sizeof(*priv), GFP_KERNEL);
+
+	if (!priv)
+		return -ENOMEM;
+
+	priv->pin_names =
+		devm_kcalloc(ch->dev, config->npins, sizeof(*priv->pin_names), GFP_KERNEL);
+	if (!priv->pin_names)
+		return -ENOMEM;
+
+	priv->led_pins = devm_bitmap_zalloc(ch->dev, config->npins, GFP_KERNEL);
+	if (!priv->led_pins)
+		return -ENOMEM;
+
+	priv->ch = ch;
+	priv->npins = config->npins;
+
+	/* gpiolib bounds chip and line names by GPIO_MAX_NAME_SIZE. */
+	if (strlen(ch->display_name) >= GPIO_MAX_NAME_SIZE) {
+		dev_warn(ch->dev, "%s: ch_%d: display_name truncated to '%.*s' (max %d chars)\n",
+			 __func__, ch->ch_id, GPIO_MAX_NAME_SIZE - 1, ch->display_name,
+			 GPIO_MAX_NAME_SIZE - 1);
+		ch->display_name[GPIO_MAX_NAME_SIZE - 1] = '\0';
+	}
+
+	ret = lstp_gpio_create_gpiochip(priv);
+	if (ret)
+		return ret;
+
+	ret = lstp_read_config_table(ch, sizeof(struct lstp_gpio_config),
+				     sizeof(struct lstp_gpio_pin), priv->npins,
+				     lstp_gpio_pin_setup_on_entry, priv);
+	if (ret) {
+		dev_err(ch->dev, "%s: ch_%d: Could not setup pins (%pe)\n", __func__, ch->ch_id,
+			ERR_PTR(ret));
+		return ret;
+	}
+	if (priv->ngpio == 0 && priv->nleds == 0) {
+		dev_err(ch->dev, "%s: ch_%d: No GPIO or LED pins discovered\n", __func__,
+			ch->ch_id);
+		return -EINVAL;
+	}
+
+	ret = lstp_gpio_create_irq_chip(priv);
+	if (ret)
+		return ret;
+
+	ch->priv = priv;
+
+	if (priv->ngpio) {
+		ret = lstp_gpio_sync_hw_irq_state(priv);
+		if (ret)
+			return ret;
+
+		ret = devm_gpiochip_add_data(ch->dev, &priv->gc, priv);
+		if (ret) {
+			dev_err(ch->dev, "%s: ch_%d: Could not register GPIO chip (%pe)\n",
+				__func__, ch->ch_id, ERR_PTR(ret));
+			return ret;
+		}
+
+		/*
+		 * Booked after the chip so it runs before the chip goes: removal
+		 * frees gc->irq.domain without clearing it, and
+		 * lstp_gpio_irq_event_work() reads it. Dispatch is retracted
+		 * before the auxdev's devres runs, so no batch is queued behind
+		 * the flush.
+		 */
+		ret = devm_add_action_or_reset(ch->dev, lstp_gpio_flush_irq_work, priv->wq);
+		if (ret)
+			return ret;
+	}
+
+	lstp_gpio_add_leds(priv, auxdev->id);
+
+	dev_info(ch->dev, "%s: ch_%d: Started as %s with %u GPIOs and %u LEDs\n", __func__,
+		 ch->ch_id, priv->gc.label, priv->ngpio, priv->nleds);
+	return 0;
+}
+
+static const struct lstp_ops lstp_gpio_ops = {
+	.irq_packet = lstp_gpio_irq_event,
+};
+
+static const struct auxiliary_device_id lstp_gpio_id_table[] = { { .name = "lstp.gpio" }, {} };
+MODULE_DEVICE_TABLE(auxiliary, lstp_gpio_id_table);
+
+static struct lstp_auxiliary_driver lstp_gpio_drv = {
+	.adrv = {
+		.name = "gpio",
+		.probe = lstp_gpio_probe,
+		.id_table = lstp_gpio_id_table,
+	},
+	.ops = &lstp_gpio_ops,
+};
+
+static int __init lstp_gpio_module_init(void)
+{
+	int ret = lstp_auxiliary_driver_register(&lstp_gpio_drv);
+
+	/* A registration that fails after binding has deleted LED children. */
+	if (ret)
+		wait_var_event(&lstp_gpio_live_leds, !atomic_read(&lstp_gpio_live_leds));
+	return ret;
+}
+module_init(lstp_gpio_module_init);
+
+/* The unregister unbinds every channel, which deletes its LED child. */
+static void __exit lstp_gpio_module_exit(void)
+{
+	lstp_auxiliary_driver_unregister(&lstp_gpio_drv);
+	wait_var_event(&lstp_gpio_live_leds, !atomic_read(&lstp_gpio_live_leds));
+}
+module_exit(lstp_gpio_module_exit);
+
+MODULE_IMPORT_NS("LSTP");
+MODULE_DESCRIPTION("LSTP GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 07/11] leds: Add NVIDIA LSTP LED driver
  2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
  2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
  2026-10-12 17:36 ` [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver Adrian Ambrozewicz
@ 2026-10-12 17:36 ` Adrian Ambrozewicz
  2026-10-12 17:46   ` sashiko-bot
  2026-10-13 11:37 ` [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Greg Kroah-Hartman
  2026-10-13 11:39 ` Greg Kroah-Hartman
  4 siblings, 1 reply; 10+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 17:36 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek
  Cc: Donald Shannon, Linus Walleij, Bartosz Golaszewski, linux-kernel,
	linux-leds, linux-usb, linux-gpio, Greg Kroah-Hartman

Add a driver for the LEDs of NVIDIA LSTP USB adapters: the pins of a
GPIO channel that the firmware reports as LEDs, which the GPIO driver
offers as an auxiliary device. Each is registered with the LED class;
blink and pattern requests are handed to the adapter, which plays them
on its own. A leds child node of the channel can name the LEDs and set
their default state, trigger and pattern.

Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
 MAINTAINERS              |   1 +
 drivers/leds/Kconfig     |  14 ++
 drivers/leds/Makefile    |   1 +
 drivers/leds/leds-lstp.c | 404 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 420 insertions(+)
 create mode 100644 drivers/leds/leds-lstp.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 639cd7b6b146..c8ed59cd4b11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19578,6 +19578,7 @@ F:	Documentation/devicetree/bindings/usb/nvidia,lstp.yaml
 F:	Documentation/driver-api/usb/lstp.rst
 F:	Documentation/firmware-guide/acpi/lstp.rst
 F:	drivers/gpio/gpio-lstp.c
+F:	drivers/leds/leds-lstp.c
 F:	drivers/usb/misc/lstp.c
 F:	include/linux/usb/lstp.h
 
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index d0d3182236be..aa843e72a28d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -1050,6 +1050,20 @@ source "drivers/leds/flash/Kconfig"
 comment "RGB LED drivers"
 source "drivers/leds/rgb/Kconfig"
 
+config LEDS_LSTP
+	tristate "LED support for NVIDIA LSTP adapters"
+	depends on LEDS_CLASS
+	depends on GPIO_LSTP
+	default GPIO_LSTP
+	help
+	  Say Y or M here to support the LEDs of NVIDIA LSTP USB adapters,
+	  the pins of the adapter's GPIO channels that the firmware reports
+	  as LEDs. They are registered with the LED class, with blink and
+	  pattern requests handed to the adapter.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called leds-lstp.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index a68244bd50fb..115b88c6b61a 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_LEDS_LP8501)		+= leds-lp8501.o
 obj-$(CONFIG_LEDS_LP8788)		+= leds-lp8788.o
 obj-$(CONFIG_LEDS_LP8860)		+= leds-lp8860.o
 obj-$(CONFIG_LEDS_LP8864)		+= leds-lp8864.o
+obj-$(CONFIG_LEDS_LSTP)			+= leds-lstp.o
 obj-$(CONFIG_LEDS_LT3593)		+= leds-lt3593.o
 obj-$(CONFIG_LEDS_LTC3220)		+= leds-ltc3220.o
 obj-$(CONFIG_LEDS_MAX5970)		+= leds-max5970.o
diff --git a/drivers/leds/leds-lstp.c b/drivers/leds/leds-lstp.c
new file mode 100644
index 000000000000..352e484eb919
--- /dev/null
+++ b/drivers/leds/leds-lstp.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * LED driver for LSTP USB interface.
+ *
+ * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.  All rights reserved.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/build_bug.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#include <linux/usb/lstp.h>
+
+#define LSTP_LED_REPEAT_FOREVER 0 /* wire: repeats == 0 plays forever */
+#define LSTP_LED_LINUX_REPEAT_FOREVER (-1) /* LED-class pattern_set: repeat == -1 plays forever */
+#define LSTP_LED_DEFAULT_HOLD_TIME_MS 1 /* delta_t for single-step holds */
+#define LSTP_LED_DEFAULT_BLINK_MS 500 /* LED-class default when delay_on/off are both 0 */
+
+/* A GPIO channel command; the others are in gpio-lstp.c. */
+#define LSTP_GPIO_CMD_SET_LED_PATTERN 0x05
+
+struct lstp_led_pattern {
+	u8 brightness;
+	__le32 time_ms;
+} __packed;
+
+struct lstp_gpio_set_led_pattern_req {
+	__le16 pin;
+	__le32 repeats;
+	u8 num_steps;
+	struct lstp_led_pattern pattern[];
+} __packed;
+
+struct lstp_led {
+	struct led_classdev cdev;
+	struct lstp_channel *ch;
+	u16 pin;
+};
+
+/* Send SET_LED_PATTERN. @num_steps 0 clears the pattern, 1 holds forever. */
+static int lstp_led_set_pattern(struct lstp_led *led, u32 repeats,
+				const struct led_pattern *pattern, u8 num_steps)
+{
+	struct lstp_channel *ch = led->ch;
+	struct lstp_gpio_set_led_pattern_req *req;
+	size_t req_len = struct_size(req, pattern, num_steps);
+	/* The LED core applies a firmware-node max-brightness; the wire is 8-bit */
+	unsigned int max_brightness = min_t(unsigned int, led->cdev.max_brightness, U8_MAX);
+
+	/* Probe enforces bulk_tx_size >= LSTP_USB_EP_MIN_SIZE; anchor base here. */
+	BUILD_BUG_ON(sizeof(struct lstp_header) + sizeof(*req) > LSTP_USB_EP_MIN_SIZE);
+
+	for (u8 i = 0; i < num_steps; i++) {
+		if (pattern[i].brightness > max_brightness)
+			return -EINVAL;
+	}
+
+	if (req_len > ch->max_tx_payload) {
+		dev_err_ratelimited(ch->dev,
+				    "%s: ch_%d: SET_LED_PATTERN too long for led '%s' (%zu bytes, max %zu)\n",
+				    __func__, ch->ch_id, led->cdev.name, req_len,
+				    ch->max_tx_payload);
+		return -EMSGSIZE;
+	}
+
+	req = kmalloc(req_len, GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->pin = cpu_to_le16(led->pin);
+	req->repeats = cpu_to_le32(repeats);
+	req->num_steps = num_steps;
+	for (u8 i = 0; i < num_steps; i++) {
+		req->pattern[i].brightness = (u8)pattern[i].brightness;
+		req->pattern[i].time_ms = cpu_to_le32(pattern[i].delta_t);
+	}
+
+	int ret =
+		lstp_request(ch, LSTP_GPIO_CMD_SET_LED_PATTERN, req, req_len, NULL, 0, NULL, NULL);
+	if (ret && ret != -ENODEV)
+		dev_err_ratelimited(ch->dev,
+				    "%s: ch_%d: SET_LED_PATTERN failed for led '%s' (repeats=%u, num_steps=%u) (%pe)\n",
+				    __func__, ch->ch_id, led->cdev.name, repeats, num_steps,
+				    ERR_PTR(ret));
+
+	kfree(req);
+	return ret;
+}
+
+/*
+ * blink_set: @delay_on 0 holds OFF, @delay_off 0 holds ON, both 0 selects the
+ * LED-class 500 ms default. On-brightness is cdev->brightness, falling back to
+ * max_brightness when the LED is currently off.
+ */
+static int lstp_led_blink_set(struct led_classdev *cdev, unsigned long *delay_on,
+			      unsigned long *delay_off)
+{
+	struct lstp_led *led = container_of(cdev, struct lstp_led, cdev);
+	u8 on_value = cdev->brightness ? (u8)cdev->brightness : (u8)cdev->max_brightness;
+	struct led_pattern steps[2];
+	u8 len;
+
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = LSTP_LED_DEFAULT_BLINK_MS;
+		*delay_off = LSTP_LED_DEFAULT_BLINK_MS;
+	}
+
+	if (*delay_on == 0) {
+		steps[0].delta_t = LSTP_LED_DEFAULT_HOLD_TIME_MS;
+		steps[0].brightness = LED_OFF;
+		len = 1;
+	} else if (*delay_off == 0) {
+		steps[0].delta_t = LSTP_LED_DEFAULT_HOLD_TIME_MS;
+		steps[0].brightness = on_value;
+		len = 1;
+	} else {
+		steps[0].delta_t = *delay_on;
+		steps[0].brightness = on_value;
+		steps[1].delta_t = *delay_off;
+		steps[1].brightness = LED_OFF;
+		len = 2;
+	}
+	return lstp_led_set_pattern(led, LSTP_LED_REPEAT_FOREVER, steps, len);
+}
+
+/*
+ * pattern_set: LED-class repeat -1 maps to the wire's repeats == 0 ("forever").
+ * repeat == 0 and anything below -1 are invalid.
+ */
+static int lstp_led_pattern_set(struct led_classdev *cdev, struct led_pattern *pattern, u32 len,
+				int repeat)
+{
+	struct lstp_led *led = container_of(cdev, struct lstp_led, cdev);
+
+	if (len > U8_MAX)
+		return -EMSGSIZE;
+	if (repeat == 0 || repeat < LSTP_LED_LINUX_REPEAT_FOREVER)
+		return -EINVAL;
+
+	u32 mapped_repeat = (repeat == LSTP_LED_LINUX_REPEAT_FOREVER) ? LSTP_LED_REPEAT_FOREVER :
+									(u32)repeat;
+	return lstp_led_set_pattern(led, mapped_repeat, pattern, (u8)len);
+}
+
+static int lstp_led_pattern_clear(struct led_classdev *cdev)
+{
+	struct lstp_led *led = container_of(cdev, struct lstp_led, cdev);
+
+	return lstp_led_set_pattern(led, LSTP_LED_REPEAT_FOREVER, NULL, 0);
+}
+
+/* brightness_set_blocking: a one-step pattern; @brightness is core-clamped. */
+static int lstp_led_brightness_set_blocking(struct led_classdev *cdev,
+					    enum led_brightness brightness)
+{
+	struct lstp_led *led = container_of(cdev, struct lstp_led, cdev);
+	struct led_pattern step = { .delta_t = LSTP_LED_DEFAULT_HOLD_TIME_MS,
+				    .brightness = brightness };
+
+	return lstp_led_set_pattern(led, LSTP_LED_REPEAT_FOREVER, &step, 1);
+}
+
+/*
+ * Send @fwnode's led-pattern once, repeating forever; the device then plays it
+ * without host involvement. A pattern that does not fit in one request is not
+ * sent at all.
+ */
+static void lstp_led_send_fwnode_pattern(struct lstp_led *led, struct fwnode_handle *fwnode)
+{
+	struct lstp_channel *ch = led->ch;
+	struct lstp_gpio_set_led_pattern_req *req;
+	size_t max_steps = min_t(size_t, U8_MAX,
+				 (ch->max_tx_payload - sizeof(*req)) / sizeof(req->pattern[0]));
+	int count = fwnode_property_count_u32(fwnode, "led-pattern");
+	struct led_pattern *steps = NULL;
+	unsigned int num_steps;
+	u32 *raw = NULL;
+
+	if (count <= 0 || count % 2) {
+		dev_err(ch->dev,
+			"%s: ch_%d: led-pattern of LED '%s' is not a list of <brightness duration> pairs\n",
+			__func__, ch->ch_id, led->cdev.name);
+		return;
+	}
+
+	num_steps = count / 2;
+	if (num_steps > max_steps) {
+		dev_err(ch->dev,
+			"%s: ch_%d: led-pattern of LED '%s' has %u steps, the device takes at most %zu\n",
+			__func__, ch->ch_id, led->cdev.name, num_steps, max_steps);
+		return;
+	}
+
+	raw = kcalloc(count, sizeof(*raw), GFP_KERNEL);
+	steps = kzalloc_objs(*steps, num_steps);
+	if (!raw || !steps || fwnode_property_read_u32_array(fwnode, "led-pattern", raw, count))
+		goto out;
+
+	for (unsigned int i = 0; i < num_steps; i++) {
+		if (raw[2 * i] > led->cdev.max_brightness) {
+			dev_err(ch->dev,
+				"%s: ch_%d: led-pattern of LED '%s': step %u brightness %u exceeds %u\n",
+				__func__, ch->ch_id, led->cdev.name, i, raw[2 * i],
+				led->cdev.max_brightness);
+			goto out;
+		}
+		steps[i].brightness = raw[2 * i];
+		steps[i].delta_t = raw[2 * i + 1];
+	}
+
+	lstp_led_set_pattern(led, LSTP_LED_REPEAT_FOREVER, steps, num_steps);
+out:
+	kfree(steps);
+	kfree(raw);
+}
+
+/*
+ * default-state is written once, and a missing one means "off". "keep" leaves
+ * the LED as the device firmware set it: the device cannot report its current
+ * state, so there is nothing to read back.
+ */
+static void lstp_led_apply_default_state(struct lstp_led *led, struct fwnode_handle *fwnode)
+{
+	switch (led_init_default_state_get(fwnode)) {
+	case LEDS_DEFSTATE_KEEP:
+		return;
+	case LEDS_DEFSTATE_ON:
+		led->cdev.brightness = led->cdev.max_brightness;
+		break;
+	default:
+		led->cdev.brightness = LED_OFF;
+		break;
+	}
+
+	lstp_led_brightness_set_blocking(&led->cdev, led->cdev.brightness);
+}
+
+static void lstp_led_put_fwnode(void *fwnode)
+{
+	fwnode_handle_put(fwnode);
+}
+
+/*
+ * led_classdev_unregister() flushes set_brightness_work before it removes the
+ * sysfs files, and the LED core does not refuse brightness or trigger writes in
+ * between: such a write would queue work or attach a trigger that outlives
+ * @data. Disable sysfs first, under led_access, so they fail with -EBUSY.
+ */
+static void lstp_led_unregister(void *data)
+{
+	struct led_classdev *cdev = data;
+
+	mutex_lock(&cdev->led_access);
+	led_sysfs_disable(cdev);
+	mutex_unlock(&cdev->led_access);
+	led_classdev_unregister(cdev);
+}
+
+static int lstp_led_classdev_register(struct device *dev, struct led_classdev *cdev,
+				      struct led_init_data *init_data)
+{
+	int ret = led_classdev_register_ext(dev, cdev, init_data);
+
+	if (ret)
+		return ret;
+	return devm_add_action_or_reset(dev, lstp_led_unregister, cdev);
+}
+
+/* Register @led, initialised from @fwnode if any. A failure leaves the other LEDs up. */
+static int lstp_led_register(struct device *dev, struct lstp_led *led, struct fwnode_handle *fwnode)
+{
+	int ret;
+
+	if (!fwnode) {
+		ret = lstp_led_classdev_register(dev, &led->cdev, NULL);
+	} else {
+		struct led_init_data init_data = { .fwnode = fwnode };
+
+		/*
+		 * The class device keeps @fwnode as its node, and default_trigger
+		 * points into it. Queued first, the put runs after unregistration.
+		 */
+		ret = devm_add_action_or_reset(dev, lstp_led_put_fwnode, fwnode_handle_get(fwnode));
+		if (!ret) {
+			bool pattern = fwnode_property_present(fwnode, "led-pattern");
+			bool trigger = fwnode_property_present(fwnode, "linux,default-trigger");
+
+			/* A trigger takes precedence; the LED core applies it from @fwnode. */
+			if (pattern && trigger)
+				dev_err(dev,
+					"%s: ch_%d: Ignoring led-pattern of LED '%s': %pfw also sets linux,default-trigger\n",
+					__func__, led->ch->ch_id, led->cdev.name, fwnode);
+			if (pattern && !trigger)
+				lstp_led_send_fwnode_pattern(led, fwnode);
+			else
+				lstp_led_apply_default_state(led, fwnode);
+			ret = lstp_led_classdev_register(dev, &led->cdev, &init_data);
+		}
+	}
+
+	if (ret)
+		dev_warn(dev, "%s: ch_%d: Could not register LED '%s', pin %u (%pe)\n", __func__,
+			 led->ch->ch_id, led->cdev.name, led->pin, ERR_PTR(ret));
+	return ret;
+}
+
+static struct lstp_led *lstp_led_by_pin(struct lstp_led *leds, u16 nleds, u32 pin)
+{
+	for (u16 i = 0; i < nleds; i++) {
+		if (leds[i].pin == pin)
+			return &leds[i];
+	}
+	return NULL;
+}
+
+/*
+ * Register the LEDs described under the channel's "leds" node, then every other
+ * LED pin under its firmware pin name. A node that does not describe a
+ * firmware-reported LED pin, or that names no LED, is ignored.
+ */
+static int lstp_leds_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
+{
+	struct lstp_leds *pins = container_of(auxdev, struct lstp_leds, auxdev);
+	struct lstp_channel *ch = pins->ch;
+	struct device *dev = &auxdev->dev;
+	struct fwnode_handle *node;
+	struct fwnode_handle *child;
+	struct lstp_led *leds;
+
+	leds = devm_kcalloc(dev, pins->npins, sizeof(*leds), GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	for (u16 i = 0; i < pins->npins; i++) {
+		leds[i].ch = ch;
+		leds[i].pin = pins->pins[i].pin;
+		leds[i].cdev.name = pins->pins[i].name;
+		leds[i].cdev.max_brightness = U8_MAX;
+		leds[i].cdev.flags = LED_HW_PLUGGABLE;
+		leds[i].cdev.brightness_set_blocking = lstp_led_brightness_set_blocking;
+		leds[i].cdev.blink_set = lstp_led_blink_set;
+		leds[i].cdev.pattern_set = lstp_led_pattern_set;
+		leds[i].cdev.pattern_clear = lstp_led_pattern_clear;
+	}
+
+	node = fwnode_get_named_child_node(ch->fwnode, "leds");
+	/*
+	 * Not the available-child iterator; see Documentation/driver-api/usb/lstp.rst.
+	 * On OF, fwnode_for_each_child_node() already skips disabled nodes.
+	 */
+	fwnode_for_each_child_node(node, child) {
+		struct lstp_led *led = NULL;
+		u32 pin;
+
+		if (!fwnode_property_read_u32(child, "reg", &pin))
+			led = lstp_led_by_pin(leds, pins->npins, pin);
+		if (!led) {
+			dev_warn(dev, "%s: ch_%d: Ignoring %pfw: not an LED pin\n", __func__,
+				 ch->ch_id, child);
+			continue;
+		}
+		if (led->cdev.dev) {
+			dev_warn(dev, "%s: ch_%d: Ignoring %pfw: pin %u is already described\n",
+				 __func__, ch->ch_id, child, pin);
+			continue;
+		}
+		if (!fwnode_property_present(child, "label") &&
+		    !fwnode_property_present(child, "function")) {
+			dev_warn(dev, "%s: ch_%d: Ignoring %pfw: neither label nor function\n",
+				 __func__, ch->ch_id, child);
+			continue;
+		}
+		lstp_led_register(dev, led, child);
+	}
+	fwnode_handle_put(node);
+
+	for (u16 i = 0; i < pins->npins; i++) {
+		if (!leds[i].cdev.dev)
+			lstp_led_register(dev, &leds[i], NULL);
+	}
+
+	return 0;
+}
+
+static const struct auxiliary_device_id lstp_leds_id_table[] = { { .name = "lstp.led" }, {} };
+MODULE_DEVICE_TABLE(auxiliary, lstp_leds_id_table);
+
+static struct auxiliary_driver lstp_leds_drv = {
+	.name = "led",
+	.probe = lstp_leds_probe,
+	.id_table = lstp_leds_id_table,
+};
+module_auxiliary_driver(lstp_leds_drv);
+
+MODULE_IMPORT_NS("LSTP");
+MODULE_DESCRIPTION("LSTP LED driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO
  2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
@ 2026-10-12 17:43   ` sashiko-bot
  2026-10-13  6:09   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-12 17:43 UTC (permalink / raw)
  To: Adrian Ambrozewicz; +Cc: robh, linux-gpio, devicetree, linux-usb, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: missing interrupt-controller in required list
--

commit aeb4a4fc6444001bea560a7fa11534922cb0e28e
Author: Adrian Ambrozewicz <aambrozewicz@nvidia.com>

dt-bindings: gpio: Add NVIDIA LSTP GPIO
    
Add the binding for a GPIO channel of an NVIDIA LSTP USB adapter. The
node is a child of the adapter's USB device or interface node, matched
to a channel the device reports by reg and compatible.

> diff --git a/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
> new file mode 100644
> index 0000000000000..4ca5081e509d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml

[ ... ]

> +dependentRequired:
> +  interrupt-controller: ["#interrupt-cells"]
> +  "#interrupt-cells": [interrupt-controller]
> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - "#gpio-cells"

[Severity: Medium]
The documentation states this hardware is also an interrupt controller, but
this required list omits the interrupt-controller and #interrupt-cells
properties.

Should interrupt-controller and #interrupt-cells be moved from
dependentRequired into this unconditional required list? Subsystem rules
mandate that core hardware provider capabilities like an interrupt controller
must be globally required with appropriate constraints. Placing them in
dependentRequired treats them as optional rather than globally required.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    /* Minimal: let firmware define pin count and names */
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio@3 {
> +            compatible = "nvidia,lstp-gpio";
> +            reg = <3>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +        };
> +    };

[Severity: Medium]
Does this example omit the interrupt provider capabilities?

Subsystem rules require that all examples include all required properties.
If the interrupt-controller properties are made globally required, examples
1, 2, and 4 will also need to be updated to include them.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-lstp-usb-adapter-v1-0-aambrozewicz@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 07/11] leds: Add NVIDIA LSTP LED driver
  2026-10-12 17:36 ` [PATCH 07/11] leds: Add NVIDIA LSTP LED driver Adrian Ambrozewicz
@ 2026-10-12 17:46   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-12 17:46 UTC (permalink / raw)
  To: Adrian Ambrozewicz; +Cc: lee, linux-usb, linux-gpio, linux-leds

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] leds: applying initial brightness before parsing DT max-brightness
- [Medium] leds: rejecting DT nodes with only color and allowing deprecated label
- [Low] leds: non-standard commit message prefix missing driver name
--

commit 231bdc9e871148317f849221691f708a54dbd399
Author: Adrian Ambrozewicz <aambrozewicz@nvidia.com>

leds: Add NVIDIA LSTP LED driver
    
This commit introduces a new driver for LEDs on NVIDIA LSTP USB adapters.
It uses the GPIO channel of the adapter, hands blink and pattern requests
directly to the hardware, and configures them via device tree child nodes.

> leds: Add NVIDIA LSTP LED driver

[Severity: Low]
This isn't a bug, but the commit message prefix seems to be missing the
driver name. Should this follow the standard format, such as
"leds: lstp: Add NVIDIA LSTP LED driver" to help with git history
categorization?

diff --git a/drivers/leds/leds-lstp.c b/drivers/leds/leds-lstp.c
new file mode 100644
index 0000000000000..352e484eb9194
--- /dev/null
+++ b/drivers/leds/leds-lstp.c
[ ... ]
@@ -275,20 +275,21 @@ static int lstp_led_register(struct device *dev, struct lstp_led *led, struct fw
+		if (!ret) {
+			bool pattern = fwnode_property_present(fwnode, "led-pattern");
+			bool trigger = fwnode_property_present(fwnode, "linux,default-trigger");
+
+			/* A trigger takes precedence; the LED core applies it from @fwnode. */
+			if (pattern && trigger)
+				dev_err(dev,
+					"%s: ch_%d: Ignoring led-pattern of LED '%s': %pfw also sets linux,default-trigger\n",
+					__func__, led->ch->ch_id, led->cdev.name, fwnode);
+			if (pattern && !trigger)
+				lstp_led_send_fwnode_pattern(led, fwnode);
+			else
+				lstp_led_apply_default_state(led, fwnode);

[Severity: High]
Does applying the initial hardware state here happen too early? At this
point, led_classdev_register_ext() has not yet been called, so the device
tree max-brightness property hasn't been parsed into led->cdev.max_brightness.

As a result, lstp_led_apply_default_state() will use the uninitialized
max_brightness value of 255 (U8_MAX) to drive the hardware. Could this risk
over-driving the hardware before the registration layer correctly enforces
the bound?

+			ret = lstp_led_classdev_register(dev, &led->cdev, &init_data);
+		}
+	}
+
+	if (ret)
+		dev_warn(dev, "%s: ch_%d: Could not register LED '%s', pin %u (%pe)\n", __func__,
+			 led->ch->ch_id, led->cdev.name, led->pin, ERR_PTR(ret));
+	return ret;
+}
[ ... ]
@@ -328,34 +328,41 @@ static int lstp_leds_probe(struct auxiliary_device *auxdev, const struct auxilia
[ ... ]
+		if (led->cdev.dev) {
+			dev_warn(dev, "%s: ch_%d: Ignoring %pfw: pin %u is already described\n",
+				 __func__, ch->ch_id, child, pin);
+			continue;
+		}
+		if (!fwnode_property_present(child, "label") &&
+		    !fwnode_property_present(child, "function")) {

[Severity: Medium]
Does this logic incorrectly reject modern device tree nodes? The LED
subsystem guidelines dictate that the label property is deprecated and
should be avoided, while color and function are preferred.

By warning and skipping nodes that lack both label and function, valid
modern DT nodes that rely solely on the color property (e.g.,
color = <LED_COLOR_ID_GREEN>) would be entirely rejected. Should this check
be updated to account for the color property and align with modern naming
requirements?

+			dev_warn(dev, "%s: ch_%d: Ignoring %pfw: neither label nor function\n",
+				 __func__, ch->ch_id, child);
+			continue;
+		}
+		lstp_led_register(dev, led, child);
+	}
+	fwnode_handle_put(node);
+
+	for (u16 i = 0; i < pins->npins; i++) {
+		if (!leds[i].cdev.dev)
+			lstp_led_register(dev, &leds[i], NULL);
+	}
+
+	return 0;
+}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-lstp-usb-adapter-v1-0-aambrozewicz@nvidia.com?part=7

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver
  2026-10-12 17:36 ` [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver Adrian Ambrozewicz
@ 2026-10-12 17:53   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-12 17:53 UTC (permalink / raw)
  To: Adrian Ambrozewicz; +Cc: linux-gpio, linux-usb

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] core: module unload race in lstp_gpio_leds_release() waking module_exit
--

commit 4aeddbd8eb4f935f2de9273a79a09c2407988002
Author: Adrian Ambrozewicz <aambrozewicz@nvidia.com>

gpio: Add NVIDIA LSTP GPIO driver

Add a driver for the GPIO channels of NVIDIA LSTP USB adapters. Each
channel is registered as a GPIO chip with an interrupt controller for
its pins. Pins the firmware reports as LEDs are handed to the LED driver
as an auxiliary device.

> diff --git a/drivers/gpio/gpio-lstp.c b/drivers/gpio/gpio-lstp.c
> new file mode 100644
> index 0000000000000..cc988a09582b0
> --- /dev/null
> +++ b/drivers/gpio/gpio-lstp.c
[ ... ]
> +static void lstp_gpio_leds_release(struct device *dev)
> +{
> +	struct lstp_leds *leds = container_of(to_auxiliary_dev(dev), struct lstp_leds, auxdev);
> +	struct device *parent = leds->ch->dev;
> +
> +	kfree(leds);
> +	put_device(parent);
> +	if (atomic_dec_and_test(&lstp_gpio_live_leds))
> +		wake_up_var(&lstp_gpio_live_leds);
> +}

[Severity: High]
Does this create a module unload race condition?

If a user-space process holds an active sysfs reference to the LED auxiliary
device, the device release callback is delayed until userspace drops its
reference (for example, closing a file descriptor). By this time, the rmmod
command could be executing and waiting in lstp_gpio_module_exit().

When lstp_gpio_leds_release() wakes up the module_exit waiter, the rmmod
thread finishes and the kernel unmaps the module's text memory. However, the
thread running the release callback is still executing the epilogue of
lstp_gpio_leds_release(), which resides in this module.

Returning to unmapped memory causes a kernel panic. A device release function
must not wake the module_exit thread that unloads its own code.

[ ... ]
> +/* The unregister unbinds every channel, which deletes its LED child. */
> +static void __exit lstp_gpio_module_exit(void)
> +{
> +	lstp_auxiliary_driver_unregister(&lstp_gpio_drv);
> +	wait_var_event(&lstp_gpio_live_leds, !atomic_read(&lstp_gpio_live_leds));
> +}
> +module_exit(lstp_gpio_module_exit);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-lstp-usb-adapter-v1-0-aambrozewicz@nvidia.com?part=6

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO
  2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
  2026-10-12 17:43   ` sashiko-bot
@ 2026-10-13  6:09   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-13  6:09 UTC (permalink / raw)
  To: Adrian Ambrozewicz, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Donald Shannon, linux-usb, linux-gpio, devicetree, linux-kernel,
	Greg Kroah-Hartman

On 12/10/2026 19:35, Adrian Ambrozewicz wrote:
> Add the binding for a GPIO channel of an NVIDIA LSTP USB adapter. The
> node is a child of the adapter's USB device or interface node, matched
> to a channel the device reports by reg (the channel ID) and compatible
> (the channel type).
> 
> The device firmware reports the number of lines and their names and
> directions; the node adds line names, hogs and interrupt consumers. Pins
> the firmware reports as LEDs are LEDs, not GPIO lines, and an optional
> leds child node describes them.
> 
> Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> ---
>  .../bindings/gpio/nvidia,lstp-gpio.yaml       | 276 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 283 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
> new file mode 100644
> index 000000000000..4ca5081e509d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nvidia,lstp-gpio.yaml
> @@ -0,0 +1,276 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/nvidia,lstp-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA LSTP GPIO Channel
> +
> +maintainers:
> +  - Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> +  - Donald Shannon <donalds@nvidia.com>
> +
> +description: |
> +  GPIO controller channel on an NVIDIA LSTP USB device.

What is LSTP?

> +
> +  The number of GPIO lines and their default names and directions are
> +  reported by the device firmware.
> +
> +  gpio-line-names overrides the firmware-reported pin names.  The
> +  gpio-controller and #gpio-cells properties let consumers (e.g.
> +  gpio-leds, gpio-keys) reference the lines by phandle.

Why are you describing the binding? This is pointless. Describe the
hardware.

> +
> +  The channel is also an interrupt controller.  A consumer may reference
> +  a line through a *-gpios property and use it as an interrupt, or

What? Please read other bindings to understand what is here.


> +  reference it through interrupts-extended, or interrupt-parent and
> +  interrupts, using the interrupt-controller and #interrupt-cells
> +  properties.  An interrupt line must be an input; the firmware-reported
> +  pin direction is authoritative and is not changed at runtime.
> +
> +  GPIO hog child nodes are supported for boot-time pin configuration.
> +
> +  Pins the device firmware reports as LEDs are not GPIO lines; each is
> +  an LED instead, named after the firmware pin name.  An optional leds
> +  child node describes some of them: their names, initial state, default
> +  trigger, and a pattern the device plays on its own.
> +
> +  Because the controller sits behind USB, every line access is a USB
> +  transaction that blocks until it completes.  Consumers that need
> +  non-blocking line access cannot use this controller.
> +
> +properties:
> +  compatible:
> +    const: nvidia,lstp-gpio

What is lstp?

> +
> +  reg:
> +    description: Channel ID reported by the device firmware.
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +    description: |
> +      Two cells: pin number and flags.
> +      Cell 0: GPIO pin offset (0-based).
> +      Cell 1: GPIO flags (e.g. GPIO_ACTIVE_LOW).
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +    description: |
> +      Two cells: GPIO line offset and trigger flags.
> +      Cell 0: GPIO line offset (0-based), same numbering as #gpio-cells.
> +      Cell 1: Trigger flags (see <dt-bindings/interrupt-controller/irq.h>).
> +
> +  gpio-line-names:
> +    description: |
> +      Optional list of human-readable names for each GPIO line.
> +      Overrides the firmware-reported pin names.  The number of entries
> +      must not exceed the number of GPIO lines reported by firmware.
> +    minItems: 1
> +    maxItems: 255
> +
> +  leds:
> +    type: object
> +    description: |
> +      LED pins of the channel.  A pin the device firmware reports as an
> +      LED is an LED whether or not it has a node here, and a node for a
> +      pin the firmware does not report as an LED is ignored.
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +    patternProperties:
> +      "^led@[0-9a-f]+$":
> +        type: object
> +        $ref: /schemas/leds/common.yaml#
> +        unevaluatedProperties: false
> +        description: |
> +          One LED pin.  The LED takes its name from label, or from color,
> +          function and function-enumerator, instead of the firmware pin
> +          name.
> +
> +        properties:
> +          reg:
> +            description: Pin number, in the channel's own pin numbering.
> +            maxItems: 1
> +
> +          default-state:
> +            description: |
> +              Written to the device once, when the LED is set up, and "off"
> +              when absent, as in leds/common.yaml.  "keep" leaves the LED as
> +              the device firmware set it; the device cannot report its
> +              current state.  Not written for a node with led-pattern.
> +
> +          led-pattern:
> +            description: |
> +              A pattern the device plays in a loop once the LED is set up.
> +              Each entry is <brightness duration>: brightness from 0 to
> +              255, held or faded towards for duration milliseconds,
> +              depending on the pin's firmware-reported LED mode.  The
> +              pattern is sent to the device once and played there without
> +              further USB traffic.  It must fit in a single request, which
> +              for a device with 64-byte USB endpoints is ten entries; a
> +              longer pattern is not sent.
> +            minItems: 1
> +            maxItems: 255
> +            items:
> +              items:
> +                - description: Brightness
> +                  maximum: 255
> +                - description: Duration in milliseconds
> +
> +          active-low: false
> +          flash-max-microamp: false
> +          flash-max-timeout-us: false
> +          inactive-high-impedance: false
> +          led-max-microamp: false
> +          led-sources: false
> +          max-brightness: false
> +          panic-indicator: false
> +          retain-state-shutdown: false
> +          retain-state-suspended: false

Why are you repeating all standard properties? Drop.

> +
> +        required:
> +          - reg
> +
> +        anyOf:
> +          - required: [label]
> +          - required: [function]

I don't get why simple binding is made so complicated.

> +
> +        dependentSchemas:
> +          led-pattern:
> +            properties:
> +              default-state: false
> +              linux,default-trigger: false
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +
> +    additionalProperties: false
> +
> +patternProperties:
> +  "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
> +    type: object
> +    required:
> +      - gpio-hog
> +
> +dependentRequired:
> +  interrupt-controller: ["#interrupt-cells"]
> +  "#interrupt-cells": [interrupt-controller]
> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - "#gpio-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    /* Minimal: let firmware define pin count and names */
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio@3 {
> +            compatible = "nvidia,lstp-gpio";
> +            reg = <3>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +        };
> +    };
> +  - |
> +    /* Override pin names and add a hog */
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio_ch: gpio@3 {
> +            compatible = "nvidia,lstp-gpio";
> +            reg = <3>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            gpio-line-names = "STATUS_LED", "ALERT_INT";
> +
> +            led-hog {
> +                gpio-hog;
> +                gpios = <0 0>;
> +                output-high;
> +                line-name = "status-led-hog";
> +            };
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    /* Act as an interrupt controller for a consumer via interrupts-extended */
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio_irq_ch: gpio@3 {
> +            compatible = "nvidia,lstp-gpio";
> +            reg = <3>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;
> +        };
> +    };
> +
> +    gpio-keys {
> +        compatible = "gpio-keys";
> +
> +        key-alert {
> +            label = "alert";
> +            linux,code = <143>; /* KEY_WAKEUP */
> +            /* Line 1 as a falling-edge interrupt source */
> +            interrupts-extended = <&gpio_irq_ch 1 IRQ_TYPE_EDGE_FALLING>;
> +        };
> +    };

Drop irrelevant. Keep only one complete example. Two if they are different.

> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    /* Name two firmware-reported LED pins; one plays a pattern on its own */
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        gpio@3 {
> +            compatible = "nvidia,lstp-gpio";
> +            reg = <3>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +
> +            leds {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                led@4 {
> +                    reg = <4>;
> +                    color = <LED_COLOR_ID_GREEN>;
> +                    function = LED_FUNCTION_POWER;
> +                    default-state = "on";
> +                };
> +
> +                led@5 {
> +                    reg = <5>;
> +                    color = <LED_COLOR_ID_AMBER>;
> +                    function = LED_FUNCTION_STATUS;
> +                    /* Two short flashes, then a pause */
> +                    led-pattern = <255 100>, <0 100>, <255 100>, <0 700>;
> +                };
> +            };
> +        };

So that's the only example you should keep.

> +    };
Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers
  2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
                   ` (2 preceding siblings ...)
  2026-10-12 17:36 ` [PATCH 07/11] leds: Add NVIDIA LSTP LED driver Adrian Ambrozewicz
@ 2026-10-13 11:37 ` Greg Kroah-Hartman
  2026-10-13 11:39 ` Greg Kroah-Hartman
  4 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2026-10-13 11:37 UTC (permalink / raw)
  To: Adrian Ambrozewicz
  Cc: Donald Shannon, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Bartosz Golaszewski, Lee Jones, Pavel Machek,
	Andi Shyti, Mark Brown, Jiri Slaby, Corey Minyard,
	Rafael J. Wysocki, Len Brown, Jonathan Corbet, Ilpo Järvinen,
	Sakari Ailus, Hans de Goede, Lixu Zhang, Danilo Krummrich,
	Johan Hovold, Oliver Neukum, Quan Nguyen, Andrew Jeffery,
	linux-usb, devicetree, linux-gpio, linux-leds, linux-i2c,
	linux-spi, linux-serial, openipmi-developer, linux-acpi,
	linux-doc, driver-core, linux-kernel

On Mon, Oct 12, 2026 at 08:35:58PM +0300, Adrian Ambrozewicz wrote:
> This series adds drivers for the NVIDIA LSTP USB adapter: a USB core
> driver and GPIO, LED, I2C, SPI, UART and BMC-side IPMI function drivers,
> with their device tree bindings, an ACPI description document and the
> sysfs ABI.

Why would a USB device have anything to do with device tree?

> What LSTP is
> ------------
> 
> LSTP (Low-Speed Transport Protocol or Low-Speed Tunnelling Protocol) is
> a vendor-specific USB interface (class 0xff, subclass 0x3f) that
> multiplexes several low-speed "channels" over one pair of bulk
> endpoints. It is implemented by NVIDIA's SMA, an MCU (0955:cf11), and
> by other vendors' parts: two hub/bridge chips have it built in
> (0424:4940, 174c:2214). Boards use it to
> reach SPI flash, I2C sensors, GPIOs and LEDs, a UART and a host-to-BMC
> IPMI path, on both BMCs and host systems.

Cute, but that's a total abuse of the USB standard.  Why wasn't this
implemented by the proper "each channel is a USB interface" model
instead?  Have the firmware engineers there not read the USB spec?

> The channel types are SPI host, I2C host, GPIO (with interrupts and LED
> pins), UART, IPMI and MMIO. Channel 0 is the control channel: at probe
> the driver reads the channel table from it, which gives each channel's
> ID, type and configuration. The set of channels is decided by the
> firmware, so the same VID:PID carries different channels on different
> boards. No firmware description is needed for the channels to work.

Again cute, but that's not how USB devices are supposed to work, why
would we want to do this at all?

> Series layout
> -------------
> 
>    1  dt-bindings: gpio: Add NVIDIA LSTP GPIO
>    2  dt-bindings: i2c: Add NVIDIA LSTP I2C
>    3  dt-bindings: spi: Add NVIDIA LSTP SPI
>    4  dt-bindings: usb: Add NVIDIA LSTP USB adapter

USB devices do not have DT bindings.  It's a dynamic bus, with
self-describing devices, don't make the mistake of thinking this is
giong to work.

>    5  usb: misc: Add NVIDIA LSTP USB adapter driver
>    6  gpio: Add NVIDIA LSTP GPIO driver
>    7  leds: Add NVIDIA LSTP LED driver
>    8  i2c: Add NVIDIA LSTP I2C driver
>    9  spi: Add NVIDIA LSTP SPI driver
>   10  tty: Add NVIDIA LSTP UART driver
>   11  ipmi: Add NVIDIA LSTP IPMI BMC driver

So you have a custom "bus" for a single USB device/interface?  Ick,
again, why?  Please do this the "proper" way in your firmware and do
this one-device-per-usb-interface like the rest of the world does
please.

Doing it that way will not cost any more hardware pins at all, it's all
handled in firmware, and as you all have complete control over the
firmware, you can fix this up now to do things properly.

> Patch 4 $refs the schemas from patches 1-3, so it comes last among the
> bindings; it also defines the serial@N and ipmi@N channel nodes inline.
> The MAINTAINERS entry starts in patch 1 and gains each later patch's
> files. Patch 5 carries:
> - the core in drivers/usb/misc/;
> - include/linux/usb/lstp.h for the function drivers;
> - the sysfs ABI file;
> - Documentation/driver-api/usb/lstp.rst;
> - Documentation/firmware-guide/acpi/lstp.rst.
> 
> Patches 6 and 8-11 each depend only on patch 5; patch 7 (LEDs) also
> needs patch 6, whose GPIO driver hands the LED pins to it.
> 
> Why the auxiliary bus, and why not MFD
> --------------------------------------

Neither is ok for USB devices, that's not an argument either way.

Please go talk to some people with USB spec experience on how to make a
multi-function device correctly.  This way sure does not look like it at
all.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers
  2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
                   ` (3 preceding siblings ...)
  2026-10-13 11:37 ` [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Greg Kroah-Hartman
@ 2026-10-13 11:39 ` Greg Kroah-Hartman
  4 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2026-10-13 11:39 UTC (permalink / raw)
  To: Adrian Ambrozewicz
  Cc: Donald Shannon, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Linus Walleij, Bartosz Golaszewski, Lee Jones, Pavel Machek,
	Andi Shyti, Mark Brown, Jiri Slaby, Corey Minyard,
	Rafael J. Wysocki, Len Brown, Jonathan Corbet, Ilpo Järvinen,
	Sakari Ailus, Hans de Goede, Lixu Zhang, Danilo Krummrich,
	Johan Hovold, Oliver Neukum, Quan Nguyen, Andrew Jeffery,
	linux-usb, devicetree, linux-gpio, linux-leds, linux-i2c,
	linux-spi, linux-serial, openipmi-developer, linux-acpi,
	linux-doc, driver-core, linux-kernel

On Mon, Oct 12, 2026 at 08:35:58PM +0300, Adrian Ambrozewicz wrote:
> This series adds drivers for the NVIDIA LSTP USB adapter: a USB core
> driver and GPIO, LED, I2C, SPI, UART and BMC-side IPMI function drivers,
> with their device tree bindings, an ACPI description document and the
> sysfs ABI.
> 
> What LSTP is
> ------------
> 
> LSTP (Low-Speed Transport Protocol or Low-Speed Tunnelling Protocol) is
> a vendor-specific USB interface (class 0xff, subclass 0x3f) that
> multiplexes several low-speed "channels" over one pair of bulk
> endpoints. It is implemented by NVIDIA's SMA, an MCU (0955:cf11), and
> by other vendors' parts: two hub/bridge chips have it built in
> (0424:4940, 174c:2214). Boards use it to
> reach SPI flash, I2C sensors, GPIOs and LEDs, a UART and a host-to-BMC
> IPMI path, on both BMCs and host systems.

Also, if you really want to do this in a dynamic way, use greybus, as
that is EXPLICITLY what it was designed for, and is patent-free (i.e.
unencumbered, and we already support it in the kernel.)  Please don't
reinvent the wheel/spec just because no one did any prior-art
research...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-10-13 11:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 17:35 [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Adrian Ambrozewicz
2026-10-12 17:35 ` [PATCH 01/11] dt-bindings: gpio: Add NVIDIA LSTP GPIO Adrian Ambrozewicz
2026-10-12 17:43   ` sashiko-bot
2026-10-13  6:09   ` Krzysztof Kozlowski
2026-10-12 17:36 ` [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver Adrian Ambrozewicz
2026-10-12 17:53   ` sashiko-bot
2026-10-12 17:36 ` [PATCH 07/11] leds: Add NVIDIA LSTP LED driver Adrian Ambrozewicz
2026-10-12 17:46   ` sashiko-bot
2026-10-13 11:37 ` [PATCH 00/11] Add NVIDIA LSTP USB adapter drivers Greg Kroah-Hartman
2026-10-13 11:39 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox