load_se_firmware() derives the RX_RFR_WATERMARK value from
QUPV3_SE_HW_PARAM_1 using RX_FIFO_WIDTH_BIT/RX_FIFO_WIDTH_MASK, which
decode the RX FIFO element width (bits per FIFO word), not its depth
(number of entries). The watermark register expects a depth-based
threshold, so this produces an incorrect watermark value and, on QUP
HW versions >= 3.10 where the depth field widened to 8 bits for
256-byte-deep FIFOs, an inconsistent one depending on core revision.
This mirrors geni_i2c_get_tx_fifo_depth() in drivers/i2c/geni_i2c.c,
which already reads SE_HW_PARAM_0 with the HW-version-gated
TX_FIFO_DEPTH_MSK/TX_FIFO_DEPTH_MSK_256_BYTES masks for the same
reason on the TX side.
The same depth-vs-width distinction, and the HW-version gating for
QUP HW >= 3.10, was introduced upstream in Linux by commit
fe8aa1ba0783 ("soc: qcom: geni-se: Update Tx and Rx fifo depth based
on QUP HW version"). The Linux driver this file's firmware-loading
sequence was ported from, added by commit d4bf06592ad6 ("soc: qcom:
geni-se: Add support to load QUP SE Firmware via Linux subsystem"),
computes the RX_RFR_WATERMARK value via geni_se_get_rx_fifo_depth()
which applies that same depth mask -- confirming this is a porting
bug rather than an intentional difference.
Extract the major/minor-gated mask selection into a new
geni_se_fifo_depth_mask() helper in <soc/qcom/geni-se.h> instead of
inlining it here, so that other GENI SE drivers (SPI, I2C, UART) can
share it for their SE_HW_PARAM_0/1 TX/RX FIFO depth decoding instead of
duplicating this logic.
Reviewed-by: Balaji Selvanathan <[email protected]>
Reviewed-by: Casey Connolly <[email protected]>
Signed-off-by: Vandhiadevan Karunamoorthy
<[email protected]>
---
Changes in v4:
- Include <linux/types.h> and <linux/bitops.h> in geni-se.h to make it
self-contained
- Make commit message generic, not SPI specific
- Carry Reviewed-by from v1:
https://lore.kernel.org/u-boot/20260824-geni-rx-rfr-watermark-fix-v1-1-d83fd0788...@oss.qualcomm.com/
- Link to v3:
https://lore.kernel.org/u-boot/20261006-geni-rx-rfr-watermark-fix-v3-1-b368fe3fb...@oss.qualcomm.com/
---
drivers/misc/qcom_geni.c | 9 +++++++--
include/soc/qcom/geni-se.h | 27 +++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/qcom_geni.c b/drivers/misc/qcom_geni.c
index a62ae6a2478..f3cf05c1c0b 100644
--- a/drivers/misc/qcom_geni.c
+++ b/drivers/misc/qcom_geni.c
@@ -168,6 +168,7 @@ static int load_se_firmware(struct qup_se_rsc *rsc, struct
elf_se_hdr *hdr)
const u32 *fw_val_arr, *cfg_val_arr;
const u8 *cfg_idx_arr;
u32 i, reg_value, mask, ramn_cnt;
+ u32 hw_version, rx_fifo_depth_mask;
int ret;
fw_val_arr = (const u32 *)((u8 *)hdr + hdr->fw_offset);
@@ -207,8 +208,12 @@ static int load_se_firmware(struct qup_se_rsc *rsc, struct
elf_se_hdr *hdr)
rsc->base + GENI_CFG_REG0 + (cfg_idx_arr[i] *
sizeof(u32)));
/* Configure condition for assertion of RX_RFR_WATERMARK condition. */
- reg_value = readl_relaxed(rsc->base + QUPV3_SE_HW_PARAM_1);
- mask = (reg_value >> RX_FIFO_WIDTH_BIT) & RX_FIFO_WIDTH_MASK;
+ hw_version = readl_relaxed(rsc->wrapper_base + QUP_HW_VER_REG);
+ rx_fifo_depth_mask = geni_se_fifo_depth_mask(hw_version,
RX_FIFO_DEPTH_MSK_256_BYTES,
+ RX_FIFO_DEPTH_MSK);
+
+ reg_value = readl_relaxed(rsc->base + SE_HW_PARAM_1);
+ mask = (reg_value & rx_fifo_depth_mask) >> RX_FIFO_DEPTH_SHFT;
writel_relaxed(mask - 2, rsc->base + GENI_RX_RFR_WATERMARK_REG);
/* Let hardware control CGC */
diff --git a/include/soc/qcom/geni-se.h b/include/soc/qcom/geni-se.h
index fc9a8e82cd8..8af78c3a61d 100644
--- a/include/soc/qcom/geni-se.h
+++ b/include/soc/qcom/geni-se.h
@@ -6,6 +6,9 @@
#ifndef _QCOM_GENI_SE
#define _QCOM_GENI_SE
+#include <linux/bitops.h>
+#include <linux/types.h>
+
enum geni_se_xfer_mode {
GENI_SE_INVALID,
GENI_SE_FIFO,
@@ -277,6 +280,30 @@ enum geni_se_protocol_type {
#define GENI_SE_VERSION_MINOR(ver) ((ver & HW_VER_MINOR_MASK) >>
HW_VER_MINOR_SHFT)
#define GENI_SE_VERSION_STEP(ver) (ver & HW_VER_STEP_MASK)
+/*
+ * geni_se_fifo_depth_mask() - Pick the TX/RX SE_HW_PARAM_x fifo depth mask
+ * for a given QUP_HW_VER_REG value.
+ * @hw_version: value read from QUP_HW_VER_REG
+ * @depth_mask_256: mask to use on HW that supports 256-byte-deep fifos
+ * (QUP HW version >= 3.10, 8-bit depth field)
+ * @depth_mask: mask to use on older HW (6-bit depth field)
+ *
+ * QUP HW version >= 3.10 widened the fifo depth field in SE_HW_PARAM_0
+ * (TX) and SE_HW_PARAM_1 (RX) from 6 bits to 8 bits; both fields are
+ * gated by the same major/minor check.
+ */
+static inline u32 geni_se_fifo_depth_mask(u32 hw_version, u32 depth_mask_256,
+ u32 depth_mask)
+{
+ u32 hw_major = GENI_SE_VERSION_MAJOR(hw_version);
+ u32 hw_minor = GENI_SE_VERSION_MINOR(hw_version);
+
+ if ((hw_major == 3 && hw_minor >= 10) || hw_major > 3)
+ return depth_mask_256;
+
+ return depth_mask;
+}
+
/* QUP SE VERSION value for major number 2 and minor number 5 */
#define QUP_SE_VERSION_2_5 0x20050000
---
base-commit: 32ebe922e574a733d01c3f106fa77d9176bfecad
change-id: 20260821-geni-rx-rfr-watermark-fix-d48171a6541f
Best regards,
--
Vandhiadevan Karunamoorthy <[email protected]>
