From 97e4692dda26d047899a3f1f3f57c3d861b6b9c9 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 4 May 2020 18:53:41 -0500 Subject: net: ipa: fix a bug in ipa_endpoint_stop() In ipa_endpoint_stop(), for TX endpoints we set the number of retries to 0. When we break out of the loop, retries being 0 means we return EIO rather than the value of ret (which should be 0). Fix this by using a non-zero retry count for both RX and TX channels, and just break out of the loop after calling gsi_channel_stop() for TX channels. This way only RX channels will retry, and the retry count will be non-zero at the end for TX channels (so the proper value gets returned). Signed-off-by: Alex Elder Signed-off-by: David S. Miller (cherry picked from commit 713b6ebb4c376b3fb65fdceb3b59e401c93248f9) Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_endpoint.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index dec1dc8618ed..de5b1e42d1ed 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -1311,7 +1311,7 @@ static int ipa_endpoint_stop_rx_dma(struct ipa *ipa) */ int ipa_endpoint_stop(struct ipa_endpoint *endpoint) { - u32 retries = endpoint->toward_ipa ? 0 : IPA_ENDPOINT_STOP_RX_RETRIES; + u32 retries = IPA_ENDPOINT_STOP_RX_RETRIES; int ret; do { @@ -1319,12 +1319,9 @@ int ipa_endpoint_stop(struct ipa_endpoint *endpoint) struct gsi *gsi = &ipa->gsi; ret = gsi_channel_stop(gsi, endpoint->channel_id); - if (ret != -EAGAIN) + if (ret != -EAGAIN || endpoint->toward_ipa) break; - if (endpoint->toward_ipa) - continue; - /* For IPA v3.5.1, send a DMA read task and check again */ if (ipa->version == IPA_VERSION_3_5_1) { ret = ipa_endpoint_stop_rx_dma(ipa); -- cgit v1.2.3 From c72ddf0dbf2d9d80afd37e28f0a52e728e6db159 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 4 May 2020 18:53:42 -0500 Subject: net: ipa: get rid of workaround in ipa_endpoint_stop() In ipa_endpoint_stop(), a workaround is used for IPA version 3.5.1 where a 1-byte DMA request is issued between GSI channel stop retries. It turns out that this workaround is only required for IPA versions 3.1 and 3.2, and we don't support those. So remove the call to ipa_endpoint_stop_rx_dma() in that function. That leaves that function unused, so get rid of it. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_endpoint.c | 39 +-------------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index de5b1e42d1ed..7176ae892e75 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -33,13 +33,10 @@ #define IPA_RX_BUFFER_OVERHEAD (PAGE_SIZE - SKB_MAX_ORDER(NET_SKB_PAD, 0)) #define IPA_ENDPOINT_STOP_RX_RETRIES 10 -#define IPA_ENDPOINT_STOP_RX_SIZE 1 /* bytes */ #define IPA_ENDPOINT_RESET_AGGR_RETRY_MAX 3 #define IPA_AGGR_TIME_LIMIT_DEFAULT 1000 /* microseconds */ -#define ENDPOINT_STOP_DMA_TIMEOUT 15 /* milliseconds */ - /** enum ipa_status_opcode - status element opcode hardware values */ enum ipa_status_opcode { IPA_STATUS_OPCODE_PACKET = 0x01, @@ -1274,32 +1271,6 @@ static void ipa_endpoint_reset(struct ipa_endpoint *endpoint) ret, endpoint->channel_id, endpoint->endpoint_id); } -static int ipa_endpoint_stop_rx_dma(struct ipa *ipa) -{ - u16 size = IPA_ENDPOINT_STOP_RX_SIZE; - struct gsi_trans *trans; - dma_addr_t addr; - int ret; - - trans = ipa_cmd_trans_alloc(ipa, 1); - if (!trans) { - dev_err(&ipa->pdev->dev, - "no transaction for RX endpoint STOP workaround\n"); - return -EBUSY; - } - - /* Read into the highest part of the zero memory area */ - addr = ipa->zero_addr + ipa->zero_size - size; - - ipa_cmd_dma_task_32b_addr_add(trans, size, addr, false); - - ret = gsi_trans_commit_wait_timeout(trans, ENDPOINT_STOP_DMA_TIMEOUT); - if (ret) - gsi_trans_free(trans); - - return ret; -} - /** * ipa_endpoint_stop() - Stops a GSI channel in IPA * @client: Client whose endpoint should be stopped @@ -1315,20 +1286,12 @@ int ipa_endpoint_stop(struct ipa_endpoint *endpoint) int ret; do { - struct ipa *ipa = endpoint->ipa; - struct gsi *gsi = &ipa->gsi; + struct gsi *gsi = &endpoint->ipa->gsi; ret = gsi_channel_stop(gsi, endpoint->channel_id); if (ret != -EAGAIN || endpoint->toward_ipa) break; - /* For IPA v3.5.1, send a DMA read task and check again */ - if (ipa->version == IPA_VERSION_3_5_1) { - ret = ipa_endpoint_stop_rx_dma(ipa); - if (ret) - break; - } - msleep(1); } while (retries--); -- cgit v1.2.3 From 9928fcc76f7bb5c93d512767b039e8257ab3093e Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 4 May 2020 18:53:43 -0500 Subject: net: ipa: don't retry in ipa_endpoint_stop() The only reason ipa_endpoint_stop() had a retry loop was that the just-removed workaround required an IPA DMA command to occur between attempts. The gsi_channel_stop() call that implements the stop does its own retry loop, to cover a channel's transition from started to stop-in-progress to stopped state. Get rid of the unnecessary retry loop in ipa_endpoint_stop(). Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_endpoint.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 7176ae892e75..68ba33ec7ce9 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -32,8 +32,6 @@ /* The amount of RX buffer space consumed by standard skb overhead */ #define IPA_RX_BUFFER_OVERHEAD (PAGE_SIZE - SKB_MAX_ORDER(NET_SKB_PAD, 0)) -#define IPA_ENDPOINT_STOP_RX_RETRIES 10 - #define IPA_ENDPOINT_RESET_AGGR_RETRY_MAX 3 #define IPA_AGGR_TIME_LIMIT_DEFAULT 1000 /* microseconds */ @@ -1282,20 +1280,9 @@ static void ipa_endpoint_reset(struct ipa_endpoint *endpoint) */ int ipa_endpoint_stop(struct ipa_endpoint *endpoint) { - u32 retries = IPA_ENDPOINT_STOP_RX_RETRIES; - int ret; - - do { - struct gsi *gsi = &endpoint->ipa->gsi; - - ret = gsi_channel_stop(gsi, endpoint->channel_id); - if (ret != -EAGAIN || endpoint->toward_ipa) - break; - - msleep(1); - } while (retries--); + struct gsi *gsi = &endpoint->ipa->gsi; - return retries ? ret : -EIO; + return gsi_channel_stop(gsi, endpoint->channel_id); } static void ipa_endpoint_program(struct ipa_endpoint *endpoint) -- cgit v1.2.3 From f30dcb7dcb1aa925dfc83923c580a53c975b754b Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 4 May 2020 18:53:44 -0500 Subject: net: ipa: kill ipa_endpoint_stop() The previous commit made ipa_endpoint_stop() be a trivial wrapper around gsi_channel_stop(). Since it no longer does anything special, just open-code it in the three places it's used. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_endpoint.c | 27 ++++++--------------------- drivers/net/ipa/ipa_endpoint.h | 2 -- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 68ba33ec7ce9..82066a223a67 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -1214,7 +1214,7 @@ static int ipa_endpoint_reset_rx_aggr(struct ipa_endpoint *endpoint) gsi_trans_read_byte_done(gsi, endpoint->channel_id); - ret = ipa_endpoint_stop(endpoint); + ret = gsi_channel_stop(gsi, endpoint->channel_id); if (ret) goto out_suspend_again; @@ -1231,7 +1231,7 @@ static int ipa_endpoint_reset_rx_aggr(struct ipa_endpoint *endpoint) goto out_suspend_again; err_endpoint_stop: - ipa_endpoint_stop(endpoint); + (void)gsi_channel_stop(gsi, endpoint->channel_id); out_suspend_again: if (suspended) (void)ipa_endpoint_program_suspend(endpoint, true); @@ -1269,22 +1269,6 @@ static void ipa_endpoint_reset(struct ipa_endpoint *endpoint) ret, endpoint->channel_id, endpoint->endpoint_id); } -/** - * ipa_endpoint_stop() - Stops a GSI channel in IPA - * @client: Client whose endpoint should be stopped - * - * This function implements the sequence to stop a GSI channel - * in IPA. This function returns when the channel is is STOP state. - * - * Return value: 0 on success, negative otherwise - */ -int ipa_endpoint_stop(struct ipa_endpoint *endpoint) -{ - struct gsi *gsi = &endpoint->ipa->gsi; - - return gsi_channel_stop(gsi, endpoint->channel_id); -} - static void ipa_endpoint_program(struct ipa_endpoint *endpoint) { if (endpoint->toward_ipa) { @@ -1337,12 +1321,13 @@ void ipa_endpoint_disable_one(struct ipa_endpoint *endpoint) { u32 mask = BIT(endpoint->endpoint_id); struct ipa *ipa = endpoint->ipa; + struct gsi *gsi = &ipa->gsi; int ret; - if (!(endpoint->ipa->enabled & mask)) + if (!(ipa->enabled & mask)) return; - endpoint->ipa->enabled ^= mask; + ipa->enabled ^= mask; if (!endpoint->toward_ipa) { ipa_endpoint_replenish_disable(endpoint); @@ -1351,7 +1336,7 @@ void ipa_endpoint_disable_one(struct ipa_endpoint *endpoint) } /* Note that if stop fails, the channel's state is not well-defined */ - ret = ipa_endpoint_stop(endpoint); + ret = gsi_channel_stop(gsi, endpoint->channel_id); if (ret) dev_err(&ipa->pdev->dev, "error %d attempting to stop endpoint %u\n", ret, diff --git a/drivers/net/ipa/ipa_endpoint.h b/drivers/net/ipa/ipa_endpoint.h index 4b336a1f759d..3b297d65828e 100644 --- a/drivers/net/ipa/ipa_endpoint.h +++ b/drivers/net/ipa/ipa_endpoint.h @@ -76,8 +76,6 @@ int ipa_endpoint_modem_exception_reset_all(struct ipa *ipa); int ipa_endpoint_skb_tx(struct ipa_endpoint *endpoint, struct sk_buff *skb); -int ipa_endpoint_stop(struct ipa_endpoint *endpoint); - void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint); int ipa_endpoint_enable_one(struct ipa_endpoint *endpoint); -- cgit v1.2.3 From da1a782a7140fab22f2dfe8453d7b73c786d73de Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 4 May 2020 18:53:45 -0500 Subject: net: ipa: kill ipa_cmd_dma_task_32b_addr_add() A recent commit removed the only use of ipa_cmd_dma_task_32b_addr_add(). This function (and the IPA immediate command it implements) is no longer needed, so get rid of it, along with all of the definitions associated with it. Isolate its removal in a commit so it can be easily added back again if needed. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_cmd.c | 59 ----------------------------------------------- drivers/net/ipa/ipa_cmd.h | 11 --------- 2 files changed, 70 deletions(-) diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index d226b858742d..394f8a6df086 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -103,28 +103,6 @@ struct ipa_cmd_ip_packet_init { /* Field masks for ipa_cmd_ip_packet_init dest_endpoint field */ #define IPA_PACKET_INIT_DEST_ENDPOINT_FMASK GENMASK(4, 0) -/* IPA_CMD_DMA_TASK_32B_ADDR */ - -/* This opcode gets modified with a DMA operation count */ - -#define DMA_TASK_32B_ADDR_OPCODE_COUNT_FMASK GENMASK(15, 8) - -struct ipa_cmd_hw_dma_task_32b_addr { - __le16 flags; - __le16 size; - __le32 addr; - __le16 packet_size; - u8 reserved[6]; -}; - -/* Field masks for ipa_cmd_hw_dma_task_32b_addr flags field */ -#define DMA_TASK_32B_ADDR_FLAGS_SW_RSVD_FMASK GENMASK(10, 0) -#define DMA_TASK_32B_ADDR_FLAGS_CMPLT_FMASK GENMASK(11, 11) -#define DMA_TASK_32B_ADDR_FLAGS_EOF_FMASK GENMASK(12, 12) -#define DMA_TASK_32B_ADDR_FLAGS_FLSH_FMASK GENMASK(13, 13) -#define DMA_TASK_32B_ADDR_FLAGS_LOCK_FMASK GENMASK(14, 14) -#define DMA_TASK_32B_ADDR_FLAGS_UNLOCK_FMASK GENMASK(15, 15) - /* IPA_CMD_DMA_SHARED_MEM */ /* For IPA v4.0+, this opcode gets modified with pipeline clear options */ @@ -163,7 +141,6 @@ union ipa_cmd_payload { struct ipa_cmd_hw_hdr_init_local hdr_init_local; struct ipa_cmd_register_write register_write; struct ipa_cmd_ip_packet_init ip_packet_init; - struct ipa_cmd_hw_dma_task_32b_addr dma_task_32b_addr; struct ipa_cmd_hw_dma_mem_mem dma_shared_mem; struct ipa_cmd_ip_packet_tag_status ip_packet_tag_status; }; @@ -508,42 +485,6 @@ static void ipa_cmd_ip_packet_init_add(struct gsi_trans *trans, u8 endpoint_id) direction, opcode); } -/* Use a 32-bit DMA command to zero a block of memory */ -void ipa_cmd_dma_task_32b_addr_add(struct gsi_trans *trans, u16 size, - dma_addr_t addr, bool toward_ipa) -{ - struct ipa *ipa = container_of(trans->gsi, struct ipa, gsi); - enum ipa_cmd_opcode opcode = IPA_CMD_DMA_TASK_32B_ADDR; - struct ipa_cmd_hw_dma_task_32b_addr *payload; - union ipa_cmd_payload *cmd_payload; - enum dma_data_direction direction; - dma_addr_t payload_addr; - u16 flags; - - /* assert(addr <= U32_MAX); */ - addr &= GENMASK_ULL(31, 0); - - /* The opcode encodes the number of DMA operations in the high byte */ - opcode |= u16_encode_bits(1, DMA_TASK_32B_ADDR_OPCODE_COUNT_FMASK); - - direction = toward_ipa ? DMA_TO_DEVICE : DMA_FROM_DEVICE; - - /* complete: 0 = don't interrupt; eof: 0 = don't assert eot */ - flags = DMA_TASK_32B_ADDR_FLAGS_FLSH_FMASK; - /* lock: 0 = don't lock endpoint; unlock: 0 = don't unlock */ - - cmd_payload = ipa_cmd_payload_alloc(ipa, &payload_addr); - payload = &cmd_payload->dma_task_32b_addr; - - payload->flags = cpu_to_le16(flags); - payload->size = cpu_to_le16(size); - payload->addr = cpu_to_le32((u32)addr); - payload->packet_size = cpu_to_le16(size); - - gsi_trans_cmd_add(trans, payload, sizeof(*payload), payload_addr, - direction, opcode); -} - /* Use a DMA command to read or write a block of IPA-resident memory */ void ipa_cmd_dma_shared_mem_add(struct gsi_trans *trans, u32 offset, u16 size, dma_addr_t addr, bool toward_ipa) diff --git a/drivers/net/ipa/ipa_cmd.h b/drivers/net/ipa/ipa_cmd.h index 4917525b3a47..e440aa69c8b5 100644 --- a/drivers/net/ipa/ipa_cmd.h +++ b/drivers/net/ipa/ipa_cmd.h @@ -35,7 +35,6 @@ enum ipa_cmd_opcode { IPA_CMD_HDR_INIT_LOCAL = 9, IPA_CMD_REGISTER_WRITE = 12, IPA_CMD_IP_PACKET_INIT = 16, - IPA_CMD_DMA_TASK_32B_ADDR = 17, IPA_CMD_DMA_SHARED_MEM = 19, IPA_CMD_IP_PACKET_TAG_STATUS = 20, }; @@ -147,16 +146,6 @@ void ipa_cmd_hdr_init_local_add(struct gsi_trans *trans, u32 offset, u16 size, void ipa_cmd_register_write_add(struct gsi_trans *trans, u32 offset, u32 value, u32 mask, bool clear_full); -/** - * ipa_cmd_dma_task_32b_addr_add() - Add a 32-bit DMA command to a transaction - * @trans: GSi transaction - * @size: Number of bytes to be memory to be transferred - * @addr: DMA address of buffer to be read into or written from - * @toward_ipa: true means write to IPA memory; false means read - */ -void ipa_cmd_dma_task_32b_addr_add(struct gsi_trans *trans, u16 size, - dma_addr_t addr, bool toward_ipa); - /** * ipa_cmd_dma_shared_mem_add() - Add a DMA memory command to a transaction * @trans: GSI transaction -- cgit v1.2.3