Skip to content

Add error for hwmc response len 0 + replace hard-coded addresses #285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 16, 2025

Conversation

ejgoldik
Copy link
Collaborator

No description provided.

@ejgoldik ejgoldik requested review from ymodlin and Nir-Az July 16, 2025 04:59
@ymodlin ymodlin requested a review from Copilot July 16, 2025 08:24
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error handling and code maintainability for the RealSense D4xx kernel driver by adding validation for zero-length HWMC responses and replacing hard-coded register addresses with predefined constants.

  • Adds error checking for zero-length HWMC responses with appropriate error logging
  • Replaces hard-coded register addresses (0x4900, 0x4904, 0x4908, 0x490c) with named constants
  • Adds explanatory comments for register operations

if (ret)
return -EBADMSG;

if (tmp_len > cmdDataLen)
return -ENOBUFS;

if (tmp_len == 0) {
dev_err(&state->client->dev,
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 1827 contains a typo: "ccompletion" should be "completion".

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a mistake... I don't see this work

Comment on lines 1845 to 1846
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message includes 'ret: %d' but 'ret' is always 0 at this point since the regmap_raw_read succeeded. This could be confusing as it will always log 'ret: 0' even when there's an error condition.

Suggested change
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);
"%s(): HWMC response length is 0\n",
__func__);

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret can be non-zero?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment on lines 1845 to 1846
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret can be non-zero?

if (ret)
return -EBADMSG;

if (tmp_len > cmdDataLen)
return -ENOBUFS;

if (tmp_len == 0) {
dev_err(&state->client->dev,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a mistake... I don't see this work

Copy link
Collaborator

@Nir-Az Nir-Az left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ejgoldik ejgoldik merged commit 4c167d7 into dev Jul 16, 2025
3 checks passed
@ymodlin ymodlin deleted the Add_error_for_HWMC_response_len_0 branch July 16, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants