Skip to content

Commit 9d30c3a

Browse files
author
Nikolai-L
committed
refactor: update coding guidelines and remove IMU pipe setup from probe
1 parent f8e4208 commit 9d30c3a

6 files changed

Lines changed: 67 additions & 66 deletions

File tree

.github/copilot-instructions.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Linux kernel driver and userspace utilities for Intel RealSense D4XX series 3D d
1414
- **Follow Linux kernel coding style**: tabs for indentation (8-space width), `/* */` block comments, max ~80–100 char lines.
1515
- **Keep changes minimal**: do not inflate code. Prefer single-line expressions over multi-line blocks, reuse existing helpers/paths instead of adding new ones, and avoid verbose comments that restate what the code already says. Every added line must earn its place — if a change can be expressed more concisely without losing clarity, use the shorter form.
1616
- **Before adding new code, check callers and existing paths**: when introducing a check, loop, or helper, first verify whether the same logic already exists elsewhere in the call chain. If a function has only one caller, consider placing the logic in the caller instead of duplicating it. Never add a second copy of logic that can be combined with an existing one — consolidate first, don't layer.
17+
- **After removing code, clean up stale references**: when deleting a function, code block, or feature, immediately search for defines, variables, struct fields, forward declarations, and comments that were only used by the removed code. Remove all of them in the same patch. Do not leave dead code behind.
1718
- **Function naming**: prefix all functions with `ds5_`. Mux-related functions use `ds5_mux_`. Examples: `ds5_read()`, `ds5_write()`, `ds5_probe()`, `ds5_mux_s_stream()`.
1819
- **Struct naming**: prefix with `ds5_`. Examples: `struct ds5`, `struct ds5_sensor`, `struct ds5_ctrls`, `struct ds5_format`.
1920
- **Macro naming**: prefix with `DS5_`. Register addresses: `DS5_FW_VERSION`, `DS5_START_STOP_STREAM`, `DS5_DEPTH_STREAM_DT`.
@@ -37,6 +38,9 @@ Each camera registers four sensor subdevices: Depth, RGB, IR (Y8/Y8I/Y12I), and
3738

3839
A deserializer abstraction layer (`struct dser_interface`) provides function pointer tables for MAX9296 vs MAX96712 variants.
3940

41+
- **SerDes pipe configuration**: the **driver** configures all four SerDes pipes (Depth, RGB, IR, IMU) at stream start via `ds5_configure()`. The D457 firmware does **not** configure any pipes. Do not add probe-time pipe setup or special-case individual pipes (e.g. IMU) in `ds5_probe()`.
42+
- **Device tree assumptions**: all supported device trees include all four sensor instances (Depth, RGB, IR, IMU). Do not add DT-scanning logic to check for the presence of individual sensor types.
43+
4044
### Video Device Layout (per camera)
4145

4246
| Device | Stream | Format |
@@ -166,3 +170,14 @@ Short summary of approaches used in the refactor of `kernel/realsense/d4xx.c`:
166170

167171
Advantages: clearer ownership of shared state, fewer global arrays and linear
168172
searches, more deterministic recovery behavior, and reduced duplicate code.
173+
174+
## Workflow Rules
175+
176+
### Post-patch configuration review (mandatory)
177+
178+
After every confirmed code patch, review and update this file and `CLAUDE.md`:
179+
180+
1. **Stale facts**: correct or remove architectural claims, assumptions, or descriptions that the patch invalidated.
181+
2. **New conventions**: if the patch exposed a coding practice gap (e.g. a cleanup step that was missed), add a general coding convention so it is enforced going forward — do not just fix the specific instance.
182+
183+
Do not leave stale or incorrect claims in configuration files after changing the code they describe.

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@
55
/.claude/skills/v4l2-test/v4l2-test_results
66
**__pycache__
77
/test.logs
8+
/.vscode/*.db*

.vscode/c_cpp_properties.json

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"configurations": [
3+
{
4+
"name": "Linux",
5+
"includePath": [
6+
"${workspaceFolder}/**",
7+
"${workspaceFolder}/kernel/**",
8+
"${workspaceFolder}/kernel/kernel-jammy-src/include",
9+
"${workspaceFolder}/kernel/kernel-jammy-src/arch/**/include",
10+
"${workspaceFolder}/kernel/realsense",
11+
"${workspaceFolder}/nvidia-oot/**",
12+
"/usr/include"
13+
],
14+
"defines": [],
15+
"compilerPath": "/usr/bin/gcc",
16+
"cStandard": "c11",
17+
"cppStandard": "c++17",
18+
"intelliSenseMode": "linux-gcc-x64",
19+
"compileCommands": "${workspaceFolder}/compile_commands.json",
20+
"browse": {
21+
"path": [
22+
"${workspaceFolder}/kernel/kernel-jammy-src/include",
23+
"${workspaceFolder}/kernel/realsense",
24+
"${workspaceFolder}/**"
25+
],
26+
"limitSymbolsToIncludedHeaders": false,
27+
"databaseFilename": "${workspaceFolder}/.vscode/browse.vc.db"
28+
}
29+
}
30+
],
31+
"version": 4
32+
}

.vscode/settings.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"C_Cpp.default.configurationProvider": "ms-vscode.cpptools",
3+
"C_Cpp.default.compileCommands": "${workspaceFolder}/compile_commands.json",
4+
"C_Cpp.intelliSenseEngine": "Default",
5+
"files.exclude": {
6+
"**/.cache": true
7+
}
8+
}

