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 <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
We currently assume the IPA driver is built only for a 64 bit kernel.
When this constraint was put in place it eliminated some do_div()
calls, replacing them with the "/" and "%" operators. We now only
use these operations on u32 and size_t objects. In a 32-bit kernel
build, size_t will be 32 bits wide, so there remains no reason to
use do_div() for divide and modulo.
A few recent commits also fix some code that assumes that DMA
addresses are 64 bits wide.
With that, we can get rid of the 64-bit build requirement.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
We currently have a build-time check to ensure that the minimum DMA
allocation alignment satisfies the constraint that IPA filter and
route tables must point to rules that are 128-byte aligned.
But what's really important is that the actual allocated DMA memory
has that alignment, even if the minimum is smaller than that.
Remove the BUILD_BUG_ON() call checking against minimim DMA alignment
and instead verify at rutime that the allocated memory is properly
aligned.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Use upper_32_bits() to extract the high-order 32 bits of a DMA
address. This avoids doing a 32-position shift on a DMA address
if it happens not to be 64 bits wide. Use lower_32_bits() to
extract the low-order 32 bits (because that's what it's for).
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Some build time checks in ipa_table_validate_build() assume that a
DMA address is 64 bits wide. That is more restrictive than it has
to be. A route or filter table is 64 bits wide no matter what the
size of a DMA address is on the AP. The code actually uses a
pointer to __le64 to access table entries, and a fixed constant
IPA_TABLE_ENTRY_SIZE to describe the size of those entries.
Loosen up two checks so they still verify some requirements, but
such that they do not assume the size of a DMA address is 64 bits.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Fix the following coccicheck report:
drivers/net/ipa/gsi.c:1341:2-9:
line 1341 is redundant because platform_get_irq() already prints an error
Remove dev_err() messages after platform_get_irq_byname() failures.
Signed-off-by: Zihao Tang <tangzihao1@hisilicon.com>
Signed-off-by: Jay Fang <f.fangjian@huawei.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
The specified format of the INDICATION_REGISTER QMI request message
has been extended to support two more optional fields:
endpoint_desc_ind:
sender wishes to receive endpoint descriptor information via
an IPA ENDP_DESC indication QMI message
bw_change_ind:
sender wishes to receive bandwidth change information via
an IPA BW_CHANGE indication QMI message
Add definitions that permit these fields to be formatted and parsed
by the QMI library code.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
The ipa_init_modem_driver_req_ei[] encoding array for the
INIT_MODEM_DRIVER request message has some errors in it.
First, the tlv_type associated with the hw_stats_quota_size field is
wrong; it duplicates the valiue used for the hw_stats_quota_base_addr
field (0x1f) and should use 0x20 instead. The tlv_type value for
the hw_stats_drop_size field also uses the same duplicate value; it
should use 0x22 instead.
Second, there is no definition for the hw_stats_drop_base_addr
field. It is an optional 32-bit enumerated type value.
Finally, the hw_stats_quota_base_addr, hw_stats_quota_size, and
hw_stats_drop_size fields are defined as enumerated types; they
should be unsigned 4-byte values.
Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
In the ipa_indication_register_req_ei[] encoding array, the tlv_type
associated with the ipa_mhi_ready_ind field is wrong. It duplicates
the value used for the data_usage_quota_reached field (0x11) and
should use value 0x12 instead. Fix this bug.
Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
In review, Alexander Duyck suggested that ipa_table_hash_support()
was trivial enough that it could be implemented as a static inline
function in the header file. But the patch had already been
accepted. Implement his suggestion.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
When a QMI handle is initialized, an array of message handler
structures is provided, defining how any received message should
be handled based on its type and message ID. The QMI core code
traverses this array when a message arrives and calls the function
associated with the (type, msg_id) found in the array.
The array is supposed to be terminated with an empty (all zero)
entry though. Without it, an unsupported message will cause
the QMI core code to go past the end of the array.
Fix this bug, by properly terminating the message handler arrays
provided when QMI handles are set up by the IPA driver.
Fixes: 530f9216a9 ("soc: qcom: ipa: AP/modem communications")
Reported-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
We configure the minimum and maximum number of various types of IPA
resources in ipa_resource_config(). It iterates over resource types
in the configuration data and assigns resource limits to each
resource group for each type.
Unfortunately, we are repeatedly initializing the resource data for
the first type, rather than initializing each of the types whose
limits are specified.
Fix this bug.
Fixes: 4a0d7579d4 ("net: ipa: avoid going past end of resource group array")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Create a simple helper function that indicates whether a channel has
been initialized. This abstacts/hides the details of how this is
determined.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Introduce a new function to abstract the knowledge of whether hashed
routing and filter tables are supported for a given IPA instance.
IPA v4.2 is the only one that doesn't support hashed tables (now
and for the foreseeable future), but the name of the helper function
is better for explaining what's going on.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
In ipa_cmd_register_write_valid() we verify that values we will
supply to a REGISTER_WRITE IPA immediate command will fit in
the fields that need to hold them. This patch fixes some issues
in that function and ipa_cmd_register_write_offset_valid().
The dev_err() call in ipa_cmd_register_write_offset_valid() has
some printf format errors:
- The name of the register (corresponding to the string format
specifier) was not supplied.
- The IPA base offset and offset need to be supplied separately to
match the other format specifiers.
Also make the ~0 constant used there to compute the maximum
supported offset value explicitly unsigned.
There are two other issues in ipa_cmd_register_write_valid():
- There's no need to check the hash flush register for platforms
(like IPA v4.2) that do not support hashed tables
- The highest possible endpoint number, whose status register
offset is computed, is COUNT - 1, not COUNT.
Fix these problems, and add some additional commentary.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
When initializing the IPA core clock and interconnects, it's
possible we'll get an EPROBE_DEFER error. This isn't really an
error, it's just means we need to be re-probed later.
Use dev_err_probe() to report the error rather than dev_err().
This avoids polluting the log with these "error" messages.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
This patch actually fixes a bug, though it doesn't affect the two
platforms supported currently. The fix implements GSI memory
pointers a bit differently.
For IPA version 4.5 and above, the address space for almost all GSI
registers is adjusted downward by a fixed amount. This is currently
handled by adjusting the I/O virtual address pointer after it has
been mapped. The bug is that the pointer is not "de-adjusted" as it
should be when it's unmapped.
This patch fixes that error, but it does so by maintaining one "raw"
pointer for the mapped memory range. This is assigned when the
memory is mapped and used to unmap the memory. This pointer is also
used to access the two registers that do *not* sit in the "adjusted"
memory space.
Rather than adjusting *that* pointer, we maintain a separate pointer
that's an adjusted copy of the "raw" pointer, and that is used for
most GSI register accesses.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
It's possible that the length passed to ipa_header_size_encoded()
is larger than what can be represented by the HDR_LEN field alone
(starting with IPA v4.5). If we attempted that, u32_encode_bits()
would trigger a build-time error.
Avoid this problem by masking off high-order bits of the value
encoded as the lower portion of the header length.
The same sort of problem exists in ipa_metadata_offset_encoded(),
so implement the same fix there.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
There is a build-time check that the packet status structure is a
multiple of 4 bytes in size. It's not clear where that constraint
comes from, but the structure defines what hardware provides so its
definition won't change. Get rid of the check; it adds no value.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The count argument to ipa_endpoint_replenish() is only ever 0 or 1,
and always will be (because we always handle each receive buffer in
a single transaction). Rename the argument to be add_one and change
it to be Boolean.
Update the function description to reflect the current code.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We do not support inter-EE channel or event ring commands. Inter-EE
interrupts are disabled (and never re-enabled) for all channels and
event rings, so we have no need for the GSI registers that clear
those interrupt conditions. So remove their definitions.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
An event ring's state only needs to be known when it is allocated,
reset, or deallocated. We check an event ring's state both before
and after performing an event ring control command that changes
its state. These are only issued at startup and shutdown, so there
is very little value in caching the state.
Stop recording a copy of the channel's last known state, and instead
fetch the true state from hardware whenever it's needed. In such
cases, *do* record the state in a local variable, in case an error
message reports it (so the value reported is the value seen).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When stopping a channel, gsi_channel_stop() will ensure NAPI
polling is complete when it calls napi_disable(). So there is no
need to call napi_synchronize() in that case.
Move the call to napi_synchronize() out of __gsi_channel_stop()
and into gsi_channel_suspend(), so it's only used where needed.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Move the mutex calls out of gsi_channel_stop_retry() and into
__gsi_channel_stop(), to make the latter more semantically similar
to __gsi_channel_start().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In gsi_channel_setup(), we check to see if the configuration data
contains any information about channels that are not supported by
the hardware. If one is found, we abort the setup process, but
the error code (ret) is not set in this case. Fix this bug.
Fixes: 650d160382 ("soc: qcom: ipa: the generic software interface")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20210204010655.15619-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Transactions to send data for a network device can be allocated at
any time up until the point the TX queue is stopped. It is possible
for ipa_start_xmit() to be called in one context just before a
the transmit queue is stopped in another.
Update gsi_channel_trans_last() so that for TX channels the
allocated and pending transaction lists are checked--in addition
to the completed and polled lists--to determine the "last"
transaction. This means any transaction that has been allocated
before the TX queue is stopped will be allowed to complete before
we conclude the channel is quiesced.
Rework the function a bit to use a list pointer and gotos.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
No completion interrupts will occur while an endpoint is suspended,
nor when a channel has been stopped for suspend. So there's no need
to disable the interrupt during suspend and re-enable it when
resuming. Without any interrupts occurring, there is no need to
disable/re-enable NAPI for channel suspend/resume either.
We'll only enable NAPI and the interrupt when we first start the
channel, and disable it again only when it's "really" stopped.
To accomplish this, move the enable/disable calls out of
__gsi_channel_start() and __gsi_channel_stop(), and into
gsi_channel_start() and gsi_channel_stop() instead.
Add a call to napi_synchronize() to gsi_channel_suspend(), to ensure
NAPI polling is done before moving on.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Disable both the I/O completion interrupt and NAPI polling on a
channel *after* we successfully stop it rather than before. This
ensures a completion occurring just before the channel is stopped
gets processed.
Enable NAPI polling and the interrupt *before* starting a channel
rather than after, to be symmetric. A stopped channel won't
generate any completion interrupts anyway.
Enable NAPI before the interrupt and disable it afterward.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Open-code gsi_channel_freeze() and gsi_channel_thaw() in all callers
and get rid of these two functions. This is part of reworking the
sequence of things done during channel suspend/resume and start/stop.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Create a new function that does most of the work of starting a
channel. What's different is that it takes a flag indicating
whether the channel should really be started or not. Create
another new function __gsi_channel_stop() that behaves similarly.
IPA v3.5.1 implements suspend using a special SUSPEND endpoint
setting. If the endpoint is suspended when an I/O completes on the
underlying GSI channel, a SUSPEND interrupt is generated.
Newer versions of IPA do not implement the SUSPEND endpoint mode.
Instead, endpoint suspend is implemented by simply stopping the
underlying GSI channel. In this case, a completing I/O on a
*stopped* channel causes the SUSPEND interrupt condition.
These new functions put all activity related to starting or stopping
a channel (including "thawing/freezing" the channel) in one place,
whether or not the channel is actually started or stopped.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Create a new helper function that encapsulates issuing a set of
channel stop commands, retrying if appropriate, with a short delay
between attempts.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
If an error occurs starting a channel, don't "thaw" it.
We should assume the channel remains in a non-started state.
Update the comment in gsi_channel_stop(); calls to this function
are no longer retried.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Fix two format specifiers that used %lu for a size_t in "ipa_mem.c".
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When extracting the destination endpoint ID from the status in
ipa_endpoint_status_skip(), u32_get_bits() is used. This happens to
work, but it's wrong: the structure field is only 8 bits wide
instead of 32.
Fix this by using u8_get_bits() to get the destination endpoint ID.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Sparse warns that the assignment of the metadata mask for a QMAP
endpoint in ipa_endpoint_init_hdr_metadata_mask() is a bad
assignment. We know we want the mask value to be big endian, even
though the value we write is in host byte order. Use a __force
tag to indicate we really mean it.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The virt local variable in gsi_channel_state() does not have an
__iomem attribute but should. Fix this.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Amy Parker <enbyamy@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The "ring->addr = addr;" assignment is done a few lines later so we
can't use "ring->addr" yet. The correct dma_handle is "addr".
Fixes: 650d160382 ("soc: qcom: ipa: the generic software interface")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/YBjpTU2oejkNIULT@mwanda
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The only time we transfer data (rather than issuing a command) out
of the AP->command TX endpoint is when we're clearing the hardware
pipeline. All that's needed is a "small" data buffer, and its
contents aren't even important.
For convenience, we just transfer a command structure in this case
(it's already mapped for DMA). The TRE is added to a transaction
using ipa_cmd_ip_tag_status_add(), but we ignore the size value
provided to that function. So just get rid of the size argument.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We only send a tagged packet from the AP->command TX endpoint when
we're clearing the hardware pipeline. And when we receive the
tagged packet we don't care what the actual tag value is.
Stop passing a tag value to ipa_cmd_ip_tag_status_add(), and just
encode 0 as the tag sent. Fix the function that encodes the tag so
it uses the proper byte ordering.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
There are times, such as when the modem crashes, when we issue
commands to clear the IPA hardware pipeline. These commands include
a data transfer command that delivers a small packet directly to the
default (AP<-LAN RX) endpoint.
The places that do this wait for the transactions that contain these
commands to complete, but the pipeline can't be assumed clear until
the sent packet has been *received*.
The small transfer will be delivered with a status structure, and
that status will indicate its tag is valid. This is the only place
we send a tagged packet, so we use the tag to determine when the
pipeline clear packet has arrived.
Add a completion to the IPA structure to to be used to signal
the receipt of a pipeline clear packet. Create a new function
ipa_cmd_pipeline_clear_wait() that will wait for that completion.
Reinitialize the completion whenever pipeline clear commands are
added to a transaction. Extend ipa_endpoint_status_tag() to check
whether a packet whose status contains a valid tag was sent from the
AP->command TX endpoint, and if so, signal the new IPA completion.
Have all callers of ipa_cmd_pipeline_clear_add() wait for the
pipeline clear indication after the transaction that clears the
pipeline has completed.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Introduce ipa_endpoint_status_tag(), which returns true if received
status indicates its tag field is valid. The endpoint parameter is
not yet used.
Call this from ipa_status_drop_packet(), and drop the packet if the
status indicates the tag was valid. Pass the endpoint pointer to
ipa_status_drop_packet(), and rename it ipa_endpoint_status_drop().
The endpoint will be used in the next patch.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Rearrange some comments and assignments made when handling a packet
that is received with status, aiming to improve understandability.
Use DIV_ROUND_CLOSEST() to get a better per-packet true size estimate.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
There is a set of functions and symbols related to performing
"tag_process" immediate commands to clear the IPA pipeline. The
name is related to one of the commands issued when doing this, but
it doesn't really convey the overall purpose of taking this action.
The purpose is to take some steps to "clear out" the hardware
pipeline, and to wait until that process completes, to ensure the
IPA hardware is in a well-defined state.
Rename these symbols to use "pipeline_clear" in their names instead.
Add some comments to explain a bit more about what's going on.
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Currently in gsi_isr_ieob(), event ring IEOB interrupts are disabled
one at a time. The loop disables the IEOB interrupt for all event
rings represented in the event mask. Instead, just disable them all
at once.
Disable them all *before* clearing the interrupt condition. This
guarantees we'll schedule NAPI for each event once, before another
IEOB interrupt could be signaled.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Rename gsi_irq_ieob_disable() to be gsi_irq_ieob_disable_one().
Introduce a new function gsi_irq_ieob_disable() that takes a mask of
events to disable rather than a single event id. This will be used
in the next patch.
Rename gsi_irq_ieob_enable() to be gsi_irq_ieob_enable_one() to be
consistent.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Have gsi_channel_update() return the first transaction in the
updated completed transaction list, or NULL if no new transactions
have been added.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Pay attention to the return value of napi_complete(), completing
polling only if it returns true.
Just use napi rather than &channel->napi as the argument passed to
napi_complete().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
