Each GSI channel has a CNTXT_1 register that encodes the size of its
ring buffer. The size of the field that records that is increased
starting at IPA v4.9. Replace the use of a fixed-size field mask
with a new inline function that encodes that size value.
Similarly, the size of GSI event rings can be larger starting with
IPA v4.9, so create a function to encode that as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Modify conditional tests throughout the IPA code so they do not
assume that IPA v3.5.1 is the oldest version supported. Also remove
assumptions that IPA v4.5 is the newest version of IPA supported.
Augment versions in comments with "+", to be clearer that the
comment applies to a version and subsequent versions. (E.g.,
"present for IPA v4.2+" instead of just "present for v4.2".)
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
It is possible for a 32 bit x86 build to use a 64 bit DMA address.
There are two remaining spots where the IPA driver does a modulo
operation to check alignment of a DMA address, and under certain
conditions this can lead to a build error on i386 (at least).
The alignment checks we're doing are for power-of-2 values, and this
means the lower 32 bits of the DMA address can be used. This ensures
both operands to the modulo operator are 32 bits wide.
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Alex Elder
Zihao Tang
David S. Miller
Jakub Kicinski
Dan Carpenter