compile_commands.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[
2+
{
3+
"directory": "/home/nlyskov/repos/realsense_mipi_platform_driver",
4+
"command": "gcc -c -D__KERNEL__ -DKBUILD_MODNAME=\"d4xx\" -DCONFIG_VIDEO_D4XX_SERDES=1 -I./kernel/kernel-jammy-src/include -I./kernel/kernel-jammy-src/arch/x86/include -I./kernel/realsense -I/usr/include -I./nvidia-oot ./kernel/realsense/d4xx.c",
5+
"file": "kernel/realsense/d4xx.c"
6+
}
7+
]

kernel/realsense/d4xx.c

Lines changed: 4 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,7 @@ struct dser_interface {
7575
#define GMSL_CSI_DT_EMBED 0x12
7676
#endif
7777

78-
/* IMU pipe defaults — used by ds5_configure() and post-reset pipe restoration.
79-
* The D457 FW configures pipes 0-2 (Depth/RGB/IR) but NOT pipe 3 (IMU).
80-
* The driver must configure pipe 3 explicitly with these values.
81-
*/
82-
#define DS5_IMU_PIPE_ID 3
8378
#define DS5_IMU_VC_ID 3
84-
#define DS5_IMU_DT1 GMSL_CSI_DT_YUV422_8
85-
#define DS5_IMU_DT2 GMSL_CSI_DT_EMBED
8679

8780
//#define DS5_DRIVER_NAME "DS5 RealSense camera driver"
8881
#define DS5_DRIVER_NAME "d4xx"
@@ -103,6 +96,7 @@ struct dser_interface {
10396
#define DS5_DEVICE_TYPE_D45X 6
10497
#define DS5_DEVICE_TYPE_D43X 5
10598
#define DS5_DEVICE_TYPE_D46X 4
99+
#define DS5_DEVICE_TYPE_UNKNOWN 0
106100

107101
#define DS5_MIPI_LANE_NUMS 0x0400
108102
#define DS5_MIPI_LANE_DATARATE 0x0402
@@ -3989,6 +3983,7 @@ static void ds5_init_ds5_dev(struct ds5 *state, struct ds5_dev *ds5_dev)
39893983
ds5_dev->ds5_primary = state;
39903984
atomic_set(&ds5_dev->ds5_probe_reset_once, 0);
39913985
ds5_reset_streaming_flags(ds5_dev);
3986+
ds5_dev->cached_device_type = DS5_DEVICE_TYPE_UNKNOWN;
39923987
}
39933988

39943989
#ifdef CONFIG_VIDEO_D4XX_SERDES
@@ -6689,67 +6684,10 @@ static int ds5_probe(struct i2c_client *c, const struct i2c_device_id *id)
66896684
goto e_chardev;
66906685
}
66916686

6692-
/* Wait for D457 FW to finish reconfiguring the MAX9295
6693-
* serializer. The FW continues writing to MAX9295 for
6694-
* ~50-100ms after reporting 0xDEAD. 200ms total provides
6695-
* margin before we touch serializer registers.
6687+
/* Wait after HW reset before touching MAX9295 serializer registers.
6688+
* This delay helps ensure the device is ready.
66966689
*/
66976690
msleep(200);
6698-
6699-
#ifdef CONFIG_VIDEO_D4XX_SERDES
6700-
/* Configure pipe 3 (IMU) on the serializer/deserializer,
6701-
* but only if an IMU peer actually exists in the device tree.
6702-
*
6703-
* The D457 FW configures pipes 0-2 (Depth/RGB/IR) during
6704-
* boot but does NOT configure pipe 3. Without pipe 3,
6705-
* the serializer has no I2C address translation for the
6706-
* IMU instance and its probe will fail with -EREMOTEIO.
6707-
*
6708-
* Only needed at first-probe time. For HWMC resets,
6709-
* ds5_configure() handles pipe reconfiguration at STREAMON.
6710-
*
6711-
* Uses ds5_setup_pipeline() (per-pipe registers only),
6712-
* NOT max9295_init_settings() which writes global registers
6713-
* (0x02, 0x308, 0x311) that disrupt the active GMSL link.
6714-
*/
6715-
{
6716-
struct device_node *bus_node, *peer;
6717-
const char *peer_cam_type;
6718-
bool has_imu_peer = false;
6719-
6720-
bus_node = of_get_parent(c->dev.of_node);
6721-
if (bus_node) {
6722-
for_each_child_of_node(bus_node, peer) {
6723-
if (!of_property_read_string(peer, "cam-type",
6724-
&peer_cam_type) &&
6725-
!strcmp(peer_cam_type, "IMU")) {
6726-
has_imu_peer = true;
6727-
of_node_put(peer);
6728-
break;
6729-
}
6730-
}
6731-
of_node_put(bus_node);
6732-
}
6733-
6734-
if (has_imu_peer) {
6735-
int pipe3_ret = ds5_setup_pipeline(state,
6736-
DS5_IMU_DT1, DS5_IMU_DT2,
6737-
DS5_IMU_PIPE_ID, DS5_IMU_VC_ID);
6738-
if (pipe3_ret)
6739-
dev_warn(&c->dev,
6740-
"%s(): first-probe pipe 3 (IMU) setup failed: %d\n",
6741-
__func__, pipe3_ret);
6742-
else
6743-
dev_info(&c->dev,
6744-
"%s(): first-probe pipe 3 (IMU) configured\n",
6745-
__func__);
6746-
} else {
6747-
dev_dbg(&c->dev,
6748-
"%s(): no IMU peer in DT, skipping pipe 3 setup\n",
6749-
__func__);
6750-
}
6751-
}
6752-
#endif
67536691
}
67546692

67556693
/* Verify communication with retries and delay.

0 commit comments

Comments
 (0)