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>
There is an off-by-one problem in gsi_channel_poll(). The count of
transactions completed is incremented each time through the loop
*before* determining whether there is any more work to do. As a
result, if we exit the loop early the counter its value is one more
than the number of transactions actually processed.
Instead, increment the count after processing, to ensure it reflects
the number of processed transactions. The result is more naturally
described as a for loop rather than a while loop, so change that.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA driver currently requires a DT property to be defined whose
value is the phandle for the modem subsystem. This was needed to
look up a remoteproc structure pointer used when registering for
notifications in the original IPA notification mechanism.
Remoteproc provides a more generic SSR notifier system, and the IPA
driver switched over to it last summer, but this remoteproc phandle
dependency was not removed at that time.
Get rid of the IPA remoteproc pointer and stop requiring the phandle
be specified.
This avoids a link error (rproc_put() not defined) for certain
configurations.
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Currently we assume that the IPA hardware has exactly three
interconnects. But that won't be guaranteed for all platforms,
so allow any number of interconnects to be specified in the
configuration data.
For each platform, define an array of interconnect data entries
(still associated with the IPA clock structure), and record the
number of entries initialized in that array.
Loop over all entries in this array when initializing, enabling,
disabling, or tearing down the set of interconnects.
With this change we no longer need the ipa_interconnect_id
enumerated type, so get rid of it.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Pass an the address of an IPA interconnect structure and its
configuration data to ipa_interconnect_init_one() and have that
function initialize all the structure's fields. Change the function
to simply return an error code.
Introduce ipa_interconnect_exit_one() to encapsulate the cleanup of
an IPA interconnect structure.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add the name to the configuration data for each interconnect. Use
this information rather than a constant string during initialization.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add fields in the ipa_interconnect structure to hold the average and
peak bandwidth values for the interconnect. Pass the configuring
data for interconnects to ipa_interconnect_init() so these values
can be recorded, and use them when enabling the interconnects.
There's no longer any need to keep a copy of the interconnect data
after initialization.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Rather than having separate pointers for the memory, imem, and
config interconnect paths, maintain an array of ipa_interconnect
structures each of which contains a pointer to a path.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
If disabling interconnects fails there's not a lot we can do. The
only two callers of ipa_interconnect_disable() ignore the return
value, so just give the function a void return type.
Print an error message if disabling any of the interconnects is not
successful. Return (and print) only the first error seen.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Use "bandwidth" rather than "rate" in describing the average and
peak values to use for IPA interconnects. They should have been
named that way to begin with.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
For RX channels we issue a stop command more than once if necessary
to allow it to stop. It turns out that TX channels could also
require retries.
Retry channel stop commands if necessary regardless of the channel
direction. Rename the symbol defining the retry count so it's not
RX-specific.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
If a GSI stop channel command leaves the channel in STOP_IN_PROC
state, we retry the stop command after a 1-2 millisecond delay.
I have been told that a 3-5 millisecond delay is a better choice.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The GSI command timeout is currently 5 seconds, which is much higher
than it should be.
Express the timeout in milliseconds rather than seconds, and reduce
it to 50 milliseconds.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
65;6003;1c
The use of msleep() for small periods (less than 20 milliseconds) is
not recommended because the actual delay can be much different than
expected.
We use msleep(1) in several places in the IPA driver to insert short
delays. Replace them with usleep_range calls, which should reliably
delay a period in the range requested.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Create a new function gsi_irq_ev_ctrl_enable() that encapsulates
enabling the event ring control GSI interrupt type, and enables a
single event ring to signal that interrupt. When an event ring
changes state as a result of an event ring command, it triggers this
interrupt.
Create an inverse function gsi_irq_ev_ctrl_disable() as well.
Because only one event ring at a time is enabled for this interrupt,
we can simply disable the interrupt for *all* channels.
Create a pair of helpers that serve the same purpose for channel
commands.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The return value of gsi_command() is true if successful or false if
we time out waiting for a completion interrupt.
Rename the variables in the three callers of gsi_command() to be
"timeout", to make it more obvious that's the only reason for
failure.
In addition, add a "gsi_" prefix to evt_ring_command() so its name
is consistent with the convention used for GSI channel and generic
commands.
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA driver depends on some SMEM functionality (qcom_smem_init(),
qcom_smem_alloc(), and qcom_smem_virt_to_phys()), but this is not
reflected in the configuration dependencies. Add a dependency on
QCOM_SMEM to avoid attempts to build the IPA driver without SMEM.
This avoids a link error for certain configurations.
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Fixes: 38a4066f59 ("net: ipa: support COMPILE_TEST")
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20210112192134.493-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Arrange for the IPA driver to be built when COMPILE_TEST is enabled.
Update the help text to reflect that we support two Qualcomm SoCs.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The second argument to gsi_trans_page_add() is a page pointer.
That declaration is found in header files used by "gsi_trans.h" for
(at least) arm64 and x86 builds, but apparently not for alpha
builds.
Fix this by adding a declaration of struct page to the top of
"gsi_trans.h".
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
At the moment it is quite hard to identify the network interface
provided by IPA in userspace components: The network interface is
created as virtual device, without any link to the IPA device.
The interface name ("rmnet_ipa%d") is the only indication that the
network interface belongs to IPA, but this is not very reliable.
Add SET_NETDEV_DEV() to associate the network interface with the
IPA parent device. This allows userspace services like ModemManager
to properly identify that this network interface is provided by IPA
and belongs to the modem.
Cc: Alex Elder <elder@kernel.org>
Fixes: a646d6ec90 ("soc: qcom: ipa: modem and microcontroller")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Link: https://lore.kernel.org/r/20210106100755.56800-1-stephan@gerhold.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Callers of evt_ring_command() no longer care whether the command
times out, and don't use what evt_ring_command() returns. Redefine
that function to have void return type.
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 428b448ee7 ("net: ipa: use state to determine event ring command success")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Callers of gsi_channel_command() no longer care whether the command
times out, and don't use what gsi_channel_command() returns. Redefine
that function to have void return type.
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 6ffddf3b3d ("net: ipa: use state to determine channel command success")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This patch implements the same basic fix for event rings as the
previous one does for channels.
The result of issuing an event ring control command should be that
the event ring changes state. If enabled, a completion interrupt
signals that the event ring state has changed. This interrupt is
enabled by gsi_evt_ring_command() and disabled again after the
command has completed (or we time out).
There is a window of time during which the command could complete
successfully without interrupting. This would cause the event ring
to transition to the desired new state.
So whether a event ring command ends via completion interrupt or
timeout, we can consider the command successful if the event ring
has entered the desired state (and a failure if it has not,
regardless of the cause).
Fixes: b4175f8731 ("net: ipa: only enable GSI event control IRQs when needed")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The result of issuing a channel control command should be that the
channel changes state. If enabled, a completion interrupt signals
that the channel state has changed. This interrupt is enabled by
gsi_channel_command() and disabled again after the command has
completed (or we time out).
There is a window of time--after the completion interrupt is disabled
but before the channel state is read--during which the command could
complete successfully without interrupting. This would cause the
channel to transition to the desired new state.
So whether a channel command ends via completion interrupt or
timeout, we can consider the command successful if the channel
has entered the desired state (and a failure if it has not,
regardless of the cause).
Fixes: d6c9e3f506 ("net: ipa: only enable generic command completion IRQ when needed");
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We enable the completion interrupt for channel or event ring
commands only when we issue them. The interrupt is disabled after
the interrupt has fired, or after we have timed out waiting for it.
If we time out, the command could complete after the interrupt has
been disabled, causing a state change in the channel or event ring.
The interrupt associated with that state change would be delivered
the next time the completion interrupt is enabled.
To avoid previous command completions interfering with new commands,
clear all pending completion interrupts before re-enabling them for
a new command.
Fixes: b4175f8731 ("net: ipa: only enable GSI event control IRQs when needed")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When the core clock rate and interconnect bandwidth specifications
were moved into configuration data, a copy/paste bug was introduced,
causing the memory interconnect bandwidth to be set three times
rather than enabling the three different interconnects.
Fix this bug.
Fixes: 91d02f9551 ("net: ipa: use config data for clocking")
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Georgi Djakov <georgi.djakov@linaro.org>
Link: https://lore.kernel.org/r/20201222151613.5730-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
xdp_return_frame_bulk() needs to pass a xdp_buff
to __xdp_return().
strlcpy got converted to strscpy but here it makes no
functional difference, so just keep the right code.
Conflicts:
net/netfilter/nf_tables_api.c
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Replace a comma between expression statements by a semicolon.
Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
When the coherent memory is freed in gsi_trans_pool_exit_dma(), we
are mistakenly passing the size of a single element in the pool
rather than the actual allocated size. Fix this bug.
Fixes: 9dd441e4ed ("soc: qcom: ipa: GSI transactions")
Reported-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20201203215106.17450-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Jon Hunter reported observing a build bug in the IPA driver:
https://lore.kernel.org/netdev/5b5d9d40-94d5-5dad-b861-fd9bef8260e2@nvidia.com
The problem is that the QMB0 max read value set for IPA v4.5 (16) is
too large to fit in the 4-bit field.
The actual value we want is 0, which requests that the hardware use
the maximum it is capable of.
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201202141502.21265-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Extend ipa_reg_init_hol_block_timer_val() so it properly calculates
the head-of-line block timeout to use for IPA v4.5.
Introduce hol_block_timer_qtime_val() to compute the value to use
for IPA v4.5, where Qtime is used as the basis of the timer. Call
that function from hol_block_timer_val() for IPA v4.5.
Both of these are private functions, so shorten their names a bit so
they don't take up so much space on the line.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Change aggr_time_limit_encoded() to properly calculate the
aggregation time limit to use for IPA v4.5.
Older IPA versions program the AGGR_GRANULARITY field of the
of the COUNTER_CFG register to set the granularity of the
aggregation timer, which we configure to be 500 microseconds.
Instead, IPA v4.5 selects between two possible granularity values
derived from the 19.2 MHz Qtime clock. These granularities are
100 microseconds or 1 millisecond per tick. We use the smaller
granularity if possible, unless the desired period is too large
to be specified that way.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA v4.5 introduces a new unified timer architecture driven on the
19.2 MHz SoC crystal oscillator (XO). It is independent of the IPA
core clock and avoids some duplication.
Lower-resolution time stamps are derived from this by using only the
high-order bits of the 19.2 MHz Qtime clock. And timers are derived
from this based on "pulse generators" configured to fire at a fixed
rate based on the Qtime clock.
This patch introduces ipa_qtime_config(), which configures the Qtime
mechanism for use. It also adds to the IPA register definitions
related to timers and time stamping.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA v4.5 significantly changes the format of the configuration
register used for endpoint aggregation. The AGGR_BYTE_LIMIT field
is now larger, and the positions of other fields are shifted. This
complicates the way we have to access this register because functions
like u32_encode_bits() require their field mask argument to be constant.
A further complication is that we want to know the maximum value
representable by at least one of these fields, and that too requires
a constant field mask.
This patch adds support for IPA v4.5 endpoint aggregation registers
in a way that continues to support "legacy" IPA hardware. It does
so in a way that keeps field masks constant.
First, for each variable field mask, we define an inline function
whose return value is either the legacy value or the IPA v4.5 value.
Second, we define functions for these fields that encode a value
to use in each field based on the IPA version (this approach is
already used elsewhere). The field mask provided is supplied by
the function mentioned above.
Finally, for the aggregation byte limit fields where we want to
know the maximum representable value, we define a function that
returns that maximum, computed from the appropriate field mask.
We can no longer verify at build time that our buffer size is
in the range that can be represented by the aggregation byte
limit field. So remove the test done by a BUILD_BUG_ON() call
in ipa_endpoint_validate_build(), and implement a comparable check
at the top of ipa_endpoint_data_valid().
Doing that makes ipa_endpoint_validate_build() contain a single
line BUILD_BUG_ON() call, so just remove that function and move
the remaining line into ipa_endpoint_data_valid().
One final note: the aggregation time limit value for IPA v4.5 needs
to be computed differently. That is handled in an upcoming patch.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The offsets for almost all GSI registers we use have different
offsets starting at IPA version 4.5. Only two registers remain
in their original location.
In a way though, the new register locations are not *that*
different. The entire group of affected registers has simply
been shifted down in memory by a fixed amount (0xd000). So for
example, the channel context 0 register that has a base offset of
0x0001c000 for "older" hardware now has a base offset of 0x0000f000.
This patch aims to add support for IPA v4.5 registers at their new
offets in a way that minimizes the amount of code that needs to
change. It is not ideal, but it avoids the need to maintain
a nearly complete set of additional register offset definitions.
The approach takes advantage of the fact that when accessing GSI
registers we do not access any of memory at lower end of the "gsi"
memory range (with two exceptions already noted). In particular,
we do not access anything within the bottom 0xd000 bytes of the
GSI memory range.
For IPA version 4.5, after we map the GSI memory, we adjust the
virtual memory pointer downward by the fixed amount (0xd000).
That way, register accesses using the offsets defined by the
existing GSI_REG_*() macros will resolve to the proper locations
for IPA version 4.5.
The two registers *not* affected by this offset are accessed only
in gsi_irq_setup(). There, for IPA version 4.5, we undo the general
register adjustment by adding the fixed amount back to the virtual
address to access these registers.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Very few GSI register definitions change for IPA v4.5, however
as a group their position in memory shifts a constant amount
(handled by the next commit).
Add definitions and update comments to the set of GSI registers to
support changes that come with IPA v4.5.
Update the logic in gsi_channel_program() to accommodate the new
(expanded) PREFETCH_MODE field in the CH_C_QOS register.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Update the IPA code to make use of the updated IPA v4.5 register
definitions. Generally what this patch does is, if IPA v4.5
hardware is in use:
- Ensure new registers or fields in IPA v4.5 are updated where
required
- Ensure registers or fields not supported in IPA v4.5 are not
examined when read, or are set to 0 when written
It does this while preserving the existing functionality for IPA
versions lower than v4.5.
The values to program for QSB_MAX_READS and QSB_MAX_WRITES and the
source and destination resource counts are updated to be correct for
all versions through v4.5 as well.
Note that IPA_RESOURCE_GROUP_SRC_MAX and IPA_RESOURCE_GROUP_DST_MAX
already reflect that 5 is an acceptable number of resources (which
IPA v4.5 implements).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA v4.5 adds a few fields to the endpoint header and extended
header configuration registers that represent new high-order bits
for certain offsets and sizes. Add code to incorporate these upper
bits into the registers for IPA v4.5.
This includes creating ipa_header_size_encoded(), which handles
encoding the metadata offset field for use in the ENDP_INIT_HDR
register in a way appropriate for the hardware version. This and
ipa_metadata_offset_encoded() ensure the mask argument passed to
u32_encode_bits() is constant.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Update "ipa_reg.h" so that register definitions support IPA hardware
version 4.5, in addition to versions 3.5.1 through v4.2. Most of
the register definitions are the same, but in some cases fields are
added, changed, or eliminated.
Updates for a few IPA v4.5 registers are more complex, and adding
those definition will be deferred to separate patches. This patch
only updates the register offset and field definitions, and adds
informational comments.
The only code change avoids accessing the backward compatibility
register for IPA version 4.5 in ipa_hardware_config(). Other IPA
v4.5-specific code changes will come later.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Starting with IPA v4.2 there is a GSI channel option to use an
"escape buffer" instead of prefetch buffers. This should be used
for all channels *except* the AP command TX channel. The logic
that implements this has it backwards; fix this bug.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
These are only used as input arguments to qmi_handle_init() which
accepts const pointers to both qmi_ops and qmi_msg_handler. Make them
const to allow the compiler to put them in read-only memory.
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Acked-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201122234031.33432-2-rikard.falkeborn@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A system shutdown can happen at essentially any time, and it's
possible that the IPA driver is busy when a shutdown is underway.
IPA hardware accesses IMEM and SMEM memory regions using an IOMMU,
and at some point during shutdown, needed I/O mappings could become
invalid. This could be disastrous for any "in flight" IPA activity.
Avoid this by defining a new driver shutdown callback that stops all
IPA activity and cleanly shuts down the driver. It merely calls the
driver's existing remove callback, reporting the error if it returns
one.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA driver remove callback, ipa_remove(), calls ipa_modem_stop()
if the setup stage of initialization is complete. If a concurrent
call to ipa_modem_start() or ipa_modem_stop() has begin but not
completed, ipa_modem_stop() can return an error (-EBUSY).
The next patch adds a driver shutdown callback, which will simply
call ipa_remove(). We really want our shutdown callback to clean
things up. So add a single retry to the ipa_modem_stop() call in
ipa_remove() after a short (millisecond) delay. This offers no
guarantee the shutdown will complete successfully, but we'll at
least try a little harder before giving up.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When stopping an AP RX channel, there can be a transient period
while the channel enters STOP_IN_PROC state before reaching the
final STOPPED state. In that case we make another attempt to stop
the channel.
Similarly, when stopping a modem channel (using a GSI generic
command issued from the AP), it's possible that multiple attempts
will be required before the channel reaches STOPPED state.
Add a field to the GSI structure to record an errno representing the
result code provided when a generic command completes. If the
result learned in gsi_isr_gp_int1() is RETRY, record -EAGAIN in the
result code, otherwise record 0 for success, or -EIO for any other
result.
If we time out nf gsi_generic_command() waiting for the command to
complete, return -ETIMEDOUT (as before). Otherwise return the
result stashed by gsi_isr_gp_int1().
Add a loop in gsi_modem_channel_halt() to reissue the HALT command
if the result code indicates -EAGAIN. Limit this to 10 retries
(after the initial attempt).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA v4.2 has a hardware quirk that requires the AP to allocate GSI
channels for the modem to use. It is recommended that these modem
channels get stopped (with a HALT generic command) by the AP when
its IPA driver gets removed.
The AP has no way of knowing the current state of a modem channel.
So when the IPA driver issues a HALT command it's possible the
channel is not running, and in that case we get an error indication.
This error simply means we didn't need to stop the channel, so we
can ignore it.
This patch adds an explanation for this situation, and arranges for
this condition to *not* report an error message.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
If the rmnet_ipa0 network device has not been opened at the time
we remove or shut down the IPA driver, its underlying TX and RX
GSI channels will not have been started, and they will still be
in ALLOCATED state.
The RESET command on a channel is meant to return a channel to
ALLOCATED state after it's been stopped. But if it was never
started, its state will still be ALLOCATED, the RESET command
is not required.
Quietly skip doing the reset without printing an error message if a
channel is already in ALLOCATED state when we request it be reset.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When a GSI command is used to change the state of a channel or event
ring we check the state before and after the command to ensure it is
as expected. If not, we print an error message, but it does not
include the channel or event ring id, and it easily can. Add the
channel or event ring id to these error messages.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Stop assuming a fixed IPA core clock rate and interconnect
bandwidths. Use the configuration data defined for these
things instead. Get rid of the previously-used constants.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Populate the core clock rate and interconnect average and peak
bandwidth data for SDM845 and SC7180 in their configuration data
files. At this point we still don't *use* this data.
Note that SC7180 actually defines a new core clock rate (100 MHz
instead of 75 MHz) and new interconnect bandwidth values. They
will be activated in the next commit, which uses the configured
values rather than the fixed constants.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Define a new type of configuration data, used to initialize the
IPA core clock and interconnects. This is the first of three
patches, and defines the data types and interface but doesn't
yet use them.
Switch the return value if there is no matching configuration data
to ENODEV instead of ENOTSUPP (to avoid using the nonstandard errno).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Some last cleanups for the existing IPA register definitions:
- Remove the definition of IPA_REG_ENABLED_PIPES_OFFSET, because
it is not used.
- Use "IPA_" instead of "BAM_" as the prefix on fields associated
with the FLAVOR_0 register. We use GSI (not BAM), but the
fields apply to both GSI and BAM.
- Get rid of the definition of IPA_CS_RSVD; it is never used.
- Add two missing field mask definitions for the INIT_DEAGGR
endpoint register.
- Eliminate a few of the defined sequencer types, because they
are unused. We can add them back when needed.
- Add a field mask to indicate which bit causes an interrupt on
the microcontroller.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Move the definition of the ipa_irq_id enumerated type out of
"ipa_interrupt.h" and into "ipa_reg.h", and flesh out its set of
defined values. Each interrupt id indicates a particular type of
IPA interrupt that can be signaled. Their numeric values define bit
positions in the IPA_IRQ_* registers, so should their definitions
should accompany the definition of those register offsets.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Move a few things around in "ipa_reg.h":
- Move the definition of ipa_reg_state_aggr_active_offset() down
a bit in the file so definitions are ordered by offset (for
the lowest supported IPA version) like all other definitions.
- Move the definition TIMER_FREQUENCY to be immediately above
the definition of ipa_aggr_granularity_val() where it's used.
- Move each register field value enumerated type definition to
immediately follow the definitions of the register and field
it is associated with.
No code functionality is modified by this patch.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Revise or add comments in "ipa_reg.h" for to provide more
information, and to improve clarity and consistency.
- Always provide a comment to define when a register or field is
supported (or not) for certain versions of IPA hardware.
- Try to be specific about *which* or *how many* definitions
a comment refers to.
- Move comments stating that ipa->available defines the valid
bits in various registers *above* the register offset
definition, to avoid some checkpatch.pl warnings.
No code is changed by this patch.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Consistently define numeric values for enumerated type members using
hexidecimal (rather than decimal) format values. Align the values
assigned in the same column in each file.
Only assign values where they really matter, for example don't
assign IPA_ENDPOINT_AP_MODEM_TX the value 0.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The backward compatibility register field masks are defined using
single-bit masks defined with BIT(x) rather than GENMASK(x, x).
Change this one set of definitions to follow the GENMASK() pattern
used everywhere else. Add a few missing field definitions for this
register as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Several IPA register field masks are defined without the "_FMASK"
suffix naming convention. Rename these, so all field masks are
consistently named.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Rename two suspend IRQ registers so they follow the IPA_REG_IRQ_xxx
naming convention used elsewhere.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA version 3.5.1 represents the timer used in avoiding head-of-line
blocking with a simple tick count. IPA v4.2 changes that, instead
splitting the timer field into two parts (base and scale) to
represent the ticks in the timer period.
IPA v4.0 and IPA v4.1 use the same method as IPA v3.5.1. Change the
test in ipa_reg_init_hol_block_timer_val() so the result is correct
for those versions as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
For IPA v3.5.1, the IPA filter/routing hash enable register actually
does exist, but it is at offset 0x8c into the IPA register space.
For newer versions of IPA it is at offset 0x148.
Define a new inline function ipa_reg_filt_rout_hash_en_offset() to
return the appropriate value for a given version of IPA hardware.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA filter/routing hash enable register and filter/routing hash
flush register each have four single-bit fields representing the
four hashed tables to be enabled or flushed. The field positions
are identical, so just use a single set of field masks to represent
the fields for both registers.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Transactions sit on one of several lists, depending on their state
(allocated, pending, complete, or polled). A spinlock protects
against concurrent access when transactions are moved between these
lists.
Transactions are also reference counted. A newly-allocated
transaction has an initial count of 1; a transaction is released in
gsi_trans_free() only if its decremented reference count reaches 0.
Releasing a transaction includes removing it from the polled (or if
unused, allocated) list, so the spinlock is acquired when we release
a transaction.
The reference count is used to allow a caller to synchronously wait
for a committed transaction to complete. In this case, the waiter
takes an extra reference to the transaction *before* committing it
(so it won't be freed), and releases its reference (calls
gsi_trans_free()) when it is done with it.
Similarly, gsi_channel_update() takes an extra reference to ensure a
transaction isn't released before the function is done operating on
it. Until the transaction is moved to the completed list (by this
function) it won't be freed, so this reference is taken "safely."
But in the quiesce path, we want to wait for the "last" transaction,
which we find in the completed or polled list. Transactions on
these lists can be freed at any time, so we (try to) prevent that
by taking the reference while holding the spinlock.
Currently gsi_trans_free() decrements a transaction's reference
count unconditionally, acquiring the lock to remove the transaction
from its list *only* when the count reaches 0. This does not
protect the quiesce path, which depends on the lock to ensure its
extra reference prevents release of the transaction.
Fix this by only dropping the last reference to a transaction
in gsi_trans_free() while holding the spinlock.
Fixes: 9dd441e4ed ("soc: qcom: ipa: GSI transactions")
Reported-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201114182017.28270-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA-resident microcontroller has the ability to log various
activity in an area of IPA shared memory. When the microcontroller
starts it generates an event to the AP to provide information about
the log.
We don't support reading this log, and we can safely ignore the
event. So do that rather than treating the log info event we
receive as "unsupported."
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
I have discovered that the maximum number of source packet contexts
configured for SDM845 is incorrect. Fix this error.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Replace constants defined with an "_FVAL" suffix with values defined
in enumerated types, to be consistent with other usage in the driver.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The gsi_ch_cmd_opcode, gsi_evt_cmd_opcode, and gsi_generic_cmd_opcode
enumerated types are values that fields in the GSI command registers
can take on. Move their definitions out of "gsi.c" and into "gsi_reg.h",
alongside the definition of registers they are associated with.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The gsi_err_code and gsi_err_type enumerated types are values that
fields in the GSI ERROR_LOG register can take on. Move their
definitions out of "gsi.c" and into "gsi_reg.h", alongside the
definition of the ERROR_LOG register offset and field symbols.
Drop the "_ERR" suffix in the names of the gsi_err_code members.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The gsi_channel_type enumerated type define values used for the
channel type/protocol for event rings and channels. Move its
definition out of "gsi.c" and into "gsi_reg.h", alongside the
definition of the CH_C_CNTXT_0 register offset and its fields.
Add a comment near the definition of the EV_CH_E_CNTXT_0 register
indicating this type is used for its EV_CHTYPE field.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The numeric values that represent the event ring channel type are
identical to the values that represent the matching protocol used
for a channel. Use a new gsi_channel_type enumerated type to
represent the values programmed for both cases, using "CHANNEL_TYPE"
in member names in place of "EVT_CHTYPE" and "CHANNEL_PROTOCOL".
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Define the GSI global interrupt types with an enumerated type whose
values are the bit positions representing the global interrupt types.
Similarly, define the GSI general interrupt types with an enumerated
type whose values are the bit positions of general interrupt types.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
There is no need for gsi_modem_channel_halt() to report an error,
because gsi_generic_command() will already have done that if the
command times out. So get rid of the extra message.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When we determine from hardware what the size of IPA memory is
we compare it against what we learned about it from DT.
If DT defines a region that's larger than actual memory, we use the
smaller actual size and issue a warning.
If DT defines a smaller region than actual memory we issue a warning
too. But in this case the difference is harmless; so rather than
issuing a warning, just provide a debug message instead.
Reorder these checks so the one that matters more is done first.
Reported-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Delete a spurious line of code in ipa_hardware_config(). It reads a
register value then ignores the value, so is completely unnecessary.
Add a missing word in a comment.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The following call in ipa_validate_build() is erroneous:
BUILD_BUG_ON(sizeof(struct gsi_trans) > 128);
The fact is, it is not a bug for the size of a GSI transaction to be
bigger than 128 bytes. The correct operation of the driver is not
dependent on the size of this structure. The only consequence of
the transaction being large is that the amount of memory required
is larger.
The problem this was trying to flag is that a *slight* increase in
the size of this structure will have a disproportionate effect on
the amount of memory used. E.g. if the structure grew to 132 bytes
the memory requirement for the transaction arrays would be about
double.
With various debugging build flags enabled, the size grows to 160
bytes. But there's no reason to treat that as a build-time bug.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Now that all of the GSI interrupts are handled uniformly,
change gsi_irq_type_update() so it takes a value. Have the
function assign that value to the cached mask of enabled GSI
IRQ types before writing it to hardware.
Note that gsi_irq_teardown() will only be called after
gsi_irq_disable(), so it's not necessary for the former
to disable all IRQ types. Get rid of that.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Most GSI general errors are unrecoverable without a full reset.
Despite that, we want to receive these errors so we can at least
report what happened before whatever undefined behavior ensues.
Explicitly disable all such interrupts in gsi_irq_setup(), then
enable those we want in gsi_irq_enable(). List the interrupt types
we are interested in (everything but breakpoint) explicitly rather
than using GSI_CNTXT_GSI_IRQ_ALL, and remove that symbol's
definition.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
It is possible for other execution environments (EEs, like the modem)
to request changes to local (AP) channel or event ring state. We do
not support this feature.
In gsi_irq_setup(), explicitly zero the mask that defines which
channels are permitted to generate inter-EE channel state change
interrupts. Do the same for the event ring mask.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A GSI channel must be started in order to use it to perform a
transfer data (or command) transaction. And the only time we'll see
an IEOB interrupt is if we send a transaction to a started channel.
Therefore we do not need to have the IEOB interrupt type enabled
until at least one channel has been started. And once the last
started channel has been stopped, we can disable the IEOB interrupt
type again.
We already enable the IEOB interrupt for a particular channel only
when it is started. Extend that by having the IEOB interrupt *type*
be enabled only when at least one channel is in STARTED state.
Disallow all channels from triggering the IEOB interrupt in
gsi_irq_setup(). We only enable an channel's interrupt when
needed, so there is no longer any need to zero the channel mask
in gsi_irq_disable().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The completion of a generic EE GSI command is signaled by a global
interrupt of type GP_INT1. The only other used type for a global
interrupt is a hardware error report.
First, disallow all global interrupt types in gsi_irq_setup(). We
want to know about hardware errors, so re-enable the interrupt type
in gsi_irq_enable(), to allow hardware errors to be reported.
Disable that interrupt type again in gsi_irq_disable().
We only issue generic EE commands one at a time, and there's no
reason to keep the completion interrupt enabled when no generic
EE command is pending. We furthermore have no need to enable the
GP_INT2 or GP_INT3 interrupt types (which aren't used).
The change in gsi_irq_enable() makes GSI_CNTXT_GLOB_IRQ_ALL unused,
so get rid of it. Have gsi_generic_command() enable the GP_INT1
interrupt type (in addition to the ERROR_INT type) only while a
generic command is pending.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A GSI event ring causes an event control interrupt to fire whenever
its state changes (between NOT_ALLOCATED and ALLOCATED). No event
ring should ever change state except when we request it to.
Currently, we permit *all* events rings to generate event control
interrupts--even those that are never used. And we enable event
control interrupts essentially at all times, from setup to teardown.
Instead, only enable the event control interrupt type for the
duration of an event ring command, and when doing so, only allow
the event ring being operated upon to cause the interrupt to fire.
Disallow all event rings from issuing the event control interrupt
in gsi_irq_setup().
Because an event ring's interrupt is only enabled when needed,
there is no longer any need to zero the event channel mask in
gsi_irq_disable().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A GSI channel causes a channel control interrupt to fire whenever
its state changes (between NOT_ALLOCATED, ALLOCATED, STARTED, etc.).
We do not support inter-EE channel commands (initiated by other EEs),
so no channel should ever change state except when we request it to.
Currently, we permit *all* channels to generate channel control
interrupts--even those that are never used. And we enable channel
control interrupts essentially at all times, from setup to teardown.
Instead, disable all channel control interrupts initially in
gsi_irq_setup(), and only enable the channel control interrupt
type for the duration of a channel command. When doing so, only
allow the channel being operated upon to cause the interrupt to
fire.
Because a channel's interrupt is now enabled only when needed (one
channel at a time), there is no longer any need to zero the channel
mask in gsi_irq_disable().
Add new gsi_irq_type_enable() and gsi_irq_type_disable() as helper
functions to control whether a given GSI interrupt type is enabled.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Keep track of the set of GSI interrupt types that are currently
enabled by recording the mask value to write (or last written) to
the TYPE_IRQ_MSK register.
Create a new helper function gsi_irq_type_update() to handle
actually writing the register.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Introduce gsi_irq_setup() and gsi_irq_teardown() to disable all
GSI interrupts when first setting up GSI hardware, and to clean
things up when we're done.
Re-enable all GSI interrupt types in gsi_irq_enable(), but do
so only after each of the type-specific interrupt masks has
been configured. Similarly, disable all interrupt types in
gsi_irq_disable()--first--before zeroing out the type-specific
masks.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Define the GSI interrupt types with an enumerated type whose values
are the bit positions representing each interrupt type. Include a
short comment describing how each interrupt type is used.
Build up the enabled interrupt mask explicitly in gsi_irq_enable(),
and get rid of the definition of GSI_CNTXT_TYPE_IRQ_MSK_ALL.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Rename the "event_enable_bitmap" field of the GSI structure to be
"ieob_enabled_bitmap". An upcoming patch will cache the last value
stored for another interrupt mask and this is a more direct naming
convention to follow.
Add a few comments to explain the bitmap fields in the GSI structure.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Introduce gsi_irq_init() and gsi_irq_exit(), to encapsulate looking
up the GSI IRQ and registering its handler. Call gsi_irq_init() a
little later in gsi_init(), and initialize the completion earlier.
The IRQ handler accesses both the GSI virtual memory pointer and the
completion, and this way these things will have been initialized
before the gsi_irq() can ever be called.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The GSI code is now exposed to IPA version numbers, and we handle
version-specific behavior based on the IPA version.
Modify some comments that talk about GSI versions so they reference
IPA versions instead. Correct version number errors in a couple of
these comments.
The (comment) mapping between IPA and GSI versions in the definition
of the ipa_version enumerated type remains.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We enable a channel doorbell engine only for IPA v3.5.1, and that is
now handled directly by gsi_channel_program().
When initially setting up a channel, we want that doorbell engine
enabled, and we can request that independent of the IPA version.
Doing that makes the "legacy" argument to gsi_channel_setup_one()
unnecessary. And with that gone we can get rid of the "legacy"
argument to gsi_channel_setup(), and gsi_setup() as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Use the IPA version in gsi_channel_program() to determine whether
we should enable the GSI doorbell engine when requested. This way,
callers only say whether or not it should be enabled if needed,
regardless of hardware version.
Rename the "legacy" argument to gsi_channel_reset(), and have
it indicate whether the doorbell engine should be enabled when
reprogramming following the reset.
Change all callers of gsi_channel_reset() to indicate whether to
enable the doorbell engine after reset, independent of hardware
version.
Rework a little logic in ipa_endpoint_reset() to get rid of the
"legacy" variable previously passed to gsi_channel_reset().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A quirk of IPA v3.5.1 requires a channel reset on an RX channel to
be performed twice. Use the IPA version in gsi_channel_reset()
rather than the passed-in legacy flag to determine that.
This is actually a bug fix, because this double reset is supposed
to occur independent of whether we're enabling the doorbell engine.
Now they will be independent.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
A quirk of IPA v4.2 requires the AP to allocate the GSI channels
that are owned by the modem.
Rather than pass a flag argument to gsi_channel_init(), use the
IPA version directly in that function to determine whether modem
channels need to be allocated.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Record the IPA version passed to gsi_init() in the GSI structure.
This allows that value to be used directly where needed, rather than
passing and storing certain flag arguments through the code.
In particular, for all but one supported version of IPA, the command
channel is programmed to only use an "escape buffer". By storing
the IPA version, we can do a simple version check in one location,
and avoid storing a flag field in every channel (and passing a flag
along while initializing channels to set that field properly).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Although GSI is integral to IPA, it is a separate hardware component
and the IPA code supporting it has been structured to avoid explicit
dependence on IPA details. An example of this is that gsi_init() is
passed a number of Boolean flags to indicate special behaviors,
whose values are dependent on the IPA hardware version. Looking
ahead, newer hardware versions would require even more such special
behaviors.
For any given version of IPA hardware (like 3.5.1 or 4.2), the GSI
hardware version is fixed (in this case, 1.3 and 2.2, respectively).
So the IPA version *implies* the GSI version, and the IPA version
can be used as effectively the equivalent of the GSI hardware version.
Rather than proliferating new special behavior flags, just provide
the IPA version to the GSI layer when it is initialized. The GSI
code can then use that directly to determine whether special
behaviors are required. The IPA version enumerated type is already
isolated to its own header file, so the exposure of this IPA detail
is very limited.
For now, just change gsi_init() to pass the version rather than the
Boolean flags, and set the flag values internal to that function.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
With IPA v3.5.1, if IPA aggregation is active at the time an
underlying GSI channel reset is performed, some special handling
is required.
There is logic in ipa_endpoint_reset() that arranges for that
special handling, but it's done for all hardware versions, not
just IPA v3.5.1.
Fix the logic to properly restrict the special behavior.
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201102173435.5987-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The previous commit added support for IPA having up to six source
and destination resources. But currently nothing uses more than
four. (Five of each are used in a newer version of the hardware.)
I find that in one of my build environments the compiler complains
about newly-added code in two spots. Inspection shows that the
warnings have no merit, but this compiler does not recognize that.
ipa_main.c:457:39: warning: array index 5 is past the end of the
array (which contains 4 elements) [-Warray-bounds]
(and the same warning at line 483)
We can make this warning go away by changing the number of elements
in the source and destination resource limit arrays--now rather than
waiting until we need it to support the newer hardware. This change
was coming soon anyway; make it now to get rid of the warning.
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201031151524.32132-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The minimum and maximum limits for resources assigned to a given
resource group are programmed in pairs, with the limits for two
groups set in a single register.
If the number of supported resource groups is odd, only half of the
register that defines these limits is valid for the last group; that
group has no second group in the pair.
Currently we ignore this constraint, and it turns out to be harmless,
but it is not guaranteed to be. This patch addresses that, and adds
support for programming the 5th resource group's limits.
Rework how the resource group limit registers are programmed by
having a single function program all group pairs rather than having
one function program each pair. Add the programming of the 4-5
resource group pair limits to this function. If a resource group is
not supported, pass a null pointer to ipa_resource_config_common()
for that group and have that function write zeroes in that case.
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The number of resource groups supported by the hardware can be
different for source and destination resources. Determine the
number supported for each using separate functions. Make the
functions inline end move their definitions into "ipa_reg.h",
because they determine whether certain register definitions are
valid. Pass just the IPA hardware version as argument.
IPA_RESOURCE_GROUP_COUNT represents the maximum number of resource
groups the driver supports for any hardware version. Change that
symbol to be two separate constants, one for source and the other
for destination resource groups. Rename them to end with "_MAX"
rather than "_COUNT", to reflect their true purpose.
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The IPA hardware manages various resources (e.g. descriptors)
internally to perform its functions. The resources are grouped,
allowing different endpoints to use separate resource pools. This
way one group of endpoints can be configured to operate unaffected
by the resource use of endpoints in a different group.
Endpoints should be assigned to a resource group, but we currently
don't do that.
Define a new resource_group field in the endpoint configuration
data, and use it to assign the proper resource group to use for
each AP endpoint.
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The mask for the RSRC_GRP field in the INIT_RSRC_GRP endpoint
initialization register is incorrectly defined for IPA v4.2 (where
it is only one bit wide). So we need to fix this.
The fix is not straightforward, however. Field masks are passed to
functions like u32_encode_bits(), and for that they must be constant.
To address this, we define a new inline function that returns the
*encoded* value to use for a given RSRC_GRP field, which depends on
the IPA version. The caller can then use something like this, to
assign a given endpoint resource id 1:
u32 offset = IPA_REG_ENDP_INIT_RSRC_GRP_N_OFFSET(endpoint_id);
u32 val = rsrc_grp_encoded(ipa->version, 1);
iowrite32(val, ipa->reg_virt + offset);
The next patch requires this fix.
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
At the end of ipa_mem_setup() we write the local packet processing
context base register to tell it where the processing context memory
is. But we are writing the wrong value.
The value written turns out to be the offset of the modem header
memory region (assigned earlier in the function). Fix this bug.
Tested-by: Sujit Kautkar <sujitka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPA transactions describe actions to be performed by the IPA
hardware. Three cases use IPA transactions: transmitting a socket
buffer; providing a page to receive packet data; and issuing an IPA
immediate command. An IPA transaction contains a scatter/gather
list (SGL) to hold the set of actions to be performed.
We map buffers in the SGL for DMA at the time they are added to the
transaction. For skb TX transactions, we fill the SGL with a call
to skb_to_sgvec(). Page RX transactions involve a single page
pointer, and that is recorded in the SGL with sg_set_page(). In
both of these cases we then map the SGL for DMA with a call to
dma_map_sg().
Immediate commands are different. The payload for an immediate
command comes from a region of coherent DMA memory, which must
*not* be mapped for DMA. For that reason, gsi_trans_cmd_add()
sort of hand-crafts each SGL entry added to a command transaction.
This patch fixes a problem with the code that crafts the SGL entry
for an immediate command. Previously a portion of the SGL entry was
updated using sg_set_buf(). However this is not valid because it
includes a call to virt_to_page() on the buffer, but the command
buffer pointer is not a linear address.
Since we never actually map the SGL for command transactions, there
are very few fields in the SGL we need to fill. Specifically, we
only need to record the DMA address and the length, so they can be
used by __gsi_trans_commit() to fill a TRE. We additionally need to
preserve the SGL flags so for_each_sg() still works. For that we
can simply assign a null page pointer for command SGL entries.
Fixes: 9dd441e4ed ("soc: qcom: ipa: GSI transactions")
Reported-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20201022010029.11877-1-elder@linaro.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Minor conflicts in net/mptcp/protocol.h and
tools/testing/selftests/net/Makefile.
In both cases code was added on both sides in the same place
so just keep both.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When processing a system suspend request we suspend modem endpoints
if they are enabled, and call ipa_cmd_tag_process() (which issues
IPA commands) to ensure the IPA pipeline is cleared. It is an error
to attempt to issue an IPA command before setup is complete, so this
is clearly a bug. But we also shouldn't suspend or resume any
endpoints that have not been set up.
Have ipa_endpoint_suspend() and ipa_endpoint_resume() immediately
return if setup hasn't completed, to avoid any attempt to configure
endpoints or issue IPA commands in that case.
Fixes: 84f9bd12d4 ("soc: qcom: ipa: IPA endpoints")
Tested-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In ipa_uc_response_hdlr() a comment uses the wrong function name
when it describes where a clock reference is taken. Fix this.
Also fix the comment in ipa_uc_response_hdlr() to correctly refer to
ipa_uc_setup(), which is where the clock reference described here is
taken.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
When "W=2" is supplied to the build command, we get a warning about
shadowing a global declaration (of a typedef) for a variable defined
in ipa_probe(). Rename the variable to get rid of the warning.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Fix two spots where a variable "channel_id" is unnecessarily
redefined inside loops in "gsi.c". This is warned about if
"W=2" is added to the build command.
Note that this problem is harmless, so there's no need to backport
it as a bugfix.
Remove a comment in gsi_init() about waking the system; the GSI
interrupt does not wake the system any more.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
The GSI general interrupt is managed by three registers: enable;
status; and clear. The three registers have same set of field bits
at the same locations. Use a common set of field masks for all
three registers to avoid duplication.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
The GSI global interrupt is managed by three registers: enable;
status; and clear. The three registers have same set of field bits
at the same locations. Use a common set of field masks for all
three registers to avoid duplication.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
