diff mbox series

[net-next] net: ipa: initialize ring indexes to 0

Message ID 20220719141855.245994-1-elder@linaro.org (mailing list archive)
State Accepted
Commit 5fb859f79f4f49d9df16bac2b3a84a6fa3aaccf1
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: ipa: initialize ring indexes to 0 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 76 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alex Elder July 19, 2022, 2:18 p.m. UTC
When a GSI channel is initially allocated, and after it has been
reset, the hardware assumes its ring index is 0.  And although we
do initialize channels this way, the comments in the IPA code don't
really explain this.  For event rings, it doesn't matter what value
we use initially, so using 0 is just fine.

Add some information about the assumptions made by hardware above
the definition of the gsi_ring structure in "gsi.h".

Zero the index field for all rings (channel and event) when the ring
is allocated.  As a result, that function initializes all fields in
the structure.

Stop zeroing the index the top of gsi_channel_program().  Initially
we'll use the index value set when the channel ring was allocated.
And we'll explicitly zero the index value in gsi_channel_reset()
before programming the hardware, adding a comment explaining why
it's required.

For event rings, use the index initialized by gsi_ring_alloc()
rather than 0 when ringing the doorbell in gsi_evt_ring_program().
(It'll still be zero, but we won't assume that to be the case.)

Use a local variable in gsi_evt_ring_program() that represents the
address of the event ring's ring structure.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 18 ++++++++++--------
 drivers/net/ipa/gsi.h |  5 +++--
 2 files changed, 13 insertions(+), 10 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org July 20, 2022, 10:20 a.m. UTC | #1
Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 19 Jul 2022 09:18:55 -0500 you wrote:
> When a GSI channel is initially allocated, and after it has been
> reset, the hardware assumes its ring index is 0.  And although we
> do initialize channels this way, the comments in the IPA code don't
> really explain this.  For event rings, it doesn't matter what value
> we use initially, so using 0 is just fine.
> 
> Add some information about the assumptions made by hardware above
> the definition of the gsi_ring structure in "gsi.h".
> 
> [...]

Here is the summary with links:
  - [net-next] net: ipa: initialize ring indexes to 0
    https://git.kernel.org/netdev/net-next/c/5fb859f79f4f

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4e46974a69ecd..fcd05acf893b3 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -665,7 +665,8 @@  static void gsi_evt_ring_doorbell(struct gsi *gsi, u32 evt_ring_id, u32 index)
 static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 {
 	struct gsi_evt_ring *evt_ring = &gsi->evt_ring[evt_ring_id];
-	size_t size = evt_ring->ring.count * GSI_RING_ELEMENT_SIZE;
+	struct gsi_ring *ring = &evt_ring->ring;
+	size_t size;
 	u32 val;
 
 	/* We program all event rings as GPI type/protocol */
@@ -674,6 +675,7 @@  static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 	val |= u32_encode_bits(GSI_RING_ELEMENT_SIZE, EV_ELEMENT_SIZE_FMASK);
 	iowrite32(val, gsi->virt + GSI_EV_CH_E_CNTXT_0_OFFSET(evt_ring_id));
 
+	size = ring->count * GSI_RING_ELEMENT_SIZE;
 	val = ev_r_length_encoded(gsi->version, size);
 	iowrite32(val, gsi->virt + GSI_EV_CH_E_CNTXT_1_OFFSET(evt_ring_id));
 
@@ -681,9 +683,9 @@  static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 	 * high-order 32 bits of the address of the event ring,
 	 * respectively.
 	 */
-	val = lower_32_bits(evt_ring->ring.addr);
+	val = lower_32_bits(ring->addr);
 	iowrite32(val, gsi->virt + GSI_EV_CH_E_CNTXT_2_OFFSET(evt_ring_id));
-	val = upper_32_bits(evt_ring->ring.addr);
+	val = upper_32_bits(ring->addr);
 	iowrite32(val, gsi->virt + GSI_EV_CH_E_CNTXT_3_OFFSET(evt_ring_id));
 
 	/* Enable interrupt moderation by setting the moderation delay */
@@ -700,8 +702,8 @@  static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 	iowrite32(0, gsi->virt + GSI_EV_CH_E_CNTXT_12_OFFSET(evt_ring_id));
 	iowrite32(0, gsi->virt + GSI_EV_CH_E_CNTXT_13_OFFSET(evt_ring_id));
 
-	/* Finally, tell the hardware we've completed event 0 (arbitrary) */
-	gsi_evt_ring_doorbell(gsi, evt_ring_id, 0);
+	/* Finally, tell the hardware our "last processed" event (arbitrary) */
+	gsi_evt_ring_doorbell(gsi, evt_ring_id, ring->index);
 }
 
 /* Find the transaction whose completion indicates a channel is quiesced */
@@ -770,9 +772,6 @@  static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
 	u32 wrr_weight = 0;
 	u32 val;
 
-	/* Arbitrarily pick TRE 0 as the first channel element to use */
-	channel->tre_ring.index = 0;
-
 	/* We program all channels as GPI type/protocol */
 	val = chtype_protocol_encoded(gsi->version, GSI_CHANNEL_TYPE_GPI);
 	if (channel->toward_ipa)
@@ -949,6 +948,8 @@  void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell)
 	if (gsi->version < IPA_VERSION_4_0 && !channel->toward_ipa)
 		gsi_channel_reset_command(channel);
 
+	/* Hardware assumes this is 0 following reset */
+	channel->tre_ring.index = 0;
 	gsi_channel_program(channel, doorbell);
 	gsi_channel_trans_cancel_pending(channel);
 
@@ -1433,6 +1434,7 @@  static int gsi_ring_alloc(struct gsi *gsi, struct gsi_ring *ring, u32 count)
 
 	ring->addr = addr;
 	ring->count = count;
+	ring->index = 0;
 
 	return 0;
 }
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index bad1a78a96ede..982c57550ef37 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -48,12 +48,13 @@  struct gsi_ring {
 	 *
 	 * A channel ring consists of TRE entries filled by the AP and passed
 	 * to the hardware for processing.  For a channel ring, the ring index
-	 * identifies the next unused entry to be filled by the AP.
+	 * identifies the next unused entry to be filled by the AP.  In this
+	 * case the initial value is assumed by hardware to be 0.
 	 *
 	 * An event ring consists of event structures filled by the hardware
 	 * and passed to the AP.  For event rings, the ring index identifies
 	 * the next ring entry that is not known to have been filled by the
-	 * hardware.
+	 * hardware.  The initial value used is arbitrary (so we use 0).
 	 */
 	u32 index;
 };