| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
[ Upstream commit b244163f2c45c12053cb0291c955f892e79ed8a9 ]
This node pointer is returned by of_parse_phandle() with refcount
incremented in this function. of_node_put() on it before exiting
this function.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Acked-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 440c3247cba3d9433ac435d371dd7927d68772a7 ]
IPA configuration data includes an array of memory region
descriptors. That was a fixed-size array at one time, but
at some point we started defining it such that it was only
as big as required for a given platform. The actual number
of entries in the array is recorded in the configuration data
along with the array.
A loop in ipa_mem_config() still assumes the array has entries
for all defined memory region IDs. As a result, this loop can
go past the end of the actual array and attempt to write
"canary" values based on nonsensical data.
Fix this, by stashing the number of entries in the array, and
using that rather than IPA_MEM_COUNT in the initialization loop
found in ipa_mem_config().
The only remaining use of IPA_MEM_COUNT is in a validation check
to ensure configuration data doesn't have too many entries.
That's fine for now.
Fixes: 3128aae8c439a ("net: ipa: redefine struct ipa_mem_data")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit b4afd4b90a7cfe54c7cd9db49e3c36d552325eac ]
We use ipa_cmd_header_valid() to ensure certain values we will
program into hardware are within range, well in advance of when we
actually program them. This way we avoid having to check for errors
when we actually program the hardware.
Unfortunately the dev_err() call for a bad offset value does not
supply the arguments to match the format specifiers properly.
Fix this.
There was also supposed to be a check to ensure the size to be
programmed fits in the field that holds it. Add this missing check.
Rearrange the way we ensure the header table fits in overall IPA
memory range.
Finally, update ipa_cmd_table_valid() so the format of messages
printed for errors matches what's done in ipa_cmd_header_valid().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 2d65ed76924bc772d3974b0894d870b1aa63b34a ]
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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit d5bc5015eb9d64cbd14e467db1a56db1472d0d6c ]
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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 3a9ef3e11c5d33e5cb355b4aad1a4caad2407541 ]
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: 530f9216a9537 ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 1d23a56b0296d29e7047b41fe0a42a001036160d ]
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: 650d1603825d8 ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit 4ace7a6e287b7e3b33276cd9fe870c326f880480 ]
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: 650d1603825d ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
[ Upstream commit afba9dc1f3a5390475006061c0bdc5ad4915878e ]
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: a646d6ec9098 ("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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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: 9dd441e4ed575 ("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>
|
|
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: 9dd441e4ed575 ("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>
|
|
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: 9dd441e4ed575 ("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: 84f9bd12d46d ("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>
|
|
The GSI interrupt type register and interrupt type mask register
have the same field bits at the same locations. Use a common set of
field masks for both registers rather than essentially duplicating
them. The only place the interrupt mask register uses any of these
is in gsi_irq_enable().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Most of the field masks used for fields in a status structure are
unused. Remove their definitions; we can add them back again when
we actually use them to handle arriving status messages. These are
warned about if "W=2" is added to the build command.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Only the deaggregation status exception type is ever actually used.
If any other status exception type is reported we basically ignore
it, and consume the packet. Remove the unused definitions of status
exception type symbols; they can be added back when we actually
handle them.
Separately, two consecutive if statements test the same condition
near the top of ipa_endpoint_suspend_one(). Instead, use a single
test with a block that combines the previously-separate lines of
code.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Three status opcodes are not currently supported. Symbols
representing their numeric values are defined but never used.
Remove those unused definitions; they can be defined again
when they actually get used.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
In "gsi_trans.c", the field mask TRE_FLAGS_IEOB_FMASK is defined but
never used. Although there's no harm in defining this, remove it
for now and redefine it at some future date if it becomes needed.
This is warned about if "W=2" is added to the build command.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Two minor conflicts:
1) net/ipv4/route.c, adding a new local variable while
moving another local variable and removing it's
initial assignment.
2) drivers/net/dsa/microchip/ksz9477.c, overlapping changes.
One pretty prints the port mode differently, whilst another
changes the driver to try and obtain the port mode from
the port node rather than the switch node.
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
We now trigger a system resume when we receive an IPA SUSPEND
interrupt. We should *not* wake up on GSI interrupts.
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>
|
|
Now that we handle wakeup interrupts properly, arrange for the IPA
interrupt to be treated as a wakeup interrupt.
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>
|
|
The previous patch causes a system resume to be triggered when a
packet is available for receipt on a suspended RX endpoint.
The CLOCK_HELD flag was previously used to indicate that an extra
clock reference was held, preventing suspend. But we no longer need
such a flag:
- We take an initial reference in ipa_config().
- That reference is held until ipa_suspend() releases it.
- A subsequent system resume leads to a reference getting
re-acquired in ipa_resume().
- This can repeat until ultimately the module is removed, where
ipa_remove() releases the reference.
We no longer need a special flag to determine whether this extra
reference is held--it is, provided probe has completed successfully
and the driver is not suspended (or removed).
On the other hand, once suspended, it's possible for more than one
endpoint to trip the IPA SUSPEND interrupt, and we only want to
trigger the system resume once. So repurpose the Boolean CLOCK_HELD
flag to record whether the IPA SUSPEND handler should initiate a
system resume.
The flag will be be cleared each time ipa_suspend() is called,
*before* any endpoints are suspended. And it will be set inside the
IPA SUSPEND interrupt handler exactly once per suspend.
Rename the flag IPA_FLAG_RESUMED to reflect its new purpose.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
The call to wakeup_source_register() in ipa_probe() does not do what
it was intended to do. Call device_init_wakeup() in ipa_setup()
instead, to set the IPA device as wakeup-capable and to initially
enable wakeup capability.
When we receive a SUSPEND interrupt, call pm_wakeup_dev_event()
with a zero processing time, to simply call for a resume without
any other processing. The ipa_resume() call will take care of
waking things up again, and will handle receiving the packet.
Note that this gets rid of a clock reference counting bug that
occurred when handling an IPA SUSPEND interrupt. Specifically,
ipa_suspend_handler() took an IPA clock reference *in addition*
to the one taken by ipa_resume(). There is no need to back-port
this fix however, because it only affects code that was not
previously working (this patch is part of fixing that).
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>
|
|
Currently, when (before) the last IPA clock reference is dropped,
all endpoints are suspended. And whenever the first IPA clock
reference is taken, all endpoints are resumed (or started).
In most cases there's no need to start endpoints when the clock
starts. So move the calls to ipa_endpoint_suspend() and
ipa_endpoint_resume() out of ipa_clock_put() and ipa_clock_get(),
respectiely. Instead, only suspend endpoints when handling a system
suspend, and only resume endpoints when handling a system resume.
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 take a clock reference in ipa_config() in order to prevent the
the IPA clock from being shutdown until a power management suspend
request arrives. An atomic field in the IPA structure records
whether that extra reference had been taken.
Rather than using an atomic to represent a Boolean value, define
a new flags bitmap, and define a "clock held" flag to represent
whether the extra clock reference has been taken.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Take advantage of the checking provided by refcount_t, rather than
using a plain atomic to represent the IPA clock reference count.
Note that we need to *set* the value to 1 in ipa_clock_get() rather
than incrementing it from 0 (because doing that is considered an
error for a refcount_t).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Looks like u32p_replace_bits() should be used instead of
u32_replace_bits() which does not modifies the value but returns the
modified version.
Fixes: 2b9feef2b6c2 ("soc: qcom: ipa: filter and routing tables")
Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Remove linux/notifier.h which is included more than once
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Wang Hai <wanghai38@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/andersson/remoteproc
Pull remoteproc updates from Bjorn Andersson:
"This introduces a new "detached" state for remote processors that are
deemed to be running at the time Linux boots and the infrastructure
for "attaching" to these. It then introduces the support for
performing this operation for the STM32 platform.
The coredump functionality is moved out from the core file and gains
support for an optional mode where the recovery phase awaits the
notification from devcoredump that the dump should be released. This
allows userspace to grab the coredump in scenarios where vmalloc space
is too low for creating a complete copy of the coredump before handing
this to devcoredump.
A new character device based interface is introduced to allow tying
the stoppage of a remote processor to the termination of a user space
process. This is useful in situations when such process provides
crucial resources/operations for the firmware running on the remote
processor.
The Texas Instrument K3 driver gains support for the C66x and C71x
DSPs.
Qualcomm remoteprocs gains support for stashing relocation information
in IMEM, to aid post mortem debugging and the crash notification
mechanism is generalized to be reusable in cases where loosely coupled
drivers needs to know about the status of a remote processor. One such
example is the IPA hardware block, which is jointly owned with the
modem and migrated to this improved interface.
It also introduces a number of bug fixes and debug improvements for
the Qualcomm modem remoteproc driver.
And it cleans up the inconsistent interface for remoteproc drivers to
implement power management"
* tag 'rproc-v5.9' of git://git.kernel.org/pub/scm/linux/kernel/git/andersson/remoteproc: (56 commits)
remoteproc: core: Register the character device interface
remoteproc: Add remoteproc character device interface
remoteproc: kill IPA notify code
net: ipa: new notification infrastructure
remoteproc: k3-dsp: Add support for C71x DSPs
dt-bindings: remoteproc: k3-dsp: Update bindings for C71x DSPs
remoteproc: k3-dsp: Add support for L2RAM loading on C66x DSPs
remoteproc: k3-dsp: Add a remoteproc driver of K3 C66x DSPs
dt-bindings: remoteproc: Add bindings for C66x DSPs on TI K3 SoCs
remoteproc: k3: Add TI-SCI processor control helper functions
remoteproc: Introduce rproc_of_parse_firmware() helper
dt-bindings: arm: keystone: Add common TI SCI bindings
remoteproc: qcom_q6v5_mss: Remove redundant running state
remoteproc: qcom: q6v5: Update running state before requesting stop
remoteproc: qcom_q6v5_mss: Add modem debug policy support
remoteproc: qcom_q6v5_mss: Validate modem blob firmware size before load
remoteproc: qcom_q6v5_mss: Validate MBA firmware size before load
rpmsg: update documentation
remoteproc: qcom_q6v5_mss: Add MBA log extraction support
remoteproc: Add coredump debugfs entry
...
|
|
Use the new SSR notifier infrastructure to request notifications of
modem events, rather than the remoteproc IPA notification system.
The latter was put in place temporarily with the knowledge that the
new mechanism would become available.
Acked-by: David S. Miller <davem@davemloft.net>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Alex Elder <elder@linaro.org>
Link: https://lore.kernel.org/r/20200724181142.13581-2-elder@linaro.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
|
|
This commit affects comments (and in one case, whitespace) only.
Throughout the IPA code, return statements are documented using
"@Return:", whereas they should use "Return:" instead. Fix these
mistakes.
In function definitions, some parameters are missing their comment
to describe them. And in structure definitions, some fields are
missing their comment to describe them. Add these missing
descriptions.
Some arguments changed name and type along the way, but their
descriptions were not updated (an endpoint pointer is now used in
many places that previously used an endpoint ID). Fix these
incorrect parameter descriptions.
In the description for the ipa_clock structure, one field had a
semicolon instead of a colon in its description. Fix this.
Add a missing function description for ipa_gsi_endpoint_data_empty().
All of these issues were identified when building with "W=1".
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
All conflicts seemed rather trivial, with some guidance from
Saeed Mameed on the tc_ct.c one.
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Include "ipa_gsi.h" in "ipa_gsi.c", so the public functions are
defined before they are used in "ipa_gsi.c". This addresses some
warnings that are reported with a "W=1" build.
Fixes: c3f398b141a8 ("soc: qcom: ipa: IPA interface to GSI")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Pointers to two struct types are used in "ipa_gsi.h", without those
struct types being forward-declared. Add these declarations.
Fixes: c3f398b141a8 ("soc: qcom: ipa: IPA interface to GSI")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Building with "W=1" did exactly what it was supposed to do, namely
point out some suspicious-looking code to be verified not to contain
bugs.
Some QMI message structures defined in "ipa_qmi_msg.c" contained
some bad field names (duplicating the "elem_size" field instead of
defining the "offset" field), almost certainly due to copy/paste
errors that weren't obvious in a scan of the code. Fix these bugs.
Fixes: 530f9216a953 ("soc: qcom: ipa: AP/modem communications")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
For IPA v4.2, the exact interpretation of the register that defines
the timeout for avoiding head-of-line blocking was a little unclear.
We're only assigning a 0 timeout to it right now, so that wasn't
very important. But now that I know how it's supposed to work, I'm
fixing it.
The register represents a tick counter, where each tick is equal to
128 IPA core clock cycles. For IPA v3.5.1, the register contains
a simple counter value. But for IPA v4.2, the register contains two
fields, base and scale, which approximate the tick counter as:
ticks = base << scale
The base and scale values to use for a given tick count are computed
using clever bit operations, and measures are taken to make the
resulting time period as close as possible to that requested.
There's no need for ipa_endpoint_init_hol_block_timer() to return
an error, so change its return type to void.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Create a new function that returns the current rate of the IPA core
clock.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Have functions that write endpoint configuration registers return
immediately if they are not valid for the direction of transfer for
the endpoint. This allows most of the calls in ipa_endpoint_program()
to be made unconditionally. Reorder the register writes to match
the order of their definition (based on offset).
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
IPA version 4.0+ does not support endpoint suspend. Put a test at
the top of ipa_endpoint_program_suspend() that returns immediately
if suspend is not supported rather than making that check in the caller.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
IPA version 3.5.1 has a hardware quirk that requires special
handling if an RX endpoint is suspended while aggregation is active.
This handling is implemented by ipa_endpoint_suspend_aggr().
Have ipa_endpoint_program_suspend() be responsible for calling
ipa_endpoint_suspend_aggr() if suspend mode is being enabled on
an endpoint. If the endpoint does not support aggregation, or if
aggregation isn't active, this call will continue to have no effect.
Move the definition of ipa_endpoint_suspend_aggr() up in the file so
its definition precedes the new earlier reference to it. This
requires ipa_endpoint_aggr_active() and ipa_endpoint_force_close()
to be moved as well.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
IPA version 4.2 has a hardware quirk that affects endpoint delay
mode, so it isn't used there. Isolate the test that avoids using
delay mode for that version inside ipa_endpoint_program_delay(),
rather than making that check in the caller.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
The convention throughout the IPA driver is to directly use
single-bit field mask values, rather than using (for example)
u32_encode_bits() to set or clear them.
Fix the one place that doesn't follow that convention, which sets
HOL_BLOCK_EN_FMASK in ipa_endpoint_init_hol_block_enable().
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
A handful of registers are valid only for RX endpoints, and some
others are valid only for TX endpoints. For these endpoints, add
a comment above their defined offset macro that indicates the
endpoints to which they apply.
Extend the endpoint parameter naming convention as well, to make
these constraints more explicit.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
The INIT_MODE endpoint configuration register is only valid for TX
endpoints. Rather than writing a zero to that register for RX
endpoints, avoid writing the register at all.
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
