From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: bus: mhi: Move sahara protocol driver under drivers/bus/mhi
Date: Wed, 11 Mar 2026 13:35:22 +1000
Message-ID: <review-patch2-20260310-sahara_protocol_new_v2-v3-2-994ea4b0d5ad@oss.qualcomm.com>
In-Reply-To: <20260310-sahara_protocol_new_v2-v3-2-994ea4b0d5ad@oss.qualcomm.com>
References: <20260310-sahara_protocol_new_v2-v3-0-994ea4b0d5ad@oss.qualcomm.com> <20260310-sahara_protocol_new_v2-v3-2-994ea4b0d5ad@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

Moves the driver, creates Kconfig/Makefile, and converts to `module_init`/`=
module_exit`.

**Issues:**
- **Copyright change is suspicious.** The original file had:
  ```c
  // SPDX-License-Identifier: GPL-2.0-only
  /* Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserve=
d. */
  ```
  The new file has:
  ```c
  // SPDX-License-Identifier: GPL-2.0
  /* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. */
  ```
  Changing `GPL-2.0-only` to `GPL-2.0` (which in SPDX means "GPL-2.0-only" =
anyway but is ambiguous) and replacing the copyright holder with a differen=
t entity/date range is a red flag. This needs justification.

- **QAIC init/exit error path change looks buggy.** The patch modifies `qai=
c_init()` but the goto labels appear swapped:
  ```c
  ret =3D pci_register_driver(&qaic_pci_driver);
  if (ret) {
      pr_debug("qaic: pci_register_driver failed %d\n", ret);
  -   return ret;
  +   goto free_pci;
  }
  ret =3D mhi_driver_register(&qaic_mhi_driver);
  if (ret) {
      pr_debug("qaic: mhi_driver_register failed %d\n", ret);
  -   goto free_pci;
  +   goto free_mhi;
  }
  ```
  After the `sahara_register()` call is removed, if `mhi_driver_register` f=
ails, the code now jumps to `free_mhi` which would unregister MHI (which ju=
st failed), instead of `free_pci` which should unregister the PCI driver. I=
'd need to see the full error path, but this looks inverted =E2=80=94 the o=
riginal code had `free_pci` for MHI failure (correct: unwind PCI), and now =
after removing sahara, the labels may have shifted incorrectly.

- The Kconfig `select FW_LOADER_COMPRESS` and `select FW_LOADER_COMPRESS_XZ=
` / `FW_LOADER_COMPRESS_ZSTD` is heavy-handed. Not all firmware necessarily=
 needs compressed loading. This should be documented or justified.

---
Generated by Claude Code Patch Reviewer