Hi Chintan, On Fri, 2024-08-23 at 15:22 +0530, Chintan Vankar wrote: > On 16/08/24 17:58, Sverdlin, Alexander wrote: > > Hi Chintan, Vignesh, > > > > On Fri, 2024-07-05 at 10:20 +0530, Chintan Vankar wrote: > > > From: Vignesh Raghavendra <vigne...@ti.com> > > > > > > Expectation of k3_ringacc_ring_reset_raw() is to reset the ring to > > > requested size and not to 0. Fix this. > > > > > > Signed-off-by: Vignesh Raghavendra <vigne...@ti.com> > > > Signed-off-by: Siddharth Vadapalli <s-vadapa...@ti.com> > > > Signed-off-by: Chintan Vankar <c-van...@ti.com> > > > --- > > > > > > Link to v2: > > > https://lore.kernel.org/r/20240425120822.2048012-5-c-van...@ti.com/ > > > > > > Changes from v2 to v3: > > > - No changes. > > > > > > drivers/soc/ti/k3-navss-ringacc-u-boot.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/soc/ti/k3-navss-ringacc-u-boot.c > > > b/drivers/soc/ti/k3-navss-ringacc-u-boot.c > > > index f958239c2a..5d650b9de7 100644 > > > --- a/drivers/soc/ti/k3-navss-ringacc-u-boot.c > > > +++ b/drivers/soc/ti/k3-navss-ringacc-u-boot.c > > > @@ -25,9 +25,16 @@ struct k3_nav_ring_cfg_regs { > > > #define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_MASK GENMASK(26, 24) > > > #define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_SHIFT (24) > > > > > > +#define KNAV_RINGACC_CFG_RING_SIZE_MASK GENMASK(15, 0) > > > + > > > static void k3_ringacc_ring_reset_raw(struct k3_nav_ring *ring) > > > { > > > - writel(0, &ring->cfg->size); > > > + u32 reg; > > > + > > > + reg = readl(&ring->cfg->size); > > > + reg &= KNAV_RINGACC_CFG_RING_SIZE_MASK; > > > > should it actually be "reg &= ~KNAV_RINGACC_CFG_RING_SIZE_MASK;"? > > Otherwise you could potentially bump the size to some number higher than > > ring->size > > if you "OR" ring->size with something lower than ring->size and ring->size > > is not > > N^2-1? > > > > Hello Alexander, I didn't get your comment, can you explain in more > detail?
What is the exact purpose of "reg &= KNAV_RINGACC_CFG_RING_SIZE_MASK;"? Which corner case is handled by it? What happens if ring->cfg->size contains "1" and ring->size contains "2"? What will be written into ring->cfg->size? > > > + reg |= ring->size; > > > + writel(reg, &ring->cfg->size); > > > } > > > > > > static void k3_ringacc_ring_reconfig_qmode_raw(struct k3_nav_ring > > > *ring, enum k3_nav_ring_mode mode) -- Alexander Sverdlin Siemens AG www.siemens.com
