rpmsg: glink: use only lower 16-bits of param2 for CMD_OPEN name length

The name len field of the CMD_OPEN packet is only 16-bits and the upper
16-bits of "param2" are a different "prio" field, whic

rpmsg: glink: use only lower 16-bits of param2 for CMD_OPEN name length

The name len field of the CMD_OPEN packet is only 16-bits and the upper
16-bits of "param2" are a different "prio" field, which can be nonzero in
certain situations, and CMD_OPEN packets can be unexpectedly dropped
because of this.

Fix this by masking out the upper 16 bits of param2.

Fixes: b4f8e52b89f6 ("rpmsg: Introduce Qualcomm RPM glink driver")
Signed-off-by: Jonathan Marek <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Handle rejected intent request better

GLINK operates using pre-allocated buffers, aka intents, where incoming
messages are aggregated before being passed up the stack. In the case
that

rpmsg: glink: Handle rejected intent request better

GLINK operates using pre-allocated buffers, aka intents, where incoming
messages are aggregated before being passed up the stack. In the case
that no suitable intents have been announced by the receiver, the sender
can request an intent to be allocated.

The initial implementation of the response to such request dealt
with two outcomes; granted allocations, and all other cases being
considered -ECANCELLED (likely from "cancelling the operation as the
remote is going down").

But on some channels intent allocation is not supported, instead the
remote will pre-allocate and announce a fixed number of intents for the
sender to use. If for such channels an rpmsg_send() is being invoked
before any channels have been announced, an intent request will be
issued and as this comes back rejected the call fails with -ECANCELED.

Given that this is reported in the same way as the remote being shut
down, there's no way for the client to differentiate the two cases.

In line with the original GLINK design, change the return value to
-EAGAIN for the case where the remote rejects an intent allocation
request.

It's tempting to handle this case in the GLINK core, as we expect
intents to show up in this case. But there's no way to distinguish
between this case and a rejection for a too big allocation, nor is it
possible to predict if a currently used (and seemingly suitable) intent
will be returned for reuse or not. As such, returning the error to the
client and allow it to react seems to be the only sensible solution.

In addition to this, commit 'c05dfce0b89e ("rpmsg: glink: Wait for
intent, not just request ack")' changed the logic such that the code
always wait for an intent request response and an intent. This works out
in most cases, but in the event that an intent request is rejected and no
further intent arrives (e.g. client asks for a too big intent), the code
will stall for 10 seconds and then return -ETIMEDOUT; instead of a more
suitable error.

This change also resulted in intent requests racing with the shutdown of
the remote would be exposed to this same problem, unless some intent
happens to arrive. A patch for this was developed and posted by Sarannya
S [1], and has been incorporated here.

To summarize, the intent request can end in 4 ways:
- Timeout, no response arrived => return -ETIMEDOUT
- Abort TX, the edge is going away => return -ECANCELLED
- Intent request was rejected => return -EAGAIN
- Intent request was accepted, and an intent arrived => return 0

This patch was developed with input from Sarannya S, Deepak Kumar Singh,
and Chris Lew.

[1] https://lore.kernel.org/all/[email protected]/

Fixes: c05dfce0b89e ("rpmsg: glink: Wait for intent, not just request ack")
Cc: [email protected]
Tested-by: Johan Hovold <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Link: https://lore.kernel.org/r/20241023-pmic-glink-ecancelled-v2-1-ebc268129407@oss.qualcomm.com
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

So, in order to avoid ending up

rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

So, in order to avoid ending up with a flexible-array member in the
middle of multiple other structs, we use the `__struct_group()`
helper to create a new tagged `struct glink_msg_hdr`. This structure
groups together all the members of the flexible `struct glink_msg`
except the flexible array.

As a result, the array is effectively separated from the rest of the
members without modifying the memory layout of the flexible structure.
We then change the type of the middle struct members currently causing
trouble from `struct glink_msg` to `struct glink_msg_hdr`.

We also want to ensure that when new members need to be added to the
flexible structure, they are always included within the newly created
tagged struct. For this, we use `static_assert()`. This ensures that the
memory layout for both the flexible structure and the new tagged struct
is the same after any changes.

This approach avoids having to implement `struct glink_msg_hdr` as a
completely separate structure, thus preventing having to maintain two
independent but basically identical structures, closing the door to
potential bugs in the future.

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible-array
member, if necessary.

Additionally, we use the `DEFINE_RAW_FLEX()` helper for an on-stack
definition of a flexible structure where the size for the flexible-array
member is known at compile-time.

So, with these changes, fix the following warnings:
drivers/rpmsg/qcom_glink_native.c:51:26: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:459:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:846:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:968:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:1380:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by: Gustavo A. R. Silva <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/ZrOQa2gew5yadyt3@cute
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Introduce packet tracepoints

Introduce tracepoints to allow tracing the GLINK packets being exchanged
with other subsystems. This is useful for debugging both interaction
with remote p

rpmsg: glink: Introduce packet tracepoints

Introduce tracepoints to allow tracing the GLINK packets being exchanged
with other subsystems. This is useful for debugging both interaction
with remote processors and client driver issues, as well as tracking
latency through the communication stack.

Channel events are traced with both local and remote channel ids, as
well as the textual representation when possible. The channel ids are
useful when matching traces with traces from the firmware side logs,
while the textual representation is necessary to identify channels when
we're starting to trace an already running system.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Pass channel to qcom_glink_send_close_ack()

Align the qcom_glink_send_close_ack() arguments with other functions to
take the struct glink_channel, so that the upcoming tracepoint patch

rpmsg: glink: Pass channel to qcom_glink_send_close_ack()

Align the qcom_glink_send_close_ack() arguments with other functions to
take the struct glink_channel, so that the upcoming tracepoint patch can
access the channel attributes.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Tidy up RX advance handling

The operation of advancing the FIFO receive pointer is sprinkled between
the interrupt handler itself, and functions being called from this.

Push all the R

rpmsg: glink: Tidy up RX advance handling

The operation of advancing the FIFO receive pointer is sprinkled between
the interrupt handler itself, and functions being called from this.

Push all the RX advancement operations to the individual handlers, to
unify the style across the handling of the various messages.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Avoid dereferencing NULL channel

The newly introduced signal command handler checks for non-existing
channel and print an error message, but then continues on to dereference
that same

rpmsg: glink: Avoid dereferencing NULL channel

The newly introduced signal command handler checks for non-existing
channel and print an error message, but then continues on to dereference
that same channel.

Instead abort the handler when no channel is found.

Fixes: a2b73aa512a4 ("rpmsg: glink: Add support to handle signals command")
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mathieu Poirier <[email protected]>

show more ...

rpmsg: glink: Add check for kstrdup

Add check for the return value of kstrdup() and return the error
if it fails in order to avoid NULL pointer dereference.

Fixes: b4f8e52b89f6 ("rpmsg: Introduce Q

rpmsg: glink: Add check for kstrdup

Add check for the return value of kstrdup() and return the error
if it fails in order to avoid NULL pointer dereference.

Fixes: b4f8e52b89f6 ("rpmsg: Introduce Qualcomm RPM glink driver")
Signed-off-by: Jiasheng Jiang <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Add support to handle signals command

Remote peripherals send signal notifications over glink with commandID 15.

Add support to send and receive the signal command and based signals
e

rpmsg: glink: Add support to handle signals command

Remote peripherals send signal notifications over glink with commandID 15.

Add support to send and receive the signal command and based signals
enable or disable flow control with remote host.

Signed-off-by: Chris Lew <[email protected]>
Signed-off-by: Deepak Kumar Singh <[email protected]>
Signed-off-by: Sarannya S <[email protected]>
Acked-by: Arnaud Pouliquen <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

rpmsg: glink: Consolidate TX_DATA and TX_DATA_CONT

Rather than duplicating most of the code for constructing the initial
TX_DATA and subsequent TX_DATA_CONT packets, roll them into a single
loop.

S

rpmsg: glink: Consolidate TX_DATA and TX_DATA_CONT

Rather than duplicating most of the code for constructing the initial
TX_DATA and subsequent TX_DATA_CONT packets, roll them into a single
loop.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Propagate TX failures in intentless mode as well

As support for splitting transmission over several messages using
TX_DATA_CONT was introduced it does not immediately return the return

rpmsg: glink: Propagate TX failures in intentless mode as well

As support for splitting transmission over several messages using
TX_DATA_CONT was introduced it does not immediately return the return
value of qcom_glink_tx().

The result is that in the intentless case (i.e. intent == NULL), the
code will continue to send all additional chunks. This is wasteful, and
it's possible that the send operation could incorrectly indicate
success, if the last chunk fits in the TX fifo.

Fix the condition.

Fixes: 8956927faed3 ("rpmsg: glink: Add TX_DATA_CONT command while sending")
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Wait for intent, not just request ack

In some implementations of the remote firmware, an intent request
acknowledgment is sent when it's determined if the intent allocation
will be ful

rpmsg: glink: Wait for intent, not just request ack

In some implementations of the remote firmware, an intent request
acknowledgment is sent when it's determined if the intent allocation
will be fulfilled, but then the intent is queued after the
acknowledgment.

The result is that upon receiving a granted allocation request, the
search for the newly allocated intent will fail and an additional
request will be made. This will at best waste memory, but if the second
request is rejected the transaction will be incorrectly rejected.

Take the incoming intent into account in the wait to mitigate this
problem.

The above scenario can still happen, in the case that, on that same
channel, an unrelated intent is delivered prior to the request
acknowledgment and a separate process enters the send path and picks up
the intent. Given that there's no relationship between the
acknowledgment and the delivered (or to be delivered intent), there
doesn't seem to be a solution to this problem.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
[bjorn: Fixed spelling issues pointed out by checkpatch in commit message]
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Transition intent request signaling to wait queue

Transition the intent request acknowledgement to use a wait queue so
that it's possible, in the next commit, to extend the wait to als

rpmsg: glink: Transition intent request signaling to wait queue

Transition the intent request acknowledgement to use a wait queue so
that it's possible, in the next commit, to extend the wait to also wait
for an incoming intent.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Release driver_override

Upon termination of the rpmsg_device, driver_override needs to be freed
to avoid leaking the potentially assigned string.

Fixes: 42cd402b8fd4 ("rpmsg: Fix kfre

rpmsg: glink: Release driver_override

Upon termination of the rpmsg_device, driver_override needs to be freed
to avoid leaking the potentially assigned string.

Fixes: 42cd402b8fd4 ("rpmsg: Fix kfree() of static memory on setting driver_override")
Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device")
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Avoid infinite loop on intent for missing channel

In the event that an intent advertisement arrives on an unknown channel
the fifo is not advanced, resulting in the same message being

rpmsg: glink: Avoid infinite loop on intent for missing channel

In the event that an intent advertisement arrives on an unknown channel
the fifo is not advanced, resulting in the same message being handled
over and over.

Fixes: dacbb35e930f ("rpmsg: glink: Receive and store the remote intent buffers")
Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Fix GLINK command prefix

The upstream GLINK driver was first introduced to communicate with the
RPM on MSM8996, presumably as an artifact from that era the command
defines was prefixed

rpmsg: glink: Fix GLINK command prefix

The upstream GLINK driver was first introduced to communicate with the
RPM on MSM8996, presumably as an artifact from that era the command
defines was prefixed RPM_CMD, while they actually are GLINK_CMDs.

Let's rename these, to keep things tidy. No functional change.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Fix spelling of peek

The code is peeking into the buffers, not peaking. Fix this throughout
the glink drivers.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by:

rpmsg: glink: Fix spelling of peek

The code is peeking into the buffers, not peaking. Fix this throughout
the glink drivers.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Cancel pending intent requests at removal

During removal of the glink edge interrupts are disabled and no more
incoming messages are being serviced. In addition to the remote endpoint

rpmsg: glink: Cancel pending intent requests at removal

During removal of the glink edge interrupts are disabled and no more
incoming messages are being serviced. In addition to the remote endpoint
being defunct that means that any outstanding requests for intents will
not be serviced, and qcom_glink_request_intent() will blindly wait for
up to 10 seconds.

Mark the intent request as not granted and complete the intent request
completion to fail the waiting client immediately.

Once the current intent request is failed, any potential clients waiting
for the intent request mutex will not enter the same wait, as the
qcom_glink_tx() call will fail fast.

Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Fail qcom_glink_tx() once remove has been initiated

Upon removing the glink edge, communication is at best one-way. This
means that the very common scenario of glink requesting intents

rpmsg: glink: Fail qcom_glink_tx() once remove has been initiated

Upon removing the glink edge, communication is at best one-way. This
means that the very common scenario of glink requesting intents will not
be possible to serve.

Typically a successful transmission results in the client waiting for a
response, with some timeout and a mechanism for aborting that timeout.

Because of this, once the glink edge is defunct once removal is
commenced it's better to fail transmissions fast.

Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Move irq and mbox handling to transports

Not all GLINK transports uses an interrupt and a mailbox instance. The
interrupt for RPM needs to be IRQF_NOSUSPEND, while it seems reasonable

rpmsg: glink: Move irq and mbox handling to transports

Not all GLINK transports uses an interrupt and a mailbox instance. The
interrupt for RPM needs to be IRQF_NOSUSPEND, while it seems reasonable
for the SMEM interrupt to use irq_set_wake. The glink struct device is
constructed in the SMEM and RPM drivers but torn down in the core
driver.

Move the interrupt and kick handling into the SMEM and RPM driver, to
improve this and facilitate further improvements.

Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: glink: Extract tx kick operation

Refactor out the tx kick operations to its own function, in preparation
for pushing the details to the individual transports.

Reviewed-by: Chris Lew <quic_cl

rpmsg: glink: Extract tx kick operation

Refactor out the tx kick operations to its own function, in preparation
for pushing the details to the individual transports.

Reviewed-by: Chris Lew <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: convert sysfs snprintf to sysfs_emit

Fix the following coccicheck warning:
drivers/rpmsg/qcom_glink_native.c:1677:8-16:
WARNING: use scnprintf or sprintf

Signed-off-by: Xuezhi Zhang <zhangxu

rpmsg: convert sysfs snprintf to sysfs_emit

Fix the following coccicheck warning:
drivers/rpmsg/qcom_glink_native.c:1677:8-16:
WARNING: use scnprintf or sprintf

Signed-off-by: Xuezhi Zhang <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: qcom: glink: remove unused name

The qcom_glink.name is read from DTS but never used further, never
referenced, so drop it. This also fixes kerneldoc warning:

drivers/rpmsg/qcom_glink_nati

rpmsg: qcom: glink: remove unused name

The qcom_glink.name is read from DTS but never used further, never
referenced, so drop it. This also fixes kerneldoc warning:

drivers/rpmsg/qcom_glink_native.c:125:
warning: Function parameter or member 'name' not described in 'qcom_glink'

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: qcom: glink: replace strncpy() with strscpy_pad()

The use of strncpy() is considered deprecated for NUL-terminated
strings[1]. Replace strncpy() with strscpy_pad(), to keep existing
pad-behav

rpmsg: qcom: glink: replace strncpy() with strscpy_pad()

The use of strncpy() is considered deprecated for NUL-terminated
strings[1]. Replace strncpy() with strscpy_pad(), to keep existing
pad-behavior of strncpy, similarly to commit 08de420a8014 ("rpmsg:
glink: Replace strncpy() with strscpy_pad()"). This fixes W=1 warning:

In function ‘qcom_glink_rx_close’,
inlined from ‘qcom_glink_work’ at ../drivers/rpmsg/qcom_glink_native.c:1638:4:
drivers/rpmsg/qcom_glink_native.c:1549:17: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
1549 | strncpy(chinfo.name, channel->name, sizeof(chinfo.name));

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

rpmsg: Update rpmsg_chrdev_register_device function

The rpmsg_chrdev driver has been replaced by the rpmsg_ctrl driver
for the /dev/rpmsg_ctrlX devices management. The reference for the
driver overr

rpmsg: Update rpmsg_chrdev_register_device function

The rpmsg_chrdev driver has been replaced by the rpmsg_ctrl driver
for the /dev/rpmsg_ctrlX devices management. The reference for the
driver override is now the rpmsg_ctrl.

Update the rpmsg_chrdev_register_device function to reflect the update,
and rename the function to use the rpmsg_ctrldev prefix.

The platform drivers are updated accordingly.

Signed-off-by: Arnaud Pouliquen <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...