Hello everyone,
I’ve recently been experimenting with chaining multiple Greybus nodes to
a single host over I²C (QWIIC port [0]). This general idea is not new
(see MicroMod Qwiic Pro Kit [1]), although those setups do not use Greybus.
Since I²C does not allow a slave to initiate transfers, it is not well
suited for node-initiated events (e.g. interrupts, SVC-initiated
control). However, for my current use case I am primarily interested in
polling-based functionality, so this limitation is acceptable.
In a typical Greybus topology, an Application Processor (AP), an SVC,
and one or more modules are connected via UniPro. In practice, because
most application processors lack a native UniPro interface, they connect
through a bridge device that also implements the SVC.
For the I²C-based setup described above, I have considered the following
topologies:
1. Separate co-processor (SVC/Bridge)
This approach is reasonable on SoCs such as TI AM6254, which include
an M4F core that can serve as the SVC/bridge and control the I²C bus.
However, on devices like TI AM62L, which lack such a core, introducing
an additional processor solely for Greybus does not seem justified.
Also, this would make the above much less portable, since it expects a
hardware component, not all BeagleBoard.org boards have.
```
+----+ +---------------+ +----------+
| AP | <--- bus ----> | SVC / Bridge | <--- I2C ----> |
Module |
+----+ +---------------+ +----------+
|
| +----------+
`----------- I2C ----> | Module |
+----------+
```
2. SVC per node
Each node implements its own SVC. Since an I²C slave cannot initiate
communication, the AP must already know the address of each SVC/module.
This also seems inefficient when chaining multiple nodes.
```
+----+ +----------------+
| AP | <--- I2C ---> | SVC / Module |
+----+ +----------------+
|
| +----------------+
`---- I2C ----> | SVC / Module |
+----------------+
```
3. SVC/Bridge functionality inside the AP
For this use case, this seems to be the most practical option.
To clarify, I am not proposing any new data paths in the Greybus
pipeline. The idea is to have a reusable an SVC/bridge implementation
similar to what exists in greybus-zephyr [2][3], but hosted within the AP.
```
+-------------+ +-----------+
| AP / SVC | <--- I2C ---> | Module |
+-------------+ +-----------+
|
| +----------+
`-- I2C ---> | Module |
+----------+
```
Before proceeding further, I would appreciate feedback on this
approach—particularly whether there are concerns with option 3, or if
there are alternative designs I should consider.
Best regards,
Ayush Singh
[0]: https://www.sparkfun.com/qwiic
[1]:
https://www.digikey.in/en/maker/projects/micromod-qwiic-pro-kit-project-gui…
[2]:
https://github.com/beagleboard/greybus-zephyr/blob/main/subsys/greybus/svc.…
[3]:
https://github.com/beagleboard/greybus-zephyr/blob/main/subsys/greybus/apbr…
This series cleans up dead code in the Greybus audio manager.
gb_audio_manager_get_module() has no in-tree callers. The previously
reported NULL dereference is therefore unreachable. Per review feedback,
the unused function is removed to avoid carrying dead code.
Patch 2 performs a small cleanup in the same area.
Changes in v3:
- Replaced the NULL-deref fix with removal of gb_audio_manager_get_module()
since there are no in-tree callers (per Greg KH).
- No functional changes otherwise.
Thanks for the review.
Signed-off-by: Hardik Phalet <hardik.phalet(a)pm.me>
Hardik Phalet (2):
staging: greybus: audio: remove unused gb_audio_manager_get_module()
staging: greybus: audio: drop stale TODO comment
drivers/staging/greybus/audio_manager.c | 12 ------------
drivers/staging/greybus/audio_manager.h | 7 -------
drivers/staging/greybus/audio_manager_module.c | 1 -
3 files changed, 20 deletions(-)
--
2.53.0
Thank you for the honest feedback. I'm a new contributor and I was
trying to resolve the checkpatch.pl CHECKs to get familiar with the
process, but I see now that vague documentation can be worse than none
at all.
I'll withdraw this patch. Since I'm looking to make my first meaningful
contribution to staging, do you have any suggestions on what types of
issues in Greybus are actually helpful for a beginner to tackle?
regards, jose
Add a comment to the mutex definition in struct gb_camera to
describe what it protects, satisfying a checkpatch.pl CHECK.
Signed-off-by: Jose A. Perez de Azpillaga <azpijr(a)gmail.com>
---
Hi,
I noticed a CHECK for alignment in this file on line 267. However,
fixing it pushed the line over 100 characters. I felt that adding
extreme indentation might add more noise than value, so I've only
included the mutex comment fix here.
I'd appreciate your guidance on whether you prefer strict alignment
even if it breaks the 100-column rule in these specific cases.
---
drivers/staging/greybus/camera.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index 62b55bb28408..a6f3637b8871 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -53,7 +53,7 @@ struct gb_camera {
struct gb_connection *data_connection;
u16 data_cport_id;
- struct mutex mutex;
+ struct mutex mutex; /* Protects camera state */
enum gb_camera_state state;
struct {
--
2.53.0