Re: [PATCH net-next v3 04/14] net: ethernet: qualcomm: Initialize PPE buffer management for IPQ9574

[Andrew Lunn]

> +	/* Configure BM flow control related threshold. */
> +	PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight);
> +	PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset);
> +	PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil);
> +	PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic);
> +	PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf);
> +	PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc);

...

> +#define PPE_BM_PORT_FC_CFG_TBL_ADDR		0x601000
> +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES		15
> +#define PPE_BM_PORT_FC_CFG_TBL_INC		0x10
> +#define PPE_BM_PORT_FC_W0_REACT_LIMIT		GENMASK(8, 0)
> +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD	GENMASK(17, 9)
> +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET		GENMASK(28, 18)
> +#define PPE_BM_PORT_FC_W0_CEILING_LOW		GENMASK(31, 29)
> +#define PPE_BM_PORT_FC_W1_CEILING_HIGH		GENMASK(7, 0)
> +#define PPE_BM_PORT_FC_W1_WEIGHT		GENMASK(10, 8)
> +#define PPE_BM_PORT_FC_W1_DYNAMIC		BIT(11)
> +#define PPE_BM_PORT_FC_W1_PRE_ALLOC		GENMASK(22, 12)
> +
> +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value)	\
> +	u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT)
> +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value)	\
> +	u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD)

Where is u32p_replace_bits()?

This cast does not look good. And this does not look like anything any
other driver does. I suspect you are not using FIELD_PREP() etc when
you should.

https://elixir.bootlin.com/linux/v6.14-rc2/source/include/linux/bitfield.h

	Andrew