On 24/08/2026 13:19, 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]>
Reviewed-by: Casey Connolly <[email protected]>
> ---
> 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]>
>
--
// Casey (she/her)
