[PATCH net-next 2/2] net: ipa: optionally define firmware name via DT
IPA initialization includes loading some firmware. This step is done either by the modem or by the AP under Trust Zone. If the AP loads firmware, the name of the firmware file is currently hard-coded ("ipa_fws.mdt"). Add the ability to specify the relative path of the firmware file to use in a property in the Device Tree IPA node. If the property is not found (or if any other error occurs attempting to get it), fall back to using a default relative path. Use the "old" fixed name as the default. Rename the symbol that represents this default to emphasize its purpose. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index aad915e2ce523..9915603ed10ba 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -67,7 +67,7 @@ */ /* The name of the GSI firmware file relative to /lib/firmware */ -#define IPA_FWS_PATH "ipa_fws.mdt" +#define IPA_FW_PATH_DEFAULT"ipa_fws.mdt" #define IPA_PAS_ID 15 /* Shift of 19.2 MHz timestamp to achieve lower resolution timestamps */ @@ -517,6 +517,7 @@ static int ipa_firmware_load(struct device *dev) struct device_node *node; struct resource res; phys_addr_t phys; + const char *path; ssize_t size; void *virt; int ret; @@ -534,9 +535,17 @@ static int ipa_firmware_load(struct device *dev) return ret; } - ret = request_firmware(&fw, IPA_FWS_PATH, dev); + /* Use name from DTB if specified; use default for *any* error */ + ret = of_property_read_string(dev->of_node, "firmware-name", &path); if (ret) { - dev_err(dev, "error %d requesting \"%s\"\n", ret, IPA_FWS_PATH); + dev_dbg(dev, "error %d getting \"firmware-name\" resource\n", + ret); + path = IPA_FW_PATH_DEFAULT; + } + + ret = request_firmware(&fw, path, dev); + if (ret) { + dev_err(dev, "error %d requesting \"%s\"\n", ret, path); return ret; } @@ -549,13 +558,11 @@ static int ipa_firmware_load(struct device *dev) goto out_release_firmware; } - ret = qcom_mdt_load(dev, fw, IPA_FWS_PATH, IPA_PAS_ID, - virt, phys, size, NULL); + ret = qcom_mdt_load(dev, fw, path, IPA_PAS_ID, virt, phys, size, NULL); if (ret) - dev_err(dev, "error %d loading \"%s\"\n", ret, IPA_FWS_PATH); + dev_err(dev, "error %d loading \"%s\"\n", ret, path); else if ((ret = qcom_scm_pas_auth_and_reset(IPA_PAS_ID))) - dev_err(dev, "error %d authenticating \"%s\"\n", ret, - IPA_FWS_PATH); + dev_err(dev, "error %d authenticating \"%s\"\n", ret, path); memunmap(virt); out_release_firmware: -- 2.27.0
[PATCH net-next 1/2] dt-bindings: net: qcom,ipa: add firmware-name property
Add a new optional firmware-name property to the IPA DT node. It is used only if the modem is not doing early initialization (i.e., if the modem-init property is not present). Its value is the name of the firmware file to use; if it's not specified, a default name ("ipa_fws.mdt") is used. Signed-off-by: Alex Elder --- .../devicetree/bindings/net/qcom,ipa.yaml | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml index da5212e693e91..7443490d4cc6d 100644 --- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml @@ -125,6 +125,14 @@ properties: the firmware passed to Trust Zone for authentication. Required when Trust Zone (not the modem) performs early initialization. + firmware-name: +$ref: /schemas/types.yaml#/definitions/string +description: + If present, name (or relative path) of the file within the + firmware search path containing the firmware image used when + initializing IPA hardware. Optional, and only used when + Trust Zone performs early initialization. + required: - compatible - iommus @@ -134,12 +142,23 @@ required: - interconnects - qcom,smem-states +# Either modem-init is present, or memory-region must be present. oneOf: - required: - modem-init - required: - memory-region +# If memory-region is present, firmware-name may optionally be present. +# But if modem-init is present, firmware-name must not be present. +if: + required: +- modem-init +then: + not: +required: + - firmware-name + additionalProperties: false examples: -- 2.27.0
[PATCH net-next 0/2] net: ipa: allow different firmware names
Add the ability to define a "firmware-name" property in the IPA DT node, specifying an alternate name to use for the firmware file. Used only if the AP (Trust Zone) does early IPA initialization. -Alex Alex Elder (2): dt-bindings: net: qcom,ipa: add firmware-name property net: ipa: optionally define firmware name via DT .../devicetree/bindings/net/qcom,ipa.yaml | 19 +++ drivers/net/ipa/ipa_main.c| 23 --- 2 files changed, 34 insertions(+), 8 deletions(-) -- 2.27.0
Re: [PATCH] drivers: ipa: Fix missing IRQF_ONESHOT as only threaded handler
On 4/15/21 10:40 PM, zhuguangqin...@gmail.com wrote: From: Guangqing Zhu This is not required here. -Alex https://lore.kernel.org/netdev/d57e0a43-4d87-93cf-471c-c8185ea85...@ieee.org/ Coccinelle noticed: drivers/net/ipa/ipa_smp2p.c:186:7-27: ERROR: Threaded IRQ with no primary handler requested without IRQF_ONESHOT Signed-off-by: Guangqing Zhu --- drivers/net/ipa/ipa_smp2p.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c index a5f7a79a1923..74e04427a711 100644 --- a/drivers/net/ipa/ipa_smp2p.c +++ b/drivers/net/ipa/ipa_smp2p.c @@ -183,7 +183,8 @@ static int ipa_smp2p_irq_init(struct ipa_smp2p *smp2p, const char *name, } irq = ret; - ret = request_threaded_irq(irq, NULL, handler, 0, name, smp2p); + ret = request_threaded_irq(irq, NULL, handler, IRQF_ONESHOT, + name, smp2p); if (ret) { dev_err(dev, "error %d requesting \"%s\" IRQ\n", ret, name); return ret;
[PATCH net-next 1/2] dt-bindings: net: qcom,ipa: add support for SM8350
Add support for "qcom,sm8350-ipa", which uses IPA v4.9. Use "enum" rather than "oneOf/const ..." to specify compatible strings, as suggested by Rob Herring. Signed-off-by: Alex Elder --- Documentation/devicetree/bindings/net/qcom,ipa.yaml | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml index 2645a02cf19bf..da5212e693e91 100644 --- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml @@ -43,11 +43,12 @@ description: properties: compatible: -oneOf: - - const: "qcom,sc7180-ipa" - - const: "qcom,sc7280-ipa" - - const: "qcom,sdm845-ipa" - - const: "qcom,sdx55-ipa" +enum: + - qcom,sc7180-ipa + - qcom,sc7280-ipa + - qcom,sdm845-ipa + - qcom,sdx55-ipa + - qcom,sm8350-ipa reg: items: -- 2.27.0
[PATCH net-next 2/2] net: ipa: add IPA v4.9 configuration data
Add support for the SM8350 SoC, which includes IPA version 4.9. Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile| 3 +- drivers/net/ipa/ipa_data-v4.9.c | 430 drivers/net/ipa/ipa_data.h | 1 + drivers/net/ipa/ipa_main.c | 4 + 4 files changed, 437 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ipa/ipa_data-v4.9.c diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index 8c0ac87903549..1efe1a88104b3 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -10,4 +10,5 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_resource.o ipa_qmi.o ipa_qmi_msg.o ipa-y += ipa_data-v3.5.1.o ipa_data-v4.2.o \ - ipa_data-v4.5.o ipa_data-v4.11.o + ipa_data-v4.5.o ipa_data-v4.9.o \ + ipa_data-v4.11.o diff --git a/drivers/net/ipa/ipa_data-v4.9.c b/drivers/net/ipa/ipa_data-v4.9.c new file mode 100644 index 0..e41be790f45e5 --- /dev/null +++ b/drivers/net/ipa/ipa_data-v4.9.c @@ -0,0 +1,430 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* Copyright (C) 2021 Linaro Ltd. */ + +#include + +#include "gsi.h" +#include "ipa_data.h" +#include "ipa_endpoint.h" +#include "ipa_mem.h" + +/** enum ipa_resource_type - IPA resource types for an SoC having IPA v4.9 */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, + IPA_RESOURCE_TYPE_DST_DPS_DMARS, +}; + +/* Resource groups used for an SoC having IPA v4.9 */ +enum ipa_rsrc_group_id { + /* Source resource group identifiers */ + IPA_RSRC_GROUP_SRC_UL_DL= 0, + IPA_RSRC_GROUP_SRC_DMA, + IPA_RSRC_GROUP_SRC_UC_RX_Q, + IPA_RSRC_GROUP_SRC_COUNT, /* Last in set; not a source group */ + + /* Destination resource group identifiers */ + IPA_RSRC_GROUP_DST_UL_DL_DPL= 0, + IPA_RSRC_GROUP_DST_DMA, + IPA_RSRC_GROUP_DST_UC, + IPA_RSRC_GROUP_DST_DRB_IP, + IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ +}; + +/* QSB configuration data for an SoC having IPA v4.9 */ +static const struct ipa_qsb_data ipa_qsb_data[] = { + [IPA_QSB_MASTER_DDR] = { + .max_writes = 8, + .max_reads = 0,/* no limit (hardware max) */ + .max_reads_beats= 120, + }, +}; + +/* Endpoint configuration data for an SoC having IPA v4.9 */ +static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { + [IPA_ENDPOINT_AP_COMMAND_TX] = { + .ee_id = GSI_EE_AP, + .channel_id = 6, + .endpoint_id= 7, + .toward_ipa = true, + .channel = { + .tre_count = 256, + .event_count= 256, + .tlv_count = 20, + }, + .endpoint = { + .config = { + .resource_group = IPA_RSRC_GROUP_SRC_UL_DL, + .dma_mode = true, + .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, + .tx = { + .seq_type = IPA_SEQ_DMA, + }, + }, + }, + }, + [IPA_ENDPOINT_AP_LAN_RX] = { + .ee_id = GSI_EE_AP, + .channel_id = 7, + .endpoint_id= 11, + .toward_ipa = false, + .channel = { + .tre_count = 256, + .event_count= 256, + .tlv_count = 9, + }, + .endpoint = { + .config = { + .resource_group = IPA_RSRC_GROUP_DST_UL_DL_DPL, + .aggregation= true, + .status_enable = true, + .rx = { + .pad_align = ilog2(sizeof(u32)), + }, + }, + }, + }, + [IPA_ENDPOINT_AP_MODEM_TX] = { + .ee_id = GSI_EE_AP, + .channel_id = 2, + .endpoint_id= 2, + .toward_
[PATCH net-next 0/2] net: ipa: add support for the SM8350 SoC
This small series adds IPA driver support for the Qualcomm SM8350 SoC, which implements IPA v4.9. The first patch updates the DT binding, and depends on a previous patch that has already been accepted into net-next. The second just defines the IPA v4.9 configuration data file. (Device Tree files to support this SoC will be sent separately and will go through the Qualcomm tree.) -Alex Alex Elder (2): dt-bindings: net: qcom,ipa: add support for SM8350 net: ipa: add IPA v4.9 configuration data .../devicetree/bindings/net/qcom,ipa.yaml | 11 +- drivers/net/ipa/Makefile | 3 +- drivers/net/ipa/ipa_data-v4.9.c | 430 ++ drivers/net/ipa/ipa_data.h| 1 + drivers/net/ipa/ipa_main.c| 4 + 5 files changed, 443 insertions(+), 6 deletions(-) create mode 100644 drivers/net/ipa/ipa_data-v4.9.c -- 2.27.0
Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an error
On 4/12/21 2:26 AM, Leon Romanovsky wrote: On Sun, Apr 11, 2021 at 08:42:15AM -0500, Alex Elder wrote: On 4/11/21 8:28 AM, Leon Romanovsky wrote: I think *not* checking an available return value is questionable practice. I'd really rather have a build option for a "__need_not_check" tag and have "must_check" be the default. __need_not_check == void ??? I'm not sure I understand your statement here, but... We are talking about the same thing. My point was that __need_not_check is actually void. The API author was supposed to declare that by declaring that function doesn't return anything. No, we are not. Functions like strcpy() return a value, but that value is almost never checked. The returned value isn't an error, so there is no real need to check that return value. This is the kind of thing I'm talking about that might be tagged __need_not_check. A function that returns a value for no reason should be void, I agree with that. In the ipa_stop() case, the value *must* be returned because it serves as an ->ndo_stop() function and has to adhere to that function prototype. The point of the current patch was to simplify the code (defined privately in the current source file), given knowledge that it never returns an error. The compiler could ensure all calls to functions that return a value actually check the return value. And because I think that's the best practice, I'd like to be able to run such a check in my code. But there are always exceptions, and that would be the purpose of a __need_not_check tag. I don't think this is worthy of any more discussion. -Alex
Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an error
On 4/11/21 8:28 AM, Leon Romanovsky wrote: >> I think *not* checking an available return value is questionable >> practice. I'd really rather have a build option for a >> "__need_not_check" tag and have "must_check" be the default. > __need_not_check == void ??? I'm not sure I understand your statement here, but... My point is, I'd rather have things like printk() and strscpy() be marked with (an imaginary) __need_not_check, than the way things are, with only certain functions being marked __must_check. In my view, if a function returns a value, all callers of that function ought to be checking it. If the return value is not necessary it should be a void function if possible. I don't expect the world to change, but I just think the default should be "must check" rather than "check optional". -Alex
Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an error
On 4/11/21 1:34 AM, Leon Romanovsky wrote: > On Fri, Apr 09, 2021 at 01:07:19PM -0500, Alex Elder wrote: >> In ipa_modem_stop(), if the modem netdev pointer is non-null we call >> ipa_stop(). We check for an error and if one is returned we handle >> it. But ipa_stop() never returns an error, so this extra handling >> is unnecessary. Simplify the code in ipa_modem_stop() based on the >> knowledge no error handling is needed at this spot. >> >> Signed-off-by: Alex Elder >> --- >> drivers/net/ipa/ipa_modem.c | 18 -- >> 1 file changed, 4 insertions(+), 14 deletions(-) > > <...> > >> +/* Stop the queue and disable the endpoints if it's open */ >> if (netdev) { >> -/* Stop the queue and disable the endpoints if it's open */ >> -ret = ipa_stop(netdev); >> -if (ret) >> -goto out_set_state; >> - >> +(void)ipa_stop(netdev); > > This void casting is not needed here and in more general case sometimes > even be seen as a mistake, for example if the returned attribute declared > as __must_check. I accept your point but I feel like it's sort of a 50/50 thing. I think *not* checking an available return value is questionable practice. I'd really rather have a build option for a "__need_not_check" tag and have "must_check" be the default. The void cast here says "I know this returns a result, but I am intentionally not checking it." If it had been __must_check I would certainly have checked it. That being said, I don't really care that much, so I'll plan to post version 2, which will drop this cast (I'll probably add a comment though). Thanks. -Alex > > Thanks >
[PATCH net-next 4/4] net: ipa: add IPA v4.11 configuration data
Add support for the SC7280 SoC, which includes IPA version 4.11. Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/ipa_data-v4.11.c | 382 +++ drivers/net/ipa/ipa_data.h | 1 + drivers/net/ipa/ipa_main.c | 4 + 4 files changed, 388 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ipa/ipa_data-v4.11.c diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index ccc4924881ac4..8c0ac87903549 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -10,4 +10,4 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_resource.o ipa_qmi.o ipa_qmi_msg.o ipa-y += ipa_data-v3.5.1.o ipa_data-v4.2.o \ - ipa_data-v4.5.o + ipa_data-v4.5.o ipa_data-v4.11.o diff --git a/drivers/net/ipa/ipa_data-v4.11.c b/drivers/net/ipa/ipa_data-v4.11.c new file mode 100644 index 0..05806ceae8b54 --- /dev/null +++ b/drivers/net/ipa/ipa_data-v4.11.c @@ -0,0 +1,382 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* Copyright (C) 2021 Linaro Ltd. */ + +#include + +#include "gsi.h" +#include "ipa_data.h" +#include "ipa_endpoint.h" +#include "ipa_mem.h" + +/** enum ipa_resource_type - IPA resource types for an SoC having IPA v4.11 */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, + IPA_RESOURCE_TYPE_DST_DPS_DMARS, +}; + +/* Resource groups used for an SoC having IPA v4.11 */ +enum ipa_rsrc_group_id { + /* Source resource group identifiers */ + IPA_RSRC_GROUP_SRC_UL_DL= 0, + IPA_RSRC_GROUP_SRC_UC_RX_Q, + IPA_RSRC_GROUP_SRC_UNUSED_2, + IPA_RSRC_GROUP_SRC_COUNT, /* Last in set; not a source group */ + + /* Destination resource group identifiers */ + IPA_RSRC_GROUP_DST_UL_DL_DPL= 0, + IPA_RSRC_GROUP_DST_UNUSED_1, + IPA_RSRC_GROUP_DST_DRB_IP, + IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ +}; + +/* QSB configuration data for an SoC having IPA v4.11 */ +static const struct ipa_qsb_data ipa_qsb_data[] = { + [IPA_QSB_MASTER_DDR] = { + .max_writes = 12, + .max_reads = 13, + .max_reads_beats= 120, + }, +}; + +/* Endpoint configuration data for an SoC having IPA v4.11 */ +static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { + [IPA_ENDPOINT_AP_COMMAND_TX] = { + .ee_id = GSI_EE_AP, + .channel_id = 5, + .endpoint_id= 7, + .toward_ipa = true, + .channel = { + .tre_count = 256, + .event_count= 256, + .tlv_count = 20, + }, + .endpoint = { + .config = { + .resource_group = IPA_RSRC_GROUP_SRC_UL_DL, + .dma_mode = true, + .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, + .tx = { + .seq_type = IPA_SEQ_DMA, + }, + }, + }, + }, + [IPA_ENDPOINT_AP_LAN_RX] = { + .ee_id = GSI_EE_AP, + .channel_id = 14, + .endpoint_id= 9, + .toward_ipa = false, + .channel = { + .tre_count = 256, + .event_count= 256, + .tlv_count = 9, + }, + .endpoint = { + .config = { + .resource_group = IPA_RSRC_GROUP_DST_UL_DL_DPL, + .aggregation= true, + .status_enable = true, + .rx = { + .pad_align = ilog2(sizeof(u32)), + }, + }, + }, + }, + [IPA_ENDPOINT_AP_MODEM_TX] = { + .ee_id = GSI_EE_AP, + .channel_id = 2, + .endpoint_id= 2, + .toward_ipa = true, + .channel = { + .tre_count = 512, +
[PATCH net-next 3/4] net: ipa: add IPA v4.5 configuration data
Add support for the SDX55 SoC, which includes IPA version 4.5. Starting with IPA v4.5, a few of the memory regions have a different number of "canary" values; update comments in the where the region identifers are defined to accurately reflect that. I'll note three differences in SDX55 versus the other two existing platforms (SDM845 and SC7180): - SDX55 uses a 32-bit Linux kernel - SDX55 has four interconnects rather than three - SDX55 uses IPA v4.5, which uses inline checksum offload Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile| 3 +- drivers/net/ipa/ipa_data-v4.5.c | 437 drivers/net/ipa/ipa_data.h | 1 + drivers/net/ipa/ipa_main.c | 4 + drivers/net/ipa/ipa_mem.h | 6 +- 5 files changed, 447 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ipa/ipa_data-v4.5.c diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index 6abd1db9fe330..ccc4924881ac4 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -9,4 +9,5 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_endpoint.o ipa_cmd.o ipa_modem.o \ ipa_resource.o ipa_qmi.o ipa_qmi_msg.o -ipa-y += ipa_data-v3.5.1.o ipa_data-v4.2.o +ipa-y += ipa_data-v3.5.1.o ipa_data-v4.2.o \ + ipa_data-v4.5.o diff --git a/drivers/net/ipa/ipa_data-v4.5.c b/drivers/net/ipa/ipa_data-v4.5.c new file mode 100644 index 0..5f67a3a909ee0 --- /dev/null +++ b/drivers/net/ipa/ipa_data-v4.5.c @@ -0,0 +1,437 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* Copyright (C) 2021 Linaro Ltd. */ + +#include + +#include "gsi.h" +#include "ipa_data.h" +#include "ipa_endpoint.h" +#include "ipa_mem.h" + +/** enum ipa_resource_type - IPA resource types for an SoC having IPA v4.5 */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, + IPA_RESOURCE_TYPE_DST_DPS_DMARS, +}; + +/* Resource groups used for an SoC having IPA v4.5 */ +enum ipa_rsrc_group_id { + /* Source resource group identifiers */ + IPA_RSRC_GROUP_SRC_UNUSED_0 = 0, + IPA_RSRC_GROUP_SRC_UL_DL, + IPA_RSRC_GROUP_SRC_UNUSED_2, + IPA_RSRC_GROUP_SRC_UNUSED_3, + IPA_RSRC_GROUP_SRC_UC_RX_Q, + IPA_RSRC_GROUP_SRC_COUNT, /* Last in set; not a source group */ + + /* Destination resource group identifiers */ + IPA_RSRC_GROUP_DST_UNUSED_0 = 0, + IPA_RSRC_GROUP_DST_UL_DL_DPL, + IPA_RSRC_GROUP_DST_UNUSED_2, + IPA_RSRC_GROUP_DST_UNUSED_3, + IPA_RSRC_GROUP_DST_UC, + IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ +}; + +/* QSB configuration data for an SoC having IPA v4.5 */ +static const struct ipa_qsb_data ipa_qsb_data[] = { + [IPA_QSB_MASTER_DDR] = { + .max_writes = 8, + .max_reads = 0,/* no limit (hardware max) */ + .max_reads_beats= 120, + }, + [IPA_QSB_MASTER_PCIE] = { + .max_writes = 8, + .max_reads = 12, + /* no outstanding read byte (beat) limit */ + }, +}; + +/* Endpoint configuration data for an SoC having IPA v4.5 */ +static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { + [IPA_ENDPOINT_AP_COMMAND_TX] = { + .ee_id = GSI_EE_AP, + .channel_id = 9, + .endpoint_id= 7, + .toward_ipa = true, + .channel = { + .tre_count = 256, + .event_count= 256, + .tlv_count = 20, + }, + .endpoint = { + .config = { + .resource_group = IPA_RSRC_GROUP_SRC_UL_DL, + .dma_mode = true, + .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, + .tx = { + .seq_type = IPA_SEQ_DMA, + }, + }, + }, + }, + [IPA_ENDPOINT_AP_LAN_RX] = { + .ee_id = GSI_EE_AP, + .channel_id = 10, + .endpoint_id= 16, + .toward_ipa = false, + .channel = { + .tre
[PATCH net-next 2/4] net: ipa: disable checksum offload for IPA v4.5+
Checksum offload for IPA v4.5+ is implemented differently, using "inline" offload (which uses a common header format for both upload and download offload). The IPA hardware must be programmed to enable MAP checksum offload, but the RMNet driver is responsible for interpreting checksum metadata supplied with messages. Currently, the RMNet driver does not support inline checksum offload. This support is imminent, but until it is available, do not allow newer versions of IPA to specify checksum offload for endpoints. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_endpoint.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index dd24179383c1c..5d8b8c68438a5 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -88,6 +88,11 @@ static bool ipa_endpoint_data_valid_one(struct ipa *ipa, u32 count, if (ipa_gsi_endpoint_data_empty(data)) return true; + /* IPA v4.5+ uses checksum offload, not yet supported by RMNet */ + if (ipa->version >= IPA_VERSION_4_5) + if (data->endpoint.config.checksum) + return false; + if (!data->toward_ipa) { if (data->endpoint.filter_support) { dev_err(dev, "filtering not supported for " @@ -230,6 +235,17 @@ static bool ipa_endpoint_data_valid(struct ipa *ipa, u32 count, static bool ipa_endpoint_data_valid(struct ipa *ipa, u32 count, const struct ipa_gsi_endpoint_data *data) { + const struct ipa_gsi_endpoint_data *dp = data; + enum ipa_endpoint_name name; + + if (ipa->version < IPA_VERSION_4_5) + return true; + + /* IPA v4.5+ uses checksum offload, not yet supported by RMNet */ + for (name = 0; name < count; name++, dp++) + if (data->endpoint.config.checksum) + return false; + return true; } -- 2.27.0
[PATCH net-next 1/4] dt-bindings: net: qcom,ipa: add some compatible strings
Add existing supported platform "qcom,sc7180-ipa" to the set of IPA compatible strings. Also add newly-supported "qcom,sdx55-ipa", "qcom,sc7280-ipa". Signed-off-by: Alex Elder --- Documentation/devicetree/bindings/net/qcom,ipa.yaml | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml index 8f86084bf12e9..2645a02cf19bf 100644 --- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml @@ -43,7 +43,11 @@ description: properties: compatible: -const: "qcom,sdm845-ipa" +oneOf: + - const: "qcom,sc7180-ipa" + - const: "qcom,sc7280-ipa" + - const: "qcom,sdm845-ipa" + - const: "qcom,sdx55-ipa" reg: items: -- 2.27.0
[PATCH net-next 0/4] net: ipa: support two more platforms
This series adds IPA support for two more Qualcomm SoCs. The first patch updates the DT binding to add compatible strings. The second temporarily disables checksum offload support for IPA version 4.5 and above. Changes are required to the RMNet driver to support the "inline" checksum offload used for IPA v4.5+, and once those are present this capability will be enabled for IPA. The third and fourth patches add configuration data for IPA versions 4.5 (used for the SDX55 SoC) and 4.11 (used for the SD7280 SoC). -Alex Alex Elder (4): dt-bindings: net: qcom,ipa: add some compatible strings net: ipa: disable checksum offload for IPA v4.5+ net: ipa: add IPA v4.5 configuration data net: ipa: add IPA v4.11 configuration data .../devicetree/bindings/net/qcom,ipa.yaml | 6 +- drivers/net/ipa/Makefile | 3 +- drivers/net/ipa/ipa_data-v4.11.c | 382 +++ drivers/net/ipa/ipa_data-v4.5.c | 437 ++ drivers/net/ipa/ipa_data.h| 2 + drivers/net/ipa/ipa_endpoint.c| 16 + drivers/net/ipa/ipa_main.c| 8 + drivers/net/ipa/ipa_mem.h | 6 +- 8 files changed, 855 insertions(+), 5 deletions(-) create mode 100644 drivers/net/ipa/ipa_data-v4.11.c create mode 100644 drivers/net/ipa/ipa_data-v4.5.c -- 2.27.0
[PATCH net-next 6/7] net: ipa: get rid of empty GSI functions
There are place holder functions in the GSI code that do nothing. Remove these, knowing we can add something back in their place if they're really needed someday. Some of these are inverse functions (such as teardown to match setup). Explicitly comment that there is no inverse in these cases. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 54 +-- 1 file changed, 6 insertions(+), 48 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 1c835b3e1a437..9f06663cef263 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -198,7 +198,7 @@ static void gsi_irq_type_disable(struct gsi *gsi, enum gsi_irq_type_id type_id) gsi_irq_type_update(gsi, gsi->type_enabled_bitmap & ~BIT(type_id)); } -/* Turn off all GSI interrupts initially */ +/* Turn off all GSI interrupts initially; there is no gsi_irq_teardown() */ static void gsi_irq_setup(struct gsi *gsi) { /* Disable all interrupt types */ @@ -217,12 +217,6 @@ static void gsi_irq_setup(struct gsi *gsi) iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET); } -/* Turn off all GSI interrupts when we're all done */ -static void gsi_irq_teardown(struct gsi *gsi) -{ - /* Nothing to do */ -} - /* Event ring commands are performed one at a time. Their completion * is signaled by the event ring control GSI interrupt type, which is * only enabled when we issue an event ring command. Only the event @@ -786,7 +780,7 @@ static void gsi_channel_trans_quiesce(struct gsi_channel *channel) } } -/* Program a channel for use */ +/* Program a channel for use; there is no gsi_channel_deprogram() */ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) { size_t size = channel->tre_ring.count * GSI_RING_ELEMENT_SIZE; @@ -874,11 +868,6 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) /* All done! */ } -static void gsi_channel_deprogram(struct gsi_channel *channel) -{ - /* Nothing to do */ -} - static int __gsi_channel_start(struct gsi_channel *channel, bool start) { struct gsi *gsi = channel->gsi; @@ -1623,18 +1612,6 @@ static u32 gsi_event_bitmap_init(u32 evt_ring_max) return event_bitmap; } -/* Setup function for event rings */ -static void gsi_evt_ring_setup(struct gsi *gsi) -{ - /* Nothing to do */ -} - -/* Inverse of gsi_evt_ring_setup() */ -static void gsi_evt_ring_teardown(struct gsi *gsi) -{ - /* Nothing to do */ -} - /* Setup function for a single channel */ static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id) { @@ -1684,7 +1661,6 @@ static void gsi_channel_teardown_one(struct gsi *gsi, u32 channel_id) netif_napi_del(&channel->napi); - gsi_channel_deprogram(channel); gsi_channel_de_alloc_command(gsi, channel_id); gsi_evt_ring_reset_command(gsi, evt_ring_id); gsi_evt_ring_de_alloc_command(gsi, evt_ring_id); @@ -1759,7 +1735,6 @@ static int gsi_channel_setup(struct gsi *gsi) u32 mask; int ret; - gsi_evt_ring_setup(gsi); gsi_irq_enable(gsi); mutex_lock(&gsi->mutex); @@ -1819,7 +1794,6 @@ static int gsi_channel_setup(struct gsi *gsi) mutex_unlock(&gsi->mutex); gsi_irq_disable(gsi); - gsi_evt_ring_teardown(gsi); return ret; } @@ -1848,7 +1822,6 @@ static void gsi_channel_teardown(struct gsi *gsi) mutex_unlock(&gsi->mutex); gsi_irq_disable(gsi); - gsi_evt_ring_teardown(gsi); } /* Setup function for GSI. GSI firmware must be loaded and initialized */ @@ -1856,7 +1829,6 @@ int gsi_setup(struct gsi *gsi) { struct device *dev = gsi->dev; u32 val; - int ret; /* Here is where we first touch the GSI hardware */ val = ioread32(gsi->virt + GSI_GSI_STATUS_OFFSET); @@ -1865,7 +1837,7 @@ int gsi_setup(struct gsi *gsi) return -EIO; } - gsi_irq_setup(gsi); + gsi_irq_setup(gsi); /* No matching teardown required */ val = ioread32(gsi->virt + GSI_GSI_HW_PARAM_2_OFFSET); @@ -1899,18 +1871,13 @@ int gsi_setup(struct gsi *gsi) /* Writing 1 indicates IRQ interrupts; 0 would be MSI */ iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET); - ret = gsi_channel_setup(gsi); - if (ret) - gsi_irq_teardown(gsi); - - return ret; + return gsi_channel_setup(gsi); } /* Inverse of gsi_setup() */ void gsi_teardown(struct gsi *gsi) { gsi_channel_teardown(gsi); - gsi_irq_teardown(gsi); } /* Initialize a channel's event ring */ @@ -1952,7 +1919,7 @@ static void gsi_channel_evt_ring_exit(struct gsi_channel *channel) gsi_evt_ring_id_free(gsi, evt_ring_id); } -/* Init function for event rings */ +/* Init function for event rings; there is n
[PATCH net-next 5/7] net: ipa: get rid of empty IPA functions
There are place holder functions in the IPA code that do nothing. For the most part these are inverse functions, for example, once the routing or filter tables are set up there is no need to perform any matching teardown activity at shutdown, or in the case of an error. These can be safely removed, resulting in some code simplification. Add comments in these spots making it explicit that there is no inverse. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 29 + drivers/net/ipa/ipa_mem.c | 9 +++-- drivers/net/ipa/ipa_mem.h | 5 ++--- drivers/net/ipa/ipa_resource.c | 8 +--- drivers/net/ipa/ipa_resource.h | 8 ++-- drivers/net/ipa/ipa_table.c| 26 +++--- drivers/net/ipa/ipa_table.h| 16 7 files changed, 24 insertions(+), 77 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index a970d10e650ef..bfed151f5d6dc 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -147,13 +147,13 @@ int ipa_setup(struct ipa *ipa) if (ret) goto err_endpoint_teardown; - ret = ipa_mem_setup(ipa); + ret = ipa_mem_setup(ipa); /* No matching teardown required */ if (ret) goto err_command_disable; - ret = ipa_table_setup(ipa); + ret = ipa_table_setup(ipa); /* No matching teardown required */ if (ret) - goto err_mem_teardown; + goto err_command_disable; /* Enable the exception handling endpoint, and tell the hardware * to use it by default. @@ -161,7 +161,7 @@ int ipa_setup(struct ipa *ipa) exception_endpoint = ipa->name_map[IPA_ENDPOINT_AP_LAN_RX]; ret = ipa_endpoint_enable_one(exception_endpoint); if (ret) - goto err_table_teardown; + goto err_command_disable; ipa_endpoint_default_route_set(ipa, exception_endpoint->endpoint_id); @@ -179,10 +179,6 @@ int ipa_setup(struct ipa *ipa) err_default_route_clear: ipa_endpoint_default_route_clear(ipa); ipa_endpoint_disable_one(exception_endpoint); -err_table_teardown: - ipa_table_teardown(ipa); -err_mem_teardown: - ipa_mem_teardown(ipa); err_command_disable: ipa_endpoint_disable_one(command_endpoint); err_endpoint_teardown: @@ -211,8 +207,6 @@ static void ipa_teardown(struct ipa *ipa) ipa_endpoint_default_route_clear(ipa); exception_endpoint = ipa->name_map[IPA_ENDPOINT_AP_LAN_RX]; ipa_endpoint_disable_one(exception_endpoint); - ipa_table_teardown(ipa); - ipa_mem_teardown(ipa); command_endpoint = ipa->name_map[IPA_ENDPOINT_AP_COMMAND_TX]; ipa_endpoint_disable_one(command_endpoint); ipa_endpoint_teardown(ipa); @@ -480,23 +474,20 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data) if (ret) goto err_endpoint_deconfig; - ipa_table_config(ipa); + ipa_table_config(ipa); /* No deconfig required */ - /* Assign resource limitation to each group */ + /* Assign resource limitation to each group; no deconfig required */ ret = ipa_resource_config(ipa, data->resource_data); if (ret) - goto err_table_deconfig; + goto err_mem_deconfig; ret = ipa_modem_config(ipa); if (ret) - goto err_resource_deconfig; + goto err_mem_deconfig; return 0; -err_resource_deconfig: - ipa_resource_deconfig(ipa); -err_table_deconfig: - ipa_table_deconfig(ipa); +err_mem_deconfig: ipa_mem_deconfig(ipa); err_endpoint_deconfig: ipa_endpoint_deconfig(ipa); @@ -514,8 +505,6 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data) static void ipa_deconfig(struct ipa *ipa) { ipa_modem_deconfig(ipa); - ipa_resource_deconfig(ipa); - ipa_table_deconfig(ipa); ipa_mem_deconfig(ipa); ipa_endpoint_deconfig(ipa); ipa_hardware_deconfig(ipa); diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c index 32907dde5dc6a..c5c3b1b7e67d5 100644 --- a/drivers/net/ipa/ipa_mem.c +++ b/drivers/net/ipa/ipa_mem.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2019-2020 Linaro Ltd. + * Copyright (C) 2019-2021 Linaro Ltd. */ #include @@ -53,6 +53,8 @@ ipa_mem_zero_region_add(struct gsi_trans *trans, const struct ipa_mem *mem) * The AP informs the modem where its portions of memory are located * in a QMI exchange that occurs at modem startup. * + * There is no need for a matching ipa_mem_teardown() function. + * * Return: 0 if successful, or a negative error code */ int ipa_mem_setup(struct ipa *ipa) @@ -97,11 +99,6 @@ int ipa_mem_setup(struct ipa *ipa) return 0; } -void i
[PATCH net-next 7/7] net: ipa: three small fixes
Some time ago changes were made to stop referring to clearing the hardware pipeline as a "tag process." Fix a comment to use the newer terminology. Get rid of a pointless double-negation of the Boolean toward_ipa flag in ipa_endpoint_config(). make ipa_endpoint_exit_one() private; it's only referenced inside "ipa_endpoint.c". Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_endpoint.c | 6 +++--- drivers/net/ipa/ipa_endpoint.h | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index dd24179383c1c..72751843b2e48 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -397,7 +397,7 @@ int ipa_endpoint_modem_exception_reset_all(struct ipa *ipa) /* We need one command per modem TX endpoint. We can get an upper * bound on that by assuming all initialized endpoints are modem->IPA. * That won't happen, and we could be more precise, but this is fine -* for now. We need to end the transaction with a "tag process." +* for now. End the transaction with commands to clear the pipeline. */ count = hweight32(initialized) + ipa_cmd_pipeline_clear_count(); trans = ipa_cmd_trans_alloc(ipa, count); @@ -1755,7 +1755,7 @@ int ipa_endpoint_config(struct ipa *ipa) /* Make sure it's pointing in the right direction */ endpoint = &ipa->endpoint[endpoint_id]; - if ((endpoint_id < rx_base) != !!endpoint->toward_ipa) { + if ((endpoint_id < rx_base) != endpoint->toward_ipa) { dev_err(dev, "endpoint id %u wrong direction\n", endpoint_id); ret = -EINVAL; @@ -1791,7 +1791,7 @@ static void ipa_endpoint_init_one(struct ipa *ipa, enum ipa_endpoint_name name, ipa->initialized |= BIT(endpoint->endpoint_id); } -void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint) +static void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint) { endpoint->ipa->initialized &= ~BIT(endpoint->endpoint_id); diff --git a/drivers/net/ipa/ipa_endpoint.h b/drivers/net/ipa/ipa_endpoint.h index f034a9e6ef215..0a859d10312dc 100644 --- a/drivers/net/ipa/ipa_endpoint.h +++ b/drivers/net/ipa/ipa_endpoint.h @@ -87,8 +87,6 @@ int ipa_endpoint_modem_exception_reset_all(struct ipa *ipa); int ipa_endpoint_skb_tx(struct ipa_endpoint *endpoint, struct sk_buff *skb); -void ipa_endpoint_exit_one(struct ipa_endpoint *endpoint); - int ipa_endpoint_enable_one(struct ipa_endpoint *endpoint); void ipa_endpoint_disable_one(struct ipa_endpoint *endpoint); -- 2.27.0
[PATCH net-next 4/7] net: ipa: ipa_stop() does not return an error
In ipa_modem_stop(), if the modem netdev pointer is non-null we call ipa_stop(). We check for an error and if one is returned we handle it. But ipa_stop() never returns an error, so this extra handling is unnecessary. Simplify the code in ipa_modem_stop() based on the knowledge no error handling is needed at this spot. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_modem.c | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index 8a6ccebde2889..af9aedbde717a 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -240,7 +240,6 @@ int ipa_modem_stop(struct ipa *ipa) { struct net_device *netdev = ipa->modem_netdev; enum ipa_modem_state state; - int ret; /* Only attempt to stop the modem if it's running */ state = atomic_cmpxchg(&ipa->modem_state, IPA_MODEM_STATE_RUNNING, @@ -257,29 +256,20 @@ int ipa_modem_stop(struct ipa *ipa) /* Prevent the modem from triggering a call to ipa_setup() */ ipa_smp2p_disable(ipa); + /* Stop the queue and disable the endpoints if it's open */ if (netdev) { - /* Stop the queue and disable the endpoints if it's open */ - ret = ipa_stop(netdev); - if (ret) - goto out_set_state; - + (void)ipa_stop(netdev); ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]->netdev = NULL; ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]->netdev = NULL; ipa->modem_netdev = NULL; unregister_netdev(netdev); free_netdev(netdev); - } else { - ret = 0; } -out_set_state: - if (ret) - atomic_set(&ipa->modem_state, IPA_MODEM_STATE_RUNNING); - else - atomic_set(&ipa->modem_state, IPA_MODEM_STATE_STOPPED); + atomic_set(&ipa->modem_state, IPA_MODEM_STATE_STOPPED); smp_mb__after_atomic(); - return ret; + return 0; } /* Treat a "clean" modem stop the same as a crash */ -- 2.27.0
[PATCH net-next 3/7] net: ipa: only set endpoint netdev pointer when in use
In ipa_modem_start(), we set endpoint netdev pointers before the network device is registered. If registration fails, we don't undo those assignments. Instead, wait to assign the netdev pointer until after registration succeeds. Set these endpoint netdev pointers to NULL in ipa_modem_stop() before unregistering the network device. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_modem.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index 9b08eb8239846..8a6ccebde2889 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2018-2020 Linaro Ltd. + * Copyright (C) 2018-2021 Linaro Ltd. */ #include @@ -213,18 +213,18 @@ int ipa_modem_start(struct ipa *ipa) goto out_set_state; } - ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]->netdev = netdev; - ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]->netdev = netdev; - SET_NETDEV_DEV(netdev, &ipa->pdev->dev); priv = netdev_priv(netdev); priv->ipa = ipa; ret = register_netdev(netdev); - if (ret) - free_netdev(netdev); - else + if (!ret) { ipa->modem_netdev = netdev; + ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]->netdev = netdev; + ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]->netdev = netdev; + } else { + free_netdev(netdev); + } out_set_state: if (ret) @@ -263,6 +263,8 @@ int ipa_modem_stop(struct ipa *ipa) if (ret) goto out_set_state; + ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]->netdev = NULL; + ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]->netdev = NULL; ipa->modem_netdev = NULL; unregister_netdev(netdev); free_netdev(netdev); -- 2.27.0
[PATCH net-next 0/7] net: ipa: a few small fixes
This series implements some minor bug fixes or improvements. The first patch removes an apparently unnecessary restriction, which results in an error on a 32-bit ARM build. The second makes a definition used for SDM845 match what is used in the downstream code. The third just ensures two netdev pointers are only non-null when valid. The fourth simplifies a little code, knowing that a called function never returns an error. The fifth and sixth just remove some empty/place holder functions. And the last patch fixes a comment, makes a function private, and removes an unnecessary double-negation of a Boolean variable. This patch produces a warning from checkpatch, indicating that a pair of parentheses is unnecessary. I agree with that advice, but it conflicts with a suggestion from the compiler. I left the "problem" in place to avoid the compiler warning. -Alex Alex Elder (7): net: ipa: relax pool entry size requirement net: ipa: update sequence type for modem TX endpoint net: ipa: only set endpoint netdev pointer when in use net: ipa: ipa_stop() does not return an error net: ipa: get rid of empty IPA functions net: ipa: get rid of empty GSI functions net: ipa: three small fixes drivers/net/ipa/gsi.c | 54 --- drivers/net/ipa/gsi_trans.c | 4 +-- drivers/net/ipa/ipa_data-v3.5.1.c | 1 + drivers/net/ipa/ipa_endpoint.c| 6 ++-- drivers/net/ipa/ipa_endpoint.h| 2 -- drivers/net/ipa/ipa_main.c| 29 ++--- drivers/net/ipa/ipa_mem.c | 9 ++ drivers/net/ipa/ipa_mem.h | 5 ++- drivers/net/ipa/ipa_modem.c | 34 --- drivers/net/ipa/ipa_resource.c| 8 + drivers/net/ipa/ipa_resource.h| 8 ++--- drivers/net/ipa/ipa_table.c | 26 ++- drivers/net/ipa/ipa_table.h | 16 +++-- 13 files changed, 49 insertions(+), 153 deletions(-) -- 2.27.0
[PATCH net-next 2/7] net: ipa: update sequence type for modem TX endpoint
On IPA v3.5.1, the sequencer type for the modem TX endpoint does not define the replication portion in the same way the downstream code does. This difference doesn't affect the behavior of the upstream code, but I'd prefer the two code bases use the same configuration value here. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-v3.5.1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ipa/ipa_data-v3.5.1.c b/drivers/net/ipa/ipa_data-v3.5.1.c index 57703e95a3f9c..ead1a82f32f5c 100644 --- a/drivers/net/ipa/ipa_data-v3.5.1.c +++ b/drivers/net/ipa/ipa_data-v3.5.1.c @@ -116,6 +116,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .status_enable = true, .tx = { .seq_type = IPA_SEQ_2_PASS_SKIP_LAST_UC, + .seq_rep_type = IPA_SEQ_REP_DMA_PARSER, .status_endpoint = IPA_ENDPOINT_MODEM_AP_RX, }, -- 2.27.0
[PATCH net-next 1/7] net: ipa: relax pool entry size requirement
I no longer know why a validation check ensured the size of an entry passed to gsi_trans_pool_init() was restricted to be a multiple of 8. For 32-bit builds, this condition doesn't always hold, and for DMA pools, the size is rounded up to a power of 2 anyway. Remove this restriction. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi_trans.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c index 70c2b585f98d6..8c795a6a85986 100644 --- a/drivers/net/ipa/gsi_trans.c +++ b/drivers/net/ipa/gsi_trans.c @@ -91,7 +91,7 @@ int gsi_trans_pool_init(struct gsi_trans_pool *pool, size_t size, u32 count, void *virt; #ifdef IPA_VALIDATE - if (!size || size % 8) + if (!size) return -EINVAL; if (count < max_alloc) return -EINVAL; @@ -141,7 +141,7 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool, void *virt; #ifdef IPA_VALIDATE - if (!size || size % 8) + if (!size) return -EINVAL; if (count < max_alloc) return -EINVAL; -- 2.27.0
Re: [RESEND net-next 4/4] net: ipa: remove repeated words
On 3/30/21 2:27 AM, Huazhong Tan wrote: From: Peng Li Remove repeated words "that" and "the". Signed-off-by: Peng Li Signed-off-by: Huazhong Tan Acked-by: Alex Elder --- 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 38e83cd..dd24179 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -809,7 +809,7 @@ static u32 hol_block_timer_val(struct ipa *ipa, u32 microseconds) * The best precision is achieved when the base value is as * large as possible. Find the highest set bit in the tick * count, and extract the number of bits in the base field -* such that that high bit is included. +* such that high bit is included. */ high = fls(ticks); /* 1..32 */ width = HWEIGHT32(BASE_VALUE_FMASK); @@ -1448,7 +1448,7 @@ static int ipa_endpoint_reset_rx_aggr(struct ipa_endpoint *endpoint) if (ret) goto out_suspend_again; - /* Finally, reset and reconfigure the channel again (re-enabling the + /* Finally, reset and reconfigure the channel again (re-enabling * the doorbell engine if appropriate). Sleep for 1 millisecond to * complete the channel reset sequence. Finish by suspending the * channel again (if necessary).
[PATCH net-next 7/7] net: ipa: kill IPA_TABLE_ENTRY_SIZE
Entries in an IPA route or filter table are 64-bit little-endian addresses, each of which refers to a routing or filtering rule. The format of these table slots are fixed, but IPA_TABLE_ENTRY_SIZE is used to define their size. This symbol doesn't really add value, and I think it unnecessarily obscures what a table entry *is*. So get rid of IPA_TABLE_ENTRY_SIZE, and just use sizeof(__le64) in its place throughout the code. Update the comments in "ipa_table.c" to provide a little better explanation of these table slots. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_cmd.c | 2 +- drivers/net/ipa/ipa_qmi.c | 10 +++ drivers/net/ipa/ipa_table.c | 59 + drivers/net/ipa/ipa_table.h | 3 -- 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index 2ac6dd8413dee..525cdf28d9ea7 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -153,7 +153,7 @@ static void ipa_cmd_validate_build(void) * of entries, as and IPv4 and IPv6 route tables have the same number * of entries. */ -#define TABLE_SIZE (TABLE_COUNT_MAX * IPA_TABLE_ENTRY_SIZE) +#define TABLE_SIZE (TABLE_COUNT_MAX * sizeof(__le64)) #define TABLE_COUNT_MAXmax_t(u32, IPA_ROUTE_COUNT_MAX, IPA_FILTER_COUNT_MAX) BUILD_BUG_ON(TABLE_SIZE > field_max(IP_FLTRT_FLAGS_HASH_SIZE_FMASK)); BUILD_BUG_ON(TABLE_SIZE > field_max(IP_FLTRT_FLAGS_NHASH_SIZE_FMASK)); diff --git a/drivers/net/ipa/ipa_qmi.c b/drivers/net/ipa/ipa_qmi.c index ccdb4a6a4c75b..593665efbcf99 100644 --- a/drivers/net/ipa/ipa_qmi.c +++ b/drivers/net/ipa/ipa_qmi.c @@ -308,12 +308,12 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi) mem = &ipa->mem[IPA_MEM_V4_ROUTE]; req.v4_route_tbl_info_valid = 1; req.v4_route_tbl_info.start = ipa->mem_offset + mem->offset; - req.v4_route_tbl_info.count = mem->size / IPA_TABLE_ENTRY_SIZE; + req.v4_route_tbl_info.count = mem->size / sizeof(__le64); mem = &ipa->mem[IPA_MEM_V6_ROUTE]; req.v6_route_tbl_info_valid = 1; req.v6_route_tbl_info.start = ipa->mem_offset + mem->offset; - req.v6_route_tbl_info.count = mem->size / IPA_TABLE_ENTRY_SIZE; + req.v6_route_tbl_info.count = mem->size / sizeof(__le64); mem = &ipa->mem[IPA_MEM_V4_FILTER]; req.v4_filter_tbl_start_valid = 1; @@ -352,8 +352,7 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi) req.v4_hash_route_tbl_info_valid = 1; req.v4_hash_route_tbl_info.start = ipa->mem_offset + mem->offset; - req.v4_hash_route_tbl_info.count = - mem->size / IPA_TABLE_ENTRY_SIZE; + req.v4_hash_route_tbl_info.count = mem->size / sizeof(__le64); } mem = &ipa->mem[IPA_MEM_V6_ROUTE_HASHED]; @@ -361,8 +360,7 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi) req.v6_hash_route_tbl_info_valid = 1; req.v6_hash_route_tbl_info.start = ipa->mem_offset + mem->offset; - req.v6_hash_route_tbl_info.count = - mem->size / IPA_TABLE_ENTRY_SIZE; + req.v6_hash_route_tbl_info.count = mem->size / sizeof(__le64); } mem = &ipa->mem[IPA_MEM_V4_FILTER_HASHED]; diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c index d9538661755f4..401b568df6a34 100644 --- a/drivers/net/ipa/ipa_table.c +++ b/drivers/net/ipa/ipa_table.c @@ -27,28 +27,38 @@ /** * DOC: IPA Filter and Route Tables * - * The IPA has tables defined in its local shared memory that define filter - * and routing rules. Each entry in these tables contains a 64-bit DMA - * address that refers to DRAM (system memory) containing a rule definition. + * The IPA has tables defined in its local (IPA-resident) memory that define + * filter and routing rules. An entry in either of these tables is a little + * endian 64-bit "slot" that holds the address of a rule definition. (The + * size of these slots is 64 bits regardless of the host DMA address size.) + * + * Separate tables (both filter and route) used for IPv4 and IPv6. There + * are normally another set of "hashed" filter and route tables, which are + * used with a hash of message metadata. Hashed operation is not supported + * by all IPA hardware (IPA v4.2 doesn't support hashed tables). + * + * Rules can be in local memory or in DRAM (system memory). The offset of + * an object (such as a route or filter table) in IPA-resident memory must + * 128-byte aligned. An object in system memory (such as a route or filter + * rule) must be at an 8-byte aligned address. We currently only place + * route or filter rules in system memory. + * * A rule consists
[PATCH net-next 6/7] net: ipa: DMA addresses are nicely aligned
A recent patch avoided doing 64-bit modulo operations by checking the alignment of some DMA allocations using only the lower 32 bits of the address. David Laight pointed out (after the fix was committed) that DMA allocations might already satisfy the alignment requirements. And he was right. Remove the alignment checks that occur after DMA allocation requests, and update comments to explain why the constraint is satisfied. The only place IPA_TABLE_ALIGN was used was to check the alignment; it is therefore no longer needed, so get rid of it. Add comments where GSI_RING_ELEMENT_SIZE and the tre_count and event_count channel data fields are defined to make explicit they are required to be powers of 2. Revise a comment in gsi_trans_pool_init_dma(), taking into account that dma_alloc_coherent() guarantees its result is aligned to a page size (or order thereof). Don't bother printing an error if a DMA allocation fails. Suggested-by: David Laight Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 13 - drivers/net/ipa/gsi_private.h | 2 +- drivers/net/ipa/gsi_trans.c | 9 - drivers/net/ipa/ipa_data.h| 4 ++-- drivers/net/ipa/ipa_table.c | 24 ++-- 5 files changed, 17 insertions(+), 35 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 585574af36ecd..1c835b3e1a437 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -1444,18 +1444,13 @@ static int gsi_ring_alloc(struct gsi *gsi, struct gsi_ring *ring, u32 count) dma_addr_t addr; /* Hardware requires a 2^n ring size, with alignment equal to size. -* The size is a power of 2, so we can check alignment using just -* the bottom 32 bits for a DMA address of any size. +* The DMA address returned by dma_alloc_coherent() is guaranteed to +* be a power-of-2 number of pages, which satisfies the requirement. */ ring->virt = dma_alloc_coherent(dev, size, &addr, GFP_KERNEL); - if (ring->virt && lower_32_bits(addr) % size) { - dma_free_coherent(dev, size, ring->virt, addr); - dev_err(dev, "unable to alloc 0x%x-aligned ring buffer\n", - size); - return -EINVAL; /* Not a good error value, but distinct */ - } else if (!ring->virt) { + if (!ring->virt) return -ENOMEM; - } + ring->addr = addr; ring->count = count; diff --git a/drivers/net/ipa/gsi_private.h b/drivers/net/ipa/gsi_private.h index ed7bc26f85e9a..ea333a244cf5e 100644 --- a/drivers/net/ipa/gsi_private.h +++ b/drivers/net/ipa/gsi_private.h @@ -14,7 +14,7 @@ struct gsi_trans; struct gsi_ring; struct gsi_channel; -#define GSI_RING_ELEMENT_SIZE 16 /* bytes */ +#define GSI_RING_ELEMENT_SIZE 16 /* bytes; must be a power of 2 */ /* Return the entry that follows one provided in a transaction pool */ void *gsi_trans_pool_next(struct gsi_trans_pool *pool, void *element); diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c index 6c3ed5b17b80c..70c2b585f98d6 100644 --- a/drivers/net/ipa/gsi_trans.c +++ b/drivers/net/ipa/gsi_trans.c @@ -153,11 +153,10 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool, size = __roundup_pow_of_two(size); total_size = (count + max_alloc - 1) * size; - /* The allocator will give us a power-of-2 number of pages. But we -* can't guarantee that, so request it. That way we won't waste any -* memory that would be available beyond the required space. -* -* Note that gsi_trans_pool_exit_dma() assumes the total allocated + /* The allocator will give us a power-of-2 number of pages +* sufficient to satisfy our request. Round up our requested +* size to avoid any unused space in the allocation. This way +* gsi_trans_pool_exit_dma() can assume the total allocated * size is exactly (count * size). */ total_size = get_order(total_size) << PAGE_SHIFT; diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index e97342e80d537..769f68923527f 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -90,8 +90,8 @@ struct ipa_qsb_data { * that can be included in a single transaction. */ struct gsi_channel_data { - u16 tre_count; - u16 event_count; + u16 tre_count; /* must be a power of 2 */ + u16 event_count;/* must be a power of 2 */ u8 tlv_count; }; diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c index 4236a50ff03ae..d9538661755f4 100644 --- a/drivers/net/ipa/ipa_table.c +++ b/drivers/net/ipa/ipa_table.c @@ -96,9 +96,6 @@ * -- */ -/* IPA hardware constrains filter and route tables alignment */ -#define
[PATCH net-next 5/7] net: ipa: use version based configuration for SC7180
Rename the SC7180 configuration data file so that its name is derived from its IPA version. Update a few other references to the code that talk about the SC7180 rather than just IPA v4.2. Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile | 2 +- .../{ipa_data-sc7180.c => ipa_data-v4.2.c}| 24 ++- drivers/net/ipa/ipa_data.h| 2 +- drivers/net/ipa/ipa_main.c| 2 +- 4 files changed, 16 insertions(+), 14 deletions(-) rename drivers/net/ipa/{ipa_data-sc7180.c => ipa_data-v4.2.c} (90%) diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index a4a42fc7b840e..6abd1db9fe330 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -9,4 +9,4 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_endpoint.o ipa_cmd.o ipa_modem.o \ ipa_resource.o ipa_qmi.o ipa_qmi_msg.o -ipa-y += ipa_data-v3.5.1.o ipa_data-sc7180.o +ipa-y += ipa_data-v3.5.1.o ipa_data-v4.2.o diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-v4.2.c similarity index 90% rename from drivers/net/ipa/ipa_data-sc7180.c rename to drivers/net/ipa/ipa_data-v4.2.c index 810c673be56ee..8744f19c64011 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-v4.2.c @@ -9,7 +9,7 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" -/** enum ipa_resource_type - IPA resource types */ +/** enum ipa_resource_type - IPA resource types for an SoC having IPA v4.2 */ enum ipa_resource_type { /* Source resource types; first must have value 0 */ IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, @@ -23,7 +23,7 @@ enum ipa_resource_type { IPA_RESOURCE_TYPE_DST_DPS_DMARS, }; -/* Resource groups used for the SC7180 SoC */ +/* Resource groups used for an SoC having IPA v4.2 */ enum ipa_rsrc_group_id { /* Source resource group identifiers */ IPA_RSRC_GROUP_SRC_UL_DL= 0, @@ -34,7 +34,7 @@ enum ipa_rsrc_group_id { IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ }; -/* QSB configuration for the SC7180 SoC. */ +/* QSB configuration data for an SoC having IPA v4.2 */ static const struct ipa_qsb_data ipa_qsb_data[] = { [IPA_QSB_MASTER_DDR] = { .max_writes = 8, @@ -43,7 +43,7 @@ static const struct ipa_qsb_data ipa_qsb_data[] = { }, }; -/* Endpoint configuration for the SC7180 SoC. */ +/* Endpoint configuration data for an SoC having IPA v4.2 */ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { [IPA_ENDPOINT_AP_COMMAND_TX] = { .ee_id = GSI_EE_AP, @@ -164,7 +164,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, }; -/* Source resource configuration data for the SC7180 SoC */ +/* Source resource configuration data for an SoC having IPA v4.2 */ static const struct ipa_resource ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { @@ -193,7 +193,7 @@ static const struct ipa_resource ipa_resource_src[] = { }, }; -/* Destination resource configuration data for the SC7180 SoC */ +/* Destination resource configuration data for an SoC having IPA v4.2 */ static const struct ipa_resource ipa_resource_dst[] = { [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { @@ -207,7 +207,7 @@ static const struct ipa_resource ipa_resource_dst[] = { }, }; -/* Resource configuration for the SC7180 SoC. */ +/* Resource configuration data for an SoC having IPA v4.2 */ static const struct ipa_resource_data ipa_resource_data = { .rsrc_group_src_count = IPA_RSRC_GROUP_SRC_COUNT, .rsrc_group_dst_count = IPA_RSRC_GROUP_DST_COUNT, @@ -217,7 +217,7 @@ static const struct ipa_resource_data ipa_resource_data = { .resource_dst = ipa_resource_dst, }; -/* IPA-resident memory region configuration for the SC7180 SoC. */ +/* IPA-resident memory region data for an SoC having IPA v4.2 */ static const struct ipa_mem ipa_mem_local_data[] = { [IPA_MEM_UC_SHARED] = { .offset = 0x, @@ -311,6 +311,7 @@ static const struct ipa_mem ipa_mem_local_data[] = { }, }; +/* Memory configuration data for an SoC having IPA v4.2 */ static const struct ipa_mem_data ipa_mem_data = { .local_count= ARRAY_SIZE(ipa_mem_local_data), .local = ipa_mem_local_data, @@ -320,7 +321,7 @@ static const struct ipa_mem_data ipa_mem_data = { .smem_size = 0x2000, }; -/* Interconnect bandwidths are in 1000 byte/second units */ +/* Interconnect rates are in 1000 byte/second units */ static const struct ipa_interconnect_
[PATCH net-next 3/7] net: ipa: don't define endpoints unnecessarily
We don't typically need much information about modem endpoints. Normally we need to specify information about modem endpoints in configuration data in only two cases: - When a modem TX endpoint supports filtering - When another endpoint's configuration refers to it For the first case, the AP initializes the filter table, and must know how many endpoints (AP and modem) support filtering. An example of the second case is the AP->modem TX endpoint, which defines the modem<-AP RX endpoint as its status endpoint. There is one exception to this, and it's due to a hardware quirk. For IPA v4.2 (only) there is a problem related to allocating GSI channels. And to work around this, the AP allocates *all* GSI channels at startup time--including those used by the modem. Get rid of the configuration information for two endpoints not required for the SDM845. SC7180 runs IPA v4.2, so we can't eliminate any modem endpoint definitions there. Two more minor changes: - Reorder the members defined for the ipa_endpoint_name enumerated type to match the order used in configuration data files when defining endpoints. - Add a new name, IPA_ENDPOINT_MODEM_DL_NLO_TX, which can be used for IPA v4.5+. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sdm845.c | 12 drivers/net/ipa/ipa_endpoint.h| 11 ++- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 49a18b1047c58..ed0bfe0634d98 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -144,12 +144,6 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, }, }, - [IPA_ENDPOINT_MODEM_COMMAND_TX] = { - .ee_id = GSI_EE_MODEM, - .channel_id = 1, - .endpoint_id= 4, - .toward_ipa = true, - }, [IPA_ENDPOINT_MODEM_LAN_TX] = { .ee_id = GSI_EE_MODEM, .channel_id = 0, @@ -159,12 +153,6 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .filter_support = true, }, }, - [IPA_ENDPOINT_MODEM_LAN_RX] = { - .ee_id = GSI_EE_MODEM, - .channel_id = 3, - .endpoint_id= 13, - .toward_ipa = false, - }, [IPA_ENDPOINT_MODEM_AP_TX] = { .ee_id = GSI_EE_MODEM, .channel_id = 4, diff --git a/drivers/net/ipa/ipa_endpoint.h b/drivers/net/ipa/ipa_endpoint.h index 0d410b0504562..f034a9e6ef215 100644 --- a/drivers/net/ipa/ipa_endpoint.h +++ b/drivers/net/ipa/ipa_endpoint.h @@ -25,15 +25,16 @@ struct ipa_gsi_endpoint_data; #define IPA_MTUETH_DATA_LEN enum ipa_endpoint_name { - IPA_ENDPOINT_AP_MODEM_TX, - IPA_ENDPOINT_MODEM_LAN_TX, - IPA_ENDPOINT_MODEM_COMMAND_TX, IPA_ENDPOINT_AP_COMMAND_TX, - IPA_ENDPOINT_MODEM_AP_TX, IPA_ENDPOINT_AP_LAN_RX, + IPA_ENDPOINT_AP_MODEM_TX, IPA_ENDPOINT_AP_MODEM_RX, - IPA_ENDPOINT_MODEM_AP_RX, + IPA_ENDPOINT_MODEM_COMMAND_TX, + IPA_ENDPOINT_MODEM_LAN_TX, IPA_ENDPOINT_MODEM_LAN_RX, + IPA_ENDPOINT_MODEM_AP_TX, + IPA_ENDPOINT_MODEM_AP_RX, + IPA_ENDPOINT_MODEM_DL_NLO_TX, IPA_ENDPOINT_COUNT, /* Number of names (not an index) */ }; -- 2.27.0
[PATCH net-next 1/7] net: ipa: fix all kernel-doc warnings
Fix all warnings produced when running: scripts/kernel-doc -none drivers/net/ipa/*.[ch] Signed-off-by: Alex Elder --- drivers/net/ipa/gsi_private.h | 2 +- drivers/net/ipa/gsi_trans.h | 5 +++-- drivers/net/ipa/ipa.h | 7 --- drivers/net/ipa/ipa_cmd.h | 19 +-- drivers/net/ipa/ipa_endpoint.h | 18 +++--- drivers/net/ipa/ipa_interrupt.h | 1 + drivers/net/ipa/ipa_mem.h | 2 +- drivers/net/ipa/ipa_qmi.h | 14 +- drivers/net/ipa/ipa_smp2p.h | 2 +- drivers/net/ipa/ipa_table.h | 3 ++- 10 files changed, 50 insertions(+), 23 deletions(-) diff --git a/drivers/net/ipa/gsi_private.h b/drivers/net/ipa/gsi_private.h index 1785c9d3344d1..ed7bc26f85e9a 100644 --- a/drivers/net/ipa/gsi_private.h +++ b/drivers/net/ipa/gsi_private.h @@ -100,7 +100,7 @@ void gsi_channel_doorbell(struct gsi_channel *channel); /** * gsi_ring_virt() - Return virtual address for a ring entry * @ring: Ring whose address is to be translated - * @addr: Index (slot number) of entry + * @index: Index (slot number) of entry */ void *gsi_ring_virt(struct gsi_ring *ring, u32 index); diff --git a/drivers/net/ipa/gsi_trans.h b/drivers/net/ipa/gsi_trans.h index 3a4ab8a94d827..17fd1822d8a9f 100644 --- a/drivers/net/ipa/gsi_trans.h +++ b/drivers/net/ipa/gsi_trans.h @@ -71,7 +71,7 @@ struct gsi_trans { /** * gsi_trans_pool_init() - Initialize a pool of structures for transactions - * @gsi: GSI pointer + * @pool: GSI transaction poll pointer * @size: Size of elements in the pool * @count: Minimum number of elements in the pool * @max_alloc: Maximum number of elements allocated at a time from pool @@ -123,7 +123,8 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool, void *gsi_trans_pool_alloc_dma(struct gsi_trans_pool *pool, dma_addr_t *addr); /** - * gsi_trans_pool_exit() - Inverse of gsi_trans_pool_init() + * gsi_trans_pool_exit_dma() - Inverse of gsi_trans_pool_init_dma() + * @dev: Device used for DMA * @pool: Pool pointer */ void gsi_trans_pool_exit_dma(struct device *dev, struct gsi_trans_pool *pool); diff --git a/drivers/net/ipa/ipa.h b/drivers/net/ipa/ipa.h index 8020776313716..e7ff376cb5b7d 100644 --- a/drivers/net/ipa/ipa.h +++ b/drivers/net/ipa/ipa.h @@ -44,6 +44,8 @@ enum ipa_flag { * @version: IPA hardware version * @pdev: Platform device * @completion:Used to signal pipeline clear transfer complete + * @nb:Notifier block used for remoteproc SSR + * @notifier: Remoteproc SSR notifier * @smp2p: SMP2P information * @clock: IPA clocking information * @table_addr:DMA address of filter/route table content @@ -58,13 +60,12 @@ enum ipa_flag { * @mem_size: Total size (bytes) of memory at @mem_virt * @mem: Array of IPA-local memory region descriptors * @imem_iova: I/O virtual address of IPA region in IMEM - * @imem_size; Size of IMEM region + * @imem_size: Size of IMEM region * @smem_iova: I/O virtual address of IPA region in SMEM - * @smem_size; Size of SMEM region + * @smem_size: Size of SMEM region * @zero_addr: DMA address of preallocated zero-filled memory * @zero_virt: Virtual address of preallocated zero-filled memory * @zero_size: Size (bytes) of preallocated zero-filled memory - * @wakeup_source: Wakeup source information * @available: Bit mask indicating endpoints hardware supports * @filter_map:Bit mask indicating endpoints that support filtering * @initialized: Bit mask indicating endpoints initialized diff --git a/drivers/net/ipa/ipa_cmd.h b/drivers/net/ipa/ipa_cmd.h index 6dd3d35cf315d..b99262281f41c 100644 --- a/drivers/net/ipa/ipa_cmd.h +++ b/drivers/net/ipa/ipa_cmd.h @@ -20,11 +20,18 @@ struct gsi_channel; /** * enum ipa_cmd_opcode:IPA immediate commands * + * @IPA_CMD_IP_V4_FILTER_INIT: Initialize IPv4 filter table + * @IPA_CMD_IP_V6_FILTER_INIT: Initialize IPv6 filter table + * @IPA_CMD_IP_V4_ROUTING_INIT:Initialize IPv4 routing table + * @IPA_CMD_IP_V6_ROUTING_INIT:Initialize IPv6 routing table + * @IPA_CMD_HDR_INIT_LOCAL:Initialize IPA-local header memory + * @IPA_CMD_REGISTER_WRITE:Register write performed by IPA + * @IPA_CMD_IP_PACKET_INIT:Set up next packet's destination endpoint + * @IPA_CMD_DMA_SHARED_MEM:DMA command performed by IPA + * @IPA_CMD_IP_PACKET_TAG_STATUS: Have next packet generate tag * status + * @IPA_CMD_NONE: Special (invalid) "not a command" value + * * All immediate commands are issued using the AP command TX endpoint. - * The numeric values here are the opcodes for IPA v3.5.1 hardware. - * - * IPA_CMD_NONE is a special (invalid) value that's used to indicate - * a
[PATCH net-next 2/7] net: ipa: store BCR register values in config data
The backward compatibility register value is a platform-specific property that is not stored in the platform data. Create a data field where this can be represented, and get rid ipa_reg_bcr_val(). This register is not present starting with IPA v4.5. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 1 + drivers/net/ipa/ipa_data-sdm845.c | 5 + drivers/net/ipa/ipa_data.h| 2 ++ drivers/net/ipa/ipa_main.c| 4 ++-- drivers/net/ipa/ipa_reg.h | 21 - 5 files changed, 10 insertions(+), 23 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index c9b6a6aaadacc..810c673be56ee 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -349,6 +349,7 @@ static const struct ipa_clock_data ipa_clock_data = { /* Configuration data for the SC7180 SoC. */ const struct ipa_data ipa_data_sc7180 = { .version= IPA_VERSION_4_2, + /* backward_compat value is 0 */ .qsb_count = ARRAY_SIZE(ipa_qsb_data), .qsb_data = ipa_qsb_data, .endpoint_count = ARRAY_SIZE(ipa_gsi_endpoint_data), diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index e14e3fb1d9700..49a18b1047c58 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -397,6 +397,11 @@ static const struct ipa_clock_data ipa_clock_data = { /* Configuration data for the SDM845 SoC. */ const struct ipa_data ipa_data_sdm845 = { .version= IPA_VERSION_3_5_1, + .backward_compat = BCR_CMDQ_L_LACK_ONE_ENTRY_FMASK | + BCR_TX_NOT_USING_BRESP_FMASK | + BCR_SUSPEND_L2_IRQ_FMASK | + BCR_HOLB_DROP_L2_IRQ_FMASK | + BCR_DUAL_TX_FMASK, .qsb_count = ARRAY_SIZE(ipa_qsb_data), .qsb_data = ipa_qsb_data, .endpoint_count = ARRAY_SIZE(ipa_gsi_endpoint_data), diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index ea8f99286228e..843d818f78e18 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -279,6 +279,7 @@ struct ipa_clock_data { /** * struct ipa_data - combined IPA/GSI configuration data * @version: IPA hardware version + * @backward_compat: BCR register value (prior to IPA v4.5 only) * @qsb_count: number of entries in the qsb_data array * @qsb_data: Qualcomm System Bus configuration data * @endpoint_count:number of entries in the endpoint_data array @@ -289,6 +290,7 @@ struct ipa_clock_data { */ struct ipa_data { enum ipa_version version; + u32 backward_compat; u32 qsb_count; /* number of entries in qsb_data[] */ const struct ipa_qsb_data *qsb_data; u32 endpoint_count; /* number of entries in endpoint_data[] */ diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index e18029152d780..afb8eb5618f73 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -397,9 +397,9 @@ static void ipa_hardware_config(struct ipa *ipa, const struct ipa_data *data) u32 granularity; u32 val; - /* IPA v4.5 has no backward compatibility register */ + /* IPA v4.5+ has no backward compatibility register */ if (version < IPA_VERSION_4_5) { - val = ipa_reg_bcr_val(version); + val = data->backward_compat; iowrite32(val, ipa->reg_virt + IPA_REG_BCR_OFFSET); } diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index de2a944bad86b..286ea9634c49d 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -235,27 +235,6 @@ static inline u32 ipa_reg_state_aggr_active_offset(enum ipa_version version) #define BCR_FILTER_PREFETCH_EN_FMASK GENMASK(8, 8) #define BCR_ROUTER_PREFETCH_EN_FMASK GENMASK(9, 9) -/* 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_5_1) - return BCR_CMDQ_L_LACK_ONE_ENTRY_FMASK | - BCR_TX_NOT_USING_BRESP_FMASK | - BCR_SUSPEND_L2_IRQ_FMASK | - BCR_HOLB_DROP_L2_IRQ_FMASK | - BCR_DUAL_TX_FMASK; - - if (version == IPA_VERSION_4_0 || version == IPA_VERSION_4_1) - return BCR_CMDQ_L_LACK_ONE_ENTRY_FMASK | - BCR_SUSPEND_L2_IRQ_FMASK | - BCR_HOLB_DROP_L2_IRQ_FMASK | - BCR_DUAL_TX_FMASK; - - /* assert(version != IPA_VERSION_4_5); */ - - return 0x; -} - /* The value of the next register must be a multiple of 8 (bottom 3 bits 0) */ #define IPA_REG_LOCAL_PKT_PROC_CNTXT_OFFSET0x01e8 -- 2.27.0
[PATCH net-next 4/7] net: ipa: switch to version based configuration
Rename the SDM845 configuration data file so that its name is derived from its IPA version. I am not aware of any special IPA behavior or handling that would be based on a specific SoC (as opposed to a specific version of the IPA it contains). Update a few other references to the code that talk about the SDM845 rather than just IPA v3.5.1. Signed-off-by: Alex Elder --- drivers/net/ipa/Kconfig | 3 +-- drivers/net/ipa/Makefile | 2 +- .../{ipa_data-sdm845.c => ipa_data-v3.5.1.c} | 22 ++- drivers/net/ipa/ipa_data.h| 2 +- drivers/net/ipa/ipa_main.c| 2 +- 5 files changed, 16 insertions(+), 15 deletions(-) rename drivers/net/ipa/{ipa_data-sdm845.c => ipa_data-v3.5.1.c} (92%) diff --git a/drivers/net/ipa/Kconfig b/drivers/net/ipa/Kconfig index 90a90262e0d07..8f99cfa14680a 100644 --- a/drivers/net/ipa/Kconfig +++ b/drivers/net/ipa/Kconfig @@ -12,8 +12,7 @@ config QCOM_IPA that is capable of generic hardware handling of IP packets, including routing, filtering, and NAT. Currently the IPA driver supports only basic transport of network traffic - between the AP and modem, on the Qualcomm SDM845 and SC7180 - SoCs. + between the AP and modem. Note that if selected, the selection type must match that of QCOM_Q6V5_COMMON (Y or M). diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index 14a7d8429baa2..a4a42fc7b840e 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -9,4 +9,4 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_endpoint.o ipa_cmd.o ipa_modem.o \ ipa_resource.o ipa_qmi.o ipa_qmi_msg.o -ipa-y += ipa_data-sdm845.o ipa_data-sc7180.o +ipa-y += ipa_data-v3.5.1.o ipa_data-sc7180.o diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-v3.5.1.c similarity index 92% rename from drivers/net/ipa/ipa_data-sdm845.c rename to drivers/net/ipa/ipa_data-v3.5.1.c index ed0bfe0634d98..57703e95a3f9c 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-v3.5.1.c @@ -11,7 +11,7 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" -/** enum ipa_resource_type - IPA resource types */ +/** enum ipa_resource_type - IPA resource types for an SoC having IPA v3.5.1 */ enum ipa_resource_type { /* Source resource types; first must have value 0 */ IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, @@ -25,7 +25,7 @@ enum ipa_resource_type { IPA_RESOURCE_TYPE_DST_DPS_DMARS, }; -/* Resource groups used for the SDM845 SoC */ +/* Resource groups used for an SoC having IPA v3.5.1 */ enum ipa_rsrc_group_id { /* Source resource group identifiers */ IPA_RSRC_GROUP_SRC_LWA_DL = 0, @@ -41,7 +41,7 @@ enum ipa_rsrc_group_id { IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ }; -/* QSB configuration for the SDM845 SoC. */ +/* QSB configuration data for an SoC having IPA v3.5.1 */ static const struct ipa_qsb_data ipa_qsb_data[] = { [IPA_QSB_MASTER_DDR] = { .max_writes = 8, @@ -53,7 +53,7 @@ static const struct ipa_qsb_data ipa_qsb_data[] = { }, }; -/* Endpoint configuration for the SDM845 SoC. */ +/* Endpoint datdata for an SoC having IPA v3.5.1 */ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { [IPA_ENDPOINT_AP_COMMAND_TX] = { .ee_id = GSI_EE_AP, @@ -170,7 +170,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, }; -/* Source resource configuration data for the SDM845 SoC */ +/* Source resource configuration data for an SoC having IPA v3.5.1 */ static const struct ipa_resource ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { @@ -232,7 +232,7 @@ static const struct ipa_resource ipa_resource_src[] = { }, }; -/* Destination resource configuration data for the SDM845 SoC */ +/* Destination resource configuration data for an SoC having IPA v3.5.1 */ static const struct ipa_resource ipa_resource_dst[] = { [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_LWA_DL] = { @@ -258,7 +258,7 @@ static const struct ipa_resource ipa_resource_dst[] = { }, }; -/* Resource configuration for the SDM845 SoC. */ +/* Resource configuration data for an SoC having IPA v3.5.1 */ static const struct ipa_resource_data ipa_resource_data = { .rsrc_group_src_count = IPA_RSRC_GROUP_SRC_COUNT, .rsrc_group_dst_count = IPA_RSRC_GROUP_DST_COUNT, @@ -268,7 +268,7 @@ static const struct ipa_resource_data ipa_resource_data = { .resource_dst = ipa_resource_dst, };
[PATCH net-next 0/7] net: ipa: a few last bits
This series incorporates a few last things that didn't fit neatly with patches I've posted recently. The first patch eliminates all remaining kernel-doc warnings. There's still room for kernel-doc improvement, but at least what's there will no longer produce warnings. The next moves the definition of the value to store in the backward compatibility register (when present) into platform data files. The third removes two endpoint definitions that do not need to be defined. The next two switch the naming convention used for configuration data files to be based on the IPA version rather than the specific platform. I was skeptical about this at first (i.e., I thought a platform might have quirks separate from the IPA version). But I'm now convinced the IPA version is enough to define the details of the hardware block. If any exceptions to this are found, we can treat those differently. Note: these two patches produce warnings from checkpatch.pl about updating MAINTAINERS: these can be ignored. The sixth removes unnecessary checks for alignment of DMA memory allocations, based comments from David Laight. And the last removes a symbol representing the size of a table entry, using sizeof(__le64) in its place. -Alex Alex Elder (7): net: ipa: fix all kernel-doc warnings net: ipa: store BCR register values in config data net: ipa: don't define endpoints unnecessarily net: ipa: switch to version based configuration net: ipa: use version based configuration for SC7180 net: ipa: DMA addresses are nicely aligned net: ipa: kill IPA_TABLE_ENTRY_SIZE drivers/net/ipa/Kconfig | 3 +- drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi.c | 13 +-- drivers/net/ipa/gsi_private.h | 4 +- drivers/net/ipa/gsi_trans.c | 9 +- drivers/net/ipa/gsi_trans.h | 5 +- drivers/net/ipa/ipa.h | 7 +- drivers/net/ipa/ipa_cmd.c | 2 +- drivers/net/ipa/ipa_cmd.h | 19 +++-- .../{ipa_data-sdm845.c => ipa_data-v3.5.1.c} | 39 - .../{ipa_data-sc7180.c => ipa_data-v4.2.c}| 25 +++--- drivers/net/ipa/ipa_data.h| 10 ++- drivers/net/ipa/ipa_endpoint.h| 29 +-- drivers/net/ipa/ipa_interrupt.h | 1 + drivers/net/ipa/ipa_main.c| 8 +- drivers/net/ipa/ipa_mem.h | 2 +- drivers/net/ipa/ipa_qmi.c | 10 +-- drivers/net/ipa/ipa_qmi.h | 14 ++-- drivers/net/ipa/ipa_reg.h | 21 - drivers/net/ipa/ipa_smp2p.h | 2 +- drivers/net/ipa/ipa_table.c | 83 +-- drivers/net/ipa/ipa_table.h | 6 +- 22 files changed, 153 insertions(+), 161 deletions(-) rename drivers/net/ipa/{ipa_data-sdm845.c => ipa_data-v3.5.1.c} (90%) rename drivers/net/ipa/{ipa_data-sc7180.c => ipa_data-v4.2.c} (90%) -- 2.27.0
[PATCH net-next 09/12] net: ipa: combine source and destation resource types
The ipa_resource_src and ipa_resource_dst structures are identical in form, so just replace them with a single structure. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 4 ++-- drivers/net/ipa/ipa_data-sdm845.c | 4 ++-- drivers/net/ipa/ipa_data.h| 18 +- drivers/net/ipa/ipa_resource.c| 8 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 24ff315175653..631b50fc8d534 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -163,7 +163,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }; /* Source resource configuration data for the SC7180 SoC */ -static const struct ipa_resource_src ipa_resource_src[] = { +static const struct ipa_resource ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 3, .max = 63, @@ -192,7 +192,7 @@ static const struct ipa_resource_src ipa_resource_src[] = { }; /* Destination resource configuration data for the SC7180 SoC */ -static const struct ipa_resource_dst ipa_resource_dst[] = { +static const struct ipa_resource ipa_resource_dst[] = { [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { .min = 3, .max = 3, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 357e8ba43a364..3c9675ce556ce 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -181,7 +181,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }; /* Source resource configuration data for the SDM845 SoC */ -static const struct ipa_resource_src ipa_resource_src[] = { +static const struct ipa_resource ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { .min = 1, .max = 255, @@ -243,7 +243,7 @@ static const struct ipa_resource_src ipa_resource_src[] = { }; /* Destination resource configuration data for the SDM845 SoC */ -static const struct ipa_resource_dst ipa_resource_dst[] = { +static const struct ipa_resource ipa_resource_dst[] = { [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_LWA_DL] = { .min = 4, .max = 4, diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index d6d14818a3968..9060586eb7cba 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -204,18 +204,10 @@ struct ipa_resource_limits { }; /** - * struct ipa_resource_src - source endpoint group resource usage - * @limits:array of source resource limits, indexed by group + * struct ipa_resource - resource group source or destination resource usage + * @limits:array of resource limits, indexed by group */ -struct ipa_resource_src { - struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX]; -}; - -/** - * struct ipa_resource_dst - destination endpoint group resource usage - * @limits:array of destination resource limits, indexed by group - */ -struct ipa_resource_dst { +struct ipa_resource { struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX]; }; @@ -233,9 +225,9 @@ struct ipa_resource_dst { */ struct ipa_resource_data { u32 resource_src_count; - const struct ipa_resource_src *resource_src; + const struct ipa_resource *resource_src; u32 resource_dst_count; - const struct ipa_resource_dst *resource_dst; + const struct ipa_resource *resource_dst; }; /** diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index 38b95b6a5193d..c7edacfa7d19d 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -87,7 +87,7 @@ static bool ipa_resource_limits_valid(struct ipa *ipa, * for a resource group not supported by hardware. */ for (i = 0; i < data->resource_src_count; i++) { - const struct ipa_resource_src *resource; + const struct ipa_resource *resource; resource = &data->resource_src[i]; for (j = group_count; j < IPA_RESOURCE_GROUP_MAX; j++) @@ -100,7 +100,7 @@ static bool ipa_resource_limits_valid(struct ipa *ipa, return false; for (i = 0; i < data->resource_dst_count; i++) { - const struct ipa_resource_dst *resource; + const struct ipa_resource *resource; resource = &data->resource_dst[i]; for (j = group_count; j < IPA_RESOURCE_GROUP_MAX; j++) @@ -129,7 +129,7 @@ ipa_resource_config_common(struct ipa *ipa, u32 offset, } static void ipa_resource_config_src(st
[PATCH net-next 11/12] net: ipa: record number of groups in data
The arrays of source and destination resource limits defined in configuration data are of a fixed size--which is the maximum number of resource groups supported for any platform. Most platforms will use fewer than that many groups. Add new members to the ipa_rsrc_group_id enumerated type to define the number of source and destination resource groups are defined for the platform. (This type is defined for each platform in its data file.) Add a new field to the resource configuration data that indicates how many of the source and destination resource groups are actually used for the platform, and initialize it with the count value. This allows us to determine the number of groups defined for the platform without exposing the ipa_rsrc_group_id enumerated type. As a result, we no longer need ipa_resource_group_src_count() and ipa_resource_group_dst_count(), because each platform now defines its supported number of resource groups. So get rid of those two functions. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 4 +++ drivers/net/ipa/ipa_data-sdm845.c | 4 +++ drivers/net/ipa/ipa_data.h| 4 +++ drivers/net/ipa/ipa_resource.c| 50 +++ 4 files changed, 16 insertions(+), 46 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 631b50fc8d534..c9b6a6aaadacc 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -27,9 +27,11 @@ enum ipa_resource_type { enum ipa_rsrc_group_id { /* Source resource group identifiers */ IPA_RSRC_GROUP_SRC_UL_DL= 0, + IPA_RSRC_GROUP_SRC_COUNT, /* Last in set; not a source group */ /* Destination resource group identifiers */ IPA_RSRC_GROUP_DST_UL_DL_DPL= 0, + IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ }; /* QSB configuration for the SC7180 SoC. */ @@ -207,6 +209,8 @@ static const struct ipa_resource ipa_resource_dst[] = { /* Resource configuration for the SC7180 SoC. */ static const struct ipa_resource_data ipa_resource_data = { + .rsrc_group_src_count = IPA_RSRC_GROUP_SRC_COUNT, + .rsrc_group_dst_count = IPA_RSRC_GROUP_DST_COUNT, .resource_src_count = ARRAY_SIZE(ipa_resource_src), .resource_src = ipa_resource_src, .resource_dst_count = ARRAY_SIZE(ipa_resource_dst), diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 3c9675ce556ce..e14e3fb1d9700 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -32,11 +32,13 @@ enum ipa_rsrc_group_id { IPA_RSRC_GROUP_SRC_UL_DL, IPA_RSRC_GROUP_SRC_MHI_DMA, IPA_RSRC_GROUP_SRC_UC_RX_Q, + IPA_RSRC_GROUP_SRC_COUNT, /* Last in set; not a source group */ /* Destination resource group identifiers */ IPA_RSRC_GROUP_DST_LWA_DL = 0, IPA_RSRC_GROUP_DST_UL_DL_DPL, IPA_RSRC_GROUP_DST_UNUSED_2, + IPA_RSRC_GROUP_DST_COUNT, /* Last; not a destination group */ }; /* QSB configuration for the SDM845 SoC. */ @@ -270,6 +272,8 @@ static const struct ipa_resource ipa_resource_dst[] = { /* Resource configuration for the SDM845 SoC. */ static const struct ipa_resource_data ipa_resource_data = { + .rsrc_group_src_count = IPA_RSRC_GROUP_SRC_COUNT, + .rsrc_group_dst_count = IPA_RSRC_GROUP_DST_COUNT, .resource_src_count = ARRAY_SIZE(ipa_resource_src), .resource_src = ipa_resource_src, .resource_dst_count = ARRAY_SIZE(ipa_resource_dst), diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index 9060586eb7cba..c5d763a3782fd 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -213,6 +213,8 @@ struct ipa_resource { /** * struct ipa_resource_data - IPA resource configuration data + * @rsrc_group_src_count: number of source resource groups supported + * @rsrc_group_dst_count: number of destination resource groups supported * @resource_src_count:number of entries in the resource_src array * @resource_src: source endpoint group resources * @resource_dst_count:number of entries in the resource_dst array @@ -224,6 +226,8 @@ struct ipa_resource { * programming it at initialization time, so we specify it here. */ struct ipa_resource_data { + u32 rsrc_group_src_count; + u32 rsrc_group_dst_count; u32 resource_src_count; const struct ipa_resource *resource_src; u32 resource_dst_count; diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index 3db4dd3bda9cc..578ff070d4055 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -26,48 +26,6 @@ * total resources of each type is assigned for use by each group. */ -/* # IPA source resource groups available based on version */ -static u32
[PATCH net-next 12/12] net: ipa: support more than 6 resource groups
IPA versions 3.0 and 3.1 support up to 8 resource groups. There is some interest in supporting these older versions of the hardware, so update the resource configuration code to program resource limits for these groups if specified. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data.h | 4 ++-- drivers/net/ipa/ipa_reg.h | 4 drivers/net/ipa/ipa_resource.c | 18 -- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index c5d763a3782fd..ea8f99286228e 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -46,8 +46,8 @@ * the IPA endpoint. */ -/* The maximum value returned by ipa_resource_group_{src,dst}_count() */ -#define IPA_RESOURCE_GROUP_MAX 5 +/* The maximum possible number of source or destination resource groups */ +#define IPA_RESOURCE_GROUP_MAX 8 /** enum ipa_qsb_master_id - array index for IPA QSB configuration data */ enum ipa_qsb_master_id { diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 9c798cef7b2e2..de2a944bad86b 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -353,12 +353,16 @@ enum ipa_pulse_gran { (0x0404 + 0x0020 * (rt)) #define IPA_REG_SRC_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(rt) \ (0x0408 + 0x0020 * (rt)) +#define IPA_REG_SRC_RSRC_GRP_67_RSRC_TYPE_N_OFFSET(rt) \ + (0x040c + 0x0020 * (rt)) #define IPA_REG_DST_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(rt) \ (0x0500 + 0x0020 * (rt)) #define IPA_REG_DST_RSRC_GRP_23_RSRC_TYPE_N_OFFSET(rt) \ (0x0504 + 0x0020 * (rt)) #define IPA_REG_DST_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(rt) \ (0x0508 + 0x0020 * (rt)) +#define IPA_REG_DST_RSRC_GRP_67_RSRC_TYPE_N_OFFSET(rt) \ + (0x050c + 0x0020 * (rt)) /* The next four fields are used for all resource group registers */ #define X_MIN_LIM_FMASKGENMASK(5, 0) #define X_MAX_LIM_FMASKGENMASK(13, 8) diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index 578ff070d4055..85f922d6f222f 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -34,8 +34,8 @@ static bool ipa_resource_limits_valid(struct ipa *ipa, u32 i; u32 j; - /* We program at most 6 source or destination resource group limits */ - BUILD_BUG_ON(IPA_RESOURCE_GROUP_MAX > 6); + /* We program at most 8 source or destination resource group limits */ + BUILD_BUG_ON(IPA_RESOURCE_GROUP_MAX > 8); group_count = data->rsrc_group_src_count; if (!group_count || group_count > IPA_RESOURCE_GROUP_MAX) @@ -113,6 +113,13 @@ static void ipa_resource_config_src(struct ipa *ipa, u32 resource_type, offset = IPA_REG_SRC_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(resource_type); ylimits = group_count == 5 ? NULL : &resource->limits[5]; ipa_resource_config_common(ipa, offset, &resource->limits[4], ylimits); + + if (group_count < 7) + return; + + offset = IPA_REG_SRC_RSRC_GRP_67_RSRC_TYPE_N_OFFSET(resource_type); + ylimits = group_count == 7 ? NULL : &resource->limits[7]; + ipa_resource_config_common(ipa, offset, &resource->limits[6], ylimits); } static void ipa_resource_config_dst(struct ipa *ipa, u32 resource_type, @@ -142,6 +149,13 @@ static void ipa_resource_config_dst(struct ipa *ipa, u32 resource_type, offset = IPA_REG_DST_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(resource_type); ylimits = group_count == 5 ? NULL : &resource->limits[5]; ipa_resource_config_common(ipa, offset, &resource->limits[4], ylimits); + + if (group_count < 7) + return; + + offset = IPA_REG_DST_RSRC_GRP_67_RSRC_TYPE_N_OFFSET(resource_type); + ylimits = group_count == 7 ? NULL : &resource->limits[7]; + ipa_resource_config_common(ipa, offset, &resource->limits[6], ylimits); } /* Configure resources */ -- 2.27.0
[PATCH net-next 10/12] net: ipa: pass data for source and dest resource config
Pass the resource data pointer to ipa_resource_config_src() and ipa_resource_config_dst() to be used for configuring resource limits. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_resource.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index c7edacfa7d19d..3db4dd3bda9cc 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -129,12 +129,15 @@ ipa_resource_config_common(struct ipa *ipa, u32 offset, } static void ipa_resource_config_src(struct ipa *ipa, u32 resource_type, - const struct ipa_resource *resource) + const struct ipa_resource_data *data) { u32 group_count = ipa_resource_group_src_count(ipa->version); const struct ipa_resource_limits *ylimits; + const struct ipa_resource *resource; u32 offset; + resource = &data->resource_src[resource_type]; + offset = IPA_REG_SRC_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource_type); ylimits = group_count == 1 ? NULL : &resource->limits[1]; ipa_resource_config_common(ipa, offset, &resource->limits[0], ylimits); @@ -155,12 +158,15 @@ static void ipa_resource_config_src(struct ipa *ipa, u32 resource_type, } static void ipa_resource_config_dst(struct ipa *ipa, u32 resource_type, - const struct ipa_resource *resource) + const struct ipa_resource_data *data) { u32 group_count = ipa_resource_group_dst_count(ipa->version); const struct ipa_resource_limits *ylimits; + const struct ipa_resource *resource; u32 offset; + resource = &data->resource_dst[resource_type]; + offset = IPA_REG_DST_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource_type); ylimits = group_count == 1 ? NULL : &resource->limits[1]; ipa_resource_config_common(ipa, offset, &resource->limits[0], ylimits); @@ -189,10 +195,10 @@ int ipa_resource_config(struct ipa *ipa, const struct ipa_resource_data *data) return -EINVAL; for (i = 0; i < data->resource_src_count; i++) - ipa_resource_config_src(ipa, i, &data->resource_src[i]); + ipa_resource_config_src(ipa, i, data); for (i = 0; i < data->resource_dst_count; i++) - ipa_resource_config_dst(ipa, i, &data->resource_dst[i]); + ipa_resource_config_dst(ipa, i, data); return 0; } -- 2.27.0
[PATCH net-next 08/12] net: ipa: combine source and destination group limits
Replace IPA_RESOURCE_GROUP_SRC_MAX and IPA_RESOURCE_GROUP_DST_MAX with a single symbol, IPA_RESOURCE_GROUP_MAX. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data.h | 11 +-- drivers/net/ipa/ipa_resource.c | 10 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index e1096d8ba5751..d6d14818a3968 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -47,8 +47,7 @@ */ /* The maximum value returned by ipa_resource_group_{src,dst}_count() */ -#define IPA_RESOURCE_GROUP_SRC_MAX 5 -#define IPA_RESOURCE_GROUP_DST_MAX 5 +#define IPA_RESOURCE_GROUP_MAX 5 /** enum ipa_qsb_master_id - array index for IPA QSB configuration data */ enum ipa_qsb_master_id { @@ -206,18 +205,18 @@ struct ipa_resource_limits { /** * struct ipa_resource_src - source endpoint group resource usage - * @limits:array of limits to use for each resource group + * @limits:array of source resource limits, indexed by group */ struct ipa_resource_src { - struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_SRC_MAX]; + struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX]; }; /** * struct ipa_resource_dst - destination endpoint group resource usage - * @limits:array of limits to use for each resource group + * @limits:array of destination resource limits, indexed by group */ struct ipa_resource_dst { - struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_DST_MAX]; + struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX]; }; /** diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index 506bcccaef64f..38b95b6a5193d 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -77,10 +77,10 @@ static bool ipa_resource_limits_valid(struct ipa *ipa, u32 j; /* We program at most 6 source or destination resource group limits */ - BUILD_BUG_ON(IPA_RESOURCE_GROUP_SRC_MAX > 6); + BUILD_BUG_ON(IPA_RESOURCE_GROUP_MAX > 6); group_count = ipa_resource_group_src_count(ipa->version); - if (!group_count || group_count > IPA_RESOURCE_GROUP_SRC_MAX) + if (!group_count || group_count > IPA_RESOURCE_GROUP_MAX) return false; /* Return an error if a non-zero resource limit is specified @@ -90,20 +90,20 @@ static bool ipa_resource_limits_valid(struct ipa *ipa, const struct ipa_resource_src *resource; resource = &data->resource_src[i]; - for (j = group_count; j < IPA_RESOURCE_GROUP_SRC_MAX; j++) + for (j = group_count; j < IPA_RESOURCE_GROUP_MAX; j++) if (resource->limits[j].min || resource->limits[j].max) return false; } group_count = ipa_resource_group_dst_count(ipa->version); - if (!group_count || group_count > IPA_RESOURCE_GROUP_DST_MAX) + if (!group_count || group_count > IPA_RESOURCE_GROUP_MAX) return false; for (i = 0; i < data->resource_dst_count; i++) { const struct ipa_resource_dst *resource; resource = &data->resource_dst[i]; - for (j = group_count; j < IPA_RESOURCE_GROUP_DST_MAX; j++) + for (j = group_count; j < IPA_RESOURCE_GROUP_MAX; j++) if (resource->limits[j].min || resource->limits[j].max) return false; } -- 2.27.0
[PATCH net-next 07/12] net: ipa: move ipa_resource_type definition
Most platforms have the same set of source and destination resource types. But some older platforms have some additional ones, and it's possible different resources will be used in the future. Move the definition of the ipa_resource_type enumerated type so it is defined for each platform in its configuration data file. This permits each to have a distinct set of resources. Shorten the data files slightly, by putting the min and max limit values on the same line. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 35 -- drivers/net/ipa/ipa_data-sdm845.c | 80 ++- drivers/net/ipa/ipa_data.h| 14 -- 3 files changed, 57 insertions(+), 72 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index eba14d7bc8ac3..24ff315175653 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -9,6 +9,20 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" +/** enum ipa_resource_type - IPA resource types */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, + IPA_RESOURCE_TYPE_DST_DPS_DMARS, +}; + /* Resource groups used for the SC7180 SoC */ enum ipa_rsrc_group_id { /* Source resource group identifiers */ @@ -152,32 +166,27 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { static const struct ipa_resource_src ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { - .min = 3, - .max = 63, + .min = 3, .max = 63, }, }, [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { - .min = 3, - .max = 3, + .min = 3, .max = 3, }, }, [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { - .min = 10, - .max = 10, + .min = 10, .max = 10, }, }, [IPA_RESOURCE_TYPE_SRC_HPS_DMARS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { - .min = 1, - .max = 1, + .min = 1, .max = 1, }, }, [IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { - .min = 5, - .max = 5, + .min = 5, .max = 5, }, }, }; @@ -186,14 +195,12 @@ static const struct ipa_resource_src ipa_resource_src[] = { static const struct ipa_resource_dst ipa_resource_dst[] = { [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { - .min = 3, - .max = 3, + .min = 3, .max = 3, }, }, [IPA_RESOURCE_TYPE_DST_DPS_DMARS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { - .min = 1, - .max = 63, + .min = 1, .max = 63, }, }, }; diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 4a4b3bd8a17c0..357e8ba43a364 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -11,6 +11,20 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" +/** enum ipa_resource_type - IPA resource types */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, + IPA_RESOURCE_TYPE_DST_DPS_DMARS, +}; + /* Resource groups used for the SDM845 SoC */ enum ipa_rsrc_group_id { /* Source resource group identifiers */ @@ -170,76 +184,60 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { static const struct ipa_resource_src ipa_resource_src[] = { [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP
[PATCH net-next 06/12] net: ipa: index resource limits with type
Remove the type field from the ipa_resource_src and ipa_resource_dst structures, and instead use that value as the index into the arrays of source and destination resources. Change ipa_resource_config_src() and ipa_resource_config_dst() so the resource type is passed in as an argument. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 21 +++-- drivers/net/ipa/ipa_data-sdm845.c | 21 +++-- drivers/net/ipa/ipa_data.h| 4 drivers/net/ipa/ipa_resource.c| 20 ++-- 4 files changed, 24 insertions(+), 42 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index e9b741832a1d7..eba14d7bc8ac3 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -150,36 +150,31 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { /* Source resource configuration data for the SC7180 SoC */ static const struct ipa_resource_src ipa_resource_src[] = { - { - .type = IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS, + [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 3, .max = 63, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 3, .max = 3, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 10, .max = 10, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + [IPA_RESOURCE_TYPE_SRC_HPS_DMARS] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 1, .max = 1, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, + [IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES] = { .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 5, .max = 5, @@ -189,15 +184,13 @@ static const struct ipa_resource_src ipa_resource_src[] = { /* Destination resource configuration data for the SC7180 SoC */ static const struct ipa_resource_dst ipa_resource_dst[] = { - { - .type = IPA_RESOURCE_TYPE_DST_DATA_SECTORS, + [IPA_RESOURCE_TYPE_DST_DATA_SECTORS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { .min = 3, .max = 3, }, }, - { - .type = IPA_RESOURCE_TYPE_DST_DPS_DMARS, + [IPA_RESOURCE_TYPE_DST_DPS_DMARS] = { .limits[IPA_RSRC_GROUP_DST_UL_DL_DPL] = { .min = 1, .max = 63, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 3bc5fcfdf960c..4a4b3bd8a17c0 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -168,8 +168,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { /* Source resource configuration data for the SDM845 SoC */ static const struct ipa_resource_src ipa_resource_src[] = { - { - .type = IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS, + [IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { .min = 1, .max = 255, @@ -183,8 +182,7 @@ static const struct ipa_resource_src ipa_resource_src[] = { .max = 63, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, + [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { .min = 10, .max = 10, @@ -198,8 +196,7 @@ static const struct ipa_resource_src ipa_resource_src[] = { .max = 8, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, + [IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { .min = 12, .max = 12, @@ -213,8 +210,7 @@ static const struct ipa_resource_src ipa_resource_src[] = { .max = 8, }, }, - { - .type = IPA_RESOURCE_TYPE_SRC_HPS_DMARS, + [IPA_RESOURCE_TYPE_SRC_HPS_DMARS] = { .limits[IPA_RSRC_GROUP_SRC_LWA_DL] = { .min = 0, .max = 63, @@ -232,8 +228,7 @@ static const struct ipa_resource_src ipa_resource_src
[PATCH net-next 05/12] net: ipa: combine resource type definitions
Combine the ipa_resource_type_src and ipa_resource_type_dst enumerated types into a single enumerated type, ipa_resource_type. Assign value 0 to the first element for the source and destination types, so their numeric values are preserved. Add some additional commentary where these are defined, stating explicitly that code assumes the first source and first destination member must have numeric value 0. Fix the kerneldoc comments for the ipa_gsi_endpoint_data structure. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data.h | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index 7816583fc14aa..ccd7fd0b801aa 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2019-2020 Linaro Ltd. + * Copyright (C) 2019-2021 Linaro Ltd. */ #ifndef _IPA_DATA_H_ #define _IPA_DATA_H_ @@ -177,12 +177,12 @@ struct ipa_endpoint_data { /** * struct ipa_gsi_endpoint_data - GSI channel/IPA endpoint data - * ee: GSI execution environment ID - * channel_id: GSI channel ID - * endpoint_id:IPA endpoint ID - * toward_ipa: direction of data transfer - * gsi:GSI channel configuration data (see above) - * ipa:IPA endpoint configuration data (see above) + * @ee_id: GSI execution environment ID + * @channel_id:GSI channel ID + * @endpoint_id: IPA endpoint ID + * @toward_ipa:direction of data transfer + * @channel: GSI channel configuration data (see above) + * @endpoint: IPA endpoint configuration data (see above) */ struct ipa_gsi_endpoint_data { u8 ee_id; /* enum gsi_ee_id */ @@ -194,18 +194,17 @@ struct ipa_gsi_endpoint_data { struct ipa_endpoint_data endpoint; }; -/** enum ipa_resource_type_src - source resource types */ -enum ipa_resource_type_src { - IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS, +/** enum ipa_resource_type - IPA resource types */ +enum ipa_resource_type { + /* Source resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS = 0, IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, IPA_RESOURCE_TYPE_SRC_HPS_DMARS, IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, -}; -/** enum ipa_resource_type_dst - destination resource types */ -enum ipa_resource_type_dst { - IPA_RESOURCE_TYPE_DST_DATA_SECTORS, + /* Destination resource types; first must have value 0 */ + IPA_RESOURCE_TYPE_DST_DATA_SECTORS = 0, IPA_RESOURCE_TYPE_DST_DPS_DMARS, }; @@ -225,7 +224,7 @@ struct ipa_resource_limits { * @limits:array of limits to use for each resource group */ struct ipa_resource_src { - enum ipa_resource_type_src type; + enum ipa_resource_type type;/* IPA_RESOURCE_TYPE_SRC_* */ struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_SRC_MAX]; }; @@ -235,7 +234,7 @@ struct ipa_resource_src { * @limits:array of limits to use for each resource group */ struct ipa_resource_dst { - enum ipa_resource_type_dst type; + enum ipa_resource_type type;/* IPA_RESOURCE_TYPE_DST_* */ struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_DST_MAX]; }; -- 2.27.0
[PATCH net-next 02/12] net: ipa: fix bug in resource group limit programming
If the number of resource groups supported by the hardware is less than a certain number, we return early in ipa_resource_config_src() and ipa_resource_config_dst() (to avoid programming resource limits for non-existent groups). Unfortunately, these checks are off by one. Fix this problem in the four places it occurs. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_resource.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_resource.c b/drivers/net/ipa/ipa_resource.c index 2f0f2dca36785..edd9d1e5cbb62 100644 --- a/drivers/net/ipa/ipa_resource.c +++ b/drivers/net/ipa/ipa_resource.c @@ -139,14 +139,14 @@ static void ipa_resource_config_src(struct ipa *ipa, ylimits = group_count == 1 ? NULL : &resource->limits[1]; ipa_resource_config_common(ipa, offset, &resource->limits[0], ylimits); - if (group_count < 2) + if (group_count < 3) return; offset = IPA_REG_SRC_RSRC_GRP_23_RSRC_TYPE_N_OFFSET(resource->type); ylimits = group_count == 3 ? NULL : &resource->limits[3]; ipa_resource_config_common(ipa, offset, &resource->limits[2], ylimits); - if (group_count < 4) + if (group_count < 5) return; offset = IPA_REG_SRC_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(resource->type); @@ -165,14 +165,14 @@ static void ipa_resource_config_dst(struct ipa *ipa, ylimits = group_count == 1 ? NULL : &resource->limits[1]; ipa_resource_config_common(ipa, offset, &resource->limits[0], ylimits); - if (group_count < 2) + if (group_count < 3) return; offset = IPA_REG_DST_RSRC_GRP_23_RSRC_TYPE_N_OFFSET(resource->type); ylimits = group_count == 3 ? NULL : &resource->limits[3]; ipa_resource_config_common(ipa, offset, &resource->limits[2], ylimits); - if (group_count < 4) + if (group_count < 5) return; offset = IPA_REG_DST_RSRC_GRP_45_RSRC_TYPE_N_OFFSET(resource->type); -- 2.27.0
[PATCH net-next 03/12] net: ipa: identify resource groups
Define a new ipa_resource_group_id enumerated type, whose members have numeric values that match the resource group number used when programming the hardware. Each platform supports a different number of source and destination resource groups, so define the type separately for each platform in its configuration data file. Use these new symbolic values when specifying the resource group an endpoint is associated with. And use them to index the limits arrays for source and destination resources, making it clearer how these values are used. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 38 - drivers/net/ipa/ipa_data-sdm845.c | 56 +++ 2 files changed, 57 insertions(+), 37 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 621ad15c9e67d..e9b741832a1d7 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 -/* Copyright (C) 2019-2020 Linaro Ltd. */ +/* Copyright (C) 2019-2021 Linaro Ltd. */ #include @@ -9,6 +9,15 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" +/* Resource groups used for the SC7180 SoC */ +enum ipa_rsrc_group_id { + /* Source resource group identifiers */ + IPA_RSRC_GROUP_SRC_UL_DL= 0, + + /* Destination resource group identifiers */ + IPA_RSRC_GROUP_DST_UL_DL_DPL= 0, +}; + /* QSB configuration for the SC7180 SoC. */ static const struct ipa_qsb_data ipa_qsb_data[] = { [IPA_QSB_MASTER_DDR] = { @@ -32,7 +41,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .config = { - .resource_group = 0, + .resource_group = IPA_RSRC_GROUP_SRC_UL_DL, .dma_mode = true, .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, .tx = { @@ -53,7 +62,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .config = { - .resource_group = 0, + .resource_group = IPA_RSRC_GROUP_DST_UL_DL_DPL, .aggregation= true, .status_enable = true, .rx = { @@ -75,7 +84,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .endpoint = { .filter_support = true, .config = { - .resource_group = 0, + .resource_group = IPA_RSRC_GROUP_SRC_UL_DL, .checksum = true, .qmap = true, .status_enable = true, @@ -100,7 +109,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .config = { - .resource_group = 0, + .resource_group = IPA_RSRC_GROUP_DST_UL_DL_DPL, .checksum = true, .qmap = true, .aggregation= true, @@ -139,58 +148,57 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, }; -/* For the SC7180, resource groups are allocated this way: - * group 0: UL_DL - */ +/* Source resource configuration data for the SC7180 SoC */ static const struct ipa_resource_src ipa_resource_src[] = { { .type = IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS, - .limits[0] = { + .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 3, .max = 63, }, }, { .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, - .limits[0] = { + .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 3, .max = 3, }, }, { .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, - .limits[0] = { + .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 10, .max = 10, }, }, { .type = IPA_RESOURCE_TYPE_SRC_HPS_DMARS, - .limits[0] = { + .limits[IPA_RSRC_GROUP_SRC_UL_DL] = { .min = 1, .max = 1, }, }, { .type = IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, - .limits[0] = { + .limits[IPA_
[PATCH net-next 04/12] net: ipa: add some missing resource limits
Currently, the SDM845 configuration data defines resource limits for the first two resource groups (for both source and destination resource types). The hardware supports additional resource groups, and we should program the resource limits for those groups as well. Even the "unused" destination resource group (number 2) should have non-zero limits programmed in some cases, to ensure correct operation. Add these missing resource group limit definitions to the SDM845 configuration data. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sdm845.c | 32 +++ 1 file changed, 32 insertions(+) diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index b6ea6295e7598..3bc5fcfdf960c 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -178,6 +178,10 @@ static const struct ipa_resource_src ipa_resource_src[] = { .min = 1, .max = 255, }, + .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = { + .min = 1, + .max = 63, + }, }, { .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS, @@ -189,6 +193,10 @@ static const struct ipa_resource_src ipa_resource_src[] = { .min = 10, .max = 10, }, + .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = { + .min = 8, + .max = 8, + }, }, { .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF, @@ -200,6 +208,10 @@ static const struct ipa_resource_src ipa_resource_src[] = { .min = 14, .max = 14, }, + .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = { + .min = 8, + .max = 8, + }, }, { .type = IPA_RESOURCE_TYPE_SRC_HPS_DMARS, @@ -211,6 +223,14 @@ static const struct ipa_resource_src ipa_resource_src[] = { .min = 0, .max = 63, }, + .limits[IPA_RSRC_GROUP_SRC_MHI_DMA] = { + .min = 0, + .max = 63, + }, + .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = { + .min = 0, + .max = 63, + }, }, { .type = IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES, @@ -222,6 +242,10 @@ static const struct ipa_resource_src ipa_resource_src[] = { .min = 20, .max = 20, }, + .limits[IPA_RSRC_GROUP_SRC_UC_RX_Q] = { + .min = 14, + .max = 14, + }, }, }; @@ -237,6 +261,10 @@ static const struct ipa_resource_dst ipa_resource_dst[] = { .min = 4, .max = 4, }, + .limits[IPA_RSRC_GROUP_DST_UNUSED_2] = { + .min = 3, + .max = 3, + } }, { .type = IPA_RESOURCE_TYPE_DST_DPS_DMARS, @@ -248,6 +276,10 @@ static const struct ipa_resource_dst ipa_resource_dst[] = { .min = 1, .max = 63, }, + .limits[IPA_RSRC_GROUP_DST_UNUSED_2] = { + .min = 1, + .max = 2, + } }, }; -- 2.27.0
[PATCH net-next 01/12] net: ipa: introduce ipa_resource.c
Separate the IPA resource-related code into a new source file, "ipa_resource.c", and matching header file "ipa_resource.h". Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/ipa_main.c | 148 +--- drivers/net/ipa/ipa_reg.h | 42 --- drivers/net/ipa/ipa_resource.c | 204 + drivers/net/ipa/ipa_resource.h | 27 + 5 files changed, 234 insertions(+), 189 deletions(-) create mode 100644 drivers/net/ipa/ipa_resource.c create mode 100644 drivers/net/ipa/ipa_resource.h diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index afe5df1e6..14a7d8429baa2 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -7,6 +7,6 @@ ipa-y := ipa_main.o ipa_clock.o ipa_reg.o ipa_mem.o \ ipa_table.o ipa_interrupt.o gsi.o gsi_trans.o \ ipa_gsi.o ipa_smp2p.o ipa_uc.o \ ipa_endpoint.o ipa_cmd.o ipa_modem.o \ - ipa_qmi.o ipa_qmi_msg.o + ipa_resource.o ipa_qmi.o ipa_qmi_msg.o ipa-y += ipa_data-sdm845.o ipa_data-sc7180.o diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index f071e90de5409..e18029152d780 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2018-2020 Linaro Ltd. + * Copyright (C) 2018-2021 Linaro Ltd. */ #include @@ -22,6 +22,7 @@ #include "ipa_clock.h" #include "ipa_data.h" #include "ipa_endpoint.h" +#include "ipa_resource.h" #include "ipa_cmd.h" #include "ipa_reg.h" #include "ipa_mem.h" @@ -452,151 +453,6 @@ static void ipa_hardware_deconfig(struct ipa *ipa) ipa_hardware_dcd_deconfig(ipa); } -#ifdef IPA_VALIDATION - -static bool ipa_resource_limits_valid(struct ipa *ipa, - const struct ipa_resource_data *data) -{ - u32 group_count; - u32 i; - u32 j; - - /* We program at most 6 source or destination resource group limits */ - BUILD_BUG_ON(IPA_RESOURCE_GROUP_SRC_MAX > 6); - - group_count = ipa_resource_group_src_count(ipa->version); - if (!group_count || group_count > IPA_RESOURCE_GROUP_SRC_MAX) - return false; - - /* Return an error if a non-zero resource limit is specified -* for a resource group not supported by hardware. -*/ - for (i = 0; i < data->resource_src_count; i++) { - const struct ipa_resource_src *resource; - - resource = &data->resource_src[i]; - for (j = group_count; j < IPA_RESOURCE_GROUP_SRC_MAX; j++) - if (resource->limits[j].min || resource->limits[j].max) - return false; - } - - group_count = ipa_resource_group_dst_count(ipa->version); - if (!group_count || group_count > IPA_RESOURCE_GROUP_DST_MAX) - return false; - - for (i = 0; i < data->resource_dst_count; i++) { - const struct ipa_resource_dst *resource; - - resource = &data->resource_dst[i]; - for (j = group_count; j < IPA_RESOURCE_GROUP_DST_MAX; j++) - if (resource->limits[j].min || resource->limits[j].max) - return false; - } - - return true; -} - -#else /* !IPA_VALIDATION */ - -static bool ipa_resource_limits_valid(struct ipa *ipa, - const struct ipa_resource_data *data) -{ - return true; -} - -#endif /* !IPA_VALIDATION */ - -static void -ipa_resource_config_common(struct ipa *ipa, u32 offset, - const struct ipa_resource_limits *xlimits, - const struct ipa_resource_limits *ylimits) -{ - u32 val; - - val = u32_encode_bits(xlimits->min, X_MIN_LIM_FMASK); - val |= u32_encode_bits(xlimits->max, X_MAX_LIM_FMASK); - if (ylimits) { - val |= u32_encode_bits(ylimits->min, Y_MIN_LIM_FMASK); - val |= u32_encode_bits(ylimits->max, Y_MAX_LIM_FMASK); - } - - iowrite32(val, ipa->reg_virt + offset); -} - -static void ipa_resource_config_src(struct ipa *ipa, - const struct ipa_resource_src *resource) -{ - u32 group_count = ipa_resource_group_src_count(ipa->version); - const struct ipa_resource_limits *ylimits; - u32 offset; - - offset = IPA_REG_SRC_RSRC_GRP_01_RSRC_TYPE_N_OFFSET(resource->type); - ylimits = group_count == 1 ? NULL : &am
[PATCH net-next 00/12] net: ipa: rework resource programming
This series reworks the way IPA resources are defined and programmed. It is a little long--and I apologize for that--but I think the patches are best taken together as a single unit. The IPA hardware operates with a set of distinct "resources." Each hardware instance has a fixed number of each resource type available. Available resources are divided into smaller pools, with each pool shared by endpoints in a "resource group." Each endpoint is thus assigned to a resource group that determines which pools supply resources the IPA hardware uses to handle the endpoint's processing. The exact set of resources used can differ for each version of IPA. Except for IPA v3.0 and v3.1, there are 5 source and 2 destination resource types, but there's no reason to assume this won't change. The number of resource groups used *does* typically change based on the hardware version. For example, some versions target reduced functionality and support fewer resource groups. With that as background... The net result of this series is to improve the flexibility with which IPA resources and resource groups are defined, permitting each version of IPA to define its own set of resources and groups. Along the way it isolates the resource-related code, and fixes a few bugs related to resource handling. The first patch moves resource-related code to a new C file (and header). It generates a checkpatch warning about updating MAINTAINERS, which can be ignored. The second patch fixes a bug, but the bug does not affect SDM845 or SC7180. The third patch defines an enumerated type whose members provide symbolic names for resource groups. The fourth defines some resource limits for SDM845 that were not previously being programmed. That platform "works" without this, but to be correct, these limits should really be programmed. The fifth patch uses a single enumerated type to define both source and destination resource type IDs, and the sixth uses those IDs to index the resource limit arrays. The seventh moves the definition of that enumerated type into the platform data files, allowing each platform to define its own set of resource types. The eighth and ninth are fairly trivial changes. One replaces two "max" symbols having the same value with a single symbol. And the other replaces two distinct but otherwise identical structure types with a single common one. The 10th is a small preparatory patch for the 11th, passing a different argument to a function that programs resource values. The 11th allows the actual number of source and destination resource groups for a platform to be specified in its configuration data. That way the number is based on the actual number of groups defined. This removes the need for a sort of clunky pair of functions that defined that information previously. Finally, the last patch just increases the number of resource groups that can be defined to 8. -Alex Alex Elder (12): net: ipa: introduce ipa_resource.c net: ipa: fix bug in resource group limit programming net: ipa: identify resource groups net: ipa: add some missing resource limits net: ipa: combine resource type definitions net: ipa: index resource limits with type net: ipa: move ipa_resource_type definition net: ipa: combine source and destination group limits net: ipa: combine source and destation resource types net: ipa: pass data for source and dest resource config net: ipa: record number of groups in data net: ipa: support more than 6 resource groups drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/ipa_data-sc7180.c | 102 + drivers/net/ipa/ipa_data-sdm845.c | 169 --- drivers/net/ipa/ipa_data.h| 62 -- drivers/net/ipa/ipa_main.c| 148 +--- drivers/net/ipa/ipa_reg.h | 46 +--- drivers/net/ipa/ipa_resource.c| 182 ++ drivers/net/ipa/ipa_resource.h| 27 + 8 files changed, 393 insertions(+), 345 deletions(-) create mode 100644 drivers/net/ipa/ipa_resource.c create mode 100644 drivers/net/ipa/ipa_resource.h -- 2.27.0
[PATCH net-next 5/6] net: ipa: update GSI ring size registers
Each GSI channel has a CNTXT_1 register that encodes the size of its ring buffer. The size of the field that records that is increased starting at IPA v4.9. Replace the use of a fixed-size field mask with a new inline function that encodes that size value. Similarly, the size of GSI event rings can be larger starting with IPA v4.9, so create a function to encode that as well. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 7 +-- drivers/net/ipa/gsi_reg.h | 17 +++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 7f2a8fce5e0db..6f146288f9e41 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -701,7 +701,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)); - val = u32_encode_bits(size, EV_R_LENGTH_FMASK); + val = ev_r_length_encoded(gsi->version, size); iowrite32(val, gsi->virt + GSI_EV_CH_E_CNTXT_1_OFFSET(evt_ring_id)); /* The context 2 and 3 registers store the low-order and @@ -808,7 +808,7 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) val |= u32_encode_bits(GSI_RING_ELEMENT_SIZE, ELEMENT_SIZE_FMASK); iowrite32(val, gsi->virt + GSI_CH_C_CNTXT_0_OFFSET(channel_id)); - val = u32_encode_bits(size, R_LENGTH_FMASK); + val = r_length_encoded(gsi->version, size); iowrite32(val, gsi->virt + GSI_CH_C_CNTXT_1_OFFSET(channel_id)); /* The context 2 and 3 registers store the low-order and @@ -842,6 +842,9 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) val |= u32_encode_bits(GSI_ESCAPE_BUF_ONLY, PREFETCH_MODE_FMASK); } + /* All channels set DB_IN_BYTES */ + if (gsi->version >= IPA_VERSION_4_9) + val |= DB_IN_BYTES; iowrite32(val, gsi->virt + GSI_CH_C_QOS_OFFSET(channel_id)); diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h index 6b53adbc667af..d964015a4409f 100644 --- a/drivers/net/ipa/gsi_reg.h +++ b/drivers/net/ipa/gsi_reg.h @@ -90,7 +90,14 @@ enum gsi_channel_type { GSI_EE_N_CH_C_CNTXT_1_OFFSET((ch), GSI_EE_AP) #define GSI_EE_N_CH_C_CNTXT_1_OFFSET(ch, ee) \ (0x0001c004 + 0x4000 * (ee) + 0x80 * (ch)) -#define R_LENGTH_FMASK GENMASK(15, 0) + +/* Encoded value for CH_C_CNTXT_1 register R_LENGTH field */ +static inline u32 r_length_encoded(enum ipa_version version, u32 length) +{ + if (version < IPA_VERSION_4_9) + return u32_encode_bits(length, GENMASK(15, 0)); + return u32_encode_bits(length, GENMASK(19, 0)); +} #define GSI_CH_C_CNTXT_2_OFFSET(ch) \ GSI_EE_N_CH_C_CNTXT_2_OFFSET((ch), GSI_EE_AP) @@ -161,7 +168,13 @@ enum gsi_prefetch_mode { GSI_EE_N_EV_CH_E_CNTXT_1_OFFSET((ev), GSI_EE_AP) #define GSI_EE_N_EV_CH_E_CNTXT_1_OFFSET(ev, ee) \ (0x0001d004 + 0x4000 * (ee) + 0x80 * (ev)) -#define EV_R_LENGTH_FMASK GENMASK(15, 0) +/* Encoded value for EV_CH_C_CNTXT_1 register EV_R_LENGTH field */ +static inline u32 ev_r_length_encoded(enum ipa_version version, u32 length) +{ + if (version < IPA_VERSION_4_9) + return u32_encode_bits(length, GENMASK(15, 0)); + return u32_encode_bits(length, GENMASK(19, 0)); +} #define GSI_EV_CH_E_CNTXT_2_OFFSET(ev) \ GSI_EE_N_EV_CH_E_CNTXT_2_OFFSET((ev), GSI_EE_AP) -- 2.27.0
[PATCH net-next 6/6] net: ipa: expand GSI channel types
IPA v4.5 (GSI v2.5) supports a larger set of channel protocols, and adds an additional field to hold the most-significant bits of the protocol identifier on a channel. Add an inline function that encodes the protocol (including the extra bits for newer versions of IPA), and define some additional protocols. At this point we still use only GPI protocol. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 2 +- drivers/net/ipa/gsi_reg.h | 38 +++--- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 6f146288f9e41..585574af36ecd 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -801,7 +801,7 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell) channel->tre_ring.index = 0; /* We program all channels as GPI type/protocol */ - val = u32_encode_bits(GSI_CHANNEL_TYPE_GPI, CHTYPE_PROTOCOL_FMASK); + val = chtype_protocol_encoded(gsi->version, GSI_CHANNEL_TYPE_GPI); if (channel->toward_ipa) val |= CHTYPE_DIR_FMASK; val |= u32_encode_bits(channel->evt_ring_id, ERINDEX_FMASK); diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h index d964015a4409f..b4ac0258d6e10 100644 --- a/drivers/net/ipa/gsi_reg.h +++ b/drivers/net/ipa/gsi_reg.h @@ -64,6 +64,21 @@ (0xc01c + 0x1000 * (ee)) /* All other register offsets are relative to gsi->virt */ + +/** enum gsi_channel_type - CHTYPE_PROTOCOL field values in CH_C_CNTXT_0 */ +enum gsi_channel_type { + GSI_CHANNEL_TYPE_MHI= 0x0, + GSI_CHANNEL_TYPE_XHCI = 0x1, + GSI_CHANNEL_TYPE_GPI= 0x2, + GSI_CHANNEL_TYPE_XDCI = 0x3, + GSI_CHANNEL_TYPE_WDI2 = 0x4, + GSI_CHANNEL_TYPE_GCI= 0x5, + GSI_CHANNEL_TYPE_WDI3 = 0x6, + GSI_CHANNEL_TYPE_MHIP = 0x7, + GSI_CHANNEL_TYPE_AQC= 0x8, + GSI_CHANNEL_TYPE_11AD = 0x9, +}; + #define GSI_CH_C_CNTXT_0_OFFSET(ch) \ GSI_EE_N_CH_C_CNTXT_0_OFFSET((ch), GSI_EE_AP) #define GSI_EE_N_CH_C_CNTXT_0_OFFSET(ch, ee) \ @@ -78,13 +93,22 @@ #define CHSTATE_FMASK GENMASK(23, 20) #define ELEMENT_SIZE_FMASK GENMASK(31, 24) -/** enum gsi_channel_type - CHTYPE_PROTOCOL field values in CH_C_CNTXT_0 */ -enum gsi_channel_type { - GSI_CHANNEL_TYPE_MHI= 0x0, - GSI_CHANNEL_TYPE_XHCI = 0x1, - GSI_CHANNEL_TYPE_GPI= 0x2, - GSI_CHANNEL_TYPE_XDCI = 0x3, -}; +/* Encoded value for CH_C_CNTXT_0 register channel protocol fields */ +static inline u32 +chtype_protocol_encoded(enum ipa_version version, enum gsi_channel_type type) +{ + u32 val; + + val = u32_encode_bits(type, CHTYPE_PROTOCOL_FMASK); + if (version < IPA_VERSION_4_5) + return val; + + /* Encode upper bit(s) as well */ + type >>= hweight32(CHTYPE_PROTOCOL_FMASK); + val |= u32_encode_bits(type, CHTYPE_PROTOCOL_MSB_FMASK); + + return val; +} #define GSI_CH_C_CNTXT_1_OFFSET(ch) \ GSI_EE_N_CH_C_CNTXT_1_OFFSET((ch), GSI_EE_AP) -- 2.27.0
[PATCH net-next 4/6] net: ipa: GSI register cleanup
The main purpose of this is to extend these GSI register definitions to support additional IPA versions. This patch makes some minor updates to "gsi_reg.h": - Define a DB_IN_BYTES field in the channel QOS register - Add some comments clarifying when certain fields are valid - Add the definition of GSI_CH_DB_STOP channel command - Add a couple of blank lines - Move one comment and indent another - Delete two unused register definitions at the end. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi_reg.h | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h index 1622d8cf8dea4..6b53adbc667af 100644 --- a/drivers/net/ipa/gsi_reg.h +++ b/drivers/net/ipa/gsi_reg.h @@ -114,6 +114,9 @@ enum gsi_channel_type { /* The next two fields are present for IPA v4.5 and above */ #define PREFETCH_MODE_FMASKGENMASK(13, 10) #define EMPTY_LVL_THRSHOLD_FMASK GENMASK(23, 16) +/* The next field is present for IPA v4.9 and above */ +#define DB_IN_BYTESGENMASK(24, 24) + /** enum gsi_prefetch_mode - PREFETCH_MODE field in CH_C_QOS */ enum gsi_prefetch_mode { GSI_USE_PREFETCH_BUFS = 0x0, @@ -146,13 +149,13 @@ enum gsi_prefetch_mode { GSI_EE_N_EV_CH_E_CNTXT_0_OFFSET((ev), GSI_EE_AP) #define GSI_EE_N_EV_CH_E_CNTXT_0_OFFSET(ev, ee) \ (0x0001d000 + 0x4000 * (ee) + 0x80 * (ev)) +/* enum gsi_channel_type defines EV_CHTYPE field values in EV_CH_E_CNTXT_0 */ #define EV_CHTYPE_FMASKGENMASK(3, 0) #define EV_EE_FMASKGENMASK(7, 4) #define EV_EVCHID_FMASKGENMASK(15, 8) #define EV_INTYPE_FMASKGENMASK(16, 16) #define EV_CHSTATE_FMASK GENMASK(23, 20) #define EV_ELEMENT_SIZE_FMASK GENMASK(31, 24) -/* enum gsi_channel_type defines EV_CHTYPE field values in EV_CH_E_CNTXT_0 */ #define GSI_EV_CH_E_CNTXT_1_OFFSET(ev) \ GSI_EE_N_EV_CH_E_CNTXT_1_OFFSET((ev), GSI_EE_AP) @@ -248,6 +251,7 @@ enum gsi_ch_cmd_opcode { GSI_CH_STOP = 0x2, GSI_CH_RESET= 0x9, GSI_CH_DE_ALLOC = 0xa, + GSI_CH_DB_STOP = 0xb, }; #define GSI_EV_CH_CMD_OFFSET \ @@ -278,6 +282,7 @@ enum gsi_generic_cmd_opcode { GSI_GENERIC_ALLOCATE_CHANNEL= 0x2, }; +/* The next register is present for IPA v3.5.1 and above */ #define GSI_GSI_HW_PARAM_2_OFFSET \ GSI_EE_N_GSI_HW_PARAM_2_OFFSET(GSI_EE_AP) #define GSI_EE_N_GSI_HW_PARAM_2_OFFSET(ee) \ @@ -300,7 +305,7 @@ enum gsi_generic_cmd_opcode { enum gsi_iram_size { IRAM_SIZE_ONE_KB= 0x0, IRAM_SIZE_TWO_KB= 0x1, -/* The next two values are available for IPA v4.0 and above */ + /* The next two values are available for IPA v4.0 and above */ IRAM_SIZE_TWO_N_HALF_KB = 0x2, IRAM_SIZE_THREE_KB = 0x3, /* The next two values are available for IPA v4.5 and above */ @@ -424,6 +429,8 @@ enum gsi_general_id { GSI_EE_N_ERROR_LOG_OFFSET(GSI_EE_AP) #define GSI_EE_N_ERROR_LOG_OFFSET(ee) \ (0x0001f200 + 0x4000 * (ee)) + +/* Fields below are present for IPA v3.5.1 and above */ #define ERR_ARG3_FMASK GENMASK(3, 0) #define ERR_ARG2_FMASK GENMASK(7, 4) #define ERR_ARG1_FMASK GENMASK(11, 8) @@ -474,7 +481,4 @@ enum gsi_generic_ee_result { GENERIC_EE_NO_RESOURCES = 0x7, }; -#define USB_MAX_PACKET_FMASK GENMASK(15, 15) /* 0: HS; 1: SS */ -#define MHI_BASE_CHANNEL_FMASK GENMASK(31, 24) - #endif /* _GSI_REG_H_ */ -- 2.27.0
[PATCH net-next 2/6] net: ipa: update component config register
IPA version 4.9 and later use a different layout of some fields found in the COMP_CFG register. Define arbitration_lock_disable_encoded(), and use it to encode a value into the ATOMIC_FETCHER_ARB_LOCK_DIS field based on the IPA version. And define full_flush_rsc_closure_en_encoded() to encode a value into the FULL_FLUSH_WAIT_RSC_CLOSE_EN field based on the IPA version. The values of these fields are neither modified nor extracted by current code, but this patch makes this possible for all supported versions. Fix a mistaken comment above ipa_hardware_config_comp() intended to describe the purpose for the register. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 2 +- drivers/net/ipa/ipa_reg.h | 32 +--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index ba1bfc30210a3..f071e90de5409 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -222,7 +222,7 @@ static void ipa_teardown(struct ipa *ipa) gsi_teardown(&ipa->gsi); } -/* Configure QMB Core Master Port selection */ +/* Configure bus access behavior for IPA components */ static void ipa_hardware_config_comp(struct ipa *ipa) { u32 val; diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 735f6e809e042..8a654ccda49eb 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -88,9 +88,6 @@ struct ipa; #define GSI_SNOC_CNOC_LOOP_PROT_DISABLE_FMASK GENMASK(14, 14) #define GSI_MULTI_AXI_MASTERS_DIS_FMASKGENMASK(15, 15) #define IPA_QMB_SELECT_GLOBAL_EN_FMASK GENMASK(16, 16) -#define IPA_ATOMIC_FETCHER_ARB_LOCK_DIS_FMASK GENMASK(20, 17) -/* The next field is present for IPA v4.5 and IPA v4.7 */ -#define IPA_FULL_FLUSH_WAIT_RSC_CLOSE_EN_FMASK GENMASK(21, 21) /* The next five fields are present for IPA v4.9+ */ #define QMB_RAM_RD_CACHE_DISABLE_FMASK GENMASK(19, 19) #define GENQMB_AOOOWR_FMASKGENMASK(20, 20) @@ -98,6 +95,35 @@ struct ipa; #define GEN_QMB_1_DYNAMIC_ASIZE_FMASK GENMASK(30, 30) #define GEN_QMB_0_DYNAMIC_ASIZE_FMASK GENMASK(31, 31) +/* Encoded value for COMP_CFG register ATOMIC_FETCHER_ARB_LOCK_DIS field */ +static inline u32 arbitration_lock_disable_encoded(enum ipa_version version, + u32 mask) +{ + /* assert(version >= IPA_VERSION_4_0); */ + + if (version < IPA_VERSION_4_9) + return u32_encode_bits(mask, GENMASK(20, 17)); + + if (version == IPA_VERSION_4_9) + return u32_encode_bits(mask, GENMASK(24, 22)); + + return u32_encode_bits(mask, GENMASK(23, 22)); +} + +/* Encoded value for COMP_CFG register FULL_FLUSH_WAIT_RS_CLOSURE_EN field */ +static inline u32 full_flush_rsc_closure_en_encoded(enum ipa_version version, + bool enable) +{ + u32 val = enable ? 1 : 0; + + /* assert(version >= IPA_VERSION_4_5); */ + + if (version == IPA_VERSION_4_5 || version == IPA_VERSION_4_7) + return u32_encode_bits(val, GENMASK(21, 21)); + + return u32_encode_bits(val, GENMASK(17, 17)); +} + #define IPA_REG_CLKON_CFG_OFFSET 0x0044 #define RX_FMASK GENMASK(0, 0) #define PROC_FMASK GENMASK(1, 1) -- 2.27.0
[PATCH net-next 3/6] net: ipa: support IPA interrupt addresses for IPA v4.7
Starting with IPA v4.7, registers related to IPA interrupts are located at a fixed offset 0x1000 above than the addresses used for earlier versions. Define and use functions to provide the offset to use for these registers based on IPA version. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_interrupt.c | 54 +--- drivers/net/ipa/ipa_reg.h | 143 +++- drivers/net/ipa/ipa_uc.c| 5 +- 3 files changed, 150 insertions(+), 52 deletions(-) diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index 61dd7605bcb66..c46df0b7c4e50 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -54,12 +54,14 @@ static void ipa_interrupt_process(struct ipa_interrupt *interrupt, u32 irq_id) bool uc_irq = ipa_interrupt_uc(interrupt, irq_id); struct ipa *ipa = interrupt->ipa; u32 mask = BIT(irq_id); + u32 offset; /* For microcontroller interrupts, clear the interrupt right away, * "to avoid clearing unhandled interrupts." */ + offset = ipa_reg_irq_clr_offset(ipa->version); if (uc_irq) - iowrite32(mask, ipa->reg_virt + IPA_REG_IRQ_CLR_OFFSET); + iowrite32(mask, ipa->reg_virt + offset); if (irq_id < IPA_IRQ_COUNT && interrupt->handler[irq_id]) interrupt->handler[irq_id](interrupt->ipa, irq_id); @@ -69,7 +71,7 @@ static void ipa_interrupt_process(struct ipa_interrupt *interrupt, u32 irq_id) * so defer clearing until after the handler has been called. */ if (!uc_irq) - iowrite32(mask, ipa->reg_virt + IPA_REG_IRQ_CLR_OFFSET); + iowrite32(mask, ipa->reg_virt + offset); } /* Process all IPA interrupt types that have been signaled */ @@ -77,13 +79,15 @@ static void ipa_interrupt_process_all(struct ipa_interrupt *interrupt) { struct ipa *ipa = interrupt->ipa; u32 enabled = interrupt->enabled; + u32 offset; u32 mask; /* The status register indicates which conditions are present, * including conditions whose interrupt is not enabled. Handle * only the enabled ones. */ - mask = ioread32(ipa->reg_virt + IPA_REG_IRQ_STTS_OFFSET); + offset = ipa_reg_irq_stts_offset(ipa->version); + mask = ioread32(ipa->reg_virt + offset); while ((mask &= enabled)) { do { u32 irq_id = __ffs(mask); @@ -92,7 +96,7 @@ static void ipa_interrupt_process_all(struct ipa_interrupt *interrupt) ipa_interrupt_process(interrupt, irq_id); } while (mask); - mask = ioread32(ipa->reg_virt + IPA_REG_IRQ_STTS_OFFSET); + mask = ioread32(ipa->reg_virt + offset); } } @@ -115,14 +119,17 @@ static irqreturn_t ipa_isr(int irq, void *dev_id) { struct ipa_interrupt *interrupt = dev_id; struct ipa *ipa = interrupt->ipa; + u32 offset; u32 mask; - mask = ioread32(ipa->reg_virt + IPA_REG_IRQ_STTS_OFFSET); + offset = ipa_reg_irq_stts_offset(ipa->version); + mask = ioread32(ipa->reg_virt + offset); if (mask & interrupt->enabled) return IRQ_WAKE_THREAD; /* Nothing in the mask was supposed to cause an interrupt */ - iowrite32(mask, ipa->reg_virt + IPA_REG_IRQ_CLR_OFFSET); + offset = ipa_reg_irq_clr_offset(ipa->version); + iowrite32(mask, ipa->reg_virt + offset); dev_err(&ipa->pdev->dev, "%s: unexpected interrupt, mask 0x%08x\n", __func__, mask); @@ -136,15 +143,22 @@ static void ipa_interrupt_suspend_control(struct ipa_interrupt *interrupt, { struct ipa *ipa = interrupt->ipa; u32 mask = BIT(endpoint_id); + u32 offset; u32 val; /* assert(mask & ipa->available); */ - val = ioread32(ipa->reg_virt + IPA_REG_IRQ_SUSPEND_EN_OFFSET); + + /* IPA version 3.0 does not support TX_SUSPEND interrupt control */ + if (ipa->version == IPA_VERSION_3_0) + return; + + offset = ipa_reg_irq_suspend_en_offset(ipa->version); + val = ioread32(ipa->reg_virt + offset); if (enable) val |= mask; else val &= ~mask; - iowrite32(val, ipa->reg_virt + IPA_REG_IRQ_SUSPEND_EN_OFFSET); + iowrite32(val, ipa->reg_virt + offset); } /* Enable TX_SUSPEND for an endpoint */ @@ -165,10 +179,18 @@ ipa_interrupt_suspend_disable(struct ipa_interrupt *interrupt, u32 endpoint_id) void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) { struct ipa *ipa = interrupt->ipa; + u32 offset; u32 val; - val = ioread32(ipa->reg_virt + IPA_REG_IRQ_SUSPEND_IN
[PATCH net-next 1/6] net: ipa: update IPA register comments
Add and update IPA register definitions. Extend these definitions to incorporate a fairly small number of new symbols (register offsets and fields) to support IPA v3.0, v3.1, v3.5, v4.0, v4.1, v4.7, 4.9, and v4.11, and have the comments reflect when they are valid. None of the added symbols require changes elsewhere in the code. Update rsrc_grp_encoded() to support these other IPA versions. Add kerneldoc comments for the IPA IRQ numbers and sequencer type. Fix a few spots where the version check should be less restrictive (missed by an earlier patch). Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_reg.h | 185 ++ 1 file changed, 129 insertions(+), 56 deletions(-) diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 86fe2978e8102..735f6e809e042 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -66,14 +66,16 @@ struct ipa; */ #define IPA_REG_COMP_CFG_OFFSET0x003c -/* The next field is not supported for IPA v4.1 */ +/* The next field is not supported for IPA v4.0+, not present for IPA v4.5+ */ #define ENABLE_FMASK GENMASK(0, 0) +/* The next field is present for IPA v4.7+ */ +#define RAM_ARB_PRI_CLIENT_SAMP_FIX_DIS_FMASK GENMASK(0, 0) #define GSI_SNOC_BYPASS_DIS_FMASK GENMASK(1, 1) #define GEN_QMB_0_SNOC_BYPASS_DIS_FMASKGENMASK(2, 2) #define GEN_QMB_1_SNOC_BYPASS_DIS_FMASKGENMASK(3, 3) -/* The next field is not present for IPA v4.5 */ +/* The next field is not present for IPA v4.5+ */ #define IPA_DCMP_FAST_CLK_EN_FMASK GENMASK(4, 4) -/* The remaining fields are not present for IPA v3.5.1 */ +/* The next twelve fields are present for IPA v4.0+ */ #define IPA_QMB_SELECT_CONS_EN_FMASK GENMASK(5, 5) #define IPA_QMB_SELECT_PROD_EN_FMASK GENMASK(6, 6) #define GSI_MULTI_INORDER_RD_DIS_FMASK GENMASK(7, 7) @@ -87,8 +89,14 @@ struct ipa; #define GSI_MULTI_AXI_MASTERS_DIS_FMASKGENMASK(15, 15) #define IPA_QMB_SELECT_GLOBAL_EN_FMASK GENMASK(16, 16) #define IPA_ATOMIC_FETCHER_ARB_LOCK_DIS_FMASK GENMASK(20, 17) -/* The next field is present for IPA v4.5 */ +/* The next field is present for IPA v4.5 and IPA v4.7 */ #define IPA_FULL_FLUSH_WAIT_RSC_CLOSE_EN_FMASK GENMASK(21, 21) +/* The next five fields are present for IPA v4.9+ */ +#define QMB_RAM_RD_CACHE_DISABLE_FMASK GENMASK(19, 19) +#define GENQMB_AOOOWR_FMASKGENMASK(20, 20) +#define IF_OUT_OF_BUF_STOP_RESET_MASK_EN_FMASK GENMASK(21, 21) +#define GEN_QMB_1_DYNAMIC_ASIZE_FMASK GENMASK(30, 30) +#define GEN_QMB_0_DYNAMIC_ASIZE_FMASK GENMASK(31, 31) #define IPA_REG_CLKON_CFG_OFFSET 0x0044 #define RX_FMASK GENMASK(0, 0) @@ -108,13 +116,15 @@ struct ipa; #define ACK_MNGR_FMASK GENMASK(14, 14) #define D_DCPH_FMASK GENMASK(15, 15) #define H_DCPH_FMASK GENMASK(16, 16) -/* The next field is not present for IPA v4.5 */ +/* The next field is not present for IPA v4.5+ */ #define DCMP_FMASK GENMASK(17, 17) +/* The next three fields are present for IPA v3.5+ */ #define NTF_TX_CMDQS_FMASK GENMASK(18, 18) #define TX_0_FMASK GENMASK(19, 19) #define TX_1_FMASK GENMASK(20, 20) +/* The next field is present for IPA v3.5.1+ */ #define FNR_FMASK GENMASK(21, 21) -/* The remaining fields are not present for IPA v3.5.1 */ +/* The next eight fields are present for IPA v4.0+ */ #define QSB2AXI_CMDQ_L_FMASK GENMASK(22, 22) #define AGGR_WRAPPER_FMASK GENMASK(23, 23) #define RAM_SLAVEWAY_FMASK GENMASK(24, 24) @@ -123,8 +133,10 @@ struct ipa; #define GSI_IF_FMASK GENMASK(27, 27) #define GLOBAL_FMASK GENMASK(28, 28) #define GLOBAL_2X_CLK_FMASKGENMASK(29, 29) -/* The next field is present for IPA v4.5 */ +/* The next field is present for IPA v4.5+ */ #define DPL_FIFO_FMASK GENMASK(30, 30) +/* The next field is present for IPA v4.7+ */ +#define DRBIP_FMASKGENMASK(31, 31) #define IPA_REG_ROUTE_OFFSET 0x0048 #define ROUTE_DIS_FMASKGENMASK(0, 0) @@ -145,13 +157,13 @@ struct ipa; #define IPA_REG_QSB_MAX_READS_OFFSET 0x0078 #define GEN_QMB_0_MAX_READS_FMASK GENMASK(3, 0) #define GEN_QMB_1_MAX_READS_FMASK GENMASK(7, 4) -/* The next two fields are not present for IPA v3.5.1 */ +/* The next two fields are present for IPA v4.0+ */ #define GEN_QMB_0_MAX_READS_BEATS_FMASKGENMASK(23, 16) #define
[PATCH net-next 0/6] net: ipa: update registers for other versions
This series updates IPA and GSI register definitions to permit more versions of IPA hardware to be supported. Most of the updates are informational, updating comments to indicate which IPA versions support each register and field. But some registers are new and others are deprecated. In a few cases register fields are laid out differently, and in these cases the changes are a little more substantive. I won't claim the result is 100% correct, but it's close, and should allow all IPA versions 3.x through 4.x to be supported by the driver. -Alex Alex Elder (6): net: ipa: update IPA register comments net: ipa: update component config register net: ipa: support IPA interrupt addresses for IPA v4.7 net: ipa: GSI register cleanup net: ipa: update GSI ring size registers net: ipa: expand GSI channel types drivers/net/ipa/gsi.c | 9 +- drivers/net/ipa/gsi_reg.h | 69 +-- drivers/net/ipa/ipa_interrupt.c | 54 +++-- drivers/net/ipa/ipa_main.c | 2 +- drivers/net/ipa/ipa_reg.h | 352 +++- drivers/net/ipa/ipa_uc.c| 5 +- 6 files changed, 366 insertions(+), 125 deletions(-) -- 2.27.0
Re: [PATCH net-next] net: ipa: avoid 64-bit modulus
On 3/24/21 12:12 PM, David Laight wrote: I think 'count' is also required to be a power of 2. so you could have checked 'addr & (size - 1)'. It is required to be, and that is checked elsewhere (in gsi_channel_data_valid()). And yes, size would therefore be a power-of-2, and so your clever test would be a simple test. I'll take that into account when I implement the fix. Thanks for the suggestion. -Alex
Re: [PATCH net-next] net: ipa: avoid 64-bit modulus
On 3/24/21 11:27 AM, David Laight wrote: From: Alex Elder Sent: 23 March 2021 01:05 It is possible for a 32 bit x86 build to use a 64 bit DMA address. There are two remaining spots where the IPA driver does a modulo operation to check alignment of a DMA address, and under certain conditions this can lead to a build error on i386 (at least). The alignment checks we're doing are for power-of-2 values, and this means the lower 32 bits of the DMA address can be used. This ensures both operands to the modulo operator are 32 bits wide. Reported-by: Randy Dunlap Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 11 +++ drivers/net/ipa/ipa_table.c | 9 ++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 7f3e338ca7a72..b6355827bf900 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -1436,15 +1436,18 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index) /* Initialize a ring, including allocating DMA memory for its entries */ static int gsi_ring_alloc(struct gsi *gsi, struct gsi_ring *ring, u32 count) { - size_t size = count * GSI_RING_ELEMENT_SIZE; + u32 size = count * GSI_RING_ELEMENT_SIZE; struct device *dev = gsi->dev; dma_addr_t addr; - /* Hardware requires a 2^n ring size, with alignment equal to size */ + /* Hardware requires a 2^n ring size, with alignment equal to size. +* The size is a power of 2, so we can check alignment using just +* the bottom 32 bits for a DMA address of any size. +*/ ring->virt = dma_alloc_coherent(dev, size, &addr, GFP_KERNEL); Doesn't dma_alloc_coherent() guarantee that alignment? I doubt anywhere else checks? I normally wouldn't check something like this if it weren't guaranteed. I'm not sure why I did it here. I see it's "guaranteed to be aligned to the smallest PAGE_SIZE order which is greater than or equal to the requested size." So I think the answer to your question is "yes, it does guarantee that." I'll make a note to remove this check in a future patch, and will credit you with the suggestion. Thanks. -Alex David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH net-next v2 4/6] net: ipa: limit local processing context address
Not all of the bits of the LOCAL_PKT_PROC_CNTXT register are valid. Until IPA v4.5, there are 17 valid bits (though the bottom three must be zero). Starting with IPA v4.5, 18 bits are valid. Introduce proc_cntxt_base_addr_encoded() to encode the base address for use in the register using only the valid bits. Shorten the name of the register (omit "_BASE") to avoid the need to wrap the line in the one place it's used. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_mem.c | 6 -- drivers/net/ipa/ipa_reg.h | 14 -- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c index f25029b9ec857..32907dde5dc6a 100644 --- a/drivers/net/ipa/ipa_mem.c +++ b/drivers/net/ipa/ipa_mem.c @@ -61,6 +61,7 @@ int ipa_mem_setup(struct ipa *ipa) struct gsi_trans *trans; u32 offset; u16 size; + u32 val; /* Get a transaction to define the header memory region and to zero * the processing context and modem memory regions. @@ -89,8 +90,9 @@ int ipa_mem_setup(struct ipa *ipa) gsi_trans_commit_wait(trans); /* Tell the hardware where the processing context area is located */ - iowrite32(ipa->mem_offset + ipa->mem[IPA_MEM_MODEM_PROC_CTX].offset, - ipa->reg_virt + IPA_REG_LOCAL_PKT_PROC_CNTXT_BASE_OFFSET); + offset = ipa->mem_offset + ipa->mem[IPA_MEM_MODEM_PROC_CTX].offset; + val = proc_cntxt_base_addr_encoded(ipa->version, offset); + iowrite32(val, ipa->reg_virt + IPA_REG_LOCAL_PKT_PROC_CNTXT_OFFSET); return 0; } diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index bba088e80cd1e..cbfef27bbcf2c 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -217,8 +217,18 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) return 0x; } -/* The value of the next register must be a multiple of 8 */ -#define IPA_REG_LOCAL_PKT_PROC_CNTXT_BASE_OFFSET 0x01e8 +/* The value of the next register must be a multiple of 8 (bottom 3 bits 0) */ +#define IPA_REG_LOCAL_PKT_PROC_CNTXT_OFFSET0x01e8 + +/* Encoded value for LOCAL_PKT_PROC_CNTXT register BASE_ADDR field */ +static inline u32 proc_cntxt_base_addr_encoded(enum ipa_version version, + u32 addr) +{ + if (version < IPA_VERSION_4_5) + return u32_encode_bits(addr, GENMASK(16, 0)); + + return u32_encode_bits(addr, GENMASK(17, 0)); +} /* ipa->available defines the valid bits in the AGGR_FORCE_CLOSE register */ #define IPA_REG_AGGR_FORCE_CLOSE_OFFSET0x01ec -- 2.27.0
[PATCH net-next v2 5/6] net: ipa: move ipa_aggr_granularity_val()
We only use ipa_aggr_granularity_val() inside "ipa_main.c", so it doesn't really need to be defined in a header file. It makes some sense to be grouped with the register definitions, but it is unlike the other inline functions now defined in "ipa_reg.h". So move it into "ipa_main.c" where it's used. TIMER_FREQUENCY is used only by that function, so move that definition as well. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 15 +++ drivers/net/ipa/ipa_reg.h | 12 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 62d82d48aed69..ba1bfc30210a3 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -287,6 +287,21 @@ ipa_hardware_config_qsb(struct ipa *ipa, const struct ipa_data *data) iowrite32(val, ipa->reg_virt + IPA_REG_QSB_MAX_READS_OFFSET); } +/* The internal inactivity timer clock is used for the aggregation timer */ +#define TIMER_FREQUENCY32000 /* 32 KHz inactivity timer clock */ + +/* Compute the value to use in the COUNTER_CFG register AGGR_GRANULARITY + * field to represent the given number of microseconds. The value is one + * less than the number of timer ticks in the requested period. 0 is not + * a valid granularity value. + */ +static u32 ipa_aggr_granularity_val(u32 usec) +{ + /* assert(usec != 0); */ + + return DIV_ROUND_CLOSEST(usec * TIMER_FREQUENCY, USEC_PER_SEC) - 1; +} + /* IPA uses unified Qtime starting at IPA v4.5, implementing various * timestamps and timers independent of the IPA core clock rate. The * Qtimer is based on a 56-bit timestamp incremented at each tick of diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index cbfef27bbcf2c..86fe2978e8102 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -237,18 +237,6 @@ static inline u32 proc_cntxt_base_addr_encoded(enum ipa_version version, #define IPA_REG_COUNTER_CFG_OFFSET 0x01f0 #define AGGR_GRANULARITY_FMASK GENMASK(8, 4) -/* The internal inactivity timer clock is used for the aggregation timer */ -#define TIMER_FREQUENCY32000 /* 32 KHz inactivity timer clock */ - -/* Compute the value to use in the AGGR_GRANULARITY field representing the - * given number of microseconds. The value is one less than the number of - * timer ticks in the requested period. 0 not a valid granularity value. - */ -static inline u32 ipa_aggr_granularity_val(u32 usec) -{ - return DIV_ROUND_CLOSEST(usec * TIMER_FREQUENCY, USEC_PER_SEC) - 1; -} - /* The next register is not present for IPA v4.5 */ #define IPA_REG_TX_CFG_OFFSET 0x01fc /* The first three fields are present for IPA v3.5.1 only */ -- 2.27.0
[PATCH net-next v2 2/6] net: ipa: update version definitions
Add IPA version definitions for all IPA v3.x and v4.x. Fix the GSI version associated with IPA version 4.1. Signed-off-by: Alex Elder --- v2: - Add kernel-doc descriptions for ipa_version members. drivers/net/ipa/ipa_version.h | 29 ++--- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/ipa_version.h b/drivers/net/ipa/ipa_version.h index 2944e2a890231..ee2b3d02f3cd3 100644 --- a/drivers/net/ipa/ipa_version.h +++ b/drivers/net/ipa/ipa_version.h @@ -8,17 +8,32 @@ /** * enum ipa_version + * @IPA_VERSION_3_0: IPA version 3.0/GSI version 1.0 + * @IPA_VERSION_3_1: IPA version 3.1/GSI version 1.1 + * @IPA_VERSION_3_5: IPA version 3.5/GSI version 1.2 + * @IPA_VERSION_3_5_1: IPA version 3.5.1/GSI version 1.3 + * @IPA_VERSION_4_0: IPA version 4.0/GSI version 2.0 + * @IPA_VERSION_4_1: IPA version 4.1/GSI version 2.0 + * @IPA_VERSION_4_2: IPA version 4.2/GSI version 2.2 + * @IPA_VERSION_4_5: IPA version 4.5/GSI version 2.5 + * @IPA_VERSION_4_7: IPA version 4.7/GSI version 2.7 + * @IPA_VERSION_4_9: IPA version 4.9/GSI version 2.9 + * @IPA_VERSION_4_11: IPA version 4.11/GSI version 2.11 (2.1.1) * * Defines the version of IPA (and GSI) hardware present on the platform. - * It seems this might be better defined elsewhere, but having it here gets - * it where it's needed. */ enum ipa_version { - 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 */ - IPA_VERSION_4_2,/* GSI version 2.2 */ - IPA_VERSION_4_5,/* GSI version 2.5 */ + IPA_VERSION_3_0, + IPA_VERSION_3_1, + IPA_VERSION_3_5, + IPA_VERSION_3_5_1, + IPA_VERSION_4_0, + IPA_VERSION_4_1, + IPA_VERSION_4_2, + IPA_VERSION_4_5, + IPA_VERSION_4_7, + IPA_VERSION_4_9, + IPA_VERSION_4_11, }; #endif /* _IPA_VERSION_H_ */ -- 2.27.0
[PATCH net-next v2 6/6] net: ipa: increase channels and events
Increase the maximum number of channels and event rings supported by the driver, to allow the maximum available on the SDX55. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h index efc980f96109e..d5996bdb20ef5 100644 --- a/drivers/net/ipa/gsi.h +++ b/drivers/net/ipa/gsi.h @@ -16,8 +16,8 @@ #include "ipa_version.h" /* Maximum number of channels and event rings supported by the driver */ -#define GSI_CHANNEL_COUNT_MAX 17 -#define GSI_EVT_RING_COUNT_MAX 13 +#define GSI_CHANNEL_COUNT_MAX 23 +#define GSI_EVT_RING_COUNT_MAX 20 /* Maximum TLV FIFO size for a channel; 64 here is arbitrary (and high) */ #define GSI_TLV_MAX64 -- 2.27.0
[PATCH net-next v2 3/6] net: ipa: define the ENDP_INIT_NAT register
Define the ENDP_INIT_NAT register for setting up the NAT configuration for an endpoint. We aren't using NAT at this time, so explicitly set the type to BYPASS for all endpoints. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_endpoint.c | 17 - drivers/net/ipa/ipa_reg.h | 14 +- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 5f93bd60c7586..38e83cd467b52 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2019-2020 Linaro Ltd. + * Copyright (C) 2019-2021 Linaro Ltd. */ #include @@ -468,6 +468,20 @@ static void ipa_endpoint_init_cfg(struct ipa_endpoint *endpoint) iowrite32(val, endpoint->ipa->reg_virt + offset); } +static void ipa_endpoint_init_nat(struct ipa_endpoint *endpoint) +{ + u32 offset; + u32 val; + + if (!endpoint->toward_ipa) + return; + + offset = IPA_REG_ENDP_INIT_NAT_N_OFFSET(endpoint->endpoint_id); + val = u32_encode_bits(IPA_NAT_BYPASS, NAT_EN_FMASK); + + iowrite32(val, endpoint->ipa->reg_virt + offset); +} + /** * ipa_endpoint_init_hdr() - Initialize HDR endpoint configuration register * @endpoint: Endpoint pointer @@ -1488,6 +1502,7 @@ static void ipa_endpoint_program(struct ipa_endpoint *endpoint) else (void)ipa_endpoint_program_suspend(endpoint, false); ipa_endpoint_init_cfg(endpoint); + ipa_endpoint_init_nat(endpoint); ipa_endpoint_init_hdr(endpoint); ipa_endpoint_init_hdr_ext(endpoint); ipa_endpoint_init_hdr_metadata_mask(endpoint); diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 36fe746575f6b..bba088e80cd1e 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2018-2020 Linaro Ltd. + * Copyright (C) 2018-2021 Linaro Ltd. */ #ifndef _IPA_REG_H_ #define _IPA_REG_H_ @@ -388,6 +388,18 @@ enum ipa_cs_offload_en { IPA_CS_OFFLOAD_DL = 0x2, }; +/* Valid only for TX (IPA consumer) endpoints */ +#define IPA_REG_ENDP_INIT_NAT_N_OFFSET(ep) \ + (0x080c + 0x0070 * (ep)) +#define NAT_EN_FMASK GENMASK(1, 0) + +/** enum ipa_nat_en - ENDP_INIT_NAT register NAT_EN field value */ +enum ipa_nat_en { + IPA_NAT_BYPASS = 0x0, + IPA_NAT_SRC = 0x1, + IPA_NAT_DST = 0x2, +}; + #define IPA_REG_ENDP_INIT_HDR_N_OFFSET(ep) \ (0x0810 + 0x0070 * (ep)) #define HDR_LEN_FMASK GENMASK(5, 0) -- 2.27.0
[PATCH net-next v2 1/6] net: ipa: reduce IPA version assumptions
Modify conditional tests throughout the IPA code so they do not assume that IPA v3.5.1 is the oldest version supported. Also remove assumptions that IPA v4.5 is the newest version of IPA supported. Augment versions in comments with "+", to be clearer that the comment applies to a version and subsequent versions. (E.g., "present for IPA v4.2+" instead of just "present for v4.2".) Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 8 drivers/net/ipa/ipa_cmd.c | 26 +++--- drivers/net/ipa/ipa_endpoint.c | 25 ++--- drivers/net/ipa/ipa_main.c | 6 +++--- drivers/net/ipa/ipa_qmi.c | 2 +- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index b6355827bf900..7f2a8fce5e0db 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -827,14 +827,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) + /* No need to use the doorbell engine starting at IPA v4.0 */ + 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; @@ -973,7 +973,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_4_0 && !channel->toward_ipa) gsi_channel_reset_command(channel); gsi_channel_program(channel, doorbell); diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index 35e35852c25c5..a0be25f1f4427 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -71,13 +71,12 @@ struct ipa_cmd_hw_hdr_init_local { /* IPA_CMD_REGISTER_WRITE */ -/* For IPA v4.0+, this opcode gets modified with pipeline clear options */ - +/* For IPA v4.0+, the pipeline clear options are encoded in the opcode */ #define REGISTER_WRITE_OPCODE_SKIP_CLEAR_FMASK GENMASK(8, 8) #define REGISTER_WRITE_OPCODE_CLEAR_OPTION_FMASK GENMASK(10, 9) struct ipa_cmd_register_write { - __le16 flags; /* Unused/reserved for IPA v3.5.1 */ + __le16 flags; /* Unused/reserved prior to IPA v4.0 */ __le16 offset; __le32 value; __le32 value_mask; @@ -85,12 +84,12 @@ struct ipa_cmd_register_write { }; /* Field masks for ipa_cmd_register_write structure fields */ -/* The next field is present for IPA v4.0 and above */ +/* The next field is present for IPA v4.0+ */ #define REGISTER_WRITE_FLAGS_OFFSET_HIGH_FMASK GENMASK(14, 11) -/* The next field is present for IPA v3.5.1 only */ +/* The next field is not present for IPA v4.0+ */ #define REGISTER_WRITE_FLAGS_SKIP_CLEAR_FMASK GENMASK(15, 15) -/* The next field and its values are present for IPA v3.5.1 only */ +/* The next field and its values are not present for IPA v4.0+ */ #define REGISTER_WRITE_CLEAR_OPTIONS_FMASK GENMASK(1, 0) /* IPA_CMD_IP_PACKET_INIT */ @@ -123,7 +122,7 @@ struct ipa_cmd_hw_dma_mem_mem { /* Field masks for ipa_cmd_hw_dma_mem_mem structure fields */ #define DMA_SHARED_MEM_FLAGS_DIRECTION_FMASK GENMASK(0, 0) -/* The next two fields are present for IPA v3.5.1 only. */ +/* The next two fields are not present for IPA v4.0+ */ #define DMA_SHARED_MEM_FLAGS_SKIP_CLEAR_FMASK GENMASK(1, 1) #define DMA_SHARED_MEM_FLAGS_CLEAR_OPTIONS_FMASK GENMASK(3, 2) @@ -237,11 +236,12 @@ static bool ipa_cmd_register_write_offset_valid(struct ipa *ipa, u32 bit_count; /* The maximum offset in a register_write immediate command depends -* on the version of IPA. IPA v3.5.1 supports a 16 bit offset, but -* newer versions allow some additional high-order bits. +* on the version of IPA. A 16 bit offset is always supported, +* but starting with IPA v4.0 some additional high-order bits are +* allowed. */ bit_count = BITS_PER_BYTE * sizeof(payload->offset); - if (ipa->version != IPA_VERSION_3_5_1) + if (ipa->version >
[PATCH net-next v2 0/6] net: ipa: versions and registers
Version 2 of this series adds kernel-doc descriptions for all members of the ipa_version enumerated type in patch 2. The original description of the series is below. -Alex This series is sort of a mix of things, generally related to updating IPA versions and register definitions. The first patch fixes some version-related tests throughout the code so the conditions are valid for IPA versions other than the two that are currently supported. Support for additional versions is forthcoming, and this is a preparatory step. The second patch adds to the set of defined IPA versions, to include all versions between 3.0 and 4.11. The next defines an endpoint initialization register that was previously not being configured. We now initialize that register (so that NAT is explicitly disabled) on all AP endpoints. The fourth adds support for an extra bit in a field in a register, which is present starting at IPA v4.5. The last two are sort of standalone. One just moves a function definition and makes it private. The other increases the number of GSI channels and events supported by the driver, sufficient for IPA v4.5. -Alex Alex Elder (6): net: ipa: reduce IPA version assumptions net: ipa: update version definitions net: ipa: define the ENDP_INIT_NAT register net: ipa: limit local processing context address net: ipa: move ipa_aggr_granularity_val() net: ipa: increase channels and events drivers/net/ipa/gsi.c | 8 +++ drivers/net/ipa/gsi.h | 4 ++-- drivers/net/ipa/ipa_cmd.c | 26 - drivers/net/ipa/ipa_endpoint.c | 42 -- drivers/net/ipa/ipa_main.c | 21 ++--- drivers/net/ipa/ipa_mem.c | 6 +++-- drivers/net/ipa/ipa_qmi.c | 2 +- drivers/net/ipa/ipa_reg.h | 40 drivers/net/ipa/ipa_version.h | 29 +-- 9 files changed, 121 insertions(+), 57 deletions(-) -- 2.27.0
[PATCH net-next 6/6] net: ipa: increase channels and events
Increase the maximum number of channels and event rings supported by the driver, to allow the maximum available on the SDX55. Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h index efc980f96109e..d5996bdb20ef5 100644 --- a/drivers/net/ipa/gsi.h +++ b/drivers/net/ipa/gsi.h @@ -16,8 +16,8 @@ #include "ipa_version.h" /* Maximum number of channels and event rings supported by the driver */ -#define GSI_CHANNEL_COUNT_MAX 17 -#define GSI_EVT_RING_COUNT_MAX 13 +#define GSI_CHANNEL_COUNT_MAX 23 +#define GSI_EVT_RING_COUNT_MAX 20 /* Maximum TLV FIFO size for a channel; 64 here is arbitrary (and high) */ #define GSI_TLV_MAX64 -- 2.27.0
[PATCH net-next 5/6] net: ipa: move ipa_aggr_granularity_val()
We only use ipa_aggr_granularity_val() inside "ipa_main.c", so it doesn't really need to be defined in a header file. It makes some sense to be grouped with the register definitions, but it is unlike the other inline functions now defined in "ipa_reg.h". So move it into "ipa_main.c" where it's used. TIMER_FREQUENCY is used only by that function, so move that definition as well. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 15 +++ drivers/net/ipa/ipa_reg.h | 12 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 62d82d48aed69..ba1bfc30210a3 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -287,6 +287,21 @@ ipa_hardware_config_qsb(struct ipa *ipa, const struct ipa_data *data) iowrite32(val, ipa->reg_virt + IPA_REG_QSB_MAX_READS_OFFSET); } +/* The internal inactivity timer clock is used for the aggregation timer */ +#define TIMER_FREQUENCY32000 /* 32 KHz inactivity timer clock */ + +/* Compute the value to use in the COUNTER_CFG register AGGR_GRANULARITY + * field to represent the given number of microseconds. The value is one + * less than the number of timer ticks in the requested period. 0 is not + * a valid granularity value. + */ +static u32 ipa_aggr_granularity_val(u32 usec) +{ + /* assert(usec != 0); */ + + return DIV_ROUND_CLOSEST(usec * TIMER_FREQUENCY, USEC_PER_SEC) - 1; +} + /* IPA uses unified Qtime starting at IPA v4.5, implementing various * timestamps and timers independent of the IPA core clock rate. The * Qtimer is based on a 56-bit timestamp incremented at each tick of diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index cbfef27bbcf2c..86fe2978e8102 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -237,18 +237,6 @@ static inline u32 proc_cntxt_base_addr_encoded(enum ipa_version version, #define IPA_REG_COUNTER_CFG_OFFSET 0x01f0 #define AGGR_GRANULARITY_FMASK GENMASK(8, 4) -/* The internal inactivity timer clock is used for the aggregation timer */ -#define TIMER_FREQUENCY32000 /* 32 KHz inactivity timer clock */ - -/* Compute the value to use in the AGGR_GRANULARITY field representing the - * given number of microseconds. The value is one less than the number of - * timer ticks in the requested period. 0 not a valid granularity value. - */ -static inline u32 ipa_aggr_granularity_val(u32 usec) -{ - return DIV_ROUND_CLOSEST(usec * TIMER_FREQUENCY, USEC_PER_SEC) - 1; -} - /* The next register is not present for IPA v4.5 */ #define IPA_REG_TX_CFG_OFFSET 0x01fc /* The first three fields are present for IPA v3.5.1 only */ -- 2.27.0
[PATCH net-next 3/6] net: ipa: define the ENDP_INIT_NAT register
Define the ENDP_INIT_NAT register for setting up the NAT configuration for an endpoint. We aren't using NAT at this time, so explicitly set the type to BYPASS for all endpoints. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_endpoint.c | 17 - drivers/net/ipa/ipa_reg.h | 14 +- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 5f93bd60c7586..38e83cd467b52 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2019-2020 Linaro Ltd. + * Copyright (C) 2019-2021 Linaro Ltd. */ #include @@ -468,6 +468,20 @@ static void ipa_endpoint_init_cfg(struct ipa_endpoint *endpoint) iowrite32(val, endpoint->ipa->reg_virt + offset); } +static void ipa_endpoint_init_nat(struct ipa_endpoint *endpoint) +{ + u32 offset; + u32 val; + + if (!endpoint->toward_ipa) + return; + + offset = IPA_REG_ENDP_INIT_NAT_N_OFFSET(endpoint->endpoint_id); + val = u32_encode_bits(IPA_NAT_BYPASS, NAT_EN_FMASK); + + iowrite32(val, endpoint->ipa->reg_virt + offset); +} + /** * ipa_endpoint_init_hdr() - Initialize HDR endpoint configuration register * @endpoint: Endpoint pointer @@ -1488,6 +1502,7 @@ static void ipa_endpoint_program(struct ipa_endpoint *endpoint) else (void)ipa_endpoint_program_suspend(endpoint, false); ipa_endpoint_init_cfg(endpoint); + ipa_endpoint_init_nat(endpoint); ipa_endpoint_init_hdr(endpoint); ipa_endpoint_init_hdr_ext(endpoint); ipa_endpoint_init_hdr_metadata_mask(endpoint); diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 36fe746575f6b..bba088e80cd1e 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved. - * Copyright (C) 2018-2020 Linaro Ltd. + * Copyright (C) 2018-2021 Linaro Ltd. */ #ifndef _IPA_REG_H_ #define _IPA_REG_H_ @@ -388,6 +388,18 @@ enum ipa_cs_offload_en { IPA_CS_OFFLOAD_DL = 0x2, }; +/* Valid only for TX (IPA consumer) endpoints */ +#define IPA_REG_ENDP_INIT_NAT_N_OFFSET(ep) \ + (0x080c + 0x0070 * (ep)) +#define NAT_EN_FMASK GENMASK(1, 0) + +/** enum ipa_nat_en - ENDP_INIT_NAT register NAT_EN field value */ +enum ipa_nat_en { + IPA_NAT_BYPASS = 0x0, + IPA_NAT_SRC = 0x1, + IPA_NAT_DST = 0x2, +}; + #define IPA_REG_ENDP_INIT_HDR_N_OFFSET(ep) \ (0x0810 + 0x0070 * (ep)) #define HDR_LEN_FMASK GENMASK(5, 0) -- 2.27.0
[PATCH net-next 4/6] net: ipa: limit local processing context address
Not all of the bits of the LOCAL_PKT_PROC_CNTXT register are valid. Until IPA v4.5, there are 17 valid bits (though the bottom three must be zero). Starting with IPA v4.5, 18 bits are valid. Introduce proc_cntxt_base_addr_encoded() to encode the base address for use in the register using only the valid bits. Shorten the name of the register (omit "_BASE") to avoid the need to wrap the line in the one place it's used. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_mem.c | 6 -- drivers/net/ipa/ipa_reg.h | 14 -- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c index f25029b9ec857..32907dde5dc6a 100644 --- a/drivers/net/ipa/ipa_mem.c +++ b/drivers/net/ipa/ipa_mem.c @@ -61,6 +61,7 @@ int ipa_mem_setup(struct ipa *ipa) struct gsi_trans *trans; u32 offset; u16 size; + u32 val; /* Get a transaction to define the header memory region and to zero * the processing context and modem memory regions. @@ -89,8 +90,9 @@ int ipa_mem_setup(struct ipa *ipa) gsi_trans_commit_wait(trans); /* Tell the hardware where the processing context area is located */ - iowrite32(ipa->mem_offset + ipa->mem[IPA_MEM_MODEM_PROC_CTX].offset, - ipa->reg_virt + IPA_REG_LOCAL_PKT_PROC_CNTXT_BASE_OFFSET); + offset = ipa->mem_offset + ipa->mem[IPA_MEM_MODEM_PROC_CTX].offset; + val = proc_cntxt_base_addr_encoded(ipa->version, offset); + iowrite32(val, ipa->reg_virt + IPA_REG_LOCAL_PKT_PROC_CNTXT_OFFSET); return 0; } diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index bba088e80cd1e..cbfef27bbcf2c 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -217,8 +217,18 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) return 0x; } -/* The value of the next register must be a multiple of 8 */ -#define IPA_REG_LOCAL_PKT_PROC_CNTXT_BASE_OFFSET 0x01e8 +/* The value of the next register must be a multiple of 8 (bottom 3 bits 0) */ +#define IPA_REG_LOCAL_PKT_PROC_CNTXT_OFFSET0x01e8 + +/* Encoded value for LOCAL_PKT_PROC_CNTXT register BASE_ADDR field */ +static inline u32 proc_cntxt_base_addr_encoded(enum ipa_version version, + u32 addr) +{ + if (version < IPA_VERSION_4_5) + return u32_encode_bits(addr, GENMASK(16, 0)); + + return u32_encode_bits(addr, GENMASK(17, 0)); +} /* ipa->available defines the valid bits in the AGGR_FORCE_CLOSE register */ #define IPA_REG_AGGR_FORCE_CLOSE_OFFSET0x01ec -- 2.27.0
[PATCH net-next 2/6] net: ipa: update version definitions
Add IPA version definitions for all IPA v3.x and v4.x. Fix the GSI version associated with IPA version 4.1. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_version.h | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_version.h b/drivers/net/ipa/ipa_version.h index 2944e2a890231..ec87b0824c307 100644 --- a/drivers/net/ipa/ipa_version.h +++ b/drivers/net/ipa/ipa_version.h @@ -14,11 +14,17 @@ * it where it's needed. */ enum ipa_version { - IPA_VERSION_3_5_1, /* GSI version 1.3.0 */ + IPA_VERSION_3_0,/* GSI version 1.0 */ + IPA_VERSION_3_1,/* GSI version 1.1 */ + IPA_VERSION_3_5,/* GSI version 1.2 */ + IPA_VERSION_3_5_1, /* GSI version 1.3 */ IPA_VERSION_4_0,/* GSI version 2.0 */ - IPA_VERSION_4_1,/* GSI version 2.1 */ + IPA_VERSION_4_1,/* GSI version 2.0 */ IPA_VERSION_4_2,/* GSI version 2.2 */ IPA_VERSION_4_5,/* GSI version 2.5 */ + IPA_VERSION_4_7,/* GSI version 2.7 */ + IPA_VERSION_4_9,/* GSI version 2.9 */ + IPA_VERSION_4_11, /* GSI version 2.11 (2.1.1) */ }; #endif /* _IPA_VERSION_H_ */ -- 2.27.0
[PATCH net-next 1/6] net: ipa: reduce IPA version assumptions
Modify conditional tests throughout the IPA code so they do not assume that IPA v3.5.1 is the oldest version supported. Also remove assumptions that IPA v4.5 is the newest version of IPA supported. Augment versions in comments with "+", to be clearer that the comment applies to a version and subsequent versions. (E.g., "present for IPA v4.2+" instead of just "present for v4.2".) Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 8 drivers/net/ipa/ipa_cmd.c | 26 +++--- drivers/net/ipa/ipa_endpoint.c | 25 ++--- drivers/net/ipa/ipa_main.c | 6 +++--- drivers/net/ipa/ipa_qmi.c | 2 +- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 7f3e338ca7a72..cf54c6f4c34ca 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -827,14 +827,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) + /* No need to use the doorbell engine starting at IPA v4.0 */ + 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; @@ -973,7 +973,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_4_0 && !channel->toward_ipa) gsi_channel_reset_command(channel); gsi_channel_program(channel, doorbell); diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index 35e35852c25c5..a0be25f1f4427 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -71,13 +71,12 @@ struct ipa_cmd_hw_hdr_init_local { /* IPA_CMD_REGISTER_WRITE */ -/* For IPA v4.0+, this opcode gets modified with pipeline clear options */ - +/* For IPA v4.0+, the pipeline clear options are encoded in the opcode */ #define REGISTER_WRITE_OPCODE_SKIP_CLEAR_FMASK GENMASK(8, 8) #define REGISTER_WRITE_OPCODE_CLEAR_OPTION_FMASK GENMASK(10, 9) struct ipa_cmd_register_write { - __le16 flags; /* Unused/reserved for IPA v3.5.1 */ + __le16 flags; /* Unused/reserved prior to IPA v4.0 */ __le16 offset; __le32 value; __le32 value_mask; @@ -85,12 +84,12 @@ struct ipa_cmd_register_write { }; /* Field masks for ipa_cmd_register_write structure fields */ -/* The next field is present for IPA v4.0 and above */ +/* The next field is present for IPA v4.0+ */ #define REGISTER_WRITE_FLAGS_OFFSET_HIGH_FMASK GENMASK(14, 11) -/* The next field is present for IPA v3.5.1 only */ +/* The next field is not present for IPA v4.0+ */ #define REGISTER_WRITE_FLAGS_SKIP_CLEAR_FMASK GENMASK(15, 15) -/* The next field and its values are present for IPA v3.5.1 only */ +/* The next field and its values are not present for IPA v4.0+ */ #define REGISTER_WRITE_CLEAR_OPTIONS_FMASK GENMASK(1, 0) /* IPA_CMD_IP_PACKET_INIT */ @@ -123,7 +122,7 @@ struct ipa_cmd_hw_dma_mem_mem { /* Field masks for ipa_cmd_hw_dma_mem_mem structure fields */ #define DMA_SHARED_MEM_FLAGS_DIRECTION_FMASK GENMASK(0, 0) -/* The next two fields are present for IPA v3.5.1 only. */ +/* The next two fields are not present for IPA v4.0+ */ #define DMA_SHARED_MEM_FLAGS_SKIP_CLEAR_FMASK GENMASK(1, 1) #define DMA_SHARED_MEM_FLAGS_CLEAR_OPTIONS_FMASK GENMASK(3, 2) @@ -237,11 +236,12 @@ static bool ipa_cmd_register_write_offset_valid(struct ipa *ipa, u32 bit_count; /* The maximum offset in a register_write immediate command depends -* on the version of IPA. IPA v3.5.1 supports a 16 bit offset, but -* newer versions allow some additional high-order bits. +* on the version of IPA. A 16 bit offset is always supported, +* but starting with IPA v4.0 some additional high-order bits are +* allowed. */ bit_count = BITS_PER_BYTE * sizeof(payload->offset); - if (ipa->version != IPA_VERSION_3_5_1) + if (ipa->version >
[PATCH net-next 0/6] net: ipa: versions and registers
This series is sort of a mix of things, generally related to updating IPA versions and register definitions. The first patch fixes some version-related tests throughout the code so the conditions are valid for IPA versions other than the two that are currently supported. Support for additional versions is forthcoming, and this is a preparatory step. The second patch adds to the set of defined IPA versions, to include all versions between 3.0 and 4.11. The next defines an endpoint initialization register that was previously not being configured. We now initialize that register (so that NAT is explicitly disabled) on all AP endpoints. The fourth adds support for an extra bit in a field in a register, which is present starting at IPA v4.5. The last two are sort of standalone. One just moves a function definition and makes it private. The other increases the number of GSI channels and events supported by the driver, sufficient for IPA v4.5. -Alex Alex Elder (6): net: ipa: reduce IPA version assumptions net: ipa: update version definitions net: ipa: define the ENDP_INIT_NAT register net: ipa: limit local processing context address net: ipa: move ipa_aggr_granularity_val() net: ipa: increase channels and events drivers/net/ipa/gsi.c | 8 +++ drivers/net/ipa/gsi.h | 4 ++-- drivers/net/ipa/ipa_cmd.c | 26 - drivers/net/ipa/ipa_endpoint.c | 42 -- drivers/net/ipa/ipa_main.c | 21 ++--- drivers/net/ipa/ipa_mem.c | 6 +++-- drivers/net/ipa/ipa_qmi.c | 2 +- drivers/net/ipa/ipa_reg.h | 40 drivers/net/ipa/ipa_version.h | 10 ++-- 9 files changed, 107 insertions(+), 52 deletions(-) -- 2.27.0
[PATCH net-next] net: ipa: avoid 64-bit modulus
It is possible for a 32 bit x86 build to use a 64 bit DMA address. There are two remaining spots where the IPA driver does a modulo operation to check alignment of a DMA address, and under certain conditions this can lead to a build error on i386 (at least). The alignment checks we're doing are for power-of-2 values, and this means the lower 32 bits of the DMA address can be used. This ensures both operands to the modulo operator are 32 bits wide. Reported-by: Randy Dunlap Signed-off-by: Alex Elder --- drivers/net/ipa/gsi.c | 11 +++ drivers/net/ipa/ipa_table.c | 9 ++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 7f3e338ca7a72..b6355827bf900 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -1436,15 +1436,18 @@ static void gsi_evt_ring_rx_update(struct gsi_evt_ring *evt_ring, u32 index) /* Initialize a ring, including allocating DMA memory for its entries */ static int gsi_ring_alloc(struct gsi *gsi, struct gsi_ring *ring, u32 count) { - size_t size = count * GSI_RING_ELEMENT_SIZE; + u32 size = count * GSI_RING_ELEMENT_SIZE; struct device *dev = gsi->dev; dma_addr_t addr; - /* Hardware requires a 2^n ring size, with alignment equal to size */ + /* Hardware requires a 2^n ring size, with alignment equal to size. +* The size is a power of 2, so we can check alignment using just +* the bottom 32 bits for a DMA address of any size. +*/ ring->virt = dma_alloc_coherent(dev, size, &addr, GFP_KERNEL); - if (ring->virt && addr % size) { + if (ring->virt && lower_32_bits(addr) % size) { dma_free_coherent(dev, size, ring->virt, addr); - dev_err(dev, "unable to alloc 0x%zx-aligned ring buffer\n", + dev_err(dev, "unable to alloc 0x%x-aligned ring buffer\n", size); return -EINVAL; /* Not a good error value, but distinct */ } else if (!ring->virt) { diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c index 988f2c2886b95..4236a50ff03ae 100644 --- a/drivers/net/ipa/ipa_table.c +++ b/drivers/net/ipa/ipa_table.c @@ -658,10 +658,13 @@ int ipa_table_init(struct ipa *ipa) return -ENOMEM; /* We put the "zero rule" at the base of our table area. The IPA -* hardware requires rules to be aligned on a 128-byte boundary. -* Make sure the allocation satisfies this constraint. +* hardware requires route and filter table rules to be aligned +* on a 128-byte boundary. As long as the alignment constraint +* is a power of 2, we can check alignment using just the bottom +* 32 bits for a DMA address of any size. */ - if (addr % IPA_TABLE_ALIGN) { + BUILD_BUG_ON(!is_power_of_2(IPA_TABLE_ALIGN)); + if (lower_32_bits(addr) % IPA_TABLE_ALIGN) { dev_err(dev, "table address %pad not %u-byte aligned\n", &addr, IPA_TABLE_ALIGN); dma_free_coherent(dev, size, virt, addr); -- 2.27.0
Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation
On 3/22/21 5:56 PM, Andrew Lunn wrote: The solution is to create a user space tool inside the drivers/net/ipa directory that will link with the kernel source files and will perform all the basic one-time checks I want to make. Hi Alex Have you found any other driver doing this? Where do they keep there code? Could this be a selftest, put somewhere in tools/testing/selftests. Or can this be a test kernel module. Eg. we have crypt/testmsg.c which runs a number of tests on the crypto subsystem, ./kernel/time/test_udelay.c which runs times on udelay. Rather than inventing something new, please follow other examples already in the kernel. I will. I did see the tools/testing directory and I'll look at how people have done things there. I need to try to get it working first, then I'll figure out where it belongs. I think I'll be able to do a user space test, but it's a little tricky to be sure it's actually testing what I want. If that ends up being too hard, I'll look into kernel test module. Thanks for the suggestions. -Alex Andrew
Re: linux-next: Tree for Mar 22 (net/ipa)
On 3/22/21 12:16 PM, Randy Dunlap wrote: On 3/22/21 12:52 AM, Stephen Rothwell wrote: Hi all, Warning: Some of the branches in linux-next may still based on v5.12-rc1, so please be careful if you are trying to bisect a bug. News: if your -next included tree is based on Linus' tree tag v5.12-rc1{,-dontuse} (or somewhere between v5.11 and that tag), please consider rebasing it onto v5.12-rc2. Also, please check any branches merged into your branch. Changes since 20210319: on i386: ld: drivers/net/ipa/gsi.o: in function `gsi_ring_alloc': gsi.c:(.text+0x926): undefined reference to `__umoddi3' This code assumes a 32-bit kernel implies a 32-bit dma_addr_t. But it seems that's not the case for i386... Let me try to reproduce this. The fix should be easy. Sorry about that. -Alex
Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation
On 3/22/21 9:17 AM, Leon Romanovsky wrote: We are not talking about putting this tool in upstream repo, right? If yes, please get buy-in from David/Jakub first. I'll make the user space sanity check tool either way. It would be nice to have it upstream, the checks continue to be made as the kernel moves forward. If I need to keep it in my own repository it will be fairly isolated, and easier to carry for my own use. If David/Jakub comments here, I'll know. Otherwise I'll post it as an RFC initially when I've got something. -Alex
Re: [PATCH net-next v2 0/2] net: ipa: fix validation
On 3/20/21 9:17 AM, Alex Elder wrote: There is sanity checking code in the IPA driver that's meant to be enabled only during development. This allows the driver to make certain assumptions, but not have to verify those assumptions are true at (operational) runtime. This code is built conditional on IPA_VALIDATION, set (if desired) inside the IPA makefile. Unfortunately, this validation code has some errors. First, there are some mismatched arguments supplied to some dev_err() calls in ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are exposed if validation is enabled. Second, the tag that enables this conditional code isn't used consistently (it's IPA_VALIDATE in some spots and IPA_VALIDATION in others). This series fixes those two problems with the conditional validation code. After much back-and-forth with Leon Romanovsky: --> I retract this series <-- I will include these patches in a future series that will do cleanup of this validation code more completely. Thanks. -Alex Version 2 removes the two patches that introduced ipa_assert(). It also modifies the description in the first patch so that it mentions the changes made to ipa_cmd_table_valid(). -Alex Alex Elder (2): net: ipa: fix init header command validation net: ipa: fix IPA validation drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c| 8 ++--- drivers/net/ipa/ipa_cmd.c | 54 ++ drivers/net/ipa/ipa_cmd.h | 6 ++-- drivers/net/ipa/ipa_endpoint.c | 6 ++-- drivers/net/ipa/ipa_main.c | 6 ++-- drivers/net/ipa/ipa_mem.c | 6 ++-- drivers/net/ipa/ipa_table.c| 6 ++-- drivers/net/ipa/ipa_table.h| 6 ++-- 9 files changed, 58 insertions(+), 42 deletions(-)
Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation
On 3/22/21 1:40 AM, Leon Romanovsky wrote: I'd like to suggest a plan so I can begin to make progress, but do so in a way you/others think is satisfactory. - I would first like to fix the existing bugs, namely that if IPA_VALIDATION is defined there are build errors, and that IPA_VALIDATION is not consistently used. That is this 2-patch series. The thing is that IPA_VALIDATION is not defined in the upstream kernel. There is nothing to be fixed in netdev repository - I assure you that my goal is to simplify the code that does this sort of checking. So here are some specific things I can implement in the coming weeks toward that: - Anything that can be checked at build time, will be checked with BUILD_BUG_ON(). +1 - Anything checked with BUILD_BUG_ON() will*not* be conditional. I.e. it won't be inside an #ifdef IPA_VALIDATION block. - I will review all remaining VALIDATION code (which can't--or can't always--be checked at build time), If it looks prudent to make it*always* be checked, I will make it always be checked (not conditional on IPA_VALIDATION). +1 The result should clearly separate checks that can be done at build time from those that can't. And with what's left (especially on that third sub-bullet) I might have some better examples with which to argue for something different. Or I might just concede that you were right all along. I hope so. I came up with a solution last night that I'm going to try to implement. I will still do the things I mention above. The solution is to create a user space tool inside the drivers/net/ipa directory that will link with the kernel source files and will perform all the basic one-time checks I want to make. Building, and then running that tool will be a build dependency for the driver. If it fails, the driver build will fail. If it passes, all the one-time checks will have been satisfied and the driver will be built, but it will not have all of these silly checks littered throughout. And all checks (even those that aren't constant) will be made at build time. I don't know if that passes your "casual developer" criterion, but at least it will not be part of the kernel code proper. I'm going to withdraw this two-patch series, and post it again along with others, so I the whole set of changes can be done as a complete series. It isn't easy to have something you value be rejected. But I understand your concerns, and accept the reasoning about non-standard coding patterns making it harder for a casual reader to comprehend. As I said, cleaning up this validation code was already my goal, but I'll be doing it differently now. In the end, I might like the new result better, which is always a nice outcome from discussion during review. Thank you. -Alex
[PATCH] iwlwifi: add new pci id for 6235
lspci output: Network controller [0280]: Intel Corporation Centrino Advanced-N6235 [8086:088f] (rev 24) Subsystem: Intel Corporation Centrino Advanced-N 6235 [8086:526a] Cc: sta...@vger.kernel.org Signed-off-by: Alex Hung --- drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c index ffaf973..f85fe36 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c @@ -200,6 +200,7 @@ static const struct pci_device_id iwl_hw_card_ids[] = { {IWL_PCI_DEVICE(0x088E, 0x446A, iwl6035_2agn_sff_cfg)}, {IWL_PCI_DEVICE(0x088E, 0x4860, iwl6035_2agn_cfg)}, {IWL_PCI_DEVICE(0x088F, 0x5260, iwl6035_2agn_cfg)}, + {IWL_PCI_DEVICE(0x088F, 0x526A, iwl6035_2agn_cfg)}, /* 105 Series */ {IWL_PCI_DEVICE(0x0894, 0x0022, iwl105_bgn_cfg)}, -- 2.7.4
Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation
On 3/21/21 8:49 AM, Leon Romanovsky wrote: > On Sun, Mar 21, 2021 at 08:21:24AM -0500, Alex Elder wrote: >> On 3/21/21 3:21 AM, Leon Romanovsky wrote: >>> On Sat, Mar 20, 2021 at 09:17:29AM -0500, Alex Elder wrote: >>>> There are blocks of IPA code that sanity-check various values, at >>>> compile time where possible. Most of these checks can be done once >>>> during development but skipped for normal operation. These checks >>>> permit the driver to make certain assumptions, thereby avoiding the >>>> need for runtime error checking. >>>> >>>> The checks are defined conditionally, but not consistently. In >>>> some cases IPA_VALIDATION enables the optional checks, while in >>>> others IPA_VALIDATE is used. >>>> >>>> Fix this by using IPA_VALIDATION consistently. >>>> >>>> Signed-off-by: Alex Elder >>>> --- >>>> drivers/net/ipa/Makefile | 2 +- >>>> drivers/net/ipa/gsi_trans.c| 8 >>>> drivers/net/ipa/ipa_cmd.c | 4 ++-- >>>> drivers/net/ipa/ipa_cmd.h | 6 +++--- >>>> drivers/net/ipa/ipa_endpoint.c | 6 +++--- >>>> drivers/net/ipa/ipa_main.c | 6 +++--- >>>> drivers/net/ipa/ipa_mem.c | 6 +++--- >>>> drivers/net/ipa/ipa_table.c| 6 +++--- >>>> drivers/net/ipa/ipa_table.h| 6 +++--- >>>> 9 files changed, 25 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile >>>> index afe5df1e6..014ae36ac6004 100644 >>>> --- a/drivers/net/ipa/Makefile >>>> +++ b/drivers/net/ipa/Makefile >>>> @@ -1,5 +1,5 @@ >>>> # Un-comment the next line if you want to validate configuration data >>>> -#ccflags-y+= -DIPA_VALIDATE >>>> +# ccflags-y += -DIPA_VALIDATION >>> >>> Maybe netdev folks think differently here, but general rule that dead >>> code and closed code is such, is not acceptable to in Linux kernel. >>> >>> <...> >> >> What is the purpose of CONFIG_KGDB? Or CONFIG_DEBUG_KERNEL? >> Would you prefer I expose this through a kconfig option? I >> intentionally did not do that, because I really intended it >> to be only for development, so defined it in the Makefile. >> But I have no objection to making it configurable that way. > > I prefer you to follow netdev/linux kernel rules of development. > The upstream repository and drivers/net/* folder especially are not > the place to put code used for the development. How do I add support for new versions of the hardware as it evolves? What I started supporting (v3.5.1) was in some respects relatively old. Version 4.2 is newer, and the v4.5 and beyond are for products that are relatively new on the market. Some updates to IPA (like 4.0+ after 3.5.1, or 4.5+ after 4.2) include substantial updates to the way the hardware works. The code can't support the new hardware without being adapted and generalized to support both old and new. My goal is to get upstream support for IPA for all Qualcomm SoCs that have it. But the hardware design is evolving; Qualcomm is actively developing their architecture so they can support new technologies (e.g. cellular 5G). Development of the driver is simply *necessary*. The assertions I proposed and checks like this are intended as an *aid* to the active development I have been doing. They may look like hacky debugging--checking errors that can't happen. They aren't that at all--they're intended to the compiler help me develop correct code, given I *know* it will be evolving. But the assertions are gone, and I accept/agree that these specific checks "look funny." More below. >>>> -#ifdef IPA_VALIDATE >>>> +#ifdef IPA_VALIDATION >>>>if (!size || size % 8) >>>>return -EINVAL; >>>>if (count < max_alloc) >>>>return -EINVAL; >>>>if (!max_alloc) >>>>return -EINVAL; >>>> -#endif /* IPA_VALIDATE */ >>>> +#endif /* IPA_VALIDATION */ >>> >>> If it is possible to supply those values, the check should be always and >>> not only under some closed config option. >> >> These are assertions. >> >> There is no need to test them for working code. If >> I run the code successfully with these tests enabled >> exactly once, and they are satisfied, then every time >> the code is run thereafter they will pass. So I want >> to c
Re: [PATCH net-next v2 2/2] net: ipa: fix IPA validation
On 3/21/21 3:21 AM, Leon Romanovsky wrote: On Sat, Mar 20, 2021 at 09:17:29AM -0500, Alex Elder wrote: There are blocks of IPA code that sanity-check various values, at compile time where possible. Most of these checks can be done once during development but skipped for normal operation. These checks permit the driver to make certain assumptions, thereby avoiding the need for runtime error checking. The checks are defined conditionally, but not consistently. In some cases IPA_VALIDATION enables the optional checks, while in others IPA_VALIDATE is used. Fix this by using IPA_VALIDATION consistently. Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c| 8 drivers/net/ipa/ipa_cmd.c | 4 ++-- drivers/net/ipa/ipa_cmd.h | 6 +++--- drivers/net/ipa/ipa_endpoint.c | 6 +++--- drivers/net/ipa/ipa_main.c | 6 +++--- drivers/net/ipa/ipa_mem.c | 6 +++--- drivers/net/ipa/ipa_table.c| 6 +++--- drivers/net/ipa/ipa_table.h| 6 +++--- 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index afe5df1e6..014ae36ac6004 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -1,5 +1,5 @@ # Un-comment the next line if you want to validate configuration data -#ccflags-y += -DIPA_VALIDATE +# ccflags-y+= -DIPA_VALIDATION Maybe netdev folks think differently here, but general rule that dead code and closed code is such, is not acceptable to in Linux kernel. <...> What is the purpose of CONFIG_KGDB? Or CONFIG_DEBUG_KERNEL? Would you prefer I expose this through a kconfig option? I intentionally did not do that, because I really intended it to be only for development, so defined it in the Makefile. But I have no objection to making it configurable that way. -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION if (!size || size % 8) return -EINVAL; if (count < max_alloc) return -EINVAL; if (!max_alloc) return -EINVAL; -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ If it is possible to supply those values, the check should be always and not only under some closed config option. These are assertions. There is no need to test them for working code. If I run the code successfully with these tests enabled exactly once, and they are satisfied, then every time the code is run thereafter they will pass. So I want to check them when debugging/developing only. That way there is a mistake, it gets caught, but otherwise there's no pointless argument checking done. I'll explain the first check; the others have similar explanation. In the current code, the passed size is sizeof(struct) for three separate structures. - If the structure size changes, I want to be sure the constraint is still honored - The code will break of someone happens to pass a size of 0. I don't expect that to ever happen, but this states that requirement. This is an optimization, basically, but one that allows the assumed conditions to be optionally verified. /* By allocating a few extra entries in our pool (one less * than the maximum number that will be requested in a @@ -140,14 +140,14 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool, dma_addr_t addr; void *virt; -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION if (!size || size % 8) return -EINVAL; if (count < max_alloc) return -EINVAL; if (!max_alloc) return -EINVAL; -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ Same <...> { -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION /* At one time we assumed a 64-bit build, allowing some do_div() * calls to be replaced by simple division or modulo operations. * We currently only perform divide and modulo operations on u32, @@ -768,7 +768,7 @@ static void ipa_validate_build(void) BUILD_BUG_ON(!ipa_aggr_granularity_val(IPA_AGGR_GRANULARITY)); BUILD_BUG_ON(ipa_aggr_granularity_val(IPA_AGGR_GRANULARITY) > field_max(AGGR_GRANULARITY_FMASK)); -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ BUILD_BUG_ON()s are checked during compilation and not during runtime like IPA_VALIDATION promised. So I should update the description. But I'm not sure where you are referring to. Here is the first line of the patch description: There are blocks of IPA code that sanity-check various values, at compile time where possible. IMHO, the issue here is that this IPA code isn't release quality but some debug drop variant and it is far from expected from submitted code. Doesn't sound very humble, IMHO. This code was found acceptable and merged for mainline a year ago. At that time it suppor
[PATCH net-next 1/5] net: ipa: use configuration data for QSB settings
Use the QSB configuration data in ipa_hardware_config_qsb(), rather than determining in code what values to use based on IPA version. Pass configuration data to ipa_hardware_config() so it can be passed to ipa_hardware_config_qsb(). Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_main.c | 73 +++--- 1 file changed, 28 insertions(+), 45 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index d354e3e65ec50..1ce593b46b04c 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -249,53 +249,35 @@ static void ipa_hardware_config_comp(struct ipa *ipa) iowrite32(val, ipa->reg_virt + IPA_REG_COMP_CFG_OFFSET); } -/* Configure DDR and PCIe max read/write QSB values */ -static void ipa_hardware_config_qsb(struct ipa *ipa) +/* Configure DDR and (possibly) PCIe max read/write QSB values */ +static void +ipa_hardware_config_qsb(struct ipa *ipa, const struct ipa_data *data) { - enum ipa_version version = ipa->version; - u32 max0; - u32 max1; + const struct ipa_qsb_data *data0; + const struct ipa_qsb_data *data1; u32 val; - /* QMB_0 represents DDR; QMB_1 represents PCIe */ - val = u32_encode_bits(8, GEN_QMB_0_MAX_WRITES_FMASK); - switch (version) { - case IPA_VERSION_4_2: - max1 = 0; /* PCIe not present */ - break; - case IPA_VERSION_4_5: - max1 = 8; - break; - default: - max1 = 4; - break; - } - val |= u32_encode_bits(max1, GEN_QMB_1_MAX_WRITES_FMASK); + /* assert(data->qsb_count > 0); */ + /* assert(data->qsb_count < 3); */ + + /* QMB 0 represents DDR; QMB 1 (if present) represents PCIe */ + data0 = &data->qsb_data[IPA_QSB_MASTER_DDR]; + if (data->qsb_count > 1) + data1 = &data->qsb_data[IPA_QSB_MASTER_PCIE]; + + /* Max outstanding write accesses for QSB masters */ + val = u32_encode_bits(data0->max_writes, GEN_QMB_0_MAX_WRITES_FMASK); + if (data->qsb_count > 1) + val |= u32_encode_bits(data1->max_writes, + GEN_QMB_1_MAX_WRITES_FMASK); iowrite32(val, ipa->reg_virt + IPA_REG_QSB_MAX_WRITES_OFFSET); - max1 = 12; - switch (version) { - case IPA_VERSION_3_5_1: - max0 = 8; - break; - case IPA_VERSION_4_0: - case IPA_VERSION_4_1: - max0 = 12; - break; - case IPA_VERSION_4_2: - max0 = 12; - max1 = 0; /* PCIe not present */ - break; - case IPA_VERSION_4_5: - max0 = 0; /* No limit (hardware maximum) */ - break; - } - val = u32_encode_bits(max0, GEN_QMB_0_MAX_READS_FMASK); - val |= u32_encode_bits(max1, GEN_QMB_1_MAX_READS_FMASK); - if (version != IPA_VERSION_3_5_1) { - /* GEN_QMB_0_MAX_READS_BEATS is 0 */ - /* GEN_QMB_1_MAX_READS_BEATS is 0 */ - } + /* Max outstanding read accesses for QSB masters */ + val = u32_encode_bits(data0->max_reads, GEN_QMB_0_MAX_READS_FMASK); + /* GEN_QMB_0_MAX_READS_BEATS is 0 (IPA v4.0 and above) */ + if (data->qsb_count > 1) + val |= u32_encode_bits(data1->max_reads, + GEN_QMB_1_MAX_READS_FMASK); iowrite32(val, ipa->reg_virt + IPA_REG_QSB_MAX_READS_OFFSET); } @@ -385,8 +367,9 @@ static void ipa_hardware_dcd_deconfig(struct ipa *ipa) /** * ipa_hardware_config() - Primitive hardware initialization * @ipa: IPA pointer + * @data: IPA configuration data */ -static void ipa_hardware_config(struct ipa *ipa) +static void ipa_hardware_config(struct ipa *ipa, const struct ipa_data *data) { enum ipa_version version = ipa->version; u32 granularity; @@ -414,7 +397,7 @@ static void ipa_hardware_config(struct ipa *ipa) ipa_hardware_config_comp(ipa); /* Configure system bus limits */ - ipa_hardware_config_qsb(ipa); + ipa_hardware_config_qsb(ipa, data); if (version < IPA_VERSION_4_5) { /* Configure aggregation timer granularity */ @@ -610,7 +593,7 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data) */ ipa_clock_get(ipa); - ipa_hardware_config(ipa); + ipa_hardware_config(ipa, data); ret = ipa_endpoint_config(ipa); if (ret) -- 2.27.0
[PATCH net-next 5/5] net: ipa: update some comments in "ipa_data.h"
Fix/expand some comments in "ipa_data.h". Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data.h | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index caf9b3f9577eb..7816583fc14aa 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -18,8 +18,9 @@ * Boot-time configuration data is used to define the configuration of the * IPA and GSI resources to use for a given platform. This data is supplied * via the Device Tree match table, associated with a particular compatible - * string. The data defines information about resources, endpoints, and - * channels. + * string. The data defines information about how resources, endpoints and + * channels, memory, clocking and so on are allocated and used for the + * platform. * * Resources are data structures used internally by the IPA hardware. The * configuration data defines the number (or limits of the number) of various @@ -75,10 +76,10 @@ struct ipa_qsb_data { * * A GSI channel is a unidirectional means of transferring data to or * from (and through) the IPA. A GSI channel has a ring buffer made - * up of "transfer elements" (TREs) that specify individual data transfers - * or IPA immediate commands. TREs are filled by the AP, and control - * is passed to IPA hardware by writing the last written element - * into a doorbell register. + * up of "transfer ring elements" (TREs) that specify individual data + * transfers or IPA immediate commands. TREs are filled by the AP, + * and control is passed to IPA hardware by writing the last written + * element into a doorbell register. * * When data transfer commands have completed the GSI generates an * event (a structure of data) and optionally signals the AP with @@ -264,7 +265,7 @@ struct ipa_resource_data { * @imem_addr: physical address of IPA region within IMEM * @imem_size: size in bytes of IPA IMEM region * @smem_id: item identifier for IPA region within SMEM memory - * @imem_size: size in bytes of the IPA SMEM region + * @smem_size: size in bytes of the IPA SMEM region */ struct ipa_mem_data { u32 local_count; @@ -307,14 +308,14 @@ struct ipa_clock_data { * @endpoint_count:number of entries in the endpoint_data array * @endpoint_data: IPA endpoint/GSI channel data * @resource_data: IPA resource configuration data - * @mem_count: number of entries in the mem_data array - * @mem_data: IPA-local shared memory region data + * @mem_data: IPA memory region data + * @clock_data:IPA clock and interconnect data */ struct ipa_data { enum ipa_version version; - u32 qsb_count; /* # entries in qsb_data[] */ + u32 qsb_count; /* number of entries in qsb_data[] */ const struct ipa_qsb_data *qsb_data; - u32 endpoint_count; /* # entries in endpoint_data[] */ + u32 endpoint_count; /* number of entries in endpoint_data[] */ const struct ipa_gsi_endpoint_data *endpoint_data; const struct ipa_resource_data *resource_data; const struct ipa_mem_data *mem_data; -- 2.27.0
[PATCH net-next 3/5] net: ipa: split sequencer type in two
An IPA endpoint has a sequencer that must be configured based on how the endpoint is to be used. Currently the IPA code programs the sequencer type by splitting a value into four 4-bit nibbles. Doing that doesn't really add much value, and regardless, a better way of splitting the sequencer type is into two halves--the lower byte describing how normal packet processing is handled, and the next byte describing information about processing replicas. So split the sequencer type into two sub-parts: the sequencer type and the replication sequencer type. Define the values supported for the "main" sequencer type, and define the values supported for the replication part separately. In addition, the sequencer type names are quite verbose, encoding what the type includes, but also what it *excludes*. Rename the sequencer types in a way that mainly describes the number of passes that a packet takes through the IPA processing pipeline, and how many of those passes end by supplying the processed packet to the microprocessor. The result expands the supported types beyond what is required for now, but simplifies the way these are defined. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 8 -- drivers/net/ipa/ipa_data-sdm845.c | 7 +++-- drivers/net/ipa/ipa_data.h| 6 ++-- drivers/net/ipa/ipa_endpoint.c| 13 - drivers/net/ipa/ipa_endpoint.h| 1 + drivers/net/ipa/ipa_reg.h | 48 --- 6 files changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 8fa10d0d9a4e7..fd2265d032cc8 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -31,7 +31,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 20, }, .endpoint = { - .seq_type = IPA_SEQ_DMA_ONLY, + .seq_type = IPA_SEQ_DMA, .config = { .resource_group = 0, .dma_mode = true, @@ -51,6 +51,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .seq_type = IPA_SEQ_INVALID, + .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 0, .aggregation= true, @@ -73,8 +74,8 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .filter_support = true, - .seq_type = - IPA_SEQ_PKT_PROCESS_NO_DEC_NO_UCP_DMAP, + .seq_type = IPA_SEQ_1_PASS_SKIP_LAST_UC, + .seq_rep_type = IPA_SEQ_REP_DMA_PARSER, .config = { .resource_group = 0, .checksum = true, @@ -99,6 +100,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .seq_type = IPA_SEQ_INVALID, + .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 0, .checksum = true, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index d9659fd22322a..7f7625cd96b0d 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -36,7 +36,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 20, }, .endpoint = { - .seq_type = IPA_SEQ_DMA_ONLY, + .seq_type = IPA_SEQ_DMA, .config = { .resource_group = 1, .dma_mode = true, @@ -56,6 +56,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .seq_type = IPA_SEQ_INVALID, + .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 1, .aggregation= true, @@ -78,8 +79,7 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .filter_support = true, - .seq_type = - IPA_SEQ_2ND_PKT_PROCESS_PASS_NO_DEC_UCP, +
[PATCH net-next 4/5] net: ipa: sequencer type is for TX endpoints only
We only program the sequencer type for TX endpoints. So move the definition of the sequencer type fields into the TX-specific portion of the endpoint configuration data. There's no need to maintain this in the IPA structure; we can extract it from the configuration data it points to in the one spot it's needed. We previously specified the sequencer type for RX endpoints with INVALID values. These are no longer needed, so get rid of them. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 12 +--- drivers/net/ipa/ipa_data-sdm845.c | 10 -- drivers/net/ipa/ipa_data.h| 13 + drivers/net/ipa/ipa_endpoint.c| 7 +++ drivers/net/ipa/ipa_endpoint.h| 2 -- drivers/net/ipa/ipa_reg.h | 2 -- 6 files changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index fd2265d032cc8..621ad15c9e67d 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -31,11 +31,13 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 20, }, .endpoint = { - .seq_type = IPA_SEQ_DMA, .config = { .resource_group = 0, .dma_mode = true, .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, + .tx = { + .seq_type = IPA_SEQ_DMA, + }, }, }, }, @@ -50,8 +52,6 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 6, }, .endpoint = { - .seq_type = IPA_SEQ_INVALID, - .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 0, .aggregation= true, @@ -74,14 +74,14 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { }, .endpoint = { .filter_support = true, - .seq_type = IPA_SEQ_1_PASS_SKIP_LAST_UC, - .seq_rep_type = IPA_SEQ_REP_DMA_PARSER, .config = { .resource_group = 0, .checksum = true, .qmap = true, .status_enable = true, .tx = { + .seq_type = IPA_SEQ_1_PASS_SKIP_LAST_UC, + .seq_rep_type = IPA_SEQ_REP_DMA_PARSER, .status_endpoint = IPA_ENDPOINT_MODEM_AP_RX, }, @@ -99,8 +99,6 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 6, }, .endpoint = { - .seq_type = IPA_SEQ_INVALID, - .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 0, .checksum = true, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 7f7625cd96b0d..6b5173f47 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -36,11 +36,13 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 20, }, .endpoint = { - .seq_type = IPA_SEQ_DMA, .config = { .resource_group = 1, .dma_mode = true, .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX, + .tx = { + .seq_type = IPA_SEQ_DMA, + }, }, }, }, @@ -55,8 +57,6 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { .tlv_count = 8, }, .endpoint = { - .seq_type = IPA_SEQ_INVALID, - .seq_rep_type = IPA_SEQ_REP_INVALID, .config = { .resource_group = 1, .aggregation= true, @@ -79,13 +79,13 @@ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { },
[PATCH net-next 2/5] net: ipa: implement MAX_READS_BEATS QSB data
Starting with IPA v4.0, a limit is placed on the number of bytes outstanding in a transaction, to reduce latency. The limit is imposed only if this value is non-zero. We don't use a non-zero value for SC7180, but newer versions of IPA do. Prepare for that by allowing a programmed value to be specified in the platform configuration data. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 1 + drivers/net/ipa/ipa_data.h| 2 ++ drivers/net/ipa/ipa_main.c| 10 -- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 216f790b22b66..8fa10d0d9a4e7 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -14,6 +14,7 @@ static const struct ipa_qsb_data ipa_qsb_data[] = { [IPA_QSB_MASTER_DDR] = { .max_writes = 8, .max_reads = 12, + /* no outstanding read byte (beat) limit */ }, }; diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index d50cd5ae7714f..4162c4722c00c 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -59,10 +59,12 @@ enum ipa_qsb_master_id { * struct ipa_qsb_data - Qualcomm System Bus configuration data * @max_writes:Maximum outstanding write requests for this master * @max_reads: Maximum outstanding read requests for this master + * @max_reads_beats: Max outstanding read bytes in 8-byte "beats" (if non-zero) */ struct ipa_qsb_data { u8 max_writes; u8 max_reads; + u8 max_reads_beats; /* Not present for IPA v3.5.1 */ }; /** diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 1ce593b46b04c..64b92dfdd3f5c 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -274,10 +274,16 @@ ipa_hardware_config_qsb(struct ipa *ipa, const struct ipa_data *data) /* Max outstanding read accesses for QSB masters */ val = u32_encode_bits(data0->max_reads, GEN_QMB_0_MAX_READS_FMASK); - /* GEN_QMB_0_MAX_READS_BEATS is 0 (IPA v4.0 and above) */ - if (data->qsb_count > 1) + if (ipa->version >= IPA_VERSION_4_0) + val |= u32_encode_bits(data0->max_reads_beats, + GEN_QMB_0_MAX_READS_BEATS_FMASK); + if (data->qsb_count > 1) { val |= u32_encode_bits(data1->max_reads, GEN_QMB_1_MAX_READS_FMASK); + if (ipa->version >= IPA_VERSION_4_0) + val |= u32_encode_bits(data1->max_reads_beats, + GEN_QMB_1_MAX_READS_BEATS_FMASK); + } iowrite32(val, ipa->reg_virt + IPA_REG_QSB_MAX_READS_OFFSET); } -- 2.27.0
[PATCH net-next 0/5] net: ipa: more configuration data updates
This series starts with two patches that should have been included in an earlier series. With these in place, QSB settings are programmed from information found in the data files rather than being embedded in code. Support is then added for reprenting another QSB property (supported for IPA v4.0+). The third patch updates the definition of the sequencer type used for an endpoint. Previously a set of 2-byte symbols with fairly long names defined the sequencer type, but now those are broken into 1-byte halves whose names are a little more informative. The fourth patch moves the sequencer type definition so it only applies to TX endpoints (they aren't valid for RX endpoints). And the last makes some minor documentation updates. -Alex Alex Elder (5): net: ipa: use configuration data for QSB settings net: ipa: implement MAX_READS_BEATS QSB data net: ipa: split sequencer type in two net: ipa: sequencer type is for TX endpoints only net: ipa: update some comments in "ipa_data.h" drivers/net/ipa/ipa_data-sc7180.c | 11 +++-- drivers/net/ipa/ipa_data-sdm845.c | 9 ++-- drivers/net/ipa/ipa_data.h| 36 --- drivers/net/ipa/ipa_endpoint.c| 14 +++--- drivers/net/ipa/ipa_endpoint.h| 1 - drivers/net/ipa/ipa_main.c| 77 +-- drivers/net/ipa/ipa_reg.h | 46 +++--- 7 files changed, 97 insertions(+), 97 deletions(-) -- 2.27.0
Re: [PATCH net-next 0/4] net: ipa: fix validation
On 3/20/21 8:24 AM, Alex Elder wrote: On 3/18/21 11:29 PM, Alex Elder wrote: There is sanity checking code in the IPA driver that's meant to be enabled only during development. This allows the driver to make certain assumptions, but not have to verify those assumptions are true at (operational) runtime. This code is built conditional on IPA_VALIDATION, set (if desired) inside the IPA makefile. Given the pushback on the ipa_assert() patch I will send out version 2 of this series, omitting the two patches that involve assertions. I posted version 2, but I think dropping two patches without changing the subject might have messed up the robots. I don't know how to fix that and don't want to make any more trouble by trying. If there's something I can do, someone please tell me. -Alex I still think there's a case for my proposal, but I'm going to move on for now and try to find other ways to do what I want. In some cases BUILD_BUG_ON() or WARN_ON_DEV() could be used. In other spots, I might be able to use dev_dbg() for checking things only while developing. But there remain a few cases where none of these options is quite right. If I ever want to suggest an assertion again I'll do it as an RFC and will copy Leon and Andrew, to make sure they can provide input. Thanks. -Alex Unfortunately, this validation code has some errors. First, there are some mismatched arguments supplied to some dev_err() calls in ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are exposed if validation is enabled. Second, the tag that enables this conditional code isn't used consistently (it's IPA_VALIDATE in some spots and IPA_VALIDATION in others). This series fixes those two problems with the conditional validation code. In addition, this series introduces some new assertion macros. I have been meaning to add this for a long time. There are comments indicating places where assertions could be checked throughout the code. The macros are designed so that any asserted condition will be checked at compile time if possible. Otherwise, the condition will be checked at runtime *only* if IPA_VALIDATION is enabled, and ignored otherwise. NOTE: The third patch produces two bogus (but understandable) warnings from checkpatch.pl. It does not recognize that the "expr" argument passed to those macros aren't actually evaluated more than once. In both cases, all but one reference is consumed by the preprocessor or compiler. A final patch converts a handful of commented assertions into "real" ones. Some existing validation code can done more simply with assertions, so over time such cases will be converted. For now though, this series adds this assertion capability. -Alex Alex Elder (4): net: ipa: fix init header command validation net: ipa: fix IPA validation net: ipa: introduce ipa_assert() net: ipa: activate some commented assertions drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c | 8 ++--- drivers/net/ipa/ipa_assert.h | 50 drivers/net/ipa/ipa_cmd.c | 53 ++ drivers/net/ipa/ipa_cmd.h | 6 ++-- drivers/net/ipa/ipa_endpoint.c | 6 ++-- drivers/net/ipa/ipa_main.c | 6 ++-- drivers/net/ipa/ipa_mem.c | 6 ++-- drivers/net/ipa/ipa_reg.h | 7 +++-- drivers/net/ipa/ipa_table.c | 11 --- drivers/net/ipa/ipa_table.h | 6 ++-- 11 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 drivers/net/ipa/ipa_assert.h
[PATCH net-next v2 2/2] net: ipa: fix IPA validation
There are blocks of IPA code that sanity-check various values, at compile time where possible. Most of these checks can be done once during development but skipped for normal operation. These checks permit the driver to make certain assumptions, thereby avoiding the need for runtime error checking. The checks are defined conditionally, but not consistently. In some cases IPA_VALIDATION enables the optional checks, while in others IPA_VALIDATE is used. Fix this by using IPA_VALIDATION consistently. Signed-off-by: Alex Elder --- drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c| 8 drivers/net/ipa/ipa_cmd.c | 4 ++-- drivers/net/ipa/ipa_cmd.h | 6 +++--- drivers/net/ipa/ipa_endpoint.c | 6 +++--- drivers/net/ipa/ipa_main.c | 6 +++--- drivers/net/ipa/ipa_mem.c | 6 +++--- drivers/net/ipa/ipa_table.c| 6 +++--- drivers/net/ipa/ipa_table.h| 6 +++--- 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/net/ipa/Makefile b/drivers/net/ipa/Makefile index afe5df1e6..014ae36ac6004 100644 --- a/drivers/net/ipa/Makefile +++ b/drivers/net/ipa/Makefile @@ -1,5 +1,5 @@ # Un-comment the next line if you want to validate configuration data -#ccflags-y += -DIPA_VALIDATE +# ccflags-y+= -DIPA_VALIDATION obj-$(CONFIG_QCOM_IPA) += ipa.o diff --git a/drivers/net/ipa/gsi_trans.c b/drivers/net/ipa/gsi_trans.c index 6c3ed5b17b80c..284063b39b33c 100644 --- a/drivers/net/ipa/gsi_trans.c +++ b/drivers/net/ipa/gsi_trans.c @@ -90,14 +90,14 @@ int gsi_trans_pool_init(struct gsi_trans_pool *pool, size_t size, u32 count, { void *virt; -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION if (!size || size % 8) return -EINVAL; if (count < max_alloc) return -EINVAL; if (!max_alloc) return -EINVAL; -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ /* By allocating a few extra entries in our pool (one less * than the maximum number that will be requested in a @@ -140,14 +140,14 @@ int gsi_trans_pool_init_dma(struct device *dev, struct gsi_trans_pool *pool, dma_addr_t addr; void *virt; -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION if (!size || size % 8) return -EINVAL; if (count < max_alloc) return -EINVAL; if (!max_alloc) return -EINVAL; -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ /* Don't let allocations cross a power-of-two boundary */ size = __roundup_pow_of_two(size); diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index d73b03a80ef89..2a4db902a728b 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -162,7 +162,7 @@ static void ipa_cmd_validate_build(void) #undef TABLE_SIZE } -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION /* Validate a memory region holding a table */ bool ipa_cmd_table_valid(struct ipa *ipa, const struct ipa_mem *mem, @@ -317,7 +317,7 @@ bool ipa_cmd_data_valid(struct ipa *ipa) return true; } -#endif /* IPA_VALIDATE */ +#endif /* IPA_VALIDATION */ int ipa_cmd_pool_init(struct gsi_channel *channel, u32 tre_max) { diff --git a/drivers/net/ipa/ipa_cmd.h b/drivers/net/ipa/ipa_cmd.h index 6dd3d35cf315d..429245f075122 100644 --- a/drivers/net/ipa/ipa_cmd.h +++ b/drivers/net/ipa/ipa_cmd.h @@ -50,7 +50,7 @@ struct ipa_cmd_info { enum dma_data_direction direction; }; -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION /** * ipa_cmd_table_valid() - Validate a memory region holding a table @@ -73,7 +73,7 @@ bool ipa_cmd_table_valid(struct ipa *ipa, const struct ipa_mem *mem, */ bool ipa_cmd_data_valid(struct ipa *ipa); -#else /* !IPA_VALIDATE */ +#else /* !IPA_VALIDATION */ static inline bool ipa_cmd_table_valid(struct ipa *ipa, const struct ipa_mem *mem, bool route, @@ -87,7 +87,7 @@ static inline bool ipa_cmd_data_valid(struct ipa *ipa) return true; } -#endif /* !IPA_VALIDATE */ +#endif /* !IPA_VALIDATION */ /** * ipa_cmd_pool_init() - initialize command channel pools diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c index 7209ee3c31244..1a4de4e9eafcd 100644 --- a/drivers/net/ipa/ipa_endpoint.c +++ b/drivers/net/ipa/ipa_endpoint.c @@ -75,7 +75,7 @@ struct ipa_status { #define IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK GENMASK(31, 22) #define IPA_STATUS_FLAGS2_TAG_FMASKGENMASK_ULL(63, 16) -#ifdef IPA_VALIDATE +#ifdef IPA_VALIDATION static bool ipa_endpoint_data_valid_one(struct ipa *ipa, u32 count, const struct ipa_gsi_endpoint_data *all_data, @@ -225,7 +225,7 @@ static bool ipa_endpoint_data_valid(struct ipa *ipa, u32 count, return true; } -#else /* !IPA_VALIDATE */ +#else /* !IPA_VALIDATION */ static bool ipa_endpoint_data_valid(struct ipa *ipa
[PATCH net-next v2 0/2] net: ipa: fix validation
There is sanity checking code in the IPA driver that's meant to be enabled only during development. This allows the driver to make certain assumptions, but not have to verify those assumptions are true at (operational) runtime. This code is built conditional on IPA_VALIDATION, set (if desired) inside the IPA makefile. Unfortunately, this validation code has some errors. First, there are some mismatched arguments supplied to some dev_err() calls in ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are exposed if validation is enabled. Second, the tag that enables this conditional code isn't used consistently (it's IPA_VALIDATE in some spots and IPA_VALIDATION in others). This series fixes those two problems with the conditional validation code. Version 2 removes the two patches that introduced ipa_assert(). It also modifies the description in the first patch so that it mentions the changes made to ipa_cmd_table_valid(). -Alex Alex Elder (2): net: ipa: fix init header command validation net: ipa: fix IPA validation drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c| 8 ++--- drivers/net/ipa/ipa_cmd.c | 54 ++ drivers/net/ipa/ipa_cmd.h | 6 ++-- drivers/net/ipa/ipa_endpoint.c | 6 ++-- drivers/net/ipa/ipa_main.c | 6 ++-- drivers/net/ipa/ipa_mem.c | 6 ++-- drivers/net/ipa/ipa_table.c| 6 ++-- drivers/net/ipa/ipa_table.h| 6 ++-- 9 files changed, 58 insertions(+), 42 deletions(-) -- 2.27.0
[PATCH net-next v2 1/2] net: ipa: fix init header command validation
We use ipa_cmd_header_valid() to ensure certain values we will program into hardware are within range, well in advance of when we actually program them. This way we avoid having to check for errors when we actually program the hardware. Unfortunately the dev_err() call for a bad offset value does not supply the arguments to match the format specifiers properly. Fix this. There was also supposed to be a check to ensure the size to be programmed fits in the field that holds it. Add this missing check. Rearrange the way we ensure the header table fits in overall IPA memory range. Finally, update ipa_cmd_table_valid() so the format of messages printed for errors matches what's done in ipa_cmd_header_valid(). Signed-off-by: Alex Elder --- v2: - Updated description to mention ipa_cmd_table_valid() changes. drivers/net/ipa/ipa_cmd.c | 50 ++- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c index 35e35852c25c5..d73b03a80ef89 100644 --- a/drivers/net/ipa/ipa_cmd.c +++ b/drivers/net/ipa/ipa_cmd.c @@ -175,21 +175,23 @@ bool ipa_cmd_table_valid(struct ipa *ipa, const struct ipa_mem *mem, : field_max(IP_FLTRT_FLAGS_NHASH_ADDR_FMASK); if (mem->offset > offset_max || ipa->mem_offset > offset_max - mem->offset) { - dev_err(dev, "IPv%c %s%s table region offset too large " - "(0x%04x + 0x%04x > 0x%04x)\n", - ipv6 ? '6' : '4', hashed ? "hashed " : "", - route ? "route" : "filter", - ipa->mem_offset, mem->offset, offset_max); + dev_err(dev, "IPv%c %s%s table region offset too large\n", + ipv6 ? '6' : '4', hashed ? "hashed " : "", + route ? "route" : "filter"); + dev_err(dev, "(0x%04x + 0x%04x > 0x%04x)\n", + ipa->mem_offset, mem->offset, offset_max); + return false; } if (mem->offset > ipa->mem_size || mem->size > ipa->mem_size - mem->offset) { - dev_err(dev, "IPv%c %s%s table region out of range " - "(0x%04x + 0x%04x > 0x%04x)\n", - ipv6 ? '6' : '4', hashed ? "hashed " : "", - route ? "route" : "filter", - mem->offset, mem->size, ipa->mem_size); + dev_err(dev, "IPv%c %s%s table region out of range\n", + ipv6 ? '6' : '4', hashed ? "hashed " : "", + route ? "route" : "filter"); + dev_err(dev, "(0x%04x + 0x%04x > 0x%04x)\n", + mem->offset, mem->size, ipa->mem_size); + return false; } @@ -205,22 +207,36 @@ static bool ipa_cmd_header_valid(struct ipa *ipa) u32 size_max; u32 size; + /* In ipa_cmd_hdr_init_local_add() we record the offset and size +* of the header table memory area. Make sure the offset and size +* fit in the fields that need to hold them, and that the entire +* range is within the overall IPA memory range. +*/ offset_max = field_max(HDR_INIT_LOCAL_FLAGS_HDR_ADDR_FMASK); if (mem->offset > offset_max || ipa->mem_offset > offset_max - mem->offset) { - dev_err(dev, "header table region offset too large " - "(0x%04x + 0x%04x > 0x%04x)\n", - ipa->mem_offset + mem->offset, offset_max); + dev_err(dev, "header table region offset too large\n"); + dev_err(dev, "(0x%04x + 0x%04x > 0x%04x)\n", + ipa->mem_offset, mem->offset, offset_max); + return false; } size_max = field_max(HDR_INIT_LOCAL_FLAGS_TABLE_SIZE_FMASK); size = ipa->mem[IPA_MEM_MODEM_HEADER].size; size += ipa->mem[IPA_MEM_AP_HEADER].size; - if (mem->offset > ipa->mem_size || size > ipa->mem_size - mem->offset) { - dev_err(dev, "header table region out of range " - "(0x%04x + 0x%04x > 0x%04x)\n", - mem->offset, size, ipa->mem_size); + + if (size > size_max) { + dev_err(dev, "header table region size too large\n"); + dev_err(dev, "(0x%04x > 0x%08x)\n", size, size_max); + + return false; + } + if (size > ipa->mem_size || mem->offset > ipa->mem_size - size) { + dev_err(dev, "header table region out of range\n"); + dev_err(dev, "(0x%04x + 0x%04x > 0x%04x)\n", + mem->offset, size, ipa->mem_size); + return false; } -- 2.27.0
Re: [PATCH net-next 0/4] net: ipa: fix validation
On 3/18/21 11:29 PM, Alex Elder wrote: There is sanity checking code in the IPA driver that's meant to be enabled only during development. This allows the driver to make certain assumptions, but not have to verify those assumptions are true at (operational) runtime. This code is built conditional on IPA_VALIDATION, set (if desired) inside the IPA makefile. Given the pushback on the ipa_assert() patch I will send out version 2 of this series, omitting the two patches that involve assertions. I still think there's a case for my proposal, but I'm going to move on for now and try to find other ways to do what I want. In some cases BUILD_BUG_ON() or WARN_ON_DEV() could be used. In other spots, I might be able to use dev_dbg() for checking things only while developing. But there remain a few cases where none of these options is quite right. If I ever want to suggest an assertion again I'll do it as an RFC and will copy Leon and Andrew, to make sure they can provide input. Thanks. -Alex Unfortunately, this validation code has some errors. First, there are some mismatched arguments supplied to some dev_err() calls in ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are exposed if validation is enabled. Second, the tag that enables this conditional code isn't used consistently (it's IPA_VALIDATE in some spots and IPA_VALIDATION in others). This series fixes those two problems with the conditional validation code. In addition, this series introduces some new assertion macros. I have been meaning to add this for a long time. There are comments indicating places where assertions could be checked throughout the code. The macros are designed so that any asserted condition will be checked at compile time if possible. Otherwise, the condition will be checked at runtime *only* if IPA_VALIDATION is enabled, and ignored otherwise. NOTE: The third patch produces two bogus (but understandable) warnings from checkpatch.pl. It does not recognize that the "expr" argument passed to those macros aren't actually evaluated more than once. In both cases, all but one reference is consumed by the preprocessor or compiler. A final patch converts a handful of commented assertions into "real" ones. Some existing validation code can done more simply with assertions, so over time such cases will be converted. For now though, this series adds this assertion capability. -Alex Alex Elder (4): net: ipa: fix init header command validation net: ipa: fix IPA validation net: ipa: introduce ipa_assert() net: ipa: activate some commented assertions drivers/net/ipa/Makefile | 2 +- drivers/net/ipa/gsi_trans.c| 8 ++--- drivers/net/ipa/ipa_assert.h | 50 drivers/net/ipa/ipa_cmd.c | 53 ++ drivers/net/ipa/ipa_cmd.h | 6 ++-- drivers/net/ipa/ipa_endpoint.c | 6 ++-- drivers/net/ipa/ipa_main.c | 6 ++-- drivers/net/ipa/ipa_mem.c | 6 ++-- drivers/net/ipa/ipa_reg.h | 7 +++-- drivers/net/ipa/ipa_table.c| 11 --- drivers/net/ipa/ipa_table.h| 6 ++-- 11 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 drivers/net/ipa/ipa_assert.h
Re: [PATCH net-next 4/4] net: ipa: activate some commented assertions
On 3/19/21 1:32 PM, Andrew Lunn wrote: @@ -212,7 +213,7 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) BCR_HOLB_DROP_L2_IRQ_FMASK | BCR_DUAL_TX_FMASK; - /* assert(version != IPA_VERSION_4_5); */ + ipa_assert(NULL, version != IPA_VERSION_4_5); Hi Alex It is impossible to see just looking what the NULL means. And given its the first parameter, it should be quite important. I find this API pretty bad. I actually don't like the first argument. I would have supplied the main IPA pointer, but that isn't always visible or available (the GSI code doesn't have the IPA pointer definition). So I thought instead I could at least supply the underlying device if available, and use dev_err(). But I wouldn't mind just getting rid of the first argument and having a failed assertion always call pr_err() and not dev_err(). The thing of most value to me the asserted condition. Thoughts? -Alex
Re: [PATCH net-next 3/4] net: ipa: introduce ipa_assert()
On 3/19/21 10:32 AM, Leon Romanovsky wrote: +/* Verify the expression yields true, and fail at build time if possible */ +#define ipa_assert(dev, expr) \ + do { \ + if (__builtin_constant_p(expr)) \ + compiletime_assert(expr, __ipa_failure_msg(expr)); \ + else \ + __ipa_assert_runtime(dev, expr); \ + } while (0) + +/* Report an error if the given expression evaluates to false at runtime */ +#define ipa_assert_always(dev, expr) \ + do { \ + if (unlikely(!(expr))) { \ + struct device *__dev = (dev); \ + \ + if (__dev) \ + dev_err(__dev, __ipa_failure_msg(expr)); \ + else \ + pr_err(__ipa_failure_msg(expr)); \ + } \ + } while (0) It will be much better for everyone if you don't obfuscate existing kernel primitives and don't hide constant vs. dynamic expressions. I don't agree with this characterization. Yes, there is some complexity in this one source file, where ipa_assert() is defined. But as a result, callers are simple one-line statements (similar to WARN_ON()). It is not complexity but being explicit vs. implicit. The coding style that has explicit flows will be always better than implicit one. By adding your custom assert, you are hiding the flows and makes unclear what can be evaluated at compilation and what can't. Assertions like this are a tool. They aid readability by communicating conditions that can be assumed to hold at the time code is executed. They are *not* part of the normal code function. They are optional, and code *must* operate correctly even if all assertions are removed. Whether a condition that is asserted can be determined at compile time or build time is irrelevant. But as an optimization, if it can be checked at compile time, I want to do that, so we can catch the problem as early as possible. I understand your point about separating compile-time versus runtime code. I mean, it's a piece of really understanding what's going on when your code is executing. But what I'm trying to do here is more like a "functional comment," i.e., a comment about the state of things that can be optionally verified. I find them to be concise and useful: assert(count <= MAX_COUNT); versus /* Caller must ensure count is in range */ We might just disagree on this, and that's OK. I'm interested to hear whether others think. -Alex
Re: [PATCH net-next 4/4] net: ipa: activate some commented assertions
On 3/19/21 10:17 AM, Leon Romanovsky wrote: @@ -212,7 +213,7 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) BCR_HOLB_DROP_L2_IRQ_FMASK | BCR_DUAL_TX_FMASK; - /* assert(version != IPA_VERSION_4_5); */ + ipa_assert(NULL, version != IPA_VERSION_4_5); This assert will fire for IPA_VERSION_4_2, I doubt that this is something you want. No, it will only fail if version == IPA_VERSION_4_5. The logic of an assertion is the opposite of BUG_ON(). It fails only if the asserted condition yields false. ok, this ipa_reg_bcr_val() is called in only one place and has a protection from IPA_VERSION_4_5, why don't you code it at the same .c file instead of adding useless assert? As I mentioned in my other message, the purpose of an assertion is *communicating with the reader*. The fact that an assertion may expand to code that ensures the assertion is true is secondary. This particular assertion says that the version will never be 4.5. While looking at this function, you don't need to see if the caller ensures that, the assertion *tells* you. Whether an assertion is warranted is really subjective. You may not appreciate the value of that, but I do. -Alex
[PATCH net-next 2/5] net: ipa: fix canary count for SC7180 UC_INFO region
There should be no canary values written before the beginning of the UC_INFO memory region. This was correct for SDM845, but somehow was committed with the wrong value for SC7180. This bug seems to cause no harm, so we'll just correct it without back-porting. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 01681be402262..434869508a215 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -206,7 +206,7 @@ static const struct ipa_mem ipa_mem_local_data[] = { [IPA_MEM_UC_INFO] = { .offset = 0x0080, .size = 0x0200, - .canary_count = 2, + .canary_count = 0, }, [IPA_MEM_V4_FILTER_HASHED] = { .offset = 0x0288, -- 2.27.0
[PATCH net-next 5/5] net: ipa: define QSB limits in configuration data
Define the maximum number of reads and writes to configure for the QSB masters used for IPA in configuration data. We don't use these values yet; the next commit takes care of that. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 10 ++ drivers/net/ipa/ipa_data-sdm845.c | 14 ++ drivers/net/ipa/ipa_data.h| 24 ++-- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index f65abc19ae9d7..216f790b22b66 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -9,6 +9,14 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" +/* QSB configuration for the SC7180 SoC. */ +static const struct ipa_qsb_data ipa_qsb_data[] = { + [IPA_QSB_MASTER_DDR] = { + .max_writes = 8, + .max_reads = 12, + }, +}; + /* Endpoint configuration for the SC7180 SoC. */ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { [IPA_ENDPOINT_AP_COMMAND_TX] = { @@ -328,6 +336,8 @@ static const struct ipa_clock_data ipa_clock_data = { /* Configuration data for the SC7180 SoC. */ const struct ipa_data ipa_data_sc7180 = { .version= IPA_VERSION_4_2, + .qsb_count = ARRAY_SIZE(ipa_qsb_data), + .qsb_data = ipa_qsb_data, .endpoint_count = ARRAY_SIZE(ipa_gsi_endpoint_data), .endpoint_data = ipa_gsi_endpoint_data, .resource_data = &ipa_resource_data, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 8cae9325eb08e..d9659fd22322a 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -11,6 +11,18 @@ #include "ipa_endpoint.h" #include "ipa_mem.h" +/* QSB configuration for the SDM845 SoC. */ +static const struct ipa_qsb_data ipa_qsb_data[] = { + [IPA_QSB_MASTER_DDR] = { + .max_writes = 8, + .max_reads = 8, + }, + [IPA_QSB_MASTER_PCIE] = { + .max_writes = 4, + .max_reads = 12, + }, +}; + /* Endpoint configuration for the SDM845 SoC. */ static const struct ipa_gsi_endpoint_data ipa_gsi_endpoint_data[] = { [IPA_ENDPOINT_AP_COMMAND_TX] = { @@ -353,6 +365,8 @@ static const struct ipa_clock_data ipa_clock_data = { /* Configuration data for the SDM845 SoC. */ const struct ipa_data ipa_data_sdm845 = { .version= IPA_VERSION_3_5_1, + .qsb_count = ARRAY_SIZE(ipa_qsb_data), + .qsb_data = ipa_qsb_data, .endpoint_count = ARRAY_SIZE(ipa_gsi_endpoint_data), .endpoint_data = ipa_gsi_endpoint_data, .resource_data = &ipa_resource_data, diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h index b476fc373f7fe..d50cd5ae7714f 100644 --- a/drivers/net/ipa/ipa_data.h +++ b/drivers/net/ipa/ipa_data.h @@ -49,6 +49,22 @@ #define IPA_RESOURCE_GROUP_SRC_MAX 5 #define IPA_RESOURCE_GROUP_DST_MAX 5 +/** enum ipa_qsb_master_id - array index for IPA QSB configuration data */ +enum ipa_qsb_master_id { + IPA_QSB_MASTER_DDR, + IPA_QSB_MASTER_PCIE, +}; + +/** + * struct ipa_qsb_data - Qualcomm System Bus configuration data + * @max_writes:Maximum outstanding write requests for this master + * @max_reads: Maximum outstanding read requests for this master + */ +struct ipa_qsb_data { + u8 max_writes; + u8 max_reads; +}; + /** * struct gsi_channel_data - GSI channel configuration data * @tre_count: number of TREs in the channel ring @@ -285,14 +301,18 @@ struct ipa_clock_data { /** * struct ipa_data - combined IPA/GSI configuration data * @version: IPA hardware version - * @endpoint_count:number of entries in endpoint_data array + * @qsb_count: number of entries in the qsb_data array + * @qsb_data: Qualcomm System Bus configuration data + * @endpoint_count:number of entries in the endpoint_data array * @endpoint_data: IPA endpoint/GSI channel data * @resource_data: IPA resource configuration data - * @mem_count: number of entries in mem_data array + * @mem_count: number of entries in the mem_data array * @mem_data: IPA-local shared memory region data */ struct ipa_data { enum ipa_version version; + u32 qsb_count; /* # entries in qsb_data[] */ + const struct ipa_qsb_data *qsb_data; u32 endpoint_count; /* # entries in endpoint_data[] */ const struct ipa_gsi_endpoint_data *endpoint_data; const struct ipa_resource_data *resource_data; -- 2.27.0
[PATCH net-next 4/5] net: ipa: define some new memory regions
There are several memory regions that are defined starting with IPA v4.0, but which were not used for the SC7180 SoC (IPA v4.2). Even though they're not used (yet), define them so they are ready to be used for SoCs when they become supported. There are two QUOTA statistics memory regions, one for the modem and one for the AP. Define distinct names for these regions, and get rid of the definition of IPA_MEM_STATS_QUOTA. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 2 +- drivers/net/ipa/ipa_mem.h | 10 +- drivers/net/ipa/ipa_qmi.c | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 0bdb60f6755c4..f65abc19ae9d7 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -268,7 +268,7 @@ static const struct ipa_mem ipa_mem_local_data[] = { .size = 0x0050, .canary_count = 2, }, - [IPA_MEM_STATS_QUOTA] = { + [IPA_MEM_STATS_QUOTA_MODEM] = { .offset = 0x0a50, .size = 0x0060, .canary_count = 2, diff --git a/drivers/net/ipa/ipa_mem.h b/drivers/net/ipa/ipa_mem.h index f99180f84f0dd..f82e8939622bb 100644 --- a/drivers/net/ipa/ipa_mem.h +++ b/drivers/net/ipa/ipa_mem.h @@ -28,6 +28,7 @@ struct ipa_mem_data; * The set of memory regions is defined in configuration data. They are * subject to these constraints: * - a zero offset and zero size represents and undefined region + * - a region's size does not include space for its "canary" values * - a region's offset is defined to be *past* all "canary" values * - offset must be large enough to account for all canaries * - a region's size may be zero, but may still have canaries @@ -56,9 +57,16 @@ enum ipa_mem_id { IPA_MEM_AP_HEADER, /* 0 canaries */ IPA_MEM_MODEM_PROC_CTX, /* 2 canaries */ IPA_MEM_AP_PROC_CTX,/* 0 canaries */ + IPA_MEM_NAT_TABLE, /* 4 canaries (IPA v4.5 and above) */ IPA_MEM_PDN_CONFIG, /* 2 canaries (IPA v4.0 and above) */ - IPA_MEM_STATS_QUOTA,/* 2 canaries (IPA v4.0 and above) */ + IPA_MEM_STATS_QUOTA_MODEM, /* 2 canaries (IPA v4.0 and above) */ + IPA_MEM_STATS_QUOTA_AP, /* 0 canaries (IPA v4.0 and above) */ IPA_MEM_STATS_TETHERING,/* 0 canaries (IPA v4.0 and above) */ + IPA_MEM_STATS_V4_FILTER,/* 0 canaries (IPA v4.0-v4.2) */ + IPA_MEM_STATS_V6_FILTER,/* 0 canaries (IPA v4.0-v4.2) */ + IPA_MEM_STATS_V4_ROUTE, /* 0 canaries (IPA v4.0-v4.2) */ + IPA_MEM_STATS_V6_ROUTE, /* 0 canaries (IPA v4.0-v4.2) */ + IPA_MEM_STATS_FILTER_ROUTE, /* 0 canaries (IPA v4.5 and above) */ IPA_MEM_STATS_DROP, /* 0 canaries (IPA v4.0 and above) */ IPA_MEM_MODEM, /* 0 canaries */ IPA_MEM_UC_EVENT_RING, /* 1 canary */ diff --git a/drivers/net/ipa/ipa_qmi.c b/drivers/net/ipa/ipa_qmi.c index 2fc64483f2753..af8666b89b375 100644 --- a/drivers/net/ipa/ipa_qmi.c +++ b/drivers/net/ipa/ipa_qmi.c @@ -378,7 +378,7 @@ init_modem_driver_req(struct ipa_qmi *ipa_qmi) /* None of the stats fields are valid (IPA v4.0 and above) */ if (ipa->version != IPA_VERSION_3_5_1) { - mem = &ipa->mem[IPA_MEM_STATS_QUOTA]; + mem = &ipa->mem[IPA_MEM_STATS_QUOTA_MODEM]; if (mem->size) { req.hw_stats_quota_base_addr_valid = 1; req.hw_stats_quota_base_addr = -- 2.27.0
[PATCH net-next 3/5] net: ipa: don't define empty memory regions
The AP_HEADER memory region for both the SDM845 and SC7180 SoCs has zero size, and has no canaries. Defining an offset for such a zero-length region is not meaningful, so it's better not to define it at all. The size of this region is used in the code, but its value will still be zero because the memory regions are defined in statically initialized memory. For the SC7180, the STATS_DROP memory region has a zero size and no canaries as well. These regions are the only place where a zero-sized region is defined despite having no canaries. Remove them. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 10 -- drivers/net/ipa/ipa_data-sdm845.c | 5 - 2 files changed, 15 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 434869508a215..0bdb60f6755c4 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -253,11 +253,6 @@ static const struct ipa_mem ipa_mem_local_data[] = { .size = 0x0140, .canary_count = 2, }, - [IPA_MEM_AP_HEADER] = { - .offset = 0x05e8, - .size = 0x, - .canary_count = 0, - }, [IPA_MEM_MODEM_PROC_CTX] = { .offset = 0x05f0, .size = 0x0200, @@ -283,11 +278,6 @@ static const struct ipa_mem ipa_mem_local_data[] = { .size = 0x0140, .canary_count = 0, }, - [IPA_MEM_STATS_DROP] = { - .offset = 0x0bf0, - .size = 0, - .canary_count = 0, - }, [IPA_MEM_MODEM] = { .offset = 0x0bf0, .size = 0x140c, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 401861e3c0aa4..8cae9325eb08e 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -293,11 +293,6 @@ static const struct ipa_mem ipa_mem_local_data[] = { .size = 0x0140, .canary_count = 2, }, - [IPA_MEM_AP_HEADER] = { - .offset = 0x07c8, - .size = 0x, - .canary_count = 0, - }, [IPA_MEM_MODEM_PROC_CTX] = { .offset = 0x07d0, .size = 0x0200, -- 2.27.0
[PATCH net-next 1/5] net: ipa: make all configuration data constant
All of the platform configuration data should be constant, but that isn't the case for the memory regions, interconnects, and clocks. Fix this. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_data-sc7180.c | 6 +++--- drivers/net/ipa/ipa_data-sdm845.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c index 997b51ceb7d76..01681be402262 100644 --- a/drivers/net/ipa/ipa_data-sc7180.c +++ b/drivers/net/ipa/ipa_data-sc7180.c @@ -300,7 +300,7 @@ static const struct ipa_mem ipa_mem_local_data[] = { }, }; -static struct ipa_mem_data ipa_mem_data = { +static const struct ipa_mem_data ipa_mem_data = { .local_count= ARRAY_SIZE(ipa_mem_local_data), .local = ipa_mem_local_data, .imem_addr = 0x146a8000, @@ -310,7 +310,7 @@ static struct ipa_mem_data ipa_mem_data = { }; /* Interconnect bandwidths are in 1000 byte/second units */ -static struct ipa_interconnect_data ipa_interconnect_data[] = { +static const struct ipa_interconnect_data ipa_interconnect_data[] = { { .name = "memory", .peak_bandwidth = 465000, /* 465 MBps */ @@ -329,7 +329,7 @@ static struct ipa_interconnect_data ipa_interconnect_data[] = { }, }; -static struct ipa_clock_data ipa_clock_data = { +static const struct ipa_clock_data ipa_clock_data = { .core_clock_rate= 100 * 1000 * 1000,/* Hz */ .interconnect_count = ARRAY_SIZE(ipa_interconnect_data), .interconnect_data = ipa_interconnect_data, diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c index 88c9c3562ab79..401861e3c0aa4 100644 --- a/drivers/net/ipa/ipa_data-sdm845.c +++ b/drivers/net/ipa/ipa_data-sdm845.c @@ -320,7 +320,7 @@ static const struct ipa_mem ipa_mem_local_data[] = { }, }; -static struct ipa_mem_data ipa_mem_data = { +static const struct ipa_mem_data ipa_mem_data = { .local_count= ARRAY_SIZE(ipa_mem_local_data), .local = ipa_mem_local_data, .imem_addr = 0x146bd000, @@ -330,7 +330,7 @@ static struct ipa_mem_data ipa_mem_data = { }; /* Interconnect bandwidths are in 1000 byte/second units */ -static struct ipa_interconnect_data ipa_interconnect_data[] = { +static const struct ipa_interconnect_data ipa_interconnect_data[] = { { .name = "memory", .peak_bandwidth = 60, /* 600 MBps */ @@ -349,7 +349,7 @@ static struct ipa_interconnect_data ipa_interconnect_data[] = { }, }; -static struct ipa_clock_data ipa_clock_data = { +static const struct ipa_clock_data ipa_clock_data = { .core_clock_rate= 75 * 1000 * 1000, /* Hz */ .interconnect_count = ARRAY_SIZE(ipa_interconnect_data), .interconnect_data = ipa_interconnect_data, -- 2.27.0
[PATCH net-next 0/5] net: ipa: update configuration data
Each IPA version has a "data" file defining how various things are configured. This series gathers a few updates to this information: - The first patch makes all configuration data constant - The second fixes an incorrect (but seemingly harmless) value - The third simplifies things a bit by using implicit zero initialization for memory regions that are empty - The fourth adds definitions for memory regions that exist but are not yet used - The fifth use configuration data rather than conditional code to set some bus parameters -Alex Alex Elder (5): net: ipa: make all configuration data constant net: ipa: fix canary count for SC7180 UC_INFO region net: ipa: don't define empty memory regions net: ipa: define some new memory regions net: ipa: define QSB limits in configuration data drivers/net/ipa/ipa_data-sc7180.c | 30 +++--- drivers/net/ipa/ipa_data-sdm845.c | 25 + drivers/net/ipa/ipa_data.h| 24 ++-- drivers/net/ipa/ipa_mem.h | 10 +- drivers/net/ipa/ipa_qmi.c | 2 +- 5 files changed, 64 insertions(+), 27 deletions(-) -- 2.27.0
Re: [PATCH net-next 4/4] net: ipa: activate some commented assertions
On 3/19/21 12:00 AM, Leon Romanovsky wrote: On Thu, Mar 18, 2021 at 11:29:23PM -0500, Alex Elder wrote: Convert some commented assertion statements into real calls to ipa_assert(). If the IPA device pointer is available, provide it, otherwise pass NULL for that. There are lots more places to convert, but this serves as an initial verification of the new mechanism. The assertions here implement both runtime and build-time assertions, both with and without the device pointer. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_reg.h | 7 --- drivers/net/ipa/ipa_table.c | 5 - 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 732e691e9aa62..d0de85de9f08d 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -9,6 +9,7 @@ #include #include "ipa_version.h" +#include "ipa_assert.h" struct ipa; @@ -212,7 +213,7 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) BCR_HOLB_DROP_L2_IRQ_FMASK | BCR_DUAL_TX_FMASK; - /* assert(version != IPA_VERSION_4_5); */ + ipa_assert(NULL, version != IPA_VERSION_4_5); This assert will fire for IPA_VERSION_4_2, I doubt that this is something you want. No, it will only fail if version == IPA_VERSION_4_5. The logic of an assertion is the opposite of BUG_ON(). It fails only if the asserted condition yields false. -Alex Thanks
Re: [PATCH net-next 3/4] net: ipa: introduce ipa_assert()
On 3/18/21 11:55 PM, Leon Romanovsky wrote: On Thu, Mar 18, 2021 at 11:29:22PM -0500, Alex Elder wrote: Create a new macro ipa_assert() to verify that a condition is true. This produces a build-time error if the condition can be evaluated at build time; otherwise __ipa_assert_runtime() is called (described below). Another macro, ipa_assert_always() verifies that an expression yields true at runtime, and if it does not, reports an error message. When IPA validation is enabled, __ipa_assert_runtime() becomes a call to ipa_assert_always(), resulting in runtime verification of the asserted condition. Otherwise __ipa_assert_runtime() has no effect, so such ipa_assert() calls are effectively ignored. IPA assertion errors will be reported using the IPA device if possible. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_assert.h | 50 1 file changed, 50 insertions(+) create mode 100644 drivers/net/ipa/ipa_assert.h diff --git a/drivers/net/ipa/ipa_assert.h b/drivers/net/ipa/ipa_assert.h new file mode 100644 index 0..7e5b9d487f69d --- /dev/null +++ b/drivers/net/ipa/ipa_assert.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2021 Linaro Ltd. + */ +#ifndef _IPA_ASSERT_H_ +#define _IPA_ASSERT_H_ + +#include +#include +#include + +/* Verify the expression yields true, and fail at build time if possible */ +#define ipa_assert(dev, expr) \ + do { \ + if (__builtin_constant_p(expr)) \ + compiletime_assert(expr, __ipa_failure_msg(expr)); \ + else \ + __ipa_assert_runtime(dev, expr); \ + } while (0) + +/* Report an error if the given expression evaluates to false at runtime */ +#define ipa_assert_always(dev, expr) \ + do { \ + if (unlikely(!(expr))) { \ + struct device *__dev = (dev); \ + \ + if (__dev) \ + dev_err(__dev, __ipa_failure_msg(expr)); \ + else \ + pr_err(__ipa_failure_msg(expr)); \ + } \ + } while (0) It will be much better for everyone if you don't obfuscate existing kernel primitives and don't hide constant vs. dynamic expressions. I don't agree with this characterization. Yes, there is some complexity in this one source file, where ipa_assert() is defined. But as a result, callers are simple one-line statements (similar to WARN_ON()). Existing kernel primitives don't achieve these objectives: - Don't check things at run time under normal conditions - Do check things when validation is enabled - If you can check it at compile time, check it regardless If there is something that helps me do that, suggest it because I will be glad to use it. So any random kernel developer will be able to change the code without investing too much time to understand this custom logic. There should be almost no need to change the definition of ipa_assert(). Even so, this custom logic is not all that complicated in my view; it's broken into a few macros that are each pretty simple. It was actuallyl a little simpler before I added some things to satisfy checkpatch.pl. And constant expressions are checked with BUILD_BUG_ON(). BUILD_BUG_ON() is great. But it doesn't work for non-constant expressions. Suppose I try to simplify ipa_assert() as: #define ipa_assert(dev, expr) \ do { \ BUILD_BUG_ON(expr); \ __ipa_runtime_assert(dev, expr); \ } while (0) I can't do that, because BUILD_BUG_ON() evaluates the expression, so I can't safely do that again in the called macro. I explained how I wanted it to work earlier, but the main reason for doing this is to communicate to the reader that the asserted properties are guaranteed to be true. It can be valuable for making things understandable. You don't have to wonder, "what if the value passed is out of range?" It won't be, and the assertion tells you that. If you still feel need to provide assertion like this, it should be done in general code. I could do that but I'm afraid it's more than I intend to take on, and am not aware of anyone else asking for a feature like this. Do you want it? I honestly appreciate your input, but I don't agree with it. I can do without codifying informative assertions but I'd really like to be able to test them. -Alex Thanks + +/* Constant message used when an assertion fails */ +#define __ipa_failure_msg(expr)"IPA assertion failed: " #expr "\n" + +#ifdef IPA_VALIDATION + +/* Only do runtime checks for "normal" assertions if validating the code */ +#define __ipa_assert_runtime(dev, expr)ipa_assert_always(dev, expr) + +#else /* !IPA_VALIDATION
[PATCH net-next 4/4] net: ipa: activate some commented assertions
Convert some commented assertion statements into real calls to ipa_assert(). If the IPA device pointer is available, provide it, otherwise pass NULL for that. There are lots more places to convert, but this serves as an initial verification of the new mechanism. The assertions here implement both runtime and build-time assertions, both with and without the device pointer. Signed-off-by: Alex Elder --- drivers/net/ipa/ipa_reg.h | 7 --- drivers/net/ipa/ipa_table.c | 5 - 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/ipa/ipa_reg.h b/drivers/net/ipa/ipa_reg.h index 732e691e9aa62..d0de85de9f08d 100644 --- a/drivers/net/ipa/ipa_reg.h +++ b/drivers/net/ipa/ipa_reg.h @@ -9,6 +9,7 @@ #include #include "ipa_version.h" +#include "ipa_assert.h" struct ipa; @@ -212,7 +213,7 @@ static inline u32 ipa_reg_bcr_val(enum ipa_version version) BCR_HOLB_DROP_L2_IRQ_FMASK | BCR_DUAL_TX_FMASK; - /* assert(version != IPA_VERSION_4_5); */ + ipa_assert(NULL, version != IPA_VERSION_4_5); return 0x; } @@ -413,7 +414,7 @@ static inline u32 ipa_header_size_encoded(enum ipa_version version, val = u32_encode_bits(size, HDR_LEN_FMASK); if (version < IPA_VERSION_4_5) { - /* ipa_assert(header_size == size); */ + ipa_assert(NULL, header_size == size); return val; } @@ -433,7 +434,7 @@ static inline u32 ipa_metadata_offset_encoded(enum ipa_version version, val = u32_encode_bits(off, HDR_OFST_METADATA_FMASK); if (version < IPA_VERSION_4_5) { - /* ipa_assert(offset == off); */ + ipa_assert(NULL, offset == off); return val; } diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c index aa8b3ce7e21d9..7784b42fbaccc 100644 --- a/drivers/net/ipa/ipa_table.c +++ b/drivers/net/ipa/ipa_table.c @@ -23,6 +23,7 @@ #include "ipa_cmd.h" #include "gsi.h" #include "gsi_trans.h" +#include "ipa_assert.h" /** * DOC: IPA Filter and Route Tables @@ -237,11 +238,13 @@ static void ipa_table_validate_build(void) static dma_addr_t ipa_table_addr(struct ipa *ipa, bool filter_mask, u16 count) { u32 skip; + u32 max; if (!count) return 0; -/* assert(count <= max_t(u32, IPA_FILTER_COUNT_MAX, IPA_ROUTE_COUNT_MAX)); */ + max = max_t(u32, IPA_FILTER_COUNT_MAX, IPA_ROUTE_COUNT_MAX); + ipa_assert(&ipa->pdev->dev, max); /* Skip over the zero rule and possibly the filter mask */ skip = filter_mask ? 1 : 2; -- 2.27.0