Hi Vandhiadevan,
Thanks for the patch.
On 8/24/2026 4:49 PM, Vandhiadevan Karunamoorthy wrote:
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.
Signed-off-by: Vandhiadevan Karunamoorthy
<[email protected]>
Looks good.
Reviewed-by: Balaji Selvanathan <[email protected]>
Regards,
Balaji
---
drivers/misc/qcom_geni.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/qcom_geni.c b/drivers/misc/qcom_geni.c
index a62ae6a2478..bfbb6ff845f 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, hw_major, hw_minor, rx_fifo_depth_mask;
int ret;
fw_val_arr = (const u32 *)((u8 *)hdr + hdr->fw_offset);
@@ -207,8 +208,17 @@ 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);
+ hw_major = GENI_SE_VERSION_MAJOR(hw_version);
+ hw_minor = GENI_SE_VERSION_MINOR(hw_version);
+
+ if ((hw_major == 3 && hw_minor >= 10) || hw_major > 3)
+ rx_fifo_depth_mask = RX_FIFO_DEPTH_MSK_256_BYTES;
+ else
+ rx_fifo_depth_mask = 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 */
---
base-commit: ece349ade2973e220f524ce59e59711cc919263f
change-id: 20260821-geni-rx-rfr-watermark-fix-d48171a6541f
Best regards,
--
Vandhiadevan Karunamoorthy <[email protected]>
