* [PATCH net-next v2 1/5] net: ipa: head-of-line block registers are RX only
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
@ 2020-06-30 13:33 ` Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 2/5] net: ipa: metadata_mask register is " Alex Elder
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-06-30 13:33 UTC (permalink / raw)
To: davem, kuba
Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
The INIT_HOL_BLOCK_EN and INIT_HOL_BLOCK_TIMER endpoint registers
are only valid for RX endpoints.
Have ipa_endpoint_modem_hol_block_clear_all() skip writing these
registers for TX endpoints.
Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: The commented calls to assert() that were added are now gone.
drivers/net/ipa/ipa_endpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 9f50d0d11704..31afe282f347 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -683,7 +683,7 @@ void ipa_endpoint_modem_hol_block_clear_all(struct ipa *ipa)
for (i = 0; i < IPA_ENDPOINT_MAX; i++) {
struct ipa_endpoint *endpoint = &ipa->endpoint[i];
- if (endpoint->ee_id != GSI_EE_MODEM)
+ if (endpoint->toward_ipa || endpoint->ee_id != GSI_EE_MODEM)
continue;
(void)ipa_endpoint_init_hol_block_timer(endpoint, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 2/5] net: ipa: metadata_mask register is RX only
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 1/5] net: ipa: head-of-line block registers are RX only Alex Elder
@ 2020-06-30 13:33 ` Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 3/5] net: ipa: mode register is TX only Alex Elder
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-06-30 13:33 UTC (permalink / raw)
To: davem, kuba
Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
The INIT_HDR_METADATA_MASK endpoint configuration register is only
valid for RX endpoints. Rather than writing a zero to that register
for TX endpoints, avoid writing the register at all.
Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: The commented call to assert() that was added is now gone.
drivers/net/ipa/ipa_endpoint.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 31afe282f347..1babcfc79360 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -530,7 +530,7 @@ static void ipa_endpoint_init_hdr_metadata_mask(struct ipa_endpoint *endpoint)
offset = IPA_REG_ENDP_INIT_HDR_METADATA_MASK_N_OFFSET(endpoint_id);
/* Note that HDR_ENDIANNESS indicates big endian header fields */
- if (!endpoint->toward_ipa && endpoint->data->qmap)
+ if (endpoint->data->qmap)
val = cpu_to_be32(IPA_ENDPOINT_QMAP_METADATA_MASK);
iowrite32(val, endpoint->ipa->reg_virt + offset);
@@ -1302,10 +1302,10 @@ static void ipa_endpoint_program(struct ipa_endpoint *endpoint)
(void)ipa_endpoint_program_suspend(endpoint, false);
ipa_endpoint_init_hdr_ext(endpoint);
ipa_endpoint_init_aggr(endpoint);
+ ipa_endpoint_init_hdr_metadata_mask(endpoint);
}
ipa_endpoint_init_cfg(endpoint);
ipa_endpoint_init_hdr(endpoint);
- ipa_endpoint_init_hdr_metadata_mask(endpoint);
ipa_endpoint_init_mode(endpoint);
ipa_endpoint_status(endpoint);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 3/5] net: ipa: mode register is TX only
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 1/5] net: ipa: head-of-line block registers are RX only Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 2/5] net: ipa: metadata_mask register is " Alex Elder
@ 2020-06-30 13:33 ` Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 4/5] net: ipa: clarify endpoint register macro constraints Alex Elder
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-06-30 13:33 UTC (permalink / raw)
To: davem, kuba
Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
The INIT_MODE endpoint configuration register is only valid for TX
endpoints. Rather than writing a zero to that register for RX
endpoints, avoid writing the register at all.
Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: The commented calls to assert() that were added are now gone.
drivers/net/ipa/ipa_endpoint.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 1babcfc79360..566ff6a09e65 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -541,7 +541,7 @@ static void ipa_endpoint_init_mode(struct ipa_endpoint *endpoint)
u32 offset = IPA_REG_ENDP_INIT_MODE_N_OFFSET(endpoint->endpoint_id);
u32 val;
- if (endpoint->toward_ipa && endpoint->data->dma_mode) {
+ if (endpoint->data->dma_mode) {
enum ipa_endpoint_name name = endpoint->data->dma_endpoint;
u32 dma_endpoint_id;
@@ -552,7 +552,7 @@ static void ipa_endpoint_init_mode(struct ipa_endpoint *endpoint)
} else {
val = u32_encode_bits(IPA_BASIC, MODE_FMASK);
}
- /* Other bitfields unspecified (and 0) */
+ /* All other bits unspecified (and 0) */
iowrite32(val, endpoint->ipa->reg_virt + offset);
}
@@ -1297,6 +1297,7 @@ static void ipa_endpoint_program(struct ipa_endpoint *endpoint)
ipa_endpoint_init_aggr(endpoint);
ipa_endpoint_init_deaggr(endpoint);
ipa_endpoint_init_seq(endpoint);
+ ipa_endpoint_init_mode(endpoint);
} else {
if (endpoint->ipa->version == IPA_VERSION_3_5_1)
(void)ipa_endpoint_program_suspend(endpoint, false);
@@ -1306,7 +1307,6 @@ static void ipa_endpoint_program(struct ipa_endpoint *endpoint)
}
ipa_endpoint_init_cfg(endpoint);
ipa_endpoint_init_hdr(endpoint);
- ipa_endpoint_init_mode(endpoint);
ipa_endpoint_status(endpoint);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 4/5] net: ipa: clarify endpoint register macro constraints
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
` (2 preceding siblings ...)
2020-06-30 13:33 ` [PATCH net-next v2 3/5] net: ipa: mode register is TX only Alex Elder
@ 2020-06-30 13:33 ` Alex Elder
2020-06-30 13:33 ` [PATCH net-next v2 5/5] net: ipa: HOL_BLOCK_EN_FMASK is a 1-bit mask Alex Elder
2020-07-01 22:30 ` [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates David Miller
5 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-06-30 13:33 UTC (permalink / raw)
To: davem, kuba
Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
A handful of registers are valid only for RX endpoints, and some
others are valid only for TX endpoints. For these endpoints, add
a comment above their defined offset macro that indicates the
endpoints to which they apply.
Extend the endpoint parameter naming convention as well, to make
these constraints more explicit.
Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: No change from version 1.
drivers/net/ipa/ipa_reg.h | 43 +++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h
index 0a688d8c1d7c..10e4ac9ead68 100644
--- a/drivers/net/ipa/ipa_reg.h
+++ b/drivers/net/ipa/ipa_reg.h
@@ -32,10 +32,12 @@ struct ipa;
* parameter is supplied to the offset macro. The "ee" value is a member of
* the gsi_ee enumerated type.
*
- * The offset of a register dependent on endpoint id is computed by a macro
- * that is supplied a parameter "ep". The "ep" value is assumed to be less
- * than the maximum endpoint value for the current hardware, and that will
- * not exceed IPA_ENDPOINT_MAX.
+ * The offset of a register dependent on endpoint ID is computed by a macro
+ * that is supplied a parameter "ep", "txep", or "rxep". A register with an
+ * "ep" parameter is valid for any endpoint; a register with a "txep" or
+ * "rxep" parameter is valid only for TX or RX endpoints, respectively. The
+ * "*ep" value is assumed to be less than the maximum valid endpoint ID
+ * for the current hardware, and that will not exceed IPA_ENDPOINT_MAX.
*
* The offset of registers related to filter and route tables is computed
* by a macro that is supplied a parameter "er". The "er" represents an
@@ -293,11 +295,13 @@ static inline u32 ipa_reg_idle_indication_cfg_offset(enum ipa_version version)
#define HDR_TOTAL_LEN_OR_PAD_OFFSET_FMASK GENMASK(9, 4)
#define HDR_PAD_TO_ALIGNMENT_FMASK GENMASK(13, 10)
-#define IPA_REG_ENDP_INIT_HDR_METADATA_MASK_N_OFFSET(ep) \
- (0x00000818 + 0x0070 * (ep))
+/* Valid only for RX (IPA producer) endpoints */
+#define IPA_REG_ENDP_INIT_HDR_METADATA_MASK_N_OFFSET(rxep) \
+ (0x00000818 + 0x0070 * (rxep))
-#define IPA_REG_ENDP_INIT_MODE_N_OFFSET(ep) \
- (0x00000820 + 0x0070 * (ep))
+/* Valid only for TX (IPA consumer) endpoints */
+#define IPA_REG_ENDP_INIT_MODE_N_OFFSET(txep) \
+ (0x00000820 + 0x0070 * (txep))
#define MODE_FMASK GENMASK(2, 0)
#define DEST_PIPE_INDEX_FMASK GENMASK(8, 4)
#define BYTE_THRESHOLD_FMASK GENMASK(27, 12)
@@ -316,19 +320,21 @@ static inline u32 ipa_reg_idle_indication_cfg_offset(enum ipa_version version)
#define AGGR_FORCE_CLOSE_FMASK GENMASK(22, 22)
#define AGGR_HARD_BYTE_LIMIT_ENABLE_FMASK GENMASK(24, 24)
-#define IPA_REG_ENDP_INIT_HOL_BLOCK_EN_N_OFFSET(ep) \
- (0x0000082c + 0x0070 * (ep))
+/* Valid only for RX (IPA producer) endpoints */
+#define IPA_REG_ENDP_INIT_HOL_BLOCK_EN_N_OFFSET(rxep) \
+ (0x0000082c + 0x0070 * (rxep))
#define HOL_BLOCK_EN_FMASK GENMASK(0, 0)
-/* The next register is valid only for RX (IPA producer) endpoints */
-#define IPA_REG_ENDP_INIT_HOL_BLOCK_TIMER_N_OFFSET(ep) \
- (0x00000830 + 0x0070 * (ep))
+/* Valid only for RX (IPA producer) endpoints */
+#define IPA_REG_ENDP_INIT_HOL_BLOCK_TIMER_N_OFFSET(rxep) \
+ (0x00000830 + 0x0070 * (rxep))
/* The next fields are present for IPA v4.2 only */
#define BASE_VALUE_FMASK GENMASK(4, 0)
#define SCALE_FMASK GENMASK(12, 8)
-#define IPA_REG_ENDP_INIT_DEAGGR_N_OFFSET(ep) \
- (0x00000834 + 0x0070 * (ep))
+/* Valid only for TX (IPA consumer) endpoints */
+#define IPA_REG_ENDP_INIT_DEAGGR_N_OFFSET(txep) \
+ (0x00000834 + 0x0070 * (txep))
#define DEAGGR_HDR_LEN_FMASK GENMASK(5, 0)
#define PACKET_OFFSET_VALID_FMASK GENMASK(7, 7)
#define PACKET_OFFSET_LOCATION_FMASK GENMASK(13, 8)
@@ -338,8 +344,9 @@ static inline u32 ipa_reg_idle_indication_cfg_offset(enum ipa_version version)
(0x00000838 + 0x0070 * (ep))
#define RSRC_GRP_FMASK GENMASK(1, 0)
-#define IPA_REG_ENDP_INIT_SEQ_N_OFFSET(ep) \
- (0x0000083c + 0x0070 * (ep))
+/* Valid only for TX (IPA consumer) endpoints */
+#define IPA_REG_ENDP_INIT_SEQ_N_OFFSET(txep) \
+ (0x0000083c + 0x0070 * (txep))
#define HPS_SEQ_TYPE_FMASK GENMASK(3, 0)
#define DPS_SEQ_TYPE_FMASK GENMASK(7, 4)
#define HPS_REP_SEQ_TYPE_FMASK GENMASK(11, 8)
@@ -353,7 +360,7 @@ static inline u32 ipa_reg_idle_indication_cfg_offset(enum ipa_version version)
/* The next field is present for IPA v4.0 and above */
#define STATUS_PKT_SUPPRESS_FMASK GENMASK(9, 9)
-/* "er" is either an endpoint id (for filters) or a route id (for routes) */
+/* "er" is either an endpoint ID (for filters) or a route ID (for routes) */
#define IPA_REG_ENDP_FILTER_ROUTER_HSH_CFG_N_OFFSET(er) \
(0x0000085c + 0x0070 * (er))
#define FILTER_HASH_MSK_SRC_ID_FMASK GENMASK(0, 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 5/5] net: ipa: HOL_BLOCK_EN_FMASK is a 1-bit mask
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
` (3 preceding siblings ...)
2020-06-30 13:33 ` [PATCH net-next v2 4/5] net: ipa: clarify endpoint register macro constraints Alex Elder
@ 2020-06-30 13:33 ` Alex Elder
2020-07-01 22:30 ` [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates David Miller
5 siblings, 0 replies; 7+ messages in thread
From: Alex Elder @ 2020-06-30 13:33 UTC (permalink / raw)
To: davem, kuba
Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
The convention throughout the IPA driver is to directly use
single-bit field mask values, rather than using (for example)
u32_encode_bits() to set or clear them.
Fix the one place that doesn't follow that convention, which sets
HOL_BLOCK_EN_FMASK in ipa_endpoint_init_hol_block_enable().
Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: No change from version 1.
drivers/net/ipa/ipa_endpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 566ff6a09e65..0332dcbcaaae 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -671,7 +671,7 @@ ipa_endpoint_init_hol_block_enable(struct ipa_endpoint *endpoint, bool enable)
u32 offset;
u32 val;
- val = u32_encode_bits(enable ? 1 : 0, HOL_BLOCK_EN_FMASK);
+ val = enable ? HOL_BLOCK_EN_FMASK : 0;
offset = IPA_REG_ENDP_INIT_HOL_BLOCK_EN_N_OFFSET(endpoint_id);
iowrite32(val, endpoint->ipa->reg_virt + offset);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates
2020-06-30 13:32 [PATCH net-next v2 0/5] net: ipa: endpoint configuration updates Alex Elder
` (4 preceding siblings ...)
2020-06-30 13:33 ` [PATCH net-next v2 5/5] net: ipa: HOL_BLOCK_EN_FMASK is a 1-bit mask Alex Elder
@ 2020-07-01 22:30 ` David Miller
5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2020-07-01 22:30 UTC (permalink / raw)
To: elder
Cc: kuba, evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
From: Alex Elder <elder@linaro.org>
Date: Tue, 30 Jun 2020 08:32:59 -0500
> This series updates code that configures IPA endpoints. The changes
> made mainly affect access to registers that are valid only for RX, or
> only for TX endpoints.
>
> The first three patches avoid writing endpoint registers if they are
> not defined to be valid. The fourth patch slightly modifies the
> parameters for the offset macros used for these endpoint registers,
> to make it explicit when only some endpoints are valid.
>
> The last patch just tweaks one line of code so it uses a convention
> used everywhere else in the driver.
>
> Version 2 of this series eliminates some of the "assert()" comments
> that Jakub inquired about. The ones removed will actually go away
> in an upcoming (not-yet-posted) patch series anyway.
Series applied, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread