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,