Linux Media Controller development
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Benjamin Mugnier <benjamin.mugnier@foss.st.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Sasha Levin <sashal@kernel.org>,
	sylvain.petinot@foss.st.com, linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 6.16] media: i2c: vd55g1: Fix RATE macros not being expressed in bps
Date: Tue,  5 Aug 2025 09:08:49 -0400	[thread overview]
Message-ID: <20250805130945.471732-14-sashal@kernel.org> (raw)
In-Reply-To: <20250805130945.471732-1-sashal@kernel.org>

From: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

[ Upstream commit 5b69a84005b686365e9cacb2ca942271ed74867a ]

As a bit rate is expressed in bps, use MEGA instead of HZ_PER_MHZ.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

**Backport Status: YES**

This commit should be backported to stable kernel trees. Here's the
extensive analysis:

## Bug Analysis

The commit fixes a unit confusion bug in the vd55g1 camera sensor
driver. The macros `VD55G1_MIPI_RATE_MIN` and `VD55G1_MIPI_RATE_MAX` are
incorrectly defined using `HZ_PER_MHZ` when they should use `MEGA`:

```c
// Before (incorrect):
#define VD55G1_MIPI_RATE_MIN    (250 * HZ_PER_MHZ)   // 250 MHz
#define VD55G1_MIPI_RATE_MAX    (1200 * HZ_PER_MHZ)  // 1200 MHz

// After (correct):
#define VD55G1_MIPI_RATE_MIN    (250 * MEGA)         // 250 Mbps
#define VD55G1_MIPI_RATE_MAX    (1200 * MEGA)        // 1200 Mbps
```

Both `HZ_PER_MHZ` and `MEGA` are defined as `1000000UL` in
`/home/sasha/linux/include/linux/units.h`, so numerically they're the
same. However, semantically they represent different units:
- `HZ_PER_MHZ` is for frequency conversion (Hz)
- `MEGA` is a generic multiplier for mega units, appropriate for bit
  rates (bps)

## Impact Assessment

1. **Functional Impact**: While numerically identical, this is a
   semantic bug that affects code clarity and correctness. The driver
   uses these macros to validate MIPI data rates:
   - Lines 712-713: Range validation of `sensor->mipi_rate`
   - Lines 716-717: Error messages that explicitly show rates in Mbps
     using `/ MEGA`

2. **User-Facing Bug**: Yes - incorrect unit semantics could lead to
   confusion when debugging or maintaining the driver. The error
   messages explicitly show "MIPI rates must be in the range [%u - %u]
   Mbps", proving these are bit rates, not frequencies.

3. **Regression Risk**: Minimal - since both macros have the same
   numerical value, there's no functional change, only semantic
   correctness.

## Stable Tree Criteria

This commit meets stable tree requirements:
1. **Fixes a real bug**: Semantic incorrectness in unit usage
2. **Small and contained**: Only changes 2 lines
3. **No side effects**: Purely a semantic fix with no functional changes
4. **No architectural changes**: Simple macro definition update
5. **Subsystem impact**: Limited to a single media driver (vd55g1)
6. **Clear fix**: The commit message clearly explains what's being fixed

The fix ensures proper semantic meaning for MIPI data rate validation,
which is important for code maintainability and prevents potential
future bugs if the macro values ever diverge.

 drivers/media/i2c/vd55g1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
index 8552ce75e1aa..d00daf89be96 100644
--- a/drivers/media/i2c/vd55g1.c
+++ b/drivers/media/i2c/vd55g1.c
@@ -129,8 +129,8 @@
 #define VD55G1_FWPATCH_REVISION_MINOR			9
 #define VD55G1_XCLK_FREQ_MIN				(6 * HZ_PER_MHZ)
 #define VD55G1_XCLK_FREQ_MAX				(27 * HZ_PER_MHZ)
-#define VD55G1_MIPI_RATE_MIN				(250 * HZ_PER_MHZ)
-#define VD55G1_MIPI_RATE_MAX				(1200 * HZ_PER_MHZ)
+#define VD55G1_MIPI_RATE_MIN				(250 * MEGA)
+#define VD55G1_MIPI_RATE_MAX				(1200 * MEGA)
 
 static const u8 patch_array[] = {
 	0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,
-- 
2.39.5


  parent reply	other threads:[~2025-08-05 13:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250805130945.471732-1-sashal@kernel.org>
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] media: uvcvideo: Fix bandwidth issue for Alcor camera Sasha Levin
2025-08-05 13:08 ` Sasha Levin [this message]
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] media: usb: hdpvr: disable zero-length read messages Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.12] media: uvcvideo: Add quirk for HP Webcam HD 2300 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16] media: i2c: vd55g1: Setup sensor external clock before patching Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] media: iris: Add handling for corrupt and drop frames Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Check I2C succeeded during probe Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Return an appropriate colorspace from tc358743_set_fmt Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] media: tc358743: Increase FIFO trigger level to 374 Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.12] media: hi556: Fix reset GPIO timings Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] media: uvcvideo: Set V4L2_CTRL_FLAG_DISABLED during queryctrl errors Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250805130945.471732-14-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=benjamin.mugnier@foss.st.com \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-media@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=sakari.ailus@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=sylvain.petinot@foss.st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox