PRE_KERNEL polling support for TISCI and Secure Proxy#105343
Conversation
natto1784
force-pushed
the
early-tisci
branch
2 times, most recently
from
f9fad6c to
4c0ae26
Compare
|
Opinions required on the new exposed API - I do not think this needs to be public if it is only used by the TISCI driver, please feel free to share your opinions. |
zephyrbot
added
platform: TI K3
zephyrbot
requested review from
LaurentiuM1234,
dnltz,
glneo,
gramsay0,
ithinuel,
vaishnavachath,
wearyzen and
yongxu-wang15
|
|
||
| LOG_MODULE_REGISTER(ti_k2g_sci); | ||
|
|
||
| BUILD_ASSERT(DT_NUM_INST_STATUS_OKAY(DT_DRV_COMPAT) == 1, "There can only be one DMSC instance"); |
There was a problem hiding this comment.
You still need to work on your commit messages, I can read what is done in the diff, I need to know "why" something is done in the commit message.
Why lock this to just one instance? I know right now we only appear to have one TI-SCI controller in the system because our messages to DM are proxied though TIFS. For future devices this might not be the case, we may directly send messages to DM or TIFS as separate TI-SCI controllers/instances.
There was a problem hiding this comment.
Apologies, I cherrypicked this from a draft set of patches and forgot to update the description.
My rationale to restrict this was to just know at compile time during the send stage whether we are using secure proxy as the transport layer or not.
However, I did overlook the usecase you mentioned just now. I will make the appropriate changes to get the same behavior with the current multi-instance driver.
There was a problem hiding this comment.
removed and added a different mechanism for polling
natto1784
force-pushed
the
early-tisci
branch
2 times, most recently
from
b703bfb to
b3e6526
Compare
| #include <stdio.h> | ||
| #define DT_DRV_COMPAT ti_k2g_sci | ||
| #define DT_DRV_COMPAT ti_k2g_sci | ||
| #define DT_SECPROXY_COMPAT ti_secure_proxy |
There was a problem hiding this comment.
Do we really need this new compatible? ti_k2g_sci (MessageManager) will have the same constraints here.
There was a problem hiding this comment.
I am using this to verify the transport layer for TISCI during PRE_KERNEL. If it is secure proxy, then we poll, otherwise we return failure. The easy way out of this would be to always assume that we will be using secure proxy as the mailbox for TISCI.
natto1784
force-pushed
the
early-tisci
branch
2 times, most recently
from
9c1e849 to
8293336
Compare
|
|
||
| rv = secproxy_mailbox_send(dev, tx_channel, tx_msg); | ||
| if (rv != 0) { | ||
| LOG_ERR("failed to send message on channel %u", tx_channel); |
There was a problem hiding this comment.
the irq will keep disabled status if send message fial, it can be restore in a common code if send or read message fail
There was a problem hiding this comment.
true, thanks for pointing that out!
There was a problem hiding this comment.
added a mechanism to restore the irq status
TISCI uses ti_secproxy as the mailbox for communicating with the DM for things as power management, clock management and more. We currently use a semaphore as a notification for the received reply/message from the DM after we send a message. However, since kernel services are not initialized during PRE_KERNEL stages, we cannot rely on this. To overcome this, add a polling API that first sends a message on a specified tx_channel and then poll for a reply on a specified rx_channel. This is similar to how SCMI waits for replies during PRE_KERNEL and is also similarly, a blocking call. This is a vendor-specific API and hence requires its own public header. Signed-off-by: Amneesh Singh <amneesh@ti.com>
TISCI uses ti_secproxy as the mailbox for communicating with the DM for things as power management, clock management and more. We currently use a semaphore as a notification for the received reply/message from the DM after we send a message. However, since kernel services are not initialized during PRE_KERNEL stages, we cannot rely on this. To solve this, use the new vendor-specific API that polls the specified RX channel for a reply, but only if TISCI uses TI secure proxy as the mailbox. Note that this blocks the thread. Signed-off-by: Amneesh Singh <amneesh@ti.com>
Add the clock controller node for TISCI and also add power-domains and clocks properties for supported general peripherals in the files. Signed-off-by: Amneesh Singh <amneesh@ti.com>
Enable Kconfig options for TISCI, clock control, power domains and power management for the R5FSS0_0 core. Signed-off-by: Amneesh Singh <amneesh@ti.com>
natto1784
force-pushed
the
early-tisci
branch
2 times, most recently
from
9c2fd01 to
972f3f5
Compare
|
| @@ -1653,6 +1687,9 @@ static int tisci_init(const struct device *dev) | |||
| .max_msg_size = MAILBOX_MBOX_SIZE, \ | |||
| .max_rx_timeout_ms = 10000, \ | |||
| .is_secure = DT_INST_PROP_OR(_n, ti_is_secure, false), \ | |||
| .send_pre_kernel = COND_CODE_1(TISCI_USES_SECPROXY(_n), \ | |||
| (secproxy_mailbox_send_then_poll_rx_dt), \ | |||
There was a problem hiding this comment.
Even with the check for which transport we are using, I do not like this strong coupling between layers. Assume adding a polling API to the mailbox framework would be the better option we can work on.
5 participants
Dependency: #100642
Supersedes: #101280
Changes
drivers: firmware:
Restricted the TISCI driver to a single instance within the device tree.
drivers: mbox:
Introduced a vendor-specific polling API for ti_secproxy to support blocking RX message retrieval during PRE_KERNEL.
drivers: firmware
Modified TISCI to use polling instead of semaphores during PRE_KERNEL stages to allow early mailbox communication.
dts
Added TISCI clock controller nodes and peripheral clock/power properties for TI AM64x variants.
am243x_evm
Enabled TISCI, clock, and power management Kconfig options for R5FSS0_0.
Differences from the old PR
TODO (maybe in a separate patchset)