Message ID | 20210211175015.200772-2-angelogioacchino.delregno@somainline.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Add support for IPA v3.1, GSI v1.0, MSM8998 IPA | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Guessed tree name to be net-next |
netdev/subject_prefix | success | Link |
netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/kdoc | fail | Errors and warnings before: 29 this patch: 30 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 110 lines checked |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/header_inline | success | Link |
netdev/stable | success | Stable not CCed |
On Thu, 11 Feb 2021 18:50:09 +0100 AngeloGioacchino Del Regno wrote: > In preparation for adding support for the MSM8998 SoC's IPA, > add the necessary bits for IPA version 3.1 featuring GSI 1.0, > found on at least MSM8998. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> please check your patches with ./scripts/kernel-doc for warnings: drivers/net/ipa/ipa_version.h:24: warning: Enum value 'IPA_VERSION_3_1' not described in enum 'ipa_version'
On 2/11/21 11:50 AM, AngeloGioacchino Del Regno wrote: > In preparation for adding support for the MSM8998 SoC's IPA, > add the necessary bits for IPA version 3.1 featuring GSI 1.0, > found on at least MSM8998. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> Overall, this looks good. As I mentioned, I've implemented a very similar set of changes in my private development tree. It's part of a much larger set of changes intended to allow many IPA versions to be supported. A few minor comments, below. -Alex > --- > drivers/net/ipa/gsi.c | 8 ++++---- > drivers/net/ipa/ipa_endpoint.c | 17 +++++++++-------- > drivers/net/ipa/ipa_main.c | 8 ++++++-- > drivers/net/ipa/ipa_reg.h | 3 +++ > drivers/net/ipa/ipa_version.h | 1 + > 5 files changed, 23 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c > index 14d9a791924b..6315336b3ca8 100644 > --- a/drivers/net/ipa/gsi.c > +++ b/drivers/net/ipa/gsi.c > @@ -794,14 +794,14 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) > > /* Max prefetch is 1 segment (do not set MAX_PREFETCH_FMASK) */ > > - /* We enable the doorbell engine for IPA v3.5.1 */ > - if (gsi->version == IPA_VERSION_3_5_1 && doorbell) > + /* We enable the doorbell engine for IPA v3.x */ > + if (gsi->version < IPA_VERSION_4_0 && doorbell) My version: if (gsi->version < IPA_VERSION_4_0 && doorbell) So... You're doing the right thing. Almost all changes I made like this were identical to yours; others were (I think all) equivalent. > val |= USE_DB_ENG_FMASK; > > /* v4.0 introduces an escape buffer for prefetch. We use it > * on all but the AP command channel. > */ > - if (gsi->version != IPA_VERSION_3_5_1 && !channel->command) { > + if (gsi->version >= IPA_VERSION_4_0 && !channel->command) { > /* If not otherwise set, prefetch buffers are used */ > if (gsi->version < IPA_VERSION_4_5) > val |= USE_ESCAPE_BUF_ONLY_FMASK; . . . > diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c > index 84bb8ae92725..be191993fbec 100644 > --- a/drivers/net/ipa/ipa_main.c > +++ b/drivers/net/ipa/ipa_main.c . . . > @@ -276,6 +276,7 @@ static void ipa_hardware_config_qsb(struct ipa *ipa) > > max1 = 12; > switch (version) { > + case IPA_VERSION_3_1: I do this a little differently now. These values will be found in the "ipa_data" file for the platform. Also I think you'd need different values for IPA v3.1 than for IPA v3.5.1. > case IPA_VERSION_3_5_1: > max0 = 8; > break; > @@ -404,6 +405,9 @@ static void ipa_hardware_config(struct ipa *ipa) > /* Enable open global clocks (not needed for IPA v4.5) */ > val = GLOBAL_FMASK; > val |= GLOBAL_2X_CLK_FMASK; > + if (version == IPA_VERSION_3_1) > + val |= MISC_FMASK; I see this being set for a workaround or IPA v3.1 in the msm-4.4 tree, but the other two flags aren't set in that case. So this might not be quite right. > + > iowrite32(val, ipa->reg_virt + IPA_REG_CLKON_CFG_OFFSET); > > /* Disable PA mask to allow HOLB drop */ . . .
On 2/11/21 11:50 AM, AngeloGioacchino Del Regno wrote: > In preparation for adding support for the MSM8998 SoC's IPA, > add the necessary bits for IPA version 3.1 featuring GSI 1.0, > found on at least MSM8998. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> Almost everything in this patch is the same as what is now in the upstream code. There were minor differences, such as using version < IPA_VERSION_4_0 rather than version <= IPA_VERSION_3_5_1, and some comment changes. In this patch there is one thing you had that I did not, and I show it below. > --- > drivers/net/ipa/gsi.c | 8 ++++---- > drivers/net/ipa/ipa_endpoint.c | 17 +++++++++-------- > drivers/net/ipa/ipa_main.c | 8 ++++++-- > drivers/net/ipa/ipa_reg.h | 3 +++ > drivers/net/ipa/ipa_version.h | 1 + > 5 files changed, 23 insertions(+), 14 deletions(-) . . . > diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c > index 84bb8ae92725..be191993fbec 100644 > --- a/drivers/net/ipa/ipa_main.c > +++ b/drivers/net/ipa/ipa_main.c . . . > @@ -404,6 +405,9 @@ static void ipa_hardware_config(struct ipa *ipa) > /* Enable open global clocks (not needed for IPA v4.5) */ > val = GLOBAL_FMASK; > val |= GLOBAL_2X_CLK_FMASK; > + if (version == IPA_VERSION_3_1) > + val |= MISC_FMASK; > + This block of code is conditional on: if (version >= IPA_VERSION_4_0 && version < IPA_VERSION_4_5) Therefore adding the MISC_FMASK here won't ever happen. (It's possible the code you were working with had some different logic.) I do, however, see in the msm-4.4 code that IPA v3.1 enables the CLKON_CFG CGC_OPEN_MISC bit (only), stating that it's a workaround. So I will update the logic and add this. -Alex > iowrite32(val, ipa->reg_virt + IPA_REG_CLKON_CFG_OFFSET); > > /* Disable PA mask to allow HOLB drop */ . . .
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 14d9a791924b..6315336b3ca8 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -794,14 +794,14 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) /* Max prefetch is 1 segment (do not set MAX_PREFETCH_FMASK) */ - /* We enable the doorbell engine for IPA v3.5.1 */ - if (gsi->version == IPA_VERSION_3_5_1 && doorbell) + /* We enable the doorbell engine for IPA v3.x */ + if (gsi->version < IPA_VERSION_4_0 && doorbell) val |= USE_DB_ENG_FMASK; /* v4.0 introduces an escape buffer for prefetch. We use it * on all but the AP command channel. */ - if (gsi->version != IPA_VERSION_3_5_1 && !channel->command) { + if (gsi->version >= IPA_VERSION_4_0 && !channel->command) { /* If not otherwise set, prefetch buffers are used */ if (gsi->version < IPA_VERSION_4_5) val |= USE_ESCAPE_BUF_ONLY_FMASK; @@ -899,7 +899,7 @@ void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell) gsi_channel_reset_command(channel); /* Due to a hardware quirk we may need to reset RX channels twice. */ - if (gsi->version == IPA_VERSION_3_5_1 && !channel->toward_ipa) + if (gsi->version <= IPA_VERSION_3_5_1 && !channel->toward_ipa) gsi_channel_reset_command(channel); gsi_channel_program(channel, doorbell); diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 9f4be9812a1f..06d8aa34276e 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -347,7 +347,7 @@ ipa_endpoint_program_suspend(struct ipa_endpoint *endpoint, bool enable) { bool suspended; - if (endpoint->ipa->version != IPA_VERSION_3_5_1) + if (endpoint->ipa->version >= IPA_VERSION_4_0) return enable; /* For IPA v4.0+, no change made */ /* assert(!endpoint->toward_ipa); */ @@ -1422,11 +1422,12 @@ static void ipa_endpoint_reset(struct ipa_endpoint *endpoint) bool special; int ret = 0; - /* On IPA v3.5.1, if an RX endpoint is reset while aggregation - * is active, we need to handle things specially to recover. + /* On IPA v3.5.1 and older, if an RX endpoint is reset while + * aggregation is active, we need to handle things in a special + * way in order to recover. * All other cases just need to reset the underlying GSI channel. */ - special = ipa->version == IPA_VERSION_3_5_1 && + special = ipa->version <= IPA_VERSION_3_5_1 && !endpoint->toward_ipa && endpoint->data->aggregation; if (special && ipa_endpoint_aggr_active(endpoint)) @@ -1525,8 +1526,8 @@ void ipa_endpoint_suspend_one(struct ipa_endpoint *endpoint) (void)ipa_endpoint_program_suspend(endpoint, true); } - /* IPA v3.5.1 doesn't use channel stop for suspend */ - stop_channel = endpoint->ipa->version != IPA_VERSION_3_5_1; + /* IPA v3.x doesn't use channel stop for suspend */ + stop_channel = endpoint->ipa->version >= IPA_VERSION_4_0; ret = gsi_channel_suspend(gsi, endpoint->channel_id, stop_channel); if (ret) dev_err(dev, "error %d suspending channel %u\n", ret, @@ -1546,8 +1547,8 @@ void ipa_endpoint_resume_one(struct ipa_endpoint *endpoint) if (!endpoint->toward_ipa) (void)ipa_endpoint_program_suspend(endpoint, false); - /* IPA v3.5.1 doesn't use channel start for resume */ - start_channel = endpoint->ipa->version != IPA_VERSION_3_5_1; + /* IPA v3.x doesn't use channel start for resume */ + start_channel = endpoint->ipa->version >= IPA_VERSION_4_0; ret = gsi_channel_resume(gsi, endpoint->channel_id, start_channel); if (ret) dev_err(dev, "error %d resuming channel %u\n", ret, diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 84bb8ae92725..be191993fbec 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -228,8 +228,8 @@ static void ipa_hardware_config_comp(struct ipa *ipa) { u32 val; - /* Nothing to configure for IPA v3.5.1 */ - if (ipa->version == IPA_VERSION_3_5_1) + /* Nothing to configure for IPA v3.5.1 and below */ + if (ipa->version <= IPA_VERSION_3_5_1) return; val = ioread32(ipa->reg_virt + IPA_REG_COMP_CFG_OFFSET); @@ -276,6 +276,7 @@ static void ipa_hardware_config_qsb(struct ipa *ipa) max1 = 12; switch (version) { + case IPA_VERSION_3_1: case IPA_VERSION_3_5_1: max0 = 8; break; @@ -404,6 +405,9 @@ static void ipa_hardware_config(struct ipa *ipa) /* Enable open global clocks (not needed for IPA v4.5) */ val = GLOBAL_FMASK; val |= GLOBAL_2X_CLK_FMASK; + if (version == IPA_VERSION_3_1) + val |= MISC_FMASK; + iowrite32(val, ipa->reg_virt + IPA_REG_CLKON_CFG_OFFSET); /* Disable PA mask to allow HOLB drop */ diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index e6b0827a244e..9b9d6e0397bb 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -199,6 +199,9 @@ static inline u32 ipa_reg_state_aggr_active_offset(enum ipa_version version) /* Backward compatibility register value to use for each version */ static inline u32 ipa_reg_bcr_val(enum ipa_version version) { + if (version == IPA_VERSION_3_1) + return BCR_CMDQ_L_LACK_ONE_ENTRY_FMASK; + if (version == IPA_VERSION_3_5_1) return BCR_CMDQ_L_LACK_ONE_ENTRY_FMASK | BCR_TX_NOT_USING_BRESP_FMASK | diff --git a/drivers/net/ipa/ipa_version.h b/drivers/net/ipa/ipa_version.h index 2944e2a89023..47f68975d576 100644 --- a/drivers/net/ipa/ipa_version.h +++ b/drivers/net/ipa/ipa_version.h @@ -14,6 +14,7 @@ * it where it's needed. */ enum ipa_version { + IPA_VERSION_3_1, /* GSI version 1.0 */ IPA_VERSION_3_5_1, /* GSI version 1.3.0 */ IPA_VERSION_4_0, /* GSI version 2.0 */ IPA_VERSION_4_1, /* GSI version 2.1 */
In preparation for adding support for the MSM8998 SoC's IPA, add the necessary bits for IPA version 3.1 featuring GSI 1.0, found on at least MSM8998. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> --- drivers/net/ipa/gsi.c | 8 ++++---- drivers/net/ipa/ipa_endpoint.c | 17 +++++++++-------- drivers/net/ipa/ipa_main.c | 8 ++++++-- drivers/net/ipa/ipa_reg.h | 3 +++ drivers/net/ipa/ipa_version.h | 1 + 5 files changed, 23 insertions(+), 14 deletions(-)