Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers
@ 2026-11-03 16:11 Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 1/9] greybus: operation: keep request active across message_send Ayush Singh
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh, Conor Dooley

A Greybus network needs an SVC (Supervisory Controller) to bring
interfaces up, assign device IDs and connect CPorts to the AP. On a
UniPro network the SVC is a real entity on the bus. On transports that
merely carry Greybus messages - a UART, an I2C bus, a network link -
nothing on the wire plays that role, so the SVC has to be emulated
somewhere: in a user-space bridge (gbridge), in coprocessor firmware
(cc1352p7 in gb-beagleplay), or open-coded inside the host driver itself.

This series moves the emulation into the kernel and makes it shared.
This was discussed in a prior RFC as well [2], but to keep things short,
removing the need for an external SVC greatly simplifies the greybus
network setup when nodes are directly connected over common transports
such as UART, I2C etc.

    gb-uart-node          gb-softsvc              greybus core
   (transport, serdev) <-> (SVC + host device) <-> (bundles, protocols)
        NodeOps            module_insert()
     submit_message()      module_remove()

gb-softsvc registers itself as a Greybus host device and answers the
SVC-side operations the core expects during interface bring-up and
teardown. A host driver implements the NodeOps trait to push data
towards its node, calls module_insert() to announce a new node,
submit_message() to hand incoming Greybus messages back to the core,
and module_remove() on disconnect.

The first user is gb-uart-node, a driver for Greybus nodes attached over
a plain serial port. Framing is HDLC, with a one-byte address (0x01 for
Greybus) and control byte, followed by the 16-bit CPort ID and the
Greybus message. No SVC firmware is required on the far end, so the node
can be a bare microcontroller speaking Greybus - a BeagleConnect Freedom
over its serial link, in this case. The testing is performed with
greybus-zephyr [0] implementation.

Both drivers are written in Rust, which is why the middle of the series
is abstractions rather than drivers. Only the APIs these two drivers
need are covered: protocols.rs abstracts the SVC-facing parts of
greybus_protocols.h, and types such as Greybus Interface do not
implement AlwaysRefCounted yet. The intent is to grow this as more Rust
host drivers appear rather than to abstract the whole subsystem up
front.

Patches 1 and 2 are small C-side preparations to the Greybus core:
exporting gb_connection_get()/gb_connection_put() and adding
gb_connection_hd_find_by_intf(), a lookup by remote interface and CPort
id for callers that only know the far end of a connection. Patch 3 adds
a CRC-CCITT abstraction, needed for HDLC frame checks. Patch 4 adds the
Greybus abstractions, patches 5 and 6 the two drivers, and patch 7 the
device tree binding for BeagleConnect Freedom.

Open questions
***************

- gb-uart-node imports types from gb-softsvc, so the series carries the
  Rust-to-Rust cross-module calling setup from nova-core [1]:
  gb_softsvc_exports.c plus the Makefile plumbing that emits crate
  metadata and generates the export list. This is a workaround for the
  build system not supporting Rust cross-module dependencies natively,
  and it should go away once that lands.

- Connection create/destroy and interface activate/resume in gb-softsvc
  currently just acknowledge the request. Callbacks into NodeOps can be
  added when a transport actually needs to act on them; I did not want
  to invent an interface without a user.

- Zerocopy is currently not being used in grebeybus/protocols.rs. They
  cannot be derived yet since types generated by bindgen do not have
  them, and it seems explicitly forbindden to manually impl the traits.
  So using old traits from transmute.

- The bindings are supposed to be created for actual device, but any MCU
  that supports Zephyr, can run the greybus-zephyr firmware with UART
  transport. So not sure if adding a beagle,beagleconnect-freedom
  compatible is the correct choice here.

- The individual patches can be spun off into independent patch series
  if required. The reason for this single patch series is to provide a
  complete picture of usage.

- Since gb-softsvc currently is not being used from a C driver, no C API
  is provided. However, if required, it can be added.

- Writing to UART from gb-uart-node is currently a bit broken. I am not
  quite sure what the safe way is to go from a non-bound device to a
  bound device. Any suggestions on this front are welcome.

- I am not sure if Rust abstractions should have a seperate entry in
  MAINTAINERS with me as the maintainer, or if they should just be added
  to the respective subsystem entries. As mentioned by Miguel Ojeda
  [3], this is upto greybus maintainers. So waiting on their inputs. I
  am available to be maintainer for the Rust abstractions for greybus
  if required.

[0]: https://github.com/beagleboard/greybus-zephyr
[1]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.com/
[2]: https://lore.kernel.org/all/ecca8eb2-8e5a-4770-bcf6-3fb49773088b@beagleboard.org/
[3]: https://lore.kernel.org/all/CANiq72mA6pTmPJUk8g7y3Z=wetFMXdzHLiPEPChpY4OTg3OHiw@mail.gmail.com/

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
Changes in v5:
- Remove AlwaysRefCounted impl from Interface. No longer required.
- Fix invalid comments.
- Fix interface drop in spinlock.
- Impl AlwaysRefCounted for hd::Device only in normal context.
- Match the bus device struct defination in device.rs docs.
- Return GB_OP_PROTOCOL_BAD instead of ENOTSUP when encountering an
  unknown message.
- Fix possible race condition in SVC module removed. Flagged by
  sashiko-bot. Involved, ensuring that operation_cancel cannot drop an
  in-flight message.
- Fix timer leak in a greybus operation.
- Check write_all return.
- Return E2BIG when frame lenght is exceeded in uart node driver.
- Link to v4: https://lore.kernel.org/r/20261021-gb-uart-transport-v4-0-c308ce821d45@beagleboard.org

Changes in v4:
- Remove gb_uard_node::GbUartNode::unbind. Suggested by Markus Probst,
  since write is still valid after unbind.
- Add inline attribute where suggested.
- Remove patch 1. No longer required with modifications to the old patch
  2.
- Increse hdlc max buffer size to 512 bytes.
- Send module_removed request before removing from map.
- Also do not hold lock across interface drop.
- Link to v3: https://lore.kernel.org/r/20261013-gb-uart-transport-v3-0-81d0a8a1acf7@beagleboard.org

Changes in v3:
- Fix incorrect email in MAINTAINERS.
- Make CONFIG_RUST_GREYBUS_ABSTRACTIONS depend on sysfs like
  CONFIG_GREYBUS.
- Fix cleanup in hd::Registration::new in case of errors.
- Fix comments.
- Fix Msg AsBytes impl as suggested by sashiko-bot.
- Use SpinLock for intf_map. This should be fine since we need locking
  only for getting the interface. We do not hold locks across operations.
- Properly drop message in case of buffer overflow in gb_uart_node.
- Link to v2: https://lore.kernel.org/r/20260827-gb-uart-transport-v2-0-a03bb1f5fbd1@beagleboard.org

Changes in v2:
- Fix possible null pointer dereference in
  gb_connection_hd_find_by_intf. Flagged by sashiko-bot.
- Remove prompt CONFIG_RUST_CRC_CCITT_ABSTRACTIONS Kconfig symbol.
- Add CONFIG_RUST_GREYBUS_ABSTRACTIONS to ensure that greybus is
  built-in for rust abstractions to work.
- Use gfp_mask in message_send callback.
- Only provide a Message reference in message_cancel callback.
- Add invariant comment for Registration.
- Call Registration->add directly in new, before constructing
  Registration.
- Add Send and Sync bounds to T in Registration<T>
- Set endo_id as 0. Is not used anywhere.
- Fix module_insert intf_count. Was hardcoded to 1 by mistake. Flagged by
  sashiko-bot.
- Check for empty intfs slice in Module::new.
- Remove module in serdev::Driver::unbind instead of on drop.
- Make gb_uart_node write atomic. Using temp buffer to build frame.
- Fix import style.
- Reorder beagleconnect-freedom dtbinding patch to be before gb_uart_node.
- Add vbat-supply and reg properties to beagleconnect-freedom dtbinding.
- Reference spi-peripheral-props in beagleconnect-freedom dtbinding.
- Link to v1: https://lore.kernel.org/r/20260820-gb-uart-transport-v1-0-282da14ab7b7@beagleboard.org

---
Ayush Singh (9):
      greybus: operation: keep request active across message_send
      greybus: svc: add gb_svc_flush()
      greybus: operation: arm request timeout before sending
      greybus: connection: Add greybus_data_rcvd_by_intf()
      rust: crc_ccitt: add CRC-CCITT abstraction
      rust: kernel: Add greybus abstractions
      drivers: greybus: Add software SVC implementation
      dt-bindings: beagle: Add BeagleConnect Freedom
      greybus: Add Rust UART node driver

 .../beagle/beagle,beagleconnect-freedom.yaml       |  62 +++
 MAINTAINERS                                        |  15 +
 drivers/greybus/.gitignore                         |   1 +
 drivers/greybus/Kconfig                            |  35 ++
 drivers/greybus/Makefile                           |  50 ++
 drivers/greybus/connection.c                       |  47 ++
 drivers/greybus/gb_softsvc.rs                      | 533 +++++++++++++++++++++
 drivers/greybus/gb_softsvc_exports.c               |  15 +
 drivers/greybus/gb_uart_node.rs                    | 270 +++++++++++
 drivers/greybus/operation.c                        |  29 +-
 drivers/greybus/svc.c                              |  14 +
 include/linux/greybus/connection.h                 |   2 +
 include/linux/greybus/svc.h                        |   1 +
 lib/crc/Kconfig                                    |   7 +
 rust/bindings/bindings_helper.h                    |   2 +
 rust/kernel/alloc.rs                               |   6 +
 rust/kernel/crc_ccitt.rs                           |  26 +
 rust/kernel/greybus/hd.rs                          | 391 +++++++++++++++
 rust/kernel/greybus/mod.rs                         | 200 ++++++++
 rust/kernel/greybus/protocols.rs                   | 445 +++++++++++++++++
 rust/kernel/lib.rs                                 |   4 +
 21 files changed, 2152 insertions(+), 3 deletions(-)
---
base-commit: becb95e45ef0dda9ffb35156d909d3da7a2789c3
change-id: 20260810-gb-uart-transport-9255d6557c4c

Best regards,
-- 
Ayush Singh <ayush@beagleboard.org>


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

* [PATCH v5 1/9] greybus: operation: keep request active across message_send
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:30   ` sashiko-bot
  2026-11-03 16:11 ` [PATCH v5 2/9] greybus: svc: add gb_svc_flush() Ayush Singh
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

gb_operation_request_send() takes a single active count, and completion
drops it. This would be fine normally, but in case of cancellation (in
gb_softsvc context, this would happen in module_removed), completion
(gb_operation_work), can drop the active count, while message_send is
running.

The core does call message_cancel before dropping the active count,
but in most normal circumstances, cancelling an operation that is
already writing over a protocol (UART) is not really possible. So we
are only left with making the core wait for the said write to complete
instead of making active count 0 while message_send is in progress.

The patch takes a second reference and active count before the send
and drop them once it returns. If the connection starts tearing down
between the two calls, return 0 and let the cancel complete the
operation.

Since it took at least me a while to truely understand what the initial
sashiko-bot report meant, here is an example:

       Thread A                     Thread B + worker                  active
       ------------------------     ------------------------------     ------
  1    get_active()                                                      1
  2    gb_message_send()
  3      GbNode::write() ...
  4      ... blocked                state = DISCONNECTING
  5      ... blocked                gb_operation_cancel()
  6      ... blocked                  result_set(-ESHUTDOWN)
  7      ... blocked                  queue_work(completion)
  8      ... blocked                  wait_event(!active)  [sleeps]
  9      ... blocked                [worker] gb_operation_work()
 10      ... blocked                [worker]   put_active()   [:292]     0
 11      ... blocked                  wait_event wakes, returns       <- too early
 12      ... blocked                gb_module_del() returns
 13      ... blocked                [softsvc] Module::drop finishes
 14      ... blocked                serdev port closed, unbind done   <- bad
 15      write() returns
 16    return to caller

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/greybus/operation.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index 7e12ffb2dd60..a63bc2eb164e 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -766,7 +766,25 @@ int gb_operation_request_send(struct gb_operation *operation,
 	if (ret)
 		goto err_put;
 
+	/*
+	 * Completion can drop the reference and active count above while the
+	 * host driver is still sending. Hold another of each across the send, so
+	 * that cancelling the operation waits for the host driver to return.
+	 */
+	gb_operation_get(operation);
+	ret = gb_operation_get_active(operation);
+	if (ret) {
+		/*
+		 * The connection is being torn down, and the operation, which is
+		 * already on its list, will be cancelled and completed.
+		 */
+		gb_operation_put(operation);
+		return 0;
+	}
+
 	ret = gb_message_send(operation->request, gfp);
+	gb_operation_put_active(operation);
+	gb_operation_put(operation);
 	if (ret)
 		goto err_put_active;
 

-- 
2.56.0


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

* [PATCH v5 2/9] greybus: svc: add gb_svc_flush()
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 1/9] greybus: operation: keep request active across message_send Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 3/9] greybus: operation: arm request timeout before sending Ayush Singh
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

A host driver that is its own SVC, such as a software SVC, injects
requests like MODULE_REMOVED through greybus_data_rcvd(). The core acts
on them asynchronously: the request handler runs on the connection
workqueue and queues the actual work on svc->wq. The response is sent
before that work runs, so it cannot tell the driver that the module is
gone.

Add gb_svc_flush() to wait until every request received so far has been
fully processed. It flushes the connection workqueue, then svc->wq.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/greybus/svc.c       | 14 ++++++++++++++
 include/linux/greybus/svc.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index 490577731a19..788d95b4b87b 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -1381,6 +1381,20 @@ static void gb_svc_remove_modules(struct gb_svc *svc)
 	}
 }
 
+/**
+ * gb_svc_flush - wait for received SVC requests to be processed
+ * @svc: the SVC
+ *
+ * Requests pass through the connection workqueue on their way to svc->wq, so
+ * both have to be flushed, in this order.
+ */
+void gb_svc_flush(struct gb_svc *svc)
+{
+	flush_workqueue(svc->connection->wq);
+	flush_workqueue(svc->wq);
+}
+EXPORT_SYMBOL_GPL(gb_svc_flush);
+
 void gb_svc_del(struct gb_svc *svc)
 {
 	gb_connection_disable_rx(svc->connection);
diff --git a/include/linux/greybus/svc.h b/include/linux/greybus/svc.h
index da547fb9071b..ced5fc3a9c6b 100644
--- a/include/linux/greybus/svc.h
+++ b/include/linux/greybus/svc.h
@@ -63,6 +63,7 @@ struct gb_svc *gb_svc_create(struct gb_host_device *hd);
 int gb_svc_add(struct gb_svc *svc);
 void gb_svc_del(struct gb_svc *svc);
 void gb_svc_put(struct gb_svc *svc);
+void gb_svc_flush(struct gb_svc *svc);
 
 int gb_svc_pwrmon_intf_sample_get(struct gb_svc *svc, u8 intf_id,
 				  u8 measurement_type, u32 *value);

-- 
2.56.0


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

* [PATCH v5 3/9] greybus: operation: arm request timeout before sending
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 1/9] greybus: operation: keep request active across message_send Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 2/9] greybus: svc: add gb_svc_flush() Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:27   ` sashiko-bot
  2026-11-03 16:11 ` [PATCH v5 4/9] greybus: connection: Add greybus_data_rcvd_by_intf() Ayush Singh
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

The host driver can complete a request before add_timer is called. The
main case for this is softsvc, although, depending on which transport
is used, there is a possibility that response arrives much faster than
expected.

By populating timer earilier, we prevent the starting a timer on
already completed request (which would then leak an active timer on cleanup).

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/greybus/operation.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index a63bc2eb164e..2d37acedf36f 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -782,20 +782,25 @@ int gb_operation_request_send(struct gb_operation *operation,
 		return 0;
 	}
 
+	/*
+	 * Arm the timeout before sending: the response can arrive, and the
+	 * operation complete, before the host driver returns.
+	 */
+	if (timeout) {
+		operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
+		add_timer(&operation->timer);
+	}
+
 	ret = gb_message_send(operation->request, gfp);
 	gb_operation_put_active(operation);
 	gb_operation_put(operation);
 	if (ret)
 		goto err_put_active;
 
-	if (timeout) {
-		operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
-		add_timer(&operation->timer);
-	}
-
 	return 0;
 
 err_put_active:
+	timer_delete_sync(&operation->timer);
 	gb_operation_put_active(operation);
 err_put:
 	gb_operation_put(operation);

-- 
2.56.0


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

* [PATCH v5 4/9] greybus: connection: Add greybus_data_rcvd_by_intf()
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (2 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 3/9] greybus: operation: arm request timeout before sending Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 5/9] rust: crc_ccitt: add CRC-CCITT abstraction Ayush Singh
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

greybus_data_rcvd() finds the receiving connection by its host-side
CPort id. A host driver such as a software SVC sees only the remote end
of a connection, the interface id and the interface CPort id, and cannot
deliver data that way.

Add greybus_data_rcvd_by_intf(), which looks the connection up by
interface id and interface CPort id and passes the data to it.
It returns -EINVAL if no such connection exists. The first user is the
gb-softsvc driver, added later in this series.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/greybus/connection.c       | 47 ++++++++++++++++++++++++++++++++++++++
 include/linux/greybus/connection.h |  2 ++
 2 files changed, 49 insertions(+)

diff --git a/drivers/greybus/connection.c b/drivers/greybus/connection.c
index bd04485decb3..ce8646299ce8 100644
--- a/drivers/greybus/connection.c
+++ b/drivers/greybus/connection.c
@@ -69,6 +69,32 @@ gb_connection_hd_find(struct gb_host_device *hd, u16 cport_id)
 	return connection;
 }
 
+/*
+ * Returns a reference-counted pointer to the connection if found.
+ */
+static struct gb_connection *
+gb_connection_hd_find_by_intf(struct gb_host_device *hd, u8 intf_id,
+			      u16 intf_cport)
+{
+	struct gb_connection *connection;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gb_connections_lock, flags);
+	list_for_each_entry(connection, &hd->connections, hd_links) {
+		if (connection->intf &&
+		    connection->intf->interface_id == intf_id &&
+		    connection->intf_cport_id == intf_cport) {
+			gb_connection_get(connection);
+			goto found;
+		}
+	}
+	connection = NULL;
+found:
+	spin_unlock_irqrestore(&gb_connections_lock, flags);
+
+	return connection;
+}
+
 /*
  * Callback from the host driver to let us know that data has been
  * received on the bundle.
@@ -91,6 +117,27 @@ void greybus_data_rcvd(struct gb_host_device *hd, u16 cport_id,
 }
 EXPORT_SYMBOL_GPL(greybus_data_rcvd);
 
+/*
+ * Callback from a host driver that only knows the remote end of a connection
+ * (interface id and interface CPort id), such as a software SVC.
+ */
+int greybus_data_rcvd_by_intf(struct gb_host_device *hd, u8 intf_id,
+			      u16 intf_cport, u8 *data, size_t length)
+{
+	struct gb_connection *connection;
+
+	trace_gb_hd_in(hd);
+
+	connection = gb_connection_hd_find_by_intf(hd, intf_id, intf_cport);
+	if (!connection)
+		return -EINVAL;
+	gb_connection_recv(connection, data, length);
+	gb_connection_put(connection);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(greybus_data_rcvd_by_intf);
+
 static void gb_connection_kref_release(struct kref *kref)
 {
 	struct gb_connection *connection;
diff --git a/include/linux/greybus/connection.h b/include/linux/greybus/connection.h
index d59b7fc1de3e..e8be67a936a6 100644
--- a/include/linux/greybus/connection.h
+++ b/include/linux/greybus/connection.h
@@ -92,6 +92,8 @@ void gb_connection_mode_switch_complete(struct gb_connection *connection);
 
 void greybus_data_rcvd(struct gb_host_device *hd, u16 cport_id,
 		       u8 *data, size_t length);
+int greybus_data_rcvd_by_intf(struct gb_host_device *hd, u8 intf_id,
+			      u16 intf_cport, u8 *data, size_t length);
 
 void gb_connection_latency_tag_enable(struct gb_connection *connection);
 void gb_connection_latency_tag_disable(struct gb_connection *connection);

-- 
2.56.0


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

* [PATCH v5 5/9] rust: crc_ccitt: add CRC-CCITT abstraction
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (3 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 4/9] greybus: connection: Add greybus_data_rcvd_by_intf() Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 6/9] rust: kernel: Add greybus abstractions Ayush Singh
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

The module is gated behind the new RUST_CRC_CCITT_ABSTRACTIONS Kconfig
symbol, which selects CRC_CCITT, so the C library is only built when a
Rust user actually needs it.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS                     |  1 +
 lib/crc/Kconfig                 |  7 +++++++
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/crc_ccitt.rs        | 26 ++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  2 ++
 5 files changed, 37 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 252b6d7ee163..772023e0e5a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7036,6 +7036,7 @@ T:	git https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git crc-ne
 F:	Documentation/staging/crc*
 F:	include/linux/crc*
 F:	lib/crc/
+F:	rust/kernel/crc*
 F:	scripts/gen-crc-consts.py
 
 CREATIVE SB0540
diff --git a/lib/crc/Kconfig b/lib/crc/Kconfig
index 927fc6a6b2b9..628da2d30fc8 100644
--- a/lib/crc/Kconfig
+++ b/lib/crc/Kconfig
@@ -32,6 +32,13 @@ config CRC_CCITT
 	  The CRC-CCITT library functions.  Select this if your module uses any
 	  of the functions from <linux/crc-ccitt.h>.
 
+config RUST_CRC_CCITT_ABSTRACTIONS
+	bool
+	depends on RUST
+	select CRC_CCITT
+	help
+	  This enables the Rust abstraction for the CRC-CCITT API.
+
 config CRC_ITU_T
 	tristate
 	help
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3791af0aadc7..55d1a2d7655a 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -49,6 +49,7 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
+#include <linux/crc-ccitt.h>
 #include <linux/cred.h>
 #include <linux/debugfs.h>
 #include <linux/device/faux.h>
diff --git a/rust/kernel/crc_ccitt.rs b/rust/kernel/crc_ccitt.rs
new file mode 100644
index 000000000000..6042ada16967
--- /dev/null
+++ b/rust/kernel/crc_ccitt.rs
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! CRC-CCITT computation.
+//!
+//! C header: [`include/linux/crc-ccitt.h`](srctree/include/linux/crc-ccitt.h)
+
+/// Computes the CRC-CCITT of `data`, starting from the seed value `crc`.
+///
+/// Pass the result back in as `crc` to compute the checksum of a buffer
+/// incrementally.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::crc_ccitt::crc_ccitt;
+///
+/// let one_shot = crc_ccitt(0xffff, b"hello world");
+/// let split = crc_ccitt(crc_ccitt(0xffff, b"hello "), b"world");
+/// assert_eq!(one_shot, split);
+/// ```
+#[inline]
+pub fn crc_ccitt(crc: u16, data: &[u8]) -> u16 {
+    // SAFETY: `data.as_ptr()` is valid for reads of `data.len()` bytes for the
+    // duration of the call, since it is derived from a live shared slice.
+    unsafe { bindings::crc_ccitt(crc, data.as_ptr(), data.len()) }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f9ef36217bb5..637442761d34 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -61,6 +61,8 @@
 #[cfg(CONFIG_CPU_FREQ)]
 pub mod cpufreq;
 pub mod cpumask;
+#[cfg(CONFIG_RUST_CRC_CCITT_ABSTRACTIONS)]
+pub mod crc_ccitt;
 pub mod cred;
 pub mod debugfs;
 pub mod device;

-- 
2.56.0


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

* [PATCH v5 6/9] rust: kernel: Add greybus abstractions
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (4 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 5/9] rust: crc_ccitt: add CRC-CCITT abstraction Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:28   ` sashiko-bot
  2026-11-03 16:11 ` [PATCH v5 7/9] drivers: greybus: Add software SVC implementation Ayush Singh
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

Add Rust abstractions for the Greybus core, enough to implement a
Greybus host driver in Rust.

The abstractions are split into 3 modules:

- Top level mod.rs: Basic abstractions greybus structures.
- hd.rs: Greybus Host Device specific abstractions.
- protocols.rs: Abstractions for greybus_protocols.h, i.e. greybus
  protocol types and constants.

Only the APIs used by gb-softsvc and gb-uart-node driver are covered.
As such, protocols.rs also only contains abstractions for greybus
structures that are used by SVC and some types such as Greybus Interface
do not implement AlwaysRefCounted at the moment.

HdDriver Note
**************

- In C message_send callback, it is possible for the caller to defer
  calling gb_message_sent. This is used in es2 driver. However, looking at
  the internals, it does not seem like the message owenership is actually
  transferred to the caller in real terms. Instead, it is more like the
  caller is shared ownership. This is evident by the fact that
  message_cancel can be called at any point, and depending on wheather the
  message is request or response, the message can become invalid after the
  message_cancel callback. Thus the driver is expected to ensure that no
  dangling references to that message exist.

  Since gb_message is not an internally ref counted structure in true
  terms (it does seem to use the ref counting in the internal operation),
  and since double calls to gb_message_sent are not really allowed, I have
  made the decision to call gb_message_sent on a message in the
  abstractions rather than making the driver call it. This means the
  driver only gets a reference to the message, and never owns it. This
  does prevent a driver to use it in the way es2 currently does, but seems
  much less of a footgun.

  Another possibility would be to implement AlwaysRefCounted on Message
  and pass the calls to gb_operation_get/put, but I have not gone through
  the internals enough to figure out if this is correct in all cases.

- At the moment, nothing in-tree calls message_send from an atomic
  context. However, since it does have gfp_mask in param, and an
  exported function, it is possible to call it from atomic context. So
  not sure if the doc comment should specific non-sleep requirement.

- message_cancel does not directly provide access to driver data. This
  was flagged by sashiko-bot. It actually is possible for the Rust
  abstractions to provide the driver data though. This is because we can
  do gb_message->operation->connection->host_device->priv_data. However,
  not sure if we should do this just in Rust abstractions. Maybe it
  would be preferable to change the C API itself instead.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS                      |   1 +
 drivers/greybus/Kconfig          |   8 +
 rust/bindings/bindings_helper.h  |   1 +
 rust/kernel/alloc.rs             |   6 +
 rust/kernel/greybus/hd.rs        | 391 ++++++++++++++++++++++++++++++++++
 rust/kernel/greybus/mod.rs       | 200 ++++++++++++++++++
 rust/kernel/greybus/protocols.rs | 445 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs               |   2 +
 8 files changed, 1054 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 772023e0e5a4..4d98db7618c3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11526,6 +11526,7 @@ F:	drivers/greybus/
 F:	drivers/staging/greybus/
 F:	include/linux/greybus.h
 F:	include/linux/greybus/
+F:	rust/kernel/greybus/
 
 GREYBUS UART PROTOCOLS DRIVERS
 M:	David Lin <dtwlin@gmail.com>
diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
index c3f056d28b01..d9f52024d1c2 100644
--- a/drivers/greybus/Kconfig
+++ b/drivers/greybus/Kconfig
@@ -15,6 +15,14 @@ menuconfig GREYBUS
 	  To compile this code as a module, choose M here: the module
 	  will be called greybus.ko
 
+config RUST_GREYBUS_ABSTRACTIONS
+	bool "Rust greybus abstractions"
+	depends on RUST
+	depends on SYSFS
+	select GREYBUS
+	help
+	  This enables the Rust abstraction for the Greybus API.
+
 if GREYBUS
 
 config GREYBUS_BEAGLEPLAY
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 55d1a2d7655a..91ac64687d23 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -64,6 +64,7 @@
 #include <linux/firmware.h>
 #include <linux/fwctl.h>
 #include <linux/fs.h>
+#include <linux/greybus.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io-pgtable.h>
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index 68d80f3fa6c8..b3c55fb2c2f1 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -38,6 +38,12 @@
 pub struct Flags(u32);
 
 impl Flags {
+    /// Creates [`Flags`] from a raw `gfp_t`.
+    #[cfg_attr(not(CONFIG_RUST_GREYBUS_ABSTRACTIONS), expect(dead_code))]
+    pub(crate) fn new(v: bindings::gfp_t) -> Self {
+        Self(v)
+    }
+
     /// Get the raw representation of this flag.
     pub(crate) fn as_raw(self) -> u32 {
         self.0
diff --git a/rust/kernel/greybus/hd.rs b/rust/kernel/greybus/hd.rs
new file mode 100644
index 000000000000..9e1a5fc645cd
--- /dev/null
+++ b/rust/kernel/greybus/hd.rs
@@ -0,0 +1,391 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Greybus host device abstractions.
+//!
+//! A driver implements the transmit path through [`HdDriver`] and feeds received data back into
+//! the core with [`Device::data_rcvd`].
+//!
+//! A host device is created, populated with driver private data and added to the Greybus bus by
+//! constructing a [`Registration`], which owns the underlying `struct gb_host_device`. Dropping
+//! the registration drops the private data and removes the device from the bus, so it is normally
+//! stored in the driver data of the parent device and torn down implicitly when that device goes
+//! away.
+//!
+//! Individual references to a live host device are represented by
+//! [`ARef<Device>`](crate::sync::aref::ARef), which keeps the embedded `struct device` reference
+//! count balanced.
+//!
+//! C header: [`include/linux/greybus/hd.h`](srctree/include/linux/greybus/hd.h)
+//!
+//! # Examples
+//!
+//! ```ignore
+//! use kernel::{
+//!     alloc::Flags,
+//!     device,
+//!     greybus::{
+//!         hd::{
+//!             Device,
+//!             HdDriver,
+//!             Registration, //
+//!         },
+//!         Message, //
+//!     },
+//!     prelude::*, //
+//! };
+//!
+//! struct MyHd {
+//!     // transport state
+//! }
+//!
+//! #[vtable]
+//! impl HdDriver for MyHd {
+//!     fn message_send(
+//!         data: &Self,
+//!         hd: &Device,
+//!         dest_cport_id: u16,
+//!         msg: &Message,
+//!         gfp_mask: Flags,
+//!     ) -> Result {
+//!         // Copy what is needed out of `msg` and queue it, allocating with `gfp_mask`.
+//!         // Returning `Ok(())` reports the message as sent to the core.
+//!         Ok(())
+//!     }
+//!
+//!     fn message_cancel(_msg: &Message) {}
+//! }
+//!
+//! fn probe(parent: &device::Device) -> Result<Registration<MyHd>> {
+//!     Registration::new(parent, BUFFER_SIZE_MAX, NUM_CPORTS, try_pin_init!(MyHd {}))
+//! }
+//! ```
+
+use core::{
+    marker::PhantomData,
+    ptr::{
+        addr_of_mut,
+        NonNull, //
+    },
+};
+
+use crate::{
+    alloc::Flags,
+    device,
+    error::{
+        code,
+        from_err_ptr,
+        to_result, //
+    },
+    greybus::Message,
+    prelude::*,
+    sync::aref::AlwaysRefCounted,
+    types::Opaque,
+};
+
+/// The set of callbacks a Greybus host driver can provide.
+#[vtable]
+pub trait HdDriver: Send + Sync + Sized + 'static {
+    /// Transmits `msg` to `dest_cport_id`.
+    ///
+    /// The message is only borrowed for the duration of this call: copy what is needed out of it,
+    /// do not stash the reference. Returning `Ok(())` reports the message as sent to the core.
+    ///
+    /// `hd` is the host device the message is sent on. It is only guaranteed to be alive for the
+    /// duration of this call.
+    fn message_send(
+        data: &Self,
+        hd: &Device,
+        dest_cport_id: u16,
+        msg: &Message,
+        gfp_mask: Flags,
+    ) -> Result;
+
+    /// Aborts the transmission of a message previously handed to [`HdDriver::message_send`].
+    ///
+    /// Always called in process context.
+    fn message_cancel(msg: &Message);
+}
+
+/// Builds the C callback table for a [`HdDriver`] implementation.
+struct HdDriverVTable<T: HdDriver>(PhantomData<T>);
+
+impl<T: HdDriver> HdDriverVTable<T> {
+    const DRIVER: bindings::gb_hd_driver = bindings::gb_hd_driver {
+        // Both are mandatory, hence no `HAS_*` check. See [`HdDriver`].
+        message_send: Some(Self::message_send),
+        message_cancel: Some(Self::message_cancel),
+        // Every other callback is left `NULL`, which the Greybus core takes as "use the default
+        // behaviour".
+        ..pin_init::zeroed()
+    };
+
+    const fn build() -> &'static bindings::gb_hd_driver {
+        &Self::DRIVER
+    }
+
+    /// # Safety
+    ///
+    /// `hd` must point at a registered host device whose private area holds a pointer to a live
+    /// `T`, and `msg` must point at a valid message.
+    unsafe extern "C" fn message_send(
+        hd: *mut bindings::gb_host_device,
+        dest_cport_id: u16,
+        msg: *mut bindings::gb_message,
+        gfp_mask: bindings::gfp_t,
+    ) -> c_int {
+        // SAFETY: `gb_host_device` and `Device` have the same layout.
+        let hd = unsafe { Device::<device::CoreInternal<'_>>::from_raw(hd) };
+        // SAFETY: `message_send` is only ever called after a successful call to
+        // `gb_hd_add`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `Pin<KBox<T>>`.
+        let data = unsafe { hd.as_ref().drvdata_borrow() };
+        // SAFETY: The caller guarantees `msg` is valid for the duration of this call.
+        let msg = unsafe { Message::from_raw(msg) };
+
+        // SAFETY: The caller guarantees `hd` is a registered host device, alive for the duration
+        // of this call.
+        let hd_normal = unsafe { Device::<device::Normal>::from_raw(hd.as_raw()) };
+
+        match T::message_send(&data, hd_normal, dest_cport_id, msg, Flags::new(gfp_mask)) {
+            Ok(()) => {
+                // SAFETY: `msg` is still valid since `sent` is not called anywhere else.
+                unsafe { msg.sent(hd, 0) };
+                0
+            }
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `msg` must point at a valid message of a registered host device of this driver.
+    unsafe extern "C" fn message_cancel(msg: *mut bindings::gb_message) {
+        // SAFETY: The caller guarantees `msg` is valid for the duration of this call.
+        let msg = unsafe { Message::from_raw(msg) };
+        T::message_cancel(msg);
+    }
+}
+
+/// A Greybus host device.
+///
+/// # Invariants
+///
+/// The wrapped value is a valid `struct gb_host_device` created by `gb_hd_create()`, and every
+/// [`ARef<Device>`](crate::sync::aref::ARef) owns an increment on its reference count.
+#[repr(transparent)]
+pub struct Device<Ctx = device::Normal>(Opaque<bindings::gb_host_device>, PhantomData<Ctx>);
+
+// SAFETY: `gb_host_device` is reference counted through its embedded `struct device`, which may be
+// used from any thread.
+unsafe impl<Ctx> Send for Device<Ctx> {}
+
+// SAFETY: `gb_host_device` has its own internal locking, so it is safe to share references to it
+// across threads.
+unsafe impl<Ctx> Sync for Device<Ctx> {}
+
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: The embedded `struct device` carries the reference count, and `gb_hd_put()` is just
+// `put_device()` on it, so the object stays alive for as long as increments are outstanding.
+unsafe impl AlwaysRefCounted for Device {
+    #[inline]
+    fn inc_ref(&self) {
+        // SAFETY: By the type invariant there is a live reference to the host device, and `dev` is
+        // its embedded `struct device`.
+        unsafe { bindings::get_device(&raw mut (*self.0.get()).dev) };
+    }
+
+    #[inline]
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The caller guarantees it owns an increment on the reference count.
+        unsafe { bindings::gb_hd_put(obj.as_ptr().cast()) }
+    }
+}
+
+impl<Ctx> Device<Ctx> {
+    /// # Safety
+    ///
+    /// `ptr` must be a valid pointer to a `struct gb_host_device`.
+    #[inline]
+    pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::gb_host_device) -> &'a Self {
+        // SAFETY: `Device` is a transparent wrapper of `Opaque<bindings::gb_host_device>`.
+        unsafe { &*ptr.cast() }
+    }
+
+    #[inline]
+    pub(crate) fn as_raw(&self) -> *mut bindings::gb_host_device {
+        self.0.get()
+    }
+
+    /// Hands a message received on `hd_cport_id` to the Greybus core.
+    #[inline]
+    pub fn data_rcvd(&self, cport_id: u16, msg: &[u8]) {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`. `msg` is valid for reads of ``msg.len()` bytes for the duration
+        // of the call, and the core only reads through the pointer — it copies the payload into
+        // the operation before returning — so handing it a `*mut` derived from a shared reference
+        // is sound.
+        //
+        // TODO: The C signature of this function should be changed to take const pointer for msg.
+        unsafe {
+            bindings::greybus_data_rcvd(self.as_raw(), cport_id, msg.as_ptr().cast_mut(), msg.len())
+        }
+    }
+
+    /// Hands `msg`, received from CPort `cport` of interface `intf_id`, to the Greybus core.
+    ///
+    /// Fails with `EINVAL` if no connection is bound to that pair.
+    #[inline]
+    pub fn data_rcvd_by_intf(&self, intf_id: u8, cport: u16, msg: &[u8]) -> Result {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`. `msg` is valid for reads of `msg.len()` bytes for the duration
+        // of the call, and the core only reads through the pointer — it copies the payload into
+        // the operation before returning — so handing it a `*mut` derived from a shared reference
+        // is sound.
+        //
+        // TODO: The C signature of this function should be changed to take const pointer for msg.
+        to_result(unsafe {
+            bindings::greybus_data_rcvd_by_intf(
+                self.as_raw(),
+                intf_id,
+                cport,
+                msg.as_ptr().cast_mut(),
+                msg.len(),
+            )
+        })
+    }
+
+    /// Waits until the SVC has processed every request handed to [`Device::data_rcvd`] so far.
+    ///
+    /// This may sleep.
+    #[inline]
+    pub fn svc_flush(&self) {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`. Its SVC is created with it and only freed when it is released,
+        // so `svc` is valid for as long as `self` is.
+        unsafe { bindings::gb_svc_flush((*self.as_raw()).svc) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    #[inline]
+    fn as_ref(&self) -> &device::Device<Ctx> {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`. `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(addr_of_mut!((*self.as_raw()).dev)) }
+    }
+}
+
+/// A host device owned by its driver, together with the driver's private data `T`.
+///
+/// Created and added to the Greybus bus on construction, removed on drop.
+///
+/// # Invariants
+///
+/// `ptr` points at a valid `struct gb_host_device` obtained from `gb_hd_create()`, whose driver
+/// data holds a live `T`
+#[repr(transparent)]
+pub struct Registration<T> {
+    ptr: NonNull<bindings::gb_host_device>,
+    _data: PhantomData<T>,
+}
+
+impl<T: HdDriver> Registration<T> {
+    /// Creates a host device under `parent` and adds it to the Greybus bus.
+    ///
+    /// `buffer_size_max` is the largest message the transport can carry in one go, header
+    /// included. `num_cports` is the number of cports that the greybus host device can connect to.
+    ///
+    /// `data` is initialised in place before the device is added, so callbacks may run against it
+    /// from the moment `gb_hd_add()` succeeds.
+    pub fn new(
+        parent: &device::Device,
+        buffer_size_max: usize,
+        num_cports: usize,
+        data: impl PinInit<T, Error>,
+    ) -> Result<Self> {
+        // SAFETY: `parent` is a valid device, and the driver table is `'static`. The core only
+        // ever reads through the driver pointer, so casting away `const` is fine.
+        let hd = from_err_ptr(unsafe {
+            bindings::gb_hd_create(
+                core::ptr::from_ref(HdDriverVTable::<T>::build()).cast_mut(),
+                parent.as_raw(),
+                buffer_size_max,
+                num_cports,
+            )
+        })?;
+
+        // SAFETY: `hd` is a valid pointer to `gb_host_device`.
+        let hd_dev: &Device<device::CoreInternal<'_>> = unsafe { Device::from_raw(hd) };
+        if let Err(e) = hd_dev.as_ref().set_drvdata(data) {
+            // SAFETY: `hd` is a pointer to a valid `struct gb_host_device`.
+            unsafe { bindings::gb_hd_put(hd) };
+
+            return Err(e);
+        }
+
+        // SAFETY: By the type invariant the host device is valid, and it has not been added yet.
+        let res = to_result(unsafe { bindings::gb_hd_add(hd) });
+        if let Err(e) = res {
+            // SAFETY: The driver data was set to a `T` above and has not been taken since.
+            drop(unsafe { hd_dev.as_ref().drvdata_obtain::<T>() });
+            // SAFETY: `hd` is a pointer to a valid `struct gb_host_device`.
+            unsafe { bindings::gb_hd_put(hd) };
+
+            return Err(e);
+        }
+
+        // INVARIANT: `gb_hd_create()` returns a valid `gb_host_device` on success,
+        // and we own the reference it returns until `Drop`.
+        Ok(Self {
+            ptr: NonNull::new(hd).ok_or(code::ENOMEM)?,
+            _data: PhantomData,
+        })
+    }
+}
+
+impl<T> Registration<T> {
+    #[inline]
+    fn as_raw(&self) -> *mut bindings::gb_host_device {
+        self.ptr.as_ptr()
+    }
+}
+
+impl<T> AsRef<Device> for Registration<T> {
+    #[inline]
+    fn as_ref(&self) -> &Device {
+        // SAFETY: By the type invariant the host device is valid.
+        unsafe { Device::from_raw(self.as_raw()) }
+    }
+}
+
+impl<T> Drop for Registration<T> {
+    fn drop<'a>(&'a mut self) {
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`.
+        unsafe { bindings::gb_hd_del(self.as_raw()) }
+
+        {
+            // SAFETY: `hd` is a valid pointer to `gb_host_device`.
+            let hd_dev = unsafe { Device::<device::CoreInternal<'_>>::from_raw(self.as_raw()) };
+            // SAFETY: The driver data was set to a `T` in `Registration::new()` and has not been
+            // taken since, and this is the only place that takes it.
+            drop(unsafe { hd_dev.as_ref().drvdata_obtain::<T>() });
+        }
+
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct gb_host_device`.
+        unsafe { bindings::gb_hd_put(self.as_raw()) };
+    }
+}
+
+// SAFETY: The greybus host device API is thread-safe as guaranteed by the device core, as long as
+// gb_hd_del() is guaranteed to only be called once - which is guaranteed by our type not
+// having Copy/Clone.
+unsafe impl<T: Send> Send for Registration<T> {}
+
+// SAFETY: The greybus device API is thread-safe as guaranteed by the device core, as long as
+// gb_hd_del() is guaranteed to only be called once - which is guaranteed by our type not
+// having Copy/Clone.
+unsafe impl<T: Sync> Sync for Registration<T> {}
diff --git a/rust/kernel/greybus/mod.rs b/rust/kernel/greybus/mod.rs
new file mode 100644
index 000000000000..908356af08e6
--- /dev/null
+++ b/rust/kernel/greybus/mod.rs
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the Greybus core.
+//!
+//! C header: [`include/linux/greybus.h`](srctree/include/linux/greybus.h)
+//!
+//! Greybus host drivers ("host devices") bridge the Greybus core to whatever transport actually
+//! carries the traffic. A host driver registers an [`hd::Device`] through an [`hd::Registration`],
+//! and implements [`hd::HdDriver`] to transmit the messages the core hands it.
+
+use core::ptr::NonNull;
+
+use crate::{
+    transmute::FromBytes,
+    types::Opaque, //
+};
+
+pub mod hd;
+pub mod protocols;
+
+/// The largest Greybus message, header included.
+///
+/// Bounded by the 16-bit `size` field in the operation header.
+pub const GB_OPERATION_SIZE_MAX: usize = u16::MAX as usize;
+
+/// The largest valid CPort id.
+///
+/// Ids above this are reserved by the protocol; a host device's `num_cports` cannot exceed
+/// `CPORT_ID_MAX + 1`.
+pub const CPORT_ID_MAX: usize = bindings::CPORT_ID_MAX as usize;
+
+/// A Greybus message handed to a host driver for transmission.
+///
+/// # Invariants
+///
+/// The shared reference is only ever handed out for the duration of a [`hd::HdDriver`] callback,
+/// during which the Greybus core guarantees the message and its buffer stay alive.
+#[repr(transparent)]
+pub struct Message(Opaque<bindings::gb_message>);
+
+impl Message {
+    /// # Safety
+    ///
+    /// `ptr` must be non-null and point at a valid `struct gb_message` which outlives the
+    /// returned `Self`.
+    #[inline]
+    pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::gb_message) -> &'a Self {
+        // SAFETY: `Message` is a transparent wrapper of `Opaque<bindings::gb_message>`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the operation header at the start of the message.
+    #[inline]
+    pub const fn header(&self) -> &protocols::GbOperationMsgHdr {
+        // SAFETY: `header` points at a valid `gb_operation_msg_hdr` for as long as the message is
+        // alive, and `GbOperationMsgHdr` is a transparent wrapper of it.
+        unsafe { &*(*self.0.get()).header.cast() }
+    }
+
+    /// # Safety
+    ///
+    /// `Message` is not valid after this call.
+    #[inline]
+    pub(crate) unsafe fn sent<Ctx>(&self, hd: &hd::Device<Ctx>, status: i32) {
+        // SAFETY: By the type invariant the message is valid and still owned by the transport,
+        // and its connection and host device are alive for as long as it is.
+        unsafe {
+            bindings::greybus_message_sent(hd.as_raw(), self.0.get(), status);
+        }
+    }
+
+    /// Returns the payload, i.e. the message without its operation header.
+    #[inline]
+    pub const fn payload_bytes(&self) -> &[u8] {
+        // SAFETY: By the type invariant the message is valid.
+        let payload = unsafe { (*self.0.get()).payload };
+        // SAFETY: By the type invariant the message is valid.
+        let payload_size = unsafe { (*self.0.get()).payload_size };
+
+        if payload.is_null() || payload_size == 0 {
+            return &[];
+        }
+
+        // SAFETY: A non-null `payload` points at `payload_size` initialized bytes.
+        unsafe { core::slice::from_raw_parts(payload.cast::<u8>(), payload_size) }
+    }
+
+    /// Interprets the message payload as a `T`.
+    ///
+    /// Returns `None` if the payload is too short or misaligned for `T`.
+    #[inline]
+    pub fn payload<T: FromBytes>(&self) -> Option<&T> {
+        T::from_bytes(self.payload_bytes())
+    }
+
+    /// Returns the operation this message belongs to.
+    #[inline]
+    pub const fn operation(&self) -> &Operation {
+        // SAFETY: By the type invariant the message is valid, and its `operation` is set for as
+        // long as the message is alive.
+        unsafe { Operation::from_raw((*self.0.get()).operation) }
+    }
+}
+
+/// A Greybus operation.
+///
+/// # Invariants
+///
+/// The wrapped value is a valid `struct gb_operation`.
+#[repr(transparent)]
+pub struct Operation(Opaque<bindings::gb_operation>);
+
+impl Operation {
+    /// # Safety
+    ///
+    /// `ptr` must be a valid pointer to a `struct gb_operation`.
+    #[inline]
+    pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::gb_operation) -> &'a Self {
+        // SAFETY: `Operation` is a transparent wrapper of `Opaque<bindings::gb_operation>`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the connection this operation travels on.
+    #[inline]
+    pub const fn connection(&self) -> &Connection {
+        // SAFETY: By the type invariant the operation is valid, and its `connection` is alive for
+        // as long as the operation is. The returned borrow is tied to `&self`, so it cannot
+        // outlive the host-device callback that handed out the operation, as `Connection`'s
+        // invariant requires.
+        unsafe { Connection::from_raw((*self.0.get()).connection) }
+    }
+}
+
+/// A Greybus connection.
+///
+/// # Invariants
+///
+/// The wrapped value is a valid `struct gb_connection`.
+///
+/// References to a `Connection` are only handed out for the duration of a host-device callback on
+/// a message travelling over it. During that time the connection is enabled, and its interface,
+/// if any, is alive: the core disables and destroys a bundle's connections before it tears down
+/// their interface.
+#[repr(transparent)]
+pub struct Connection(Opaque<bindings::gb_connection>);
+
+impl Connection {
+    /// # Safety
+    ///
+    /// `ptr` must be a valid pointer to a `struct gb_connection`.
+    #[inline]
+    pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::gb_connection) -> &'a Self {
+        // SAFETY: `Connection` is a transparent wrapper of `Opaque<bindings::gb_connection>`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the interface at the far end of the connection. In cases like SVC connection,
+    /// interface can be NULL.
+    #[inline]
+    pub fn interface(&self) -> Option<&Interface> {
+        // SAFETY: By the type invariant the connection is valid.
+        let intf_ptr = NonNull::new(unsafe { (*self.0.get()).intf })?;
+
+        // SAFETY: `intf_ptr` is non-NULL, and by the type invariant the interface is alive for as
+        // long as `&self` is. `Interface` is a transparent wrapper of
+        // `Opaque<bindings::gb_interface>`.
+        Some(unsafe { intf_ptr.cast().as_ref() })
+    }
+
+    /// Returns the CPort id this connection uses on the interface.
+    #[inline]
+    pub const fn intf_cport_id(&self) -> u16 {
+        // SAFETY: By the type invariant the connection is valid.
+        unsafe { (*self.0.get()).intf_cport_id }
+    }
+
+    /// Returns the CPort id this connection uses on the host device.
+    #[inline]
+    pub const fn hd_cport_id(&self) -> u16 {
+        // SAFETY: By the type invariant the connection is valid.
+        unsafe { (*self.0.get()).hd_cport_id }
+    }
+}
+
+/// A Greybus interface.
+///
+/// # Invariants
+///
+/// The wrapped value is a valid `struct gb_interface`.
+#[repr(transparent)]
+pub struct Interface(Opaque<bindings::gb_interface>);
+
+impl Interface {
+    /// Returns the interface id, unique within its host device.
+    #[inline]
+    pub const fn id(&self) -> u8 {
+        // SAFETY: By the type invariant the interface is valid.
+        unsafe { (*self.0.get()).interface_id }
+    }
+}
diff --git a/rust/kernel/greybus/protocols.rs b/rust/kernel/greybus/protocols.rs
new file mode 100644
index 000000000000..5bdd480ef744
--- /dev/null
+++ b/rust/kernel/greybus/protocols.rs
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Greybus wire format definitions.
+//!
+//! Thin `repr(transparent)` wrappers over the generated bindings for the operation header and the
+//! SVC protocol messages, plus the constants that go in their type and result fields. The
+//! wrappers exist to keep the byte-order conversions in one place: constructors take native-endian
+//! values and store little-endian, accessors convert back.
+
+/// Set in the header type field to mark a message as a response to the operation of the same id.
+pub const MESSAGE_TYPE_RESPONSE: u8 = 0x80;
+
+/// Operation result: the receiver does not support the request type.
+pub const GB_OP_PROTOCOL_BAD: u8 = bindings::gb_operation_result_GB_OP_PROTOCOL_BAD as u8;
+
+/// The CPort id reserved for the SVC connection on every host device.
+pub const GB_SVC_CPORT_ID: u16 = bindings::GB_SVC_CPORT_ID as u16;
+
+/// Defines `u8` constants from same-named `bindings` values.
+macro_rules! gb_u8_consts {
+    ($($name:ident),* $(,)?) => {
+        $(
+            #[allow(missing_docs)]
+            pub const $name: u8 = bindings::$name as u8;
+        )*
+    };
+}
+
+// SVC Operation Types
+gb_u8_consts! {
+    GB_SVC_TYPE_PROTOCOL_VERSION,
+    GB_SVC_TYPE_SVC_HELLO,
+    GB_SVC_TYPE_INTF_DEVICE_ID,
+    GB_SVC_TYPE_INTF_RESET,
+    GB_SVC_TYPE_CONN_CREATE,
+    GB_SVC_TYPE_CONN_DESTROY,
+    GB_SVC_TYPE_DME_PEER_GET,
+    GB_SVC_TYPE_DME_PEER_SET,
+    GB_SVC_TYPE_ROUTE_CREATE,
+    GB_SVC_TYPE_ROUTE_DESTROY,
+    GB_SVC_TYPE_TIMESYNC_ENABLE,
+    GB_SVC_TYPE_TIMESYNC_DISABLE,
+    GB_SVC_TYPE_TIMESYNC_AUTHORITATIVE,
+    GB_SVC_TYPE_INTF_SET_PWRM,
+    GB_SVC_TYPE_INTF_EJECT,
+    GB_SVC_TYPE_PING,
+    GB_SVC_TYPE_PWRMON_RAIL_COUNT_GET,
+    GB_SVC_TYPE_PWRMON_RAIL_NAMES_GET,
+    GB_SVC_TYPE_PWRMON_SAMPLE_GET,
+    GB_SVC_TYPE_PWRMON_INTF_SAMPLE_GET,
+    GB_SVC_TYPE_TIMESYNC_WAKE_PINS_ACQUIRE,
+    GB_SVC_TYPE_TIMESYNC_WAKE_PINS_RELEASE,
+    GB_SVC_TYPE_TIMESYNC_PING,
+    GB_SVC_TYPE_MODULE_INSERTED,
+    GB_SVC_TYPE_MODULE_REMOVED,
+    GB_SVC_TYPE_INTF_VSYS_ENABLE,
+    GB_SVC_TYPE_INTF_VSYS_DISABLE,
+    GB_SVC_TYPE_INTF_REFCLK_ENABLE,
+    GB_SVC_TYPE_INTF_REFCLK_DISABLE,
+    GB_SVC_TYPE_INTF_UNIPRO_ENABLE,
+    GB_SVC_TYPE_INTF_UNIPRO_DISABLE,
+    GB_SVC_TYPE_INTF_ACTIVATE,
+    GB_SVC_TYPE_INTF_RESUME,
+    GB_SVC_TYPE_INTF_MAILBOX_EVENT,
+    GB_SVC_TYPE_INTF_OOPS,
+}
+
+// UNIPRO modes
+gb_u8_consts! {
+    GB_SVC_UNIPRO_FAST_MODE,
+    GB_SVC_UNIPRO_SLOW_MODE,
+    GB_SVC_UNIPRO_FAST_AUTO_MODE,
+    GB_SVC_UNIPRO_SLOW_AUTO_MODE,
+    GB_SVC_UNIPRO_MODE_UNCHANGED,
+    GB_SVC_UNIPRO_HIBERNATE_MODE,
+    GB_SVC_UNIPRO_OFF_MODE,
+}
+
+// PWR States
+gb_u8_consts! {
+    GB_SVC_SETPWRM_PWR_OK,
+    GB_SVC_SETPWRM_PWR_LOCAL,
+    GB_SVC_SETPWRM_PWR_REMOTE,
+    GB_SVC_SETPWRM_PWR_BUSY,
+    GB_SVC_SETPWRM_PWR_ERROR_CAP,
+    GB_SVC_SETPWRM_PWR_FATAL_ERROR,
+}
+
+// Vsys Result
+gb_u8_consts! {
+    GB_SVC_INTF_VSYS_OK,
+    GB_SVC_INTF_VSYS_FAIL,
+}
+
+// Refclk Result
+gb_u8_consts! {
+    GB_SVC_INTF_REFCLK_OK,
+    GB_SVC_INTF_REFCLK_FAIL,
+}
+
+// Unipro Result
+gb_u8_consts! {
+    GB_SVC_INTF_UNIPRO_OK,
+    GB_SVC_INTF_UNIPRO_FAIL,
+    GB_SVC_INTF_UNIPRO_NOT_OFF,
+}
+
+// Op Codes
+gb_u8_consts! {
+    GB_SVC_OP_SUCCESS,
+    GB_SVC_OP_UNKNOWN_ERROR,
+    GB_SVC_INTF_NOT_DETECTED,
+    GB_SVC_INTF_NO_UPRO_LINK,
+    GB_SVC_INTF_UPRO_NOT_DOWN,
+    GB_SVC_INTF_UPRO_NOT_HIBERNATED,
+    GB_SVC_INTF_NO_V_SYS,
+    GB_SVC_INTF_V_CHG,
+    GB_SVC_INTF_WAKE_BUSY,
+    GB_SVC_INTF_NO_REFCLK,
+    GB_SVC_INTF_RELEASING,
+    GB_SVC_INTF_NO_ORDER,
+    GB_SVC_INTF_MBOX_SET,
+    GB_SVC_INTF_BAD_MBOX,
+    GB_SVC_INTF_OP_TIMEOUT,
+    GB_SVC_PWRMON_OP_NOT_PRESENT,
+}
+
+// Greybus Interface Types
+gb_u8_consts! {
+    GB_SVC_INTF_TYPE_UNKNOWN,
+    GB_SVC_INTF_TYPE_DUMMY,
+    GB_SVC_INTF_TYPE_UNIPRO,
+    GB_SVC_INTF_TYPE_GREYBUS,
+}
+
+/// The header every Greybus message starts with.
+///
+/// Its `size` field is the size of the whole message, this header included.
+#[repr(transparent)]
+pub struct GbOperationMsgHdr(bindings::gb_operation_msg_hdr);
+
+// SAFETY: `gb_operation_msg_hdr` is a POD type with no padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbOperationMsgHdr {}
+
+impl GbOperationMsgHdr {
+    /// Builds a header. `size` is the whole message, this header included.
+    #[inline]
+    pub const fn new(size: u16, operation_id: u16, type_: u8, result: u8) -> Self {
+        Self(bindings::gb_operation_msg_hdr {
+            size: size.to_le(),
+            operation_id: operation_id.to_le(),
+            type_,
+            result,
+            pad: [0u8; 2],
+        })
+    }
+
+    /// Returns the type field, response bit included.
+    #[inline]
+    pub const fn msg_type(&self) -> u8 {
+        self.0.type_
+    }
+
+    /// Returns whether this is a response rather than a request.
+    #[inline]
+    pub const fn is_response(&self) -> bool {
+        self.0.type_ & MESSAGE_TYPE_RESPONSE != 0
+    }
+
+    /// Returns the operation id pairing a response with its request. Zero for unidirectional
+    /// messages.
+    #[inline]
+    pub const fn operation_id(&self) -> u16 {
+        u16::from_le(self.0.operation_id)
+    }
+
+    /// Returns the type field with the response bit cleared.
+    #[inline]
+    pub const fn request_type(&self) -> u8 {
+        self.msg_type() & !MESSAGE_TYPE_RESPONSE
+    }
+
+    /// Returns the whole message size, this header included.
+    #[inline]
+    pub const fn size(&self) -> u16 {
+        u16::from_le(self.0.size)
+    }
+}
+
+/// Request for [`GB_SVC_TYPE_PROTOCOL_VERSION`].
+#[repr(transparent)]
+pub struct GbSvcVersionRequest(bindings::gb_svc_version_request);
+
+// SAFETY: `gb_svc_version_request` is a packed struct of integer fields, so it has no padding and
+// no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcVersionRequest {}
+
+impl GbSvcVersionRequest {
+    /// Creates a request advertising SVC protocol version `major`.`minor`.
+    #[inline]
+    pub const fn new(major: u8, minor: u8) -> Self {
+        Self(bindings::gb_svc_version_request { major, minor })
+    }
+}
+
+/// Request for [`GB_SVC_TYPE_SVC_HELLO`].
+#[repr(transparent)]
+pub struct GbSvcHelloRequest(bindings::gb_svc_hello_request);
+
+// SAFETY: `gb_svc_hello_request` is a packed struct of integer fields, so it has no padding and
+// no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcHelloRequest {}
+
+impl GbSvcHelloRequest {
+    /// Creates a hello request identifying the endo as `endo_id` and the AP's own interface as
+    /// `interface_id`.
+    #[inline]
+    pub const fn new(endo_id: u16, interface_id: u8) -> Self {
+        Self(bindings::gb_svc_hello_request {
+            endo_id: endo_id.to_le(),
+            interface_id,
+        })
+    }
+}
+
+/// Request for [`GB_SVC_TYPE_INTF_SET_PWRM`].
+#[repr(transparent)]
+pub struct GbSvcIntfSetPwrmRequest(bindings::gb_svc_intf_set_pwrm_request);
+
+// SAFETY: `gb_svc_intf_set_pwrm_request` is a packed struct of sized fields, so every bit pattern
+// of its size is a valid instance.
+unsafe impl kernel::transmute::FromBytes for GbSvcIntfSetPwrmRequest {}
+
+impl GbSvcIntfSetPwrmRequest {
+    /// Returns the requested TX gear, one of the `GB_SVC_UNIPRO_*` modes.
+    #[inline]
+    pub const fn tx_mode(&self) -> u8 {
+        self.0.tx_mode
+    }
+
+    /// Returns the requested RX gear, one of the `GB_SVC_UNIPRO_*` modes.
+    #[inline]
+    pub const fn rx_mode(&self) -> u8 {
+        self.0.rx_mode
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_SET_PWRM`].
+#[repr(transparent)]
+pub struct GbSvcIntfSetPwrmResponse(bindings::gb_svc_intf_set_pwrm_response);
+
+// SAFETY: `gb_svc_intf_set_pwrm_response` is a packed struct of integer fields, so it has no
+// padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfSetPwrmResponse {}
+
+impl GbSvcIntfSetPwrmResponse {
+    /// `result_code` is one of the `GB_SVC_SETPWRM_PWR_*` codes.
+    #[inline]
+    pub const fn new(result_code: u8) -> Self {
+        Self(bindings::gb_svc_intf_set_pwrm_response { result_code })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_DME_PEER_GET`].
+#[repr(transparent)]
+pub struct GbSvcDmePeerGetResponse(bindings::gb_svc_dme_peer_get_response);
+
+// SAFETY: `gb_svc_dme_peer_get_response` is a packed struct of integer fields, so it has no
+// padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcDmePeerGetResponse {}
+
+impl GbSvcDmePeerGetResponse {
+    /// `result_code` is the UniPro `ConfigResultCode`; `attr_value` is the UniPro attribute
+    /// value.
+    #[inline]
+    pub const fn new(result_code: u16, attr_value: u32) -> Self {
+        Self(bindings::gb_svc_dme_peer_get_response {
+            result_code: result_code.to_le(),
+            attr_value: attr_value.to_le(),
+        })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_DME_PEER_SET`].
+#[repr(transparent)]
+pub struct GbSvcDmePeerSetResponse(bindings::gb_svc_dme_peer_set_response);
+
+// SAFETY: `gb_svc_dme_peer_set_response` is a packed struct of integer fields, so it has no padding
+// and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcDmePeerSetResponse {}
+
+impl GbSvcDmePeerSetResponse {
+    /// `result_code` is the UniPro `ConfigResultCode`.
+    #[inline]
+    pub const fn new(result_code: u16) -> Self {
+        Self(bindings::gb_svc_dme_peer_set_response {
+            result_code: result_code.to_le(),
+        })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_PWRMON_RAIL_COUNT_GET`].
+#[repr(transparent)]
+pub struct GbSvcPwrmonRailCountGetResponse(bindings::gb_svc_pwrmon_rail_count_get_response);
+
+// SAFETY: `gb_svc_pwrmon_rail_count_get_response` is a packed struct of integer fields, so it has
+// no padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcPwrmonRailCountGetResponse {}
+
+impl GbSvcPwrmonRailCountGetResponse {
+    /// Creates a response reporting `rail_count` available rails.
+    #[inline]
+    pub const fn new(rail_count: u8) -> Self {
+        Self(bindings::gb_svc_pwrmon_rail_count_get_response { rail_count })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_VSYS_ENABLE`] and [`GB_SVC_TYPE_INTF_VSYS_DISABLE`].
+#[repr(transparent)]
+pub struct GbSvcIntfVsysResponse(bindings::gb_svc_intf_vsys_response);
+
+// SAFETY: `gb_svc_intf_vsys_response` is a packed struct of integer fields, so it has no padding
+// and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfVsysResponse {}
+
+impl GbSvcIntfVsysResponse {
+    /// `result_code` is [`GB_SVC_INTF_VSYS_OK`] or [`GB_SVC_INTF_VSYS_FAIL`].
+    #[inline]
+    pub const fn new(result_code: u8) -> Self {
+        Self(bindings::gb_svc_intf_vsys_response { result_code })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_REFCLK_ENABLE`] and [`GB_SVC_TYPE_INTF_REFCLK_DISABLE`].
+#[repr(transparent)]
+pub struct GbSvcIntfRefclkResponse(bindings::gb_svc_intf_refclk_response);
+
+// SAFETY: `gb_svc_intf_refclk_response` is a packed struct of integer fields, so it has no padding
+// and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfRefclkResponse {}
+
+impl GbSvcIntfRefclkResponse {
+    /// `result_code` is [`GB_SVC_INTF_REFCLK_OK`] or [`GB_SVC_INTF_REFCLK_FAIL`].
+    #[inline]
+    pub const fn new(result_code: u8) -> Self {
+        Self(bindings::gb_svc_intf_refclk_response { result_code })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_UNIPRO_ENABLE`] and [`GB_SVC_TYPE_INTF_UNIPRO_DISABLE`].
+#[repr(transparent)]
+pub struct GbSvcIntfUniproResponse(bindings::gb_svc_intf_unipro_response);
+
+// SAFETY: `gb_svc_intf_unipro_response` is a packed struct of integer fields, so it has no padding
+// and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfUniproResponse {}
+
+impl GbSvcIntfUniproResponse {
+    /// `result_code` is one of the `GB_SVC_INTF_UNIPRO_*` codes.
+    #[inline]
+    pub const fn new(result_code: u8) -> Self {
+        Self(bindings::gb_svc_intf_unipro_response { result_code })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_ACTIVATE`].
+#[repr(transparent)]
+pub struct GbSvcIntfActivateResponse(bindings::gb_svc_intf_activate_response);
+
+// SAFETY: `gb_svc_intf_activate_response` is a packed struct of integer fields, so it has no
+// padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfActivateResponse {}
+
+impl GbSvcIntfActivateResponse {
+    /// `status` is one of the `GB_SVC_OP_*` codes; `intf_type` is one of the
+    /// `GB_SVC_INTF_TYPE_*` values and is only meaningful when `status` is
+    /// [`GB_SVC_OP_SUCCESS`].
+    #[inline]
+    pub const fn new(status: u8, intf_type: u8) -> Self {
+        Self(bindings::gb_svc_intf_activate_response { status, intf_type })
+    }
+}
+
+/// Response to [`GB_SVC_TYPE_INTF_RESUME`].
+#[repr(transparent)]
+pub struct GbSvcIntfResumeResponse(bindings::gb_svc_intf_resume_response);
+
+// SAFETY: `gb_svc_intf_resume_response` is a packed struct of integer fields, so it has no padding
+// and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcIntfResumeResponse {}
+
+impl GbSvcIntfResumeResponse {
+    /// `status` is one of the `GB_SVC_OP_*` codes.
+    #[inline]
+    pub const fn new(status: u8) -> Self {
+        Self(bindings::gb_svc_intf_resume_response { status })
+    }
+}
+
+/// Request for [`GB_SVC_TYPE_MODULE_INSERTED`].
+#[repr(transparent)]
+pub struct GbSvcModuleInsertedRequest(bindings::gb_svc_module_inserted_request);
+
+// SAFETY: `gb_svc_module_inserted_request` is a packed struct of integer fields, so it has no
+// padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcModuleInsertedRequest {}
+
+impl GbSvcModuleInsertedRequest {
+    /// The module spans `intf_count` consecutive interfaces starting at `primary_intf_id`.
+    /// `flags` is a mask of `GB_SVC_MODULE_INSERTED_FLAG_*` values.
+    #[inline]
+    pub const fn new(primary_intf_id: u8, intf_count: u8, flags: u16) -> Self {
+        Self(bindings::gb_svc_module_inserted_request {
+            primary_intf_id,
+            intf_count,
+            flags: flags.to_le(),
+        })
+    }
+}
+
+/// Request for [`GB_SVC_TYPE_MODULE_REMOVED`].
+#[repr(transparent)]
+pub struct GbSvcModuleRemovedRequest(bindings::gb_svc_module_removed_request);
+
+// SAFETY: `gb_svc_module_removed_request` is a packed struct of integer fields, so it has no
+// padding and no interior mutability.
+unsafe impl kernel::transmute::AsBytes for GbSvcModuleRemovedRequest {}
+
+// SAFETY: `gb_svc_module_removed_request` is a struct of `u8` field, so every bit pattern of its
+// size is a valid instance.
+unsafe impl kernel::transmute::FromBytes for GbSvcModuleRemovedRequest {}
+
+impl GbSvcModuleRemovedRequest {
+    /// `primary_intf_id` identifies the module, and matches the one given when it was inserted.
+    #[inline]
+    pub const fn new(primary_intf_id: u8) -> Self {
+        Self(bindings::gb_svc_module_removed_request { primary_intf_id })
+    }
+
+    /// Returns the primary_intf_id field.
+    #[inline]
+    pub const fn primary_intf_id(&self) -> u8 {
+        self.0.primary_intf_id
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 637442761d34..931831343fd3 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -84,6 +84,8 @@
 pub mod fwctl;
 #[cfg(CONFIG_GPU_BUDDY = "y")]
 pub mod gpu;
+#[cfg(CONFIG_RUST_GREYBUS_ABSTRACTIONS)]
+pub mod greybus;
 #[cfg(CONFIG_I2C = "y")]
 pub mod i2c;
 pub mod id_pool;

-- 
2.56.0


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

* [PATCH v5 7/9] drivers: greybus: Add software SVC implementation
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (5 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 6/9] rust: kernel: Add greybus abstractions Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:32   ` sashiko-bot
  2026-11-03 16:11 ` [PATCH v5 8/9] dt-bindings: beagle: Add BeagleConnect Freedom Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 9/9] greybus: Add Rust UART node driver Ayush Singh
  8 siblings, 1 reply; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

A Greybus network requires an SVC (Supervisory Controller) to bring
interfaces up, assign device IDs and connect CPorts to the AP. On a
UniPro network the SVC is a real entity on the bus, but transports that
merely carry Greybus messages - a UART, an I2C bus, a network link -
have no such entity, so every host driver for those transports ends up
open-coding an SVC emulation of its own.

Add gb-softsvc, a Rust implementation of the SVC that lives in the
kernel and registers itself as a Greybus host device. It answers the
SVC-side operations the core expects during interface bring-up and
teardown.

Host drivers use it through a small in-kernel interface. A driver
implements the NodeOps trait to send data towards its node and then
calls module_insert() to announce a new node, submit_message() to hand
incoming Greybus messages back to the core, and module_remove() on
disconnect. Interface IDs are allocated by the SVC and returned from
module_insert().

Connection create/destroy and interface activate/resume currently just
acknowledge the request; callbacks into NodeOps can be added later if
transports need to act on them.

gb_softsvc_exports.c exports the Rust symbols for dependent modules, as
the build system does not yet support Rust cross-module dependencies
natively. This hack is taken from nova-core [0].

[0]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.com/

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS                          |   6 +
 drivers/greybus/Kconfig              |  12 +
 drivers/greybus/Makefile             |  40 +++
 drivers/greybus/gb_softsvc.rs        | 533 +++++++++++++++++++++++++++++++++++
 drivers/greybus/gb_softsvc_exports.c |  15 +
 5 files changed, 606 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d98db7618c3..c9126dc205db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11516,6 +11516,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
 F:	drivers/greybus/gb-beagleplay.c
 
+GREYBUS SOFTSVC DRIVERS
+M:	Ayush Singh <ayush@beagleboard.org>
+L:	greybus-dev@lists.linaro.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/greybus/gb_softsvc.rs
+
 GREYBUS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
 M:	Alex Elder <elder@kernel.org>
diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
index d9f52024d1c2..709563f8c0ad 100644
--- a/drivers/greybus/Kconfig
+++ b/drivers/greybus/Kconfig
@@ -49,5 +49,17 @@ config GREYBUS_ES2
 	  To compile this code as a module, choose M here: the module
 	  will be called gb-es2.ko
 
+config GREYBUS_SOFTSVC
+	tristate "Greybus software SVC"
+	depends on RUST_GREYBUS_ABSTRACTIONS
+	help
+	  Select this option to enable a software implementation of the
+	  Greybus SVC (Supervisory Controller) running in the kernel. It can
+	  be used by host drivers that talk to a Greybus network which does
+	  not provide an SVC of its own.
+
+	  To compile this code as a module, choose M here: the module
+	  will be called gb-softsvc.ko
+
 endif	# GREYBUS
 
diff --git a/drivers/greybus/Makefile b/drivers/greybus/Makefile
index d986e94f8897..0fa802fdf6ca 100644
--- a/drivers/greybus/Makefile
+++ b/drivers/greybus/Makefile
@@ -25,4 +25,44 @@ gb-es2-y := es2.o
 
 obj-$(CONFIG_GREYBUS_ES2)	+= gb-es2.o
 
+obj-$(CONFIG_GREYBUS_SOFTSVC)	+= gb-softsvc.o
+gb-softsvc-y			+= gb_softsvc.o gb_softsvc_exports.o
 
+rust_needed_exports = \
+	{ $(if $(strip $(2)),$(NM) -u $(2);,) echo "__DEFINED_RUST_SYMBOLS__"; \
+	  $(NM) -p --defined-only $(1); } | \
+	awk -v fmt='$(3)' ' \
+		/^__DEFINED_RUST_SYMBOLS__$$/ { defs = 1; next } \
+		!defs { if ($$NF ~ /^_R/) needed[$$NF] = 1; next } \
+		defs && $$2 ~ /(T|R|D|B)/ && $$3 ~ /^_R/ && \
+			$$3 !~ /_(init|cleanup)_module$$/ && \
+			$$3 !~ /__(pfx|cfi|odr_asan)/ && \
+			$$3 in needed { printf fmt, $$3 } \
+	'
+
+quiet_cmd_exports = EXPORTS $@
+      cmd_exports = \
+	$(call rust_needed_exports,$<,$(gb-softsvc-export-deps),EXPORT_SYMBOL_RUST_GPL(%s);\n) > $@
+
+$(obj)/exports_gb_softsvc_generated.h: $(obj)/gb_softsvc.o $(gb-softsvc-export-deps) FORCE
+	$(call if_changed,exports)
+
+targets += exports_gb_softsvc_generated.h
+
+$(obj)/gb_softsvc_exports.o: $(obj)/exports_gb_softsvc_generated.h
+CFLAGS_gb_softsvc_exports.o := -I $(objtree)/$(obj)
+
+ifdef CONFIG_MODVERSIONS
+# The C export shim declares Rust symbols as `extern int`, so reuse its export
+# list but generate symbol CRCs from the Rust object instead of the shim's DWARF.
+$(obj)/gb_softsvc_exports.o: private cmd_gensymtypes_c = \
+	$(call getexportsymbols,\1) | \
+	$(objtree)/scripts/gendwarfksyms/gendwarfksyms \
+		$(if $(KBUILD_GENDWARFKSYMS_STABLE), --stable) \
+		$(if $(KBUILD_SYMTYPES), --symtypes $(@:.o=.symtypes),) \
+		$(obj)/gb_softsvc.o
+endif
+
+# Output gb_softsvc's crate metadata for use by gb_uart_node at compile time.
+RUSTFLAGS_gb_softsvc.o += \
+	--emit=metadata=$(objtree)/$(obj)/libgb_softsvc.rmeta
diff --git a/drivers/greybus/gb_softsvc.rs b/drivers/greybus/gb_softsvc.rs
new file mode 100644
index 000000000000..c9bff6a5fa65
--- /dev/null
+++ b/drivers/greybus/gb_softsvc.rs
@@ -0,0 +1,533 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Greybus software SVC.
+//!
+//! A Greybus network needs an SVC to bring interfaces up and to connect their CPorts to the AP.
+//! Transports that carry Greybus without one - a UART or an I2C bus rather than a UniPro network -
+//! have to provide it themselves.
+//!
+//! This module implements that SVC in software.
+
+use kernel::{
+    alloc::Flags,
+    c_str,
+    error::code,
+    faux,
+    greybus::{
+        self,
+        hd,
+        protocols,
+        CPORT_ID_MAX,
+        GB_OPERATION_SIZE_MAX, //
+    },
+    id_pool::IdPool,
+    new_spinlock,
+    prelude::*,
+    sync::{
+        aref::ARef,
+        Arc,
+        SpinLock, //
+    },
+    transmute::AsBytes,
+};
+use pin_init::init_array_from_fn;
+
+const SVC_VERSION_MAJOR: u8 = 0;
+const SVC_VERSION_MINOR: u8 = 1;
+
+const ENDO_ID: u16 = 0;
+const AP_INF_ID: u8 = 0;
+
+const TOSHIBA_DMID: u32 = 0x0126;
+
+const MAX_INTF_ID: u8 = u8::MAX;
+const INTF_ID_START: u8 = 1;
+const INTF_MAP_LEN: usize = (MAX_INTF_ID - INTF_ID_START) as usize;
+
+#[repr(C, packed)]
+struct Msg<P> {
+    hdr: protocols::GbOperationMsgHdr,
+    payload: P,
+}
+
+// SAFETY: `Msg` is `repr(C, packed)`, so it has no padding between its fields, and both fields
+// are `AsBytes`, so neither has uninitialized bytes of its own.
+unsafe impl<P: AsBytes> AsBytes for Msg<P> {}
+
+impl<P> Msg<P> {
+    const GB_SIZE: u16 = size_of::<Self>() as u16;
+
+    /// Builds a request of type `ty`.
+    ///
+    /// An `operation_id` of zero marks a unidirectional request, for which the AP sends no
+    /// response.
+    const fn request(operation_id: u16, ty: u8, payload: P) -> Self {
+        Self {
+            hdr: protocols::GbOperationMsgHdr::new(Self::GB_SIZE, operation_id, ty, 0),
+            payload,
+        }
+    }
+
+    /// Builds the response to `request`, carrying `result`.
+    const fn response(request: &protocols::GbOperationMsgHdr, result: u8, payload: P) -> Self {
+        Self {
+            hdr: protocols::GbOperationMsgHdr::new(
+                Self::GB_SIZE,
+                request.operation_id(),
+                request.msg_type() | protocols::MESSAGE_TYPE_RESPONSE,
+                result,
+            ),
+            payload,
+        }
+    }
+}
+
+const _: () = assert!(size_of::<Msg<()>>() == size_of::<protocols::GbOperationMsgHdr>());
+
+module! {
+    type: GreybusSoftSvc,
+    name: "gb_softsvc",
+    authors: ["Ayush Singh <ayush@beagleboard.org>"],
+    description: "Greybus software SVC implementation",
+    license: "GPL",
+}
+
+kernel::sync::global_lock! {
+    unsafe(uninit) static GLOBAL_STATE: Mutex<Option<GlobalState>> = None;
+}
+
+struct GlobalState {
+    svc: Arc<Svc>,
+    gb_hd: ARef<hd::Device>,
+}
+
+impl GlobalState {
+    fn new(svc: Arc<Svc>, gb_hd: ARef<hd::Device>) -> Self {
+        Self { svc, gb_hd }
+    }
+}
+
+#[pin_data]
+struct Svc {
+    #[pin]
+    svc_operation_id: SpinLock<IdPool>,
+    // Pos 0 = INTF_ID_START
+    #[pin]
+    intf_map: SpinLock<[Option<Arc<dyn InterfaceOps>>; INTF_MAP_LEN]>,
+}
+
+impl Svc {
+    fn new() -> impl PinInit<Self, Error> {
+        pin_init!(Self {
+            intf_map <- new_spinlock!(init_array_from_fn(|_| None)),
+            svc_operation_id <- new_spinlock!(IdPool::with_capacity(u16::MAX.into(), GFP_KERNEL)?)
+        }? Error)
+    }
+
+    fn new_operation_id(&self) -> Result<u16> {
+        let mut ida = self.svc_operation_id.lock();
+        let res = ida.find_unused_id(1).ok_or(code::EOVERFLOW)?.acquire() as u16;
+
+        Ok(res)
+    }
+
+    fn release_operation_id(&self, id: u16) {
+        let mut ida = self.svc_operation_id.lock();
+        ida.release_id(id as usize);
+    }
+
+    fn map_insert(&self, intfs: &[Arc<dyn InterfaceOps>]) -> Result<u8> {
+        fn find_empty_window(
+            intf_map: &[Option<Arc<dyn InterfaceOps>>; INTF_MAP_LEN],
+            count: usize,
+        ) -> Result<u8> {
+            for (id, window) in intf_map.windows(count).enumerate() {
+                if window.iter().all(|x| x.is_none()) {
+                    return Ok(id as u8);
+                }
+            }
+
+            Err(code::EOVERFLOW)
+        }
+
+        let mut guard = self.intf_map.lock();
+
+        let pos = find_empty_window(&guard, intfs.len())?;
+        for (i, intf) in intfs.iter().cloned().enumerate() {
+            guard[usize::from(pos) + i] = Some(intf);
+        }
+
+        Ok(pos + INTF_ID_START)
+    }
+
+    fn map_remove(&self, primary_id: u8, intf_count: u8, dev: &kernel::device::Device) {
+        let id = match primary_id.checked_sub(INTF_ID_START) {
+            Some(x) => x,
+            None => {
+                dev_warn!(dev, "Invalid interface id");
+                return;
+            }
+        };
+
+        for i in 0..intf_count {
+            // Drop outside the lock: this may be the last reference, and the node's `Drop` may
+            // sleep. `let _ =` would drop it before the guard, while the lock is still held.
+            let intf = self.intf_map.lock()[usize::from(id + i)].take();
+            drop(intf);
+        }
+    }
+
+    fn intf_by_id(&self, id: u8) -> Result<Arc<dyn InterfaceOps>> {
+        let id = id - INTF_ID_START;
+        let guard = self.intf_map.lock();
+
+        guard
+            .get(usize::from(id))
+            .ok_or(code::ENODEV)?
+            .as_ref()
+            .ok_or(code::ENODEV)
+            .cloned()
+    }
+
+    fn send_request<P: AsBytes>(&self, hd: &hd::Device, ty: u8, payload: P) -> Result<()> {
+        let msg = Msg::request(self.new_operation_id()?, ty, payload);
+        hd.data_rcvd(protocols::GB_SVC_CPORT_ID, msg.as_bytes());
+        Ok(())
+    }
+
+    fn module_insert(&self, hd: &hd::Device, intfs: &[Arc<dyn InterfaceOps>]) -> Result<u8> {
+        let intf_id = self.map_insert(intfs)?;
+        // map_insert already checks that intf len fits in u8
+        let intf_count = intfs.len() as u8;
+
+        let payload = protocols::GbSvcModuleInsertedRequest::new(intf_id, intf_count, 0);
+        let res = self.send_request(hd, protocols::GB_SVC_TYPE_MODULE_INSERTED, payload);
+
+        if res.is_err() {
+            self.map_remove(intf_id, intf_count, hd.as_ref());
+        }
+
+        res.map(|()| intf_id)
+    }
+
+    fn module_remove(&self, hd: &hd::Device, intf_id: u8, intf_count: u8) {
+        let payload = protocols::GbSvcModuleRemovedRequest::new(intf_id);
+        let res = self.send_request(hd, protocols::GB_SVC_TYPE_MODULE_REMOVED, payload);
+
+        if res.is_err() {
+            dev_warn!(
+                hd.as_ref(),
+                "Failed to send module_removed svc request for {}",
+                intf_id
+            );
+        }
+
+        // The ids stay taken until the core has removed the module, or a new module could be
+        // given them while the old connections still exist.
+        hd.svc_flush();
+
+        self.map_remove(intf_id, intf_count, hd.as_ref());
+    }
+
+    fn send_version(&self, hd: &hd::Device) -> Result<()> {
+        dev_info!(hd.as_ref(), "Sending SVC version request");
+
+        let payload = protocols::GbSvcVersionRequest::new(SVC_VERSION_MAJOR, SVC_VERSION_MINOR);
+        self.send_request(hd, protocols::GB_SVC_TYPE_PROTOCOL_VERSION, payload)?;
+
+        Ok(())
+    }
+
+    fn send_svc_hello(&self, hd: &hd::Device) -> Result<()> {
+        dev_info!(hd.as_ref(), "Sending SVC Hello request");
+
+        let payload = protocols::GbSvcHelloRequest::new(ENDO_ID, AP_INF_ID);
+        self.send_request(hd, protocols::GB_SVC_TYPE_SVC_HELLO, payload)?;
+
+        Ok(())
+    }
+
+    fn send_response<P: AsBytes>(
+        &self,
+        hd: &hd::Device,
+        msg: &protocols::GbOperationMsgHdr,
+        payload: P,
+    ) {
+        let msg = Msg::response(msg, 0, payload);
+        hd.data_rcvd(protocols::GB_SVC_CPORT_ID, msg.as_bytes());
+    }
+
+    fn intf_set_pwrm(&self, hd: &hd::Device, msg: &greybus::Message) -> Result<()> {
+        let req_msg: &protocols::GbSvcIntfSetPwrmRequest = msg.payload().ok_or(code::EINVAL)?;
+        let result_code = if req_msg.tx_mode() == protocols::GB_SVC_UNIPRO_HIBERNATE_MODE
+            && req_msg.rx_mode() == protocols::GB_SVC_UNIPRO_HIBERNATE_MODE
+        {
+            protocols::GB_SVC_SETPWRM_PWR_OK
+        } else {
+            protocols::GB_SVC_SETPWRM_PWR_LOCAL
+        };
+
+        let payload = protocols::GbSvcIntfSetPwrmResponse::new(result_code);
+        self.send_response(hd, msg.header(), payload);
+
+        Ok(())
+    }
+
+    fn dme_peer_get(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcDmePeerGetResponse::new(0, TOSHIBA_DMID);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn dme_peer_set(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcDmePeerSetResponse::new(0);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn pwrmon_rail_count_get(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcPwrmonRailCountGetResponse::new(0);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn intf_vsys_enable_disable(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcIntfVsysResponse::new(protocols::GB_SVC_INTF_VSYS_OK);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn intf_refclk_enable_disable(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcIntfRefclkResponse::new(protocols::GB_SVC_INTF_REFCLK_OK);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn intf_unipro_enable_disable(&self, hd: &hd::Device, hdr: &protocols::GbOperationMsgHdr) {
+        let payload = protocols::GbSvcIntfUniproResponse::new(protocols::GB_SVC_INTF_UNIPRO_OK);
+        self.send_response(hd, hdr, payload);
+    }
+
+    fn intf_activate(&self, hd: &hd::Device, msg: &greybus::Message) {
+        let payload = protocols::GbSvcIntfActivateResponse::new(
+            protocols::GB_SVC_OP_SUCCESS,
+            protocols::GB_SVC_INTF_TYPE_GREYBUS,
+        );
+
+        // TODO: Maybe call a callback?
+
+        self.send_response(hd, msg.header(), payload);
+    }
+
+    fn intf_resume(&self, hd: &hd::Device, msg: &greybus::Message) {
+        let payload = protocols::GbSvcIntfResumeResponse::new(protocols::GB_SVC_OP_SUCCESS);
+
+        // TODO: Maybe call a callback?
+
+        self.send_response(hd, msg.header(), payload);
+    }
+
+    fn conn_create(&self, hd: &hd::Device, msg: &greybus::Message) -> Result<()> {
+        // TODO: Evaluate if we need to add callback to InterfaceOps
+        self.send_response(hd, msg.header(), ());
+
+        Ok(())
+    }
+
+    fn conn_destroy(&self, hd: &hd::Device, msg: &greybus::Message) -> Result<()> {
+        // TODO: Evaluate if we need to add callback to InterfaceOps
+        self.send_response(hd, msg.header(), ());
+
+        Ok(())
+    }
+}
+
+struct GbHdDriver(Arc<Svc>);
+
+impl GbHdDriver {
+    fn svc_handler(&self, hd: &hd::Device, msg: &greybus::Message) -> Result<()> {
+        let hdr = msg.header();
+
+        if hdr.is_response() {
+            self.0.release_operation_id(hdr.operation_id());
+        }
+
+        match (hdr.is_response(), hdr.request_type()) {
+            (
+                false,
+                protocols::GB_SVC_TYPE_INTF_DEVICE_ID
+                | protocols::GB_SVC_TYPE_ROUTE_CREATE
+                | protocols::GB_SVC_TYPE_ROUTE_DESTROY
+                | protocols::GB_SVC_TYPE_PING,
+            ) => self.0.send_response(hd, hdr, ()),
+            (false, protocols::GB_SVC_TYPE_CONN_CREATE) => self.0.conn_create(hd, msg)?,
+            (false, protocols::GB_SVC_TYPE_CONN_DESTROY) => self.0.conn_destroy(hd, msg)?,
+            (false, protocols::GB_SVC_TYPE_DME_PEER_GET) => self.0.dme_peer_get(hd, hdr),
+            (false, protocols::GB_SVC_TYPE_DME_PEER_SET) => self.0.dme_peer_set(hd, hdr),
+            (false, protocols::GB_SVC_TYPE_INTF_SET_PWRM) => self.0.intf_set_pwrm(hd, msg)?,
+            (false, protocols::GB_SVC_TYPE_PWRMON_RAIL_COUNT_GET) => {
+                self.0.pwrmon_rail_count_get(hd, hdr)
+            }
+            (
+                false,
+                protocols::GB_SVC_TYPE_INTF_VSYS_ENABLE | protocols::GB_SVC_TYPE_INTF_VSYS_DISABLE,
+            ) => self.0.intf_vsys_enable_disable(hd, hdr),
+            (
+                false,
+                protocols::GB_SVC_TYPE_INTF_REFCLK_ENABLE
+                | protocols::GB_SVC_TYPE_INTF_REFCLK_DISABLE,
+            ) => self.0.intf_refclk_enable_disable(hd, hdr),
+            (
+                false,
+                protocols::GB_SVC_TYPE_INTF_UNIPRO_ENABLE
+                | protocols::GB_SVC_TYPE_INTF_UNIPRO_DISABLE,
+            ) => self.0.intf_unipro_enable_disable(hd, hdr),
+            (false, protocols::GB_SVC_TYPE_INTF_ACTIVATE) => self.0.intf_activate(hd, msg),
+            (false, protocols::GB_SVC_TYPE_INTF_RESUME) => self.0.intf_resume(hd, msg),
+            (true, protocols::GB_SVC_TYPE_PROTOCOL_VERSION) => self.0.send_svc_hello(hd)?,
+            (true, protocols::GB_SVC_TYPE_SVC_HELLO) => {
+                let global_state = GlobalState::new(self.0.clone(), hd.into());
+                let _ = GLOBAL_STATE.lock().replace(global_state);
+            }
+            (
+                true,
+                protocols::GB_SVC_TYPE_MODULE_INSERTED | protocols::GB_SVC_TYPE_MODULE_REMOVED,
+            ) => {}
+            (false, ty) => {
+                dev_warn!(hd.as_ref(), "unsupported request 0x{:02x}\n", ty);
+                let rsp = Msg::response(hdr, protocols::GB_OP_PROTOCOL_BAD, ());
+                hd.data_rcvd(protocols::GB_SVC_CPORT_ID, rsp.as_bytes());
+            }
+            (true, ty) => dev_warn!(hd.as_ref(), "unexpected response 0x{:02x}\n", ty),
+        };
+
+        Ok(())
+    }
+}
+
+#[vtable]
+impl hd::HdDriver for GbHdDriver {
+    fn message_send(
+        data: &Self,
+        hd: &hd::Device,
+        dest_cport_id: u16,
+        msg: &greybus::Message,
+        gfp_mask: Flags,
+    ) -> Result {
+        if dest_cport_id == protocols::GB_SVC_CPORT_ID {
+            data.svc_handler(hd, msg)
+        } else {
+            let conn = msg.operation().connection();
+            let intf = data.0.intf_by_id(conn.interface().unwrap().id())?;
+
+            let mut buf = KVec::with_capacity(msg.header().size().into(), gfp_mask)?;
+
+            buf.extend_from_slice(msg.header().as_bytes(), gfp_mask)?;
+            buf.extend_from_slice(msg.payload_bytes(), gfp_mask)?;
+
+            intf.write(&buf, conn.intf_cport_id(), gfp_mask)
+        }
+    }
+
+    fn message_cancel(_msg: &greybus::Message) {}
+}
+
+struct GreybusSoftSvc {
+    _hd: hd::Registration<GbHdDriver>,
+    _faux: faux::Registration,
+}
+
+impl kernel::Module for GreybusSoftSvc {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("gb_softsvc (init)\n");
+
+        // SAFETY: This runs once at module init, before anything else can reach `GLOBAL_STATE`.
+        unsafe { GLOBAL_STATE.init() };
+
+        let faux = faux::Registration::new(c_str!("gb-softsvc"), None)?;
+        let svc = Arc::pin_init(Svc::new(), GFP_KERNEL)?;
+        let data = GbHdDriver(svc.clone());
+
+        let dev = faux.as_ref().as_ref();
+        let hd = hd::Registration::new(dev, GB_OPERATION_SIZE_MAX, CPORT_ID_MAX + 1, Ok(data))?;
+
+        svc.send_version(hd.as_ref())?;
+
+        Ok(GreybusSoftSvc {
+            _hd: hd,
+            _faux: faux,
+        })
+    }
+}
+
+impl Drop for GreybusSoftSvc {
+    fn drop(&mut self) {
+        let _ = GLOBAL_STATE.lock().take();
+        pr_info!("gb_softsvc (exit)\n");
+    }
+}
+
+/// The operations a Greybus interface provides.
+///
+/// A node is whatever sits behind an interface — a real transport, an in-kernel emulation, or
+/// anything else that can accept Greybus traffic. This module does not care which.
+pub trait InterfaceOps: Send + Sync {
+    /// Delivers `data` to the node's `cport`.
+    fn write(&self, data: &[u8], cport: u16, gfp_mask: Flags) -> Result<()>;
+}
+
+/// A module attached to the Greybus network.
+pub struct Module {
+    id: u8,
+    intf_count: u8,
+    gb_hd: ARef<hd::Device>,
+    svc: Arc<Svc>,
+}
+
+impl Module {
+    /// Attaches a new module made of `intfs` to the Greybus network.
+    ///
+    /// Dropping the returned [`Module`] removes it from the network.
+    pub fn new(intfs: &[Arc<dyn InterfaceOps>]) -> Result<Self> {
+        let Ok(intf_count) = u8::try_from(intfs.len()) else {
+            return Err(code::E2BIG);
+        };
+
+        if intf_count == 0 {
+            return Err(code::EINVAL);
+        }
+
+        let guard = GLOBAL_STATE.lock();
+        let state = guard.as_ref().ok_or(code::EAGAIN)?;
+        let id = state.svc.module_insert(&state.gb_hd, intfs)?;
+
+        Ok(Self {
+            id,
+            intf_count,
+            gb_hd: state.gb_hd.clone(),
+            svc: state.svc.clone(),
+        })
+    }
+
+    /// Delivers `msg`, received from interface `id` on its `cport`, to the Greybus core.
+    ///
+    /// `intf_id_rel` is the index of the interface within this module, e.g. 0, 1 or 2 for a
+    /// module with three interfaces. `cport` is the interface-side CPort id.
+    ///
+    /// Fails with `EINVAL` if `intf_id_rel` is out of range or no connection is bound to that
+    /// pair.
+    pub fn submit_message(&self, intf_id_rel: u8, cport: u16, msg: &[u8]) -> Result<()> {
+        if intf_id_rel >= self.intf_count {
+            return Err(code::EINVAL);
+        }
+
+        self.gb_hd
+            .data_rcvd_by_intf(self.id + intf_id_rel, cport, msg)
+    }
+}
+
+/// Dropping the module removes it from the Greybus core and waits for that to finish, so no
+/// [`InterfaceOps`] method of its interfaces runs after the drop returns.
+///
+/// This sleeps, and must not happen from within an [`InterfaceOps`] method.
+impl Drop for Module {
+    fn drop(&mut self) {
+        self.svc
+            .module_remove(&self.gb_hd, self.id, self.intf_count);
+    }
+}
diff --git a/drivers/greybus/gb_softsvc_exports.c b/drivers/greybus/gb_softsvc_exports.c
new file mode 100644
index 000000000000..a0e360c4d137
--- /dev/null
+++ b/drivers/greybus/gb_softsvc_exports.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 BeagleBoard.org. All rights reserved.
+
+/*
+ * Exports Rust symbols from the `gb_softsvc` crate for use by dependent modules.
+ *
+ * This is a workaround until the build system supports Rust cross-module
+ * dependencies natively.
+ */
+
+#include <linux/export.h>
+
+#define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym)
+
+#include "exports_gb_softsvc_generated.h"

-- 
2.56.0


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

* [PATCH v5 8/9] dt-bindings: beagle: Add BeagleConnect Freedom
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (6 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 7/9] drivers: greybus: Add software SVC implementation Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  2026-11-03 16:11 ` [PATCH v5 9/9] greybus: Add Rust UART node driver Ayush Singh
  8 siblings, 0 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh, Conor Dooley

Add devicetree bindings for the BeagleBoard.org BeagleConnect Freedom,
a wireless-enabled board based on the TI CC1352P7. The board exposes an
MCU that can be attached to a host over USB, UART, SPI or I2C, and is
described as a child node of the corresponding bus.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 .../beagle/beagle,beagleconnect-freedom.yaml       | 62 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/beagle/beagle,beagleconnect-freedom.yaml b/Documentation/devicetree/bindings/beagle/beagle,beagleconnect-freedom.yaml
new file mode 100644
index 000000000000..81dfdace33e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/beagle/beagle,beagleconnect-freedom.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/beagle/beagle,beagleconnect-freedom.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BeagleBoard.org BeagleConnect Freedom
+
+description:
+  The BeagleBoard.org BeagleConnect Freedom board connected over USB, UART, SPI or I2C.
+
+maintainers:
+  - Ayush Singh <ayush@beagleboard.org>
+
+properties:
+  compatible:
+    const: beagle,beagleconnect-freedom
+
+  reg:
+    maxItems: 1
+
+  vbat-supply: true
+
+required:
+  - compatible
+
+allOf:
+  - if:
+      required:
+        - reg
+    then:
+      $ref: /schemas/spi/spi-peripheral-props.yaml#
+    else:
+      $ref: /schemas/serial/serial-peripheral-props.yaml#
+
+      properties:
+        current-speed:
+          default: 115200
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    serial {
+      mcu {
+        compatible = "beagle,beagleconnect-freedom";
+        vbat-supply = <&vbat>;
+      };
+    };
+
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      mcu@0 {
+        compatible = "beagle,beagleconnect-freedom";
+        reg = <0>;
+        spi-max-frequency = <1000000>;
+        vbat-supply = <&vbat>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index c9126dc205db..521bc6c67979 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11522,6 +11522,12 @@ L:	greybus-dev@lists.linaro.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/greybus/gb_softsvc.rs
 
+GREYBUS UART NODE DRIVERS
+M:	Ayush Singh <ayush@beagleboard.org>
+L:	greybus-dev@lists.linaro.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/beagle/beagle,beagleconnect-freedom.yaml
+
 GREYBUS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
 M:	Alex Elder <elder@kernel.org>

-- 
2.56.0


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

* [PATCH v5 9/9] greybus: Add Rust UART node driver
  2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
                   ` (7 preceding siblings ...)
  2026-11-03 16:11 ` [PATCH v5 8/9] dt-bindings: beagle: Add BeagleConnect Freedom Ayush Singh
@ 2026-11-03 16:11 ` Ayush Singh
  8 siblings, 0 replies; 14+ messages in thread
From: Ayush Singh @ 2026-11-03 16:11 UTC (permalink / raw)
  To: Jason Kridner, robertcnelson, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
	Ayush Singh, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Markus Probst
  Cc: greybus-dev, linux-kernel, rust-for-linux, linux-crypto,
	devicetree, Ayush Singh

Add a driver for Greybus nodes attached over a plain serial port. The
node is registered with the software SVC (gb-softsvc), which handles the
SVC protocol on behalf of the AP, so no dedicated coprocessor running
SVC firmware is needed.

Greybus messages are carried over HDLC framing on the wire. Each frame
carries a one-byte address (0x01 for Greybus) and control byte, followed
by the 16-bit CPort ID and the Greybus message itself.

Port parameters are taken from the firmware node: "current-speed" if
present, otherwise 115200, with flow control and parity disabled.

Since gb-uart-node imports types from gb-softsvc, Rust to Rust calling
setup from nova-core [0] is being used.

[0]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.com/

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS                     |   1 +
 drivers/greybus/.gitignore      |   1 +
 drivers/greybus/Kconfig         |  15 +++
 drivers/greybus/Makefile        |  10 ++
 drivers/greybus/gb_uart_node.rs | 270 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 297 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 521bc6c67979..5bdd500ef63e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11527,6 +11527,7 @@ M:	Ayush Singh <ayush@beagleboard.org>
 L:	greybus-dev@lists.linaro.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/beagle/beagle,beagleconnect-freedom.yaml
+F:	drivers/greybus/gb_uart_node.rs
 
 GREYBUS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
diff --git a/drivers/greybus/.gitignore b/drivers/greybus/.gitignore
new file mode 100644
index 000000000000..ff9c4a3539b4
--- /dev/null
+++ b/drivers/greybus/.gitignore
@@ -0,0 +1 @@
+exports_gb_softsvc_generated.h
diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
index 709563f8c0ad..ce2030161f30 100644
--- a/drivers/greybus/Kconfig
+++ b/drivers/greybus/Kconfig
@@ -61,5 +61,20 @@ config GREYBUS_SOFTSVC
 	  To compile this code as a module, choose M here: the module
 	  will be called gb-softsvc.ko
 
+config GREYBUS_UART_NODE
+	tristate "Greybus UART node transport"
+	depends on GREYBUS_SOFTSVC
+	depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+	depends on RUST_GREYBUS_ABSTRACTIONS
+	select RUST_CRC_CCITT_ABSTRACTIONS
+	help
+	  Select this option if you have a Greybus node connected over a
+	  serial port. The node is registered with the software SVC, which
+	  handles the SVC protocol on behalf of the AP, so no dedicated
+	  coprocessor running SVC firmware is required.
+
+	  To compile this code as a module, choose M here: the module
+	  will be called gb-uart-node.ko
+
 endif	# GREYBUS
 
diff --git a/drivers/greybus/Makefile b/drivers/greybus/Makefile
index 0fa802fdf6ca..023f59c11e65 100644
--- a/drivers/greybus/Makefile
+++ b/drivers/greybus/Makefile
@@ -28,6 +28,12 @@ obj-$(CONFIG_GREYBUS_ES2)	+= gb-es2.o
 obj-$(CONFIG_GREYBUS_SOFTSVC)	+= gb-softsvc.o
 gb-softsvc-y			+= gb_softsvc.o gb_softsvc_exports.o
 
+obj-$(CONFIG_GREYBUS_UART_NODE)	+= gb-uart-node.o
+gb-uart-node-y			+= gb_uart_node.o
+
+# Export Rust symbols from gb-softsvc only if gb-uart-node actually references them.
+gb-softsvc-export-deps := $(if $(CONFIG_GREYBUS_UART_NODE),$(obj)/gb_uart_node.o)
+
 rust_needed_exports = \
 	{ $(if $(strip $(2)),$(NM) -u $(2);,) echo "__DEFINED_RUST_SYMBOLS__"; \
 	  $(NM) -p --defined-only $(1); } | \
@@ -66,3 +72,7 @@ endif
 # Output gb_softsvc's crate metadata for use by gb_uart_node at compile time.
 RUSTFLAGS_gb_softsvc.o += \
 	--emit=metadata=$(objtree)/$(obj)/libgb_softsvc.rmeta
+
+# Allow gb_uart_node to import gb_softsvc's types.
+$(obj)/gb_uart_node.o: $(obj)/gb_softsvc.o
+RUSTFLAGS_gb_uart_node.o := -L $(objtree)/$(obj) --extern gb_softsvc
diff --git a/drivers/greybus/gb_uart_node.rs b/drivers/greybus/gb_uart_node.rs
new file mode 100644
index 000000000000..b38e776c3e85
--- /dev/null
+++ b/drivers/greybus/gb_uart_node.rs
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Greybus UART Node driver
+
+use kernel::{
+    alloc::Flags,
+    crc_ccitt::crc_ccitt,
+    device::{
+        AsBusDevice,
+        Bound,
+        Core, //
+    },
+    error::code,
+    new_spinlock,
+    of, //
+    prelude::*,
+    serdev,
+    sync::{
+        aref::ARef,
+        Arc,
+        SpinLock, //
+    },
+};
+
+use zerocopy::little_endian;
+use zerocopy_derive::{
+    FromBytes,
+    Immutable,
+    KnownLayout, //
+};
+
+/// Most greybus implementations will have a limit on greybus message length. The theoretical limit
+/// is [`u16::MAX`], but the real limits are much more modest. 256 seems to be the limit I use on
+/// most of my peers, but let's use 512 to be on safe side in kernel. The only real big request that
+/// I frequently encounter is manifest_get. However, if camera protocol ends up being supported at
+/// some point, this might need to be increased.
+const HDLC_MAX_FRAME_LEN: usize = 512;
+const HDLC_FRAME_HEADER_LEN: usize =
+    2 * size_of_val(&HDLC_FRAME) + 2 * size_of::<u16>() + 2 * size_of::<u8>();
+const HDLC_MAX_MSG_LEN: usize = HDLC_MAX_FRAME_LEN - HDLC_FRAME_HEADER_LEN;
+
+const HDLC_FRAME: u8 = 0x7E;
+const HDLC_ESC: u8 = 0x7D;
+const HDLC_XOR: u8 = 0x20;
+const HDLC_EXPECTED_CRC: u16 = 0xf0b8;
+
+const ADDRESS_GREYBUS: u8 = 0x01;
+
+#[repr(C, packed)]
+#[derive(FromBytes, Immutable, KnownLayout)]
+struct GreybusFrame {
+    cport: little_endian::U16,
+    msg: [u8],
+}
+
+struct HdlcRx {
+    rx_buf: KVec<u8>,
+    rx_in_esc: bool,
+    rx_overflow: bool,
+    sdev: ARef<serdev::Device>,
+    node: gb_softsvc::Module,
+}
+
+impl HdlcRx {
+    fn new(sdev: ARef<serdev::Device>, node: gb_softsvc::Module) -> Result<Self> {
+        Ok(Self {
+            node,
+            sdev,
+            rx_buf: KVec::with_capacity(HDLC_MAX_FRAME_LEN, GFP_KERNEL)?,
+            rx_in_esc: false,
+            rx_overflow: false,
+        })
+    }
+
+    fn frame_finish(&self) -> Result<()> {
+        if self.rx_buf.len() < 4 {
+            return Err(code::EFAULT);
+        }
+
+        let crc = crc_ccitt(0xffff, &self.rx_buf);
+        if crc != HDLC_EXPECTED_CRC {
+            dev_warn!(self.sdev.as_ref(), "CRC failed {}", crc);
+            return Ok(());
+        }
+
+        let addr = self.rx_buf[0];
+        let _ctrl = self.rx_buf[1];
+        let payload = &self.rx_buf[2..self.rx_buf.len() - size_of::<u16>()];
+
+        match addr {
+            ADDRESS_GREYBUS => {
+                let frame = GreybusFrame::ref_from_bytes(payload).map_err(|_| code::EINVAL)?;
+                self.node.submit_message(0, frame.cport.into(), &frame.msg)
+            }
+            _ => Err(code::EINVAL),
+        }
+    }
+
+    fn rx(&mut self, data: &[u8]) -> usize {
+        for i in data.iter() {
+            match *i {
+                HDLC_FRAME => {
+                    if !self.rx_buf.is_empty() {
+                        if let Err(e) = self.frame_finish() {
+                            dev_warn!(self.sdev.as_ref(), "bad frame: {e:?}\n");
+                        }
+                    }
+
+                    self.rx_buf.clear();
+                    self.rx_in_esc = false;
+                    self.rx_overflow = false;
+                }
+                HDLC_ESC => self.rx_in_esc = true,
+                // Skip data if buffer has overflown.
+                _ if self.rx_overflow => {}
+                _ => {
+                    let c = if self.rx_in_esc { *i ^ HDLC_XOR } else { *i };
+                    self.rx_in_esc = false;
+
+                    if self.rx_buf.push_within_capacity(c).is_err() {
+                        dev_warn!(self.sdev.as_ref(), "buffer overflow. Dropping frame");
+
+                        self.rx_overflow = true;
+                        self.rx_buf.clear();
+                    }
+                }
+            }
+        }
+
+        data.len()
+    }
+}
+
+struct GbNode {
+    sdev: ARef<serdev::Device>,
+}
+
+impl GbNode {
+    const fn new(sdev: ARef<serdev::Device>) -> Self {
+        Self { sdev }
+    }
+
+    fn fill_buf(mut crc: u16, data: &[u8], buf: &mut KVec<u8>) -> Result<u16> {
+        for i in data {
+            crc = crc_ccitt(crc, &[*i]);
+            if *i == HDLC_ESC || *i == HDLC_FRAME {
+                buf.push_within_capacity(HDLC_ESC)
+                    .map_err(|_| code::E2BIG)?;
+                buf.push_within_capacity(i ^ HDLC_XOR)
+                    .map_err(|_| code::E2BIG)?;
+            } else {
+                buf.push_within_capacity(*i).map_err(|_| code::E2BIG)?;
+            }
+        }
+
+        Ok(crc)
+    }
+}
+
+impl gb_softsvc::InterfaceOps for GbNode {
+    fn write(&self, data: &[u8], cport: u16, gfp_mask: Flags) -> Result<()> {
+        // SAFETY: `GbNode` only exists while its serdev driver is bound, so the device is in the
+        // `Bound` state for the duration of this call.
+        let bound: &serdev::Device<Bound> =
+            unsafe { serdev::Device::from_device(self.sdev.as_ref().as_bound()) };
+
+        // Drop messages which are bigger than what is supported.
+        if data.len() > HDLC_MAX_MSG_LEN {
+            return Err(code::E2BIG);
+        }
+
+        let mut buf = KVec::with_capacity(HDLC_MAX_FRAME_LEN, gfp_mask)?;
+
+        let mut crc = 0xffff;
+
+        buf.push_within_capacity(HDLC_FRAME)
+            .map_err(|_| code::E2BIG)?;
+
+        crc = Self::fill_buf(crc, &[ADDRESS_GREYBUS, 0x03], &mut buf)?;
+        crc = Self::fill_buf(crc, &cport.to_le_bytes(), &mut buf)?;
+        crc = Self::fill_buf(crc, data, &mut buf)?;
+
+        crc ^= 0xffff;
+        Self::fill_buf(crc, &crc.to_le_bytes(), &mut buf)?;
+
+        buf.push_within_capacity(HDLC_FRAME)
+            .map_err(|_| code::E2BIG)?;
+
+        // A short count means the wait was interrupted by a signal after part of the frame was
+        // accepted. The rest is never sent; the receiver drops the truncated frame on the next
+        // flag, but the message itself was not sent.
+        if bound.write_all(&buf, 0)? != buf.len() {
+            return Err(code::EINTR);
+        }
+
+        Ok(())
+    }
+}
+
+#[pin_data]
+struct GbUartNode {
+    #[pin]
+    rx: SpinLock<HdlcRx>,
+}
+
+impl GbUartNode {
+    fn init(sdev: &serdev::Device<Core<'_>>) -> Result<HdlcRx> {
+        if sdev
+            .set_baudrate(
+                sdev.as_ref()
+                    .fwnode()
+                    .and_then(|fwnode| fwnode.property_read(c"current-speed").optional())
+                    .unwrap_or(115200),
+            )
+            .is_err()
+        {
+            return Err(EINVAL);
+        }
+        sdev.set_flow_control(false);
+        sdev.set_parity(serdev::Parity::None)?;
+
+        let node = gb_softsvc::Module::new(&[Arc::new(GbNode::new(sdev.into()), GFP_KERNEL)?])?;
+
+        HdlcRx::new(sdev.into(), node)
+    }
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    <GbUartNode as serdev::Driver>::IdInfo,
+    [(of::DeviceId::new(c"beagle,beagleconnect-freedom"), ())]
+);
+
+#[vtable]
+impl serdev::Driver for GbUartNode {
+    type IdInfo = ();
+    type Data<'bound> = Self;
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe<'bound>(
+        sdev: &'bound serdev::Device<Core<'_>>,
+        _info: Option<&'bound Self::IdInfo>,
+    ) -> impl PinInit<Self, Error> + 'bound {
+        dev_dbg!(sdev.as_ref(), "Probe gb_uart_node.\n");
+
+        try_pin_init!(Self {
+            rx <- new_spinlock!(Self::init(sdev)?, "gb_uart_node::rx"),
+        }? Error)
+    }
+
+    fn receive<'bound>(
+        _sdev: &'bound serdev::Device<Bound>,
+        this: Pin<&Self>,
+        data: &[u8],
+    ) -> usize {
+        if let Some(mut guard) = this.rx.try_lock() {
+            return guard.rx(data);
+        }
+
+        0
+    }
+}
+
+kernel::module_serdev_device_driver! {
+    type: GbUartNode,
+    name: "gb_uart_node",
+    authors: ["Ayush Singh <ayush@beagleboard.org>"],
+    description: "Greybus node connected over UART",
+    license: "GPL v2",
+}

-- 
2.56.0


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

* Re: [PATCH v5 3/9] greybus: operation: arm request timeout before sending
  2026-11-03 16:11 ` [PATCH v5 3/9] greybus: operation: arm request timeout before sending Ayush Singh
@ 2026-11-03 16:27   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-03 16:27 UTC (permalink / raw)
  To: Ayush Singh, Jason Kridner, robertcnelson, Johan Hovold,
	Alex Elder, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan,
	Eric Biggers, Ard Biesheuvel, Ayush Singh, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lorenzo Stoakes,
	Vlastimil Babka, Liam R. Howlett, Uladzislau Rezki, Markus Probst
  Cc: linux-crypto, devicetree

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] greybus: operation: UAF and ref underflow in gb_operation_request_send()
- [Critical] greybus: operation: UAF from early timeout in gb_operation_request_send()
- [Critical] greybus: operation: timer leak in gb_operation_request_send() on cancel
--

commit 7b73a81d11f2d699f2c8ebeafd18ae6352191433
Author: Ayush Singh <ayush@beagleboard.org>

greybus: operation: arm request timeout before sending

This commit arms the operation timeout timer before calling
gb_message_send() in gb_operation_request_send() so that a fast response
completing before gb_message_send() returns does not leave an armed timer
behind, and calls timer_delete_sync() in the err_put_active error path.

> diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
> index a63bc2eb164e9..2d37acedf36f8 100644
> --- a/drivers/greybus/operation.c
> +++ b/drivers/greybus/operation.c
> @@ -782,20 +782,25 @@ int gb_operation_request_send(struct gb_operation *operation,
>  		return 0;
>  	}
>  
> +	/*
> +	 * Arm the timeout before sending: the response can arrive, and the
> +	 * operation complete, before the host driver returns.
> +	 */
> +	if (timeout) {
> +		operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
> +		add_timer(&operation->timer);
> +	}

[Severity: Critical]
Could an operation still be cancelled between gb_operation_get_active() and
add_timer(&operation->timer) in gb_operation_request_send(), leaking an
armed timer in operation->timer?

Earlier in gb_operation_request_send(), operation->errno is initialized to
-EINPROGRESS and the first gb_operation_get_active() adds operation->links
to connection->operations.

If a concurrent thread disables the connection via gb_connection_disable()
or gb_connection_disable_forced() after the second
gb_operation_get_active() succeeds and before add_timer(&operation->timer)
runs:

CPU0                                    CPU1
gb_operation_request_send()
  gb_operation_result_set(-EINPROGRESS)
  gb_operation_get_active()
  gb_operation_get_active()
                                        gb_connection_cancel_operations()
                                          gb_operation_cancel(-ESHUTDOWN)
                                            gb_operation_result_set(-ESHUTDOWN)
                                            queue_work(&operation->work)

                                        gb_operation_work()
                                          timer_delete_sync(&operation->timer)
                                          operation->callback(operation)
                                          gb_operation_put_active(operation)
                                          gb_operation_put(operation)
  add_timer(&operation->timer)
  gb_message_send() -> returns 0

Because gb_operation_work() runs timer_delete_sync(&operation->timer) before
CPU0 calls add_timer(&operation->timer), nothing deletes operation->timer
afterward. When the caller later drops its reference via gb_operation_put(),
_gb_operation_destroy() frees struct gb_operation while operation->timer is
still queued in the timer wheel.

> +
>  	ret = gb_message_send(operation->request, gfp);

[Severity: Critical]
What happens if operation->timer expires after add_timer(&operation->timer)
is called, but before the host driver in gb_message_send() has submitted its
transport request?

For an asynchronous host driver such as drivers/greybus/es2.c, if the
sending thread is delayed or preempted before message_send() assigns
message->hcpriv and calls usb_submit_urb():

drivers/greybus/es2.c:message_send() {
	...
	spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
	message->hcpriv = urb;
	spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
	...
	retval = usb_submit_urb(urb, gfp_mask);
}

If operation->timer fires first, gb_operation_timeout() sets
operation->errno to -ETIMEDOUT and schedules gb_operation_work():

gb_operation_timeout()
  gb_operation_result_set(operation, -ETIMEDOUT)
  queue_work(gb_operation_completion_wq, &operation->work)

gb_operation_work()
  timer_delete_sync(&operation->timer)
  gb_message_cancel(operation->request)
    es2.c:message_cancel() -> no-op because message->hcpriv is not yet active
  operation->callback(operation)
  gb_operation_put_active(operation)
  gb_operation_put(operation)

When the sending thread resumes in es2.c:message_send(), it submits the URB
and returns 0 to gb_operation_request_send(). Because operation->callback()
has already completed the operation, the caller can drop the final reference
to operation via gb_operation_put(), freeing operation->request and
operation->request->buffer in _gb_operation_destroy() while the URB is still
in flight.

>  	gb_operation_put_active(operation);
>  	gb_operation_put(operation);
>  	if (ret)
>  		goto err_put_active;
>  
> -	if (timeout) {
> -		operation->timer.expires = jiffies + msecs_to_jiffies(timeout);
> -		add_timer(&operation->timer);
> -	}
> -
>  	return 0;
>  
>  err_put_active:
> +	timer_delete_sync(&operation->timer);
>  	gb_operation_put_active(operation);
>  err_put:
>  	gb_operation_put(operation);

[Severity: Critical]
Can this error path underflow operation->active and operation->kref and
trigger a use-after-free on struct gb_operation when gb_message_send() fails
after operation->timer has already expired (or after gb_operation_cancel()
has been called)?

In gb_operation_request_send(), two pairs of kref and active references are
taken before gb_message_send(): one pair for operation completion, and one
pair held across gb_message_send().

With operation->timer armed before gb_message_send(), if gb_message_send()
sleeps or takes longer than timeout before returning an error (for example,
hdlc_tx_frames() in gb-beagleplay.c waiting on a full TX circular buffer
before returning -EAGAIN, or write_all() in gb_uart_node.rs returning
-EINTR), operation->timer can fire while gb_message_send() is still running:

gb_operation_timeout()
  if (gb_operation_result_set(operation, -ETIMEDOUT))
      queue_work(gb_operation_completion_wq, &operation->work);

gb_operation_work() then runs on gb_operation_completion_wq, invokes
operation->callback(operation), and drops the completion reference pair:

gb_operation_work()
  ...
  operation->callback(operation);
  gb_operation_put_active(operation);
  gb_operation_put(operation);

When gb_message_send() returns a non-zero ret, gb_operation_request_send()
drops the send-guard reference pair and jumps to err_put_active:

  ret = gb_message_send(operation->request, gfp);
  gb_operation_put_active(operation);
  gb_operation_put(operation);
  if (ret)
      goto err_put_active;

In err_put_active, timer_delete_sync(&operation->timer) only waits for the
timer callback to finish; it does not cancel the already-queued
operation->work, nor does err_put_active check gb_operation_result_set() to
see whether completion was already claimed.

Consequently, err_put_active drops gb_operation_put_active(operation) and
gb_operation_put(operation) a third time when only two pairs were acquired,
underflowing operation->active to -1 (which also causes wait_event() in
gb_operation_cancel() to hang) and dropping the caller's own kref on
operation (or accessing operation after it was already freed if an
asynchronous callback such as gb_loopback_async_operation_callback() dropped
the caller's reference before gb_operation_put() prior to err_put_active).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-gb-uart-transport-v5-0-f8e326d6561f@beagleboard.org?part=3

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

* Re: [PATCH v5 6/9] rust: kernel: Add greybus abstractions
  2026-11-03 16:11 ` [PATCH v5 6/9] rust: kernel: Add greybus abstractions Ayush Singh
@ 2026-11-03 16:28   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-03 16:28 UTC (permalink / raw)
  To: Ayush Singh, Jason Kridner, robertcnelson, Johan Hovold,
	Alex Elder, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan,
	Eric Biggers, Ard Biesheuvel, Ayush Singh, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lorenzo Stoakes,
	Vlastimil Babka, Liam R. Howlett, Uladzislau Rezki, Markus Probst
  Cc: linux-crypto, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: greybus: blanket Send and Sync impls on hd::Device<Ctx>
--

commit d45a0cafbb6b7cfe1ad47516461c24f42e8c8a34
Author: Ayush Singh <ayush@beagleboard.org>

rust: kernel: Add greybus abstractions

This commit adds Rust abstractions for the Greybus core, including host
device registration, driver callbacks, and wire protocol definitions.

> diff --git a/rust/kernel/greybus/hd.rs b/rust/kernel/greybus/hd.rs
> new file mode 100644
> index 0000000000000..9e1a5fc645cd7
> --- /dev/null
> +++ b/rust/kernel/greybus/hd.rs
[ ... ]
> +/// A Greybus host device.
> +///
> +/// # Invariants
> +///
> +/// The wrapped value is a valid `struct gb_host_device` created by `gb_hd_create()`, and every
> +/// [`ARef<Device>`](crate::sync::aref::ARef) owns an increment on its reference count.
> +#[repr(transparent)]
> +pub struct Device<Ctx = device::Normal>(Opaque<bindings::gb_host_device>, PhantomData<Ctx>);
> +
> +// SAFETY: `gb_host_device` is reference counted through its embedded `struct device`, which may be
> +// used from any thread.
> +unsafe impl<Ctx> Send for Device<Ctx> {}
> +
> +// SAFETY: `gb_host_device` has its own internal locking, so it is safe to share references to it
> +// across threads.
> +unsafe impl<Ctx> Sync for Device<Ctx> {}

[Severity: Low]
As reported by Sashiko on v4 [1], in rust/kernel/greybus/hd.rs, Device<Ctx>
provides blanket Send and Sync implementations for all Ctx and omits the
Ctx: device::DeviceContext bound on the struct definition.

Because Device<Ctx> also implements AsRef<device::Device<Ctx>>, marking
Device<device::CoreInternal> as Sync would allow sharing a
&Device<device::CoreInternal> reference across threads and obtaining
concurrent &device::Device<device::CoreInternal> references, even though
device::Device<device::CoreInternal> is intentionally !Sync because
set_drvdata() and drvdata_obtain() mutate dev->driver_data without
synchronization.

Other bus device wrappers in the kernel crate (such as device::Device,
serdev::Device, platform::Device, pci::Device, auxiliary::Device, and
usb::Device) bound Ctx: device::DeviceContext on the struct definition and
only implement Send and Sync for Device (Device<device::Normal>) and Sync
for Device<device::Bound>, keeping Core and CoreInternal contexts !Send and
!Sync.

Should Device<Ctx> in rust/kernel/greybus/hd.rs constrain
Ctx: device::DeviceContext and restrict its Send and Sync implementations to
match the other device wrappers?

[1] https://lore.kernel.org/r/20261021-gb-uart-transport-v4-3-c308ce821d45@beagleboard.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-gb-uart-transport-v5-0-f8e326d6561f@beagleboard.org?part=6

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

* Re: [PATCH v5 1/9] greybus: operation: keep request active across message_send
  2026-11-03 16:11 ` [PATCH v5 1/9] greybus: operation: keep request active across message_send Ayush Singh
@ 2026-11-03 16:30   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-03 16:30 UTC (permalink / raw)
  To: Ayush Singh, Jason Kridner, robertcnelson, Johan Hovold,
	Alex Elder, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan,
	Eric Biggers, Ard Biesheuvel, Ayush Singh, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lorenzo Stoakes,
	Vlastimil Babka, Liam R. Howlett, Uladzislau Rezki, Markus Probst
  Cc: linux-crypto, devicetree

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] greybus: operation: double put in gb_operation_request_send() on send error
- [Critical] greybus: operation: race between gb_operation_get_active() calls on send
--

commit 1f5c8fb4470157e5c81c1041163bd49efb2ea63b
Author: Ayush Singh <ayush@beagleboard.org>

greybus: operation: keep request active across message_send

This commit takes an extra reference and active count on the Greybus
operation across gb_message_send() in gb_operation_request_send() so that
operation cancellation waits for the host driver's send path to return.

> diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
> index 7e12ffb2dd60d..a63bc2eb164e9 100644
> --- a/drivers/greybus/operation.c
> +++ b/drivers/greybus/operation.c
> @@ -766,7 +766,25 @@ int gb_operation_request_send(struct gb_operation *operation,
>  	if (ret)
>  		goto err_put;
>  
> +	/*
> +	 * Completion can drop the reference and active count above while the
> +	 * host driver is still sending. Hold another of each across the send, so
> +	 * that cancelling the operation waits for the host driver to return.
> +	 */
> +	gb_operation_get(operation);
> +	ret = gb_operation_get_active(operation);
> +	if (ret) {
> +		/*
> +		 * The connection is being torn down, and the operation, which is
> +		 * already on its list, will be cancelled and completed.
> +		 */
> +		gb_operation_put(operation);
> +		return 0;
> +	}

[Severity: Critical]
Because the first gb_operation_get_active(operation) call adds operation->links
to connection->operations with operation->active == 1 and then drops
connection->lock before the second reference and active count are acquired,
can a concurrent teardown race in the window between the two
gb_operation_get_active() calls?

1. If gb_connection_disable() or gb_connection_disable_forced() runs right
after the first gb_operation_get_active() unlocks connection->lock,
gb_connection_cancel_operations() -> gb_operation_cancel() queues
gb_operation_work(). If gb_operation_work() runs before this thread reaches
the second gb_operation_get(operation), it invokes operation->callback(),
decrements operation->active to 0 (waking gb_operation_cancel() before
gb_message_send() has even started), and drops the completion reference. For
asynchronous callers whose callback drops the caller's own reference (such as
gb_loopback_async_operation_callback() in drivers/staging/greybus/loopback.c),
operation->kref can reach 0 and free operation before the second
gb_operation_get() and gb_operation_get_active() touch it.

2. For GB_OPERATION_FLAG_CORE operations, gb_operation_get_active() permits
operations when connection->state is GB_CONNECTION_STATE_DISCONNECTING:

drivers/greybus/operation.c:gb_operation_get_active() {
	...
	case GB_CONNECTION_STATE_DISCONNECTING:
		if (!gb_operation_is_core(operation))
			goto err_unlock;
		break;
	...
}

If a core operation passes the first gb_operation_get_active() in
GB_CONNECTION_STATE_ENABLED, and gb_connection_disable() transitions to
GB_CONNECTION_STATE_DISCONNECTING and cancels/completes the operation in the
window before the second gb_operation_get_active(), won't the second
gb_operation_get_active() still succeed in DISCONNECTING, re-add the already
completed operation to connection->operations, and send it?

Conversely, in drivers/greybus/connection.c, gb_connection_disable() and
_gb_connection_enable() call gb_connection_cancel_operations() when entering
GB_CONNECTION_STATE_DISCONNECTING, but not when subsequently setting
connection->state = GB_CONNECTION_STATE_DISABLED:

drivers/greybus/connection.c:gb_connection_disable() {
	...
	spin_lock_irq(&connection->lock);
	connection->state = GB_CONNECTION_STATE_DISCONNECTING;
	gb_connection_cancel_operations(connection, -ESHUTDOWN);
	spin_unlock_irq(&connection->lock);
	...
	connection->state = GB_CONNECTION_STATE_DISABLED;
}

If a core operation passes the first gb_operation_get_active() while in
GB_CONNECTION_STATE_DISCONNECTING and connection->state transitions to
GB_CONNECTION_STATE_DISABLED before the second gb_operation_get_active(), the
second call returns -ENOTCONN and this path returns 0 without dropping the
first active count or reference and without anyone cancelling the operation,
leaking operation and hanging synchronous callers waiting on
operation->completion.

> +
>  	ret = gb_message_send(operation->request, gfp);
> +	gb_operation_put_active(operation);
> +	gb_operation_put(operation);
>  	if (ret)
>  		goto err_put_active;

[Severity: Critical]
What happens if gb_operation_cancel() cancels the operation while
gb_message_send() is in progress, and gb_message_send() then returns a
non-zero error code?

While gb_message_send() is running, operation is on connection->operations
with operation->errno set to -EINPROGRESS. If a concurrent teardown calls
gb_connection_cancel_operations() -> gb_operation_cancel(operation, -ESHUTDOWN):

drivers/greybus/operation.c:gb_operation_cancel() {
	...
	if (gb_operation_result_set(operation, errno)) {
		gb_message_cancel(operation->request);
		queue_work(gb_operation_completion_wq, &operation->work);
	}
	...
}

gb_operation_work() will run operation->callback(operation) and call
gb_operation_put_active(operation) and gb_operation_put(operation), consuming
the initial completion active count and reference.

If gb_message_send() also returns an error (for example, on disconnect or TX
failure), gb_operation_request_send() drops the send active count and
reference here, and then jumps to err_put_active:

	ret = gb_message_send(operation->request, gfp);
	gb_operation_put_active(operation);
	gb_operation_put(operation);
	if (ret)
		goto err_put_active;
	...
err_put_active:
	gb_operation_put_active(operation);
err_put:
	gb_operation_put(operation);

Wouldn't jumping to err_put_active drop the completion active count and
operation->kref a second time (three puts for two gets), underflowing
operation->active to -1, underflowing operation->kref into a use-after-free,
and running both the asynchronous completion callback and the synchronous
error return path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-gb-uart-transport-v5-0-f8e326d6561f@beagleboard.org?part=1

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

* Re: [PATCH v5 7/9] drivers: greybus: Add software SVC implementation
  2026-11-03 16:11 ` [PATCH v5 7/9] drivers: greybus: Add software SVC implementation Ayush Singh
@ 2026-11-03 16:32   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-03 16:32 UTC (permalink / raw)
  To: Ayush Singh, Jason Kridner, robertcnelson, Johan Hovold,
	Alex Elder, Greg Kroah-Hartman, Miguel Ojeda, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan,
	Eric Biggers, Ard Biesheuvel, Ayush Singh, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lorenzo Stoakes,
	Vlastimil Babka, Liam R. Howlett, Uladzislau Rezki, Markus Probst
  Cc: linux-crypto, devicetree

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] greybus: gb_softsvc: unsynchronized async SVC handshake in init() and drop()
- [Medium] greybus: gb_softsvc: unchecked response result in GbHdDriver::svc_handler()
- [Low] greybus: gb_softsvc: commit message describes outdated NodeOps API
--

commit ee105f38d9a9f65cae00900a000f4081ef4dbafb
Author: Ayush Singh <ayush@beagleboard.org>

drivers: greybus: Add software SVC implementation

This commit adds gb-softsvc, an in-kernel software implementation of a
Greybus Supervisory Controller (SVC) in Rust for transports that do not
have a hardware SVC on the bus.

>     drivers: greybus: Add software SVC implementation
>     
>     A Greybus network requires an SVC (Supervisory Controller) to bring
>     interfaces up, assign device IDs and connect CPorts to the AP. On a
>     UniPro network the SVC is a real entity on the bus, but transports that
>     merely carry Greybus messages - a UART, an I2C bus, a network link -
>     have no such entity, so every host driver for those transports ends up
>     open-coding an SVC emulation of its own.
>     
>     Add gb-softsvc, a Rust implementation of the SVC that lives in the
>     kernel and registers itself as a Greybus host device. It answers the
>     SVC-side operations the core expects during interface bring-up and
>     teardown.
>     
>     Host drivers use it through a small in-kernel interface. A driver
>     implements the NodeOps trait to send data towards its node and then
>     calls module_insert() to announce a new node, submit_message() to hand
>     incoming Greybus messages back to the core, and module_remove() on
>     disconnect. Interface IDs are allocated by the SVC and returned from
>     module_insert().
>     
>     Connection create/destroy and interface activate/resume currently just
>     acknowledge the request; callbacks into NodeOps can be added later if
>     transports need to act on them.
>     
>     gb_softsvc_exports.c exports the Rust symbols for dependent modules, as
>     the build system does not yet support Rust cross-module dependencies
>     natively. This hack is taken from nova-core [0].
>     
>     [0]: https://lore.kernel.org/all/20260622-nova-exports-v5-0-6191773fc977@nvidia.com/
>     
>     Signed-off-by: Ayush Singh <ayush@beagleboard.org>

[Severity: Low]
As reported by Sashiko on v4 [1], does the commit message still describe an
older version of the public API?

The changelog states that host drivers implement NodeOps and call
module_insert() (to obtain allocated interface IDs) and module_remove(). In
drivers/greybus/gb_softsvc.rs, the public trait is InterfaceOps,
module_insert() and module_remove() are private methods on Svc, and drivers
use Module::new(), Module::submit_message() with a module-relative interface
index, and Drop for Module.

> diff --git a/drivers/greybus/gb_softsvc.rs b/drivers/greybus/gb_softsvc.rs
> new file mode 100644
> index 0000000000000..c9bff6a5fa658
> --- /dev/null
> +++ b/drivers/greybus/gb_softsvc.rs

[ ... ]

> @@ -0,0 +1,533 @@
> +            (
> +                false,
> +                protocols::GB_SVC_TYPE_INTF_UNIPRO_ENABLE
> +                | protocols::GB_SVC_TYPE_INTF_UNIPRO_DISABLE,
> +            ) => self.0.intf_unipro_enable_disable(hd, hdr),
> +            (false, protocols::GB_SVC_TYPE_INTF_ACTIVATE) => self.0.intf_activate(hd, msg),
> +            (false, protocols::GB_SVC_TYPE_INTF_RESUME) => self.0.intf_resume(hd, msg),
> +            (true, protocols::GB_SVC_TYPE_PROTOCOL_VERSION) => self.0.send_svc_hello(hd)?,
> +            (true, protocols::GB_SVC_TYPE_SVC_HELLO) => {
> +                let global_state = GlobalState::new(self.0.clone(), hd.into());
> +                let _ = GLOBAL_STATE.lock().replace(global_state);
> +            }

[Severity: Medium]
Should GbHdDriver::svc_handler() verify that the response header result is
GB_OP_SUCCESS before advancing the handshake and populating GLOBAL_STATE?

In drivers/greybus/operation.c, gb_operation_request_handle() always calls
gb_operation_response_send(operation, status) for non-unidirectional requests:

drivers/greybus/operation.c:gb_operation_response_send() {
    ...
    /* Fill in the response header and send it */
    operation->response->header->result = gb_operation_errno_map(errno);

    ret = gb_message_send(operation->response, GFP_KERNEL);
    ...
}

If gb_svc_version_request() or gb_svc_hello() fails in drivers/greybus/svc.c,
the core still sends a response over GB_SVC_CPORT_ID with a non-zero error
code in header->result and leaves svc->state uninitialized. Because
GbHdDriver::svc_handler() only matches on (hdr.is_response(),
hdr.request_type()), it will treat the failed response as a success and
publish an uninitialized SVC in GLOBAL_STATE.

> +            (
> +                true,
> +                protocols::GB_SVC_TYPE_MODULE_INSERTED | protocols::GB_SVC_TYPE_MODULE_REMOVED,
> +            ) => {}
> +            (false, ty) => {
> +                dev_warn!(hd.as_ref(), "unsupported request 0x{:02x}\n", ty);
> +                let rsp = Msg::response(hdr, protocols::GB_OP_PROTOCOL_BAD, ());
> +                hd.data_rcvd(protocols::GB_SVC_CPORT_ID, rsp.as_bytes());
> +            }
> +            (true, ty) => dev_warn!(hd.as_ref(), "unexpected response 0x{:02x}\n", ty),
> +        };
> +
> +        Ok(())
> +    }
> +}

[ ... ]

> +impl kernel::Module for GreybusSoftSvc {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("gb_softsvc (init)\n");
> +
> +        // SAFETY: This runs once at module init, before anything else can reach `GLOBAL_STATE`.
> +        unsafe { GLOBAL_STATE.init() };
> +
> +        let faux = faux::Registration::new(c_str!("gb-softsvc"), None)?;
> +        let svc = Arc::pin_init(Svc::new(), GFP_KERNEL)?;
> +        let data = GbHdDriver(svc.clone());
> +
> +        let dev = faux.as_ref().as_ref();
> +        let hd = hd::Registration::new(dev, GB_OPERATION_SIZE_MAX, CPORT_ID_MAX + 1, Ok(data))?;
> +
> +        svc.send_version(hd.as_ref())?;
> +
> +        Ok(GreybusSoftSvc {
> +            _hd: hd,
> +            _faux: faux,
> +        })
> +    }
> +}
> +
> +impl Drop for GreybusSoftSvc {
> +    fn drop(&mut self) {
> +        let _ = GLOBAL_STATE.lock().take();
> +        pr_info!("gb_softsvc (exit)\n");
> +    }
> +}

[Severity: High]
Because the SVC handshake runs asynchronously on svc->connection->wq, can
GreybusSoftSvc::init() and GreybusSoftSvc::drop() race with the handshake work
items?

In GreybusSoftSvc::init(), svc.send_version() calls hd.data_rcvd() ->
greybus_data_rcvd() -> gb_connection_recv_request(), which only queues the
incoming GB_SVC_TYPE_PROTOCOL_VERSION request onto svc->connection->wq.
GreybusSoftSvc::init() returns before that work item and the subsequent
GB_SVC_TYPE_SVC_HELLO request complete and populate GLOBAL_STATE in
GbHdDriver::svc_handler(), so a consumer calling Module::new() right after
module init will see GLOBAL_STATE as None and fail with EAGAIN:

    pub fn new(intfs: &[Arc<dyn InterfaceOps>]) -> Result<Self> {
        ...
        let guard = GLOBAL_STATE.lock();
        let state = guard.as_ref().ok_or(code::EAGAIN)?;

In addition, when GreybusSoftSvc is dropped, GreybusSoftSvc::drop() runs
GLOBAL_STATE.lock().take() before its _hd field is dropped via
Registration::drop() -> gb_hd_del() -> gb_svc_del() ->
gb_connection_disable_rx(). If the asynchronous GB_SVC_TYPE_SVC_HELLO response
is processed on svc->connection->wq after GreybusSoftSvc::drop() clears
GLOBAL_STATE but before _hd is unregistered, GbHdDriver::svc_handler() will
reinstall GlobalState into GLOBAL_STATE and leak the Arc<Svc> and
ARef<hd::Device> references.

[1] https://lore.kernel.org/r/20261021-gb-uart-transport-v4-4-c308ce821d45@beagleboard.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-gb-uart-transport-v5-0-f8e326d6561f@beagleboard.org?part=7

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

end of thread, other threads:[~2026-11-03 16:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 16:11 [PATCH v5 0/9] Add Greybus Sotfsvc and UART Node drivers Ayush Singh
2026-11-03 16:11 ` [PATCH v5 1/9] greybus: operation: keep request active across message_send Ayush Singh
2026-11-03 16:30   ` sashiko-bot
2026-11-03 16:11 ` [PATCH v5 2/9] greybus: svc: add gb_svc_flush() Ayush Singh
2026-11-03 16:11 ` [PATCH v5 3/9] greybus: operation: arm request timeout before sending Ayush Singh
2026-11-03 16:27   ` sashiko-bot
2026-11-03 16:11 ` [PATCH v5 4/9] greybus: connection: Add greybus_data_rcvd_by_intf() Ayush Singh
2026-11-03 16:11 ` [PATCH v5 5/9] rust: crc_ccitt: add CRC-CCITT abstraction Ayush Singh
2026-11-03 16:11 ` [PATCH v5 6/9] rust: kernel: Add greybus abstractions Ayush Singh
2026-11-03 16:28   ` sashiko-bot
2026-11-03 16:11 ` [PATCH v5 7/9] drivers: greybus: Add software SVC implementation Ayush Singh
2026-11-03 16:32   ` sashiko-bot
2026-11-03 16:11 ` [PATCH v5 8/9] dt-bindings: beagle: Add BeagleConnect Freedom Ayush Singh
2026-11-03 16:11 ` [PATCH v5 9/9] greybus: Add Rust UART node driver Ayush Singh

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