Re: [PATCH v2 02/14] soc: ti: k3: add navss ringacc driver

From: Grygorii Strashko
Date: Mon Sep 09 2019 - 12:59:20 EST




On 09/09/2019 16:00, Peter Ujfalusi wrote:
Hi,

Grygorii, can you take a look?

On 09/09/2019 9.09, Tero Kristo wrote:
Hi,

Mostly some cosmetic comments below, other than that seems fine to me.

On 30/07/2019 12:34, Peter Ujfalusi wrote:
From: Grygorii Strashko <grygorii.strashko@xxxxxx>

The Ring Accelerator (RINGACC or RA) provides hardware acceleration to
enable straightforward passing of work between a producer and a consumer.
There is one RINGACC module per NAVSS on TI AM65x SoCs.

The RINGACC converts constant-address read and write accesses to
equivalent
read or write accesses to a circular data structure in memory. The
RINGACC
eliminates the need for each DMA controller which needs to access ring
elements from having to know the current state of the ring (base address,
current offset). The DMA controller performs a read or write access to a
specific address range (which maps to the source interface on the
RINGACC)
and the RINGACC replaces the address for the transaction with a new
address
which corresponds to the head or tail element of the ring (head for
reads,
tail for writes). Since the RINGACC maintains the state, multiple DMA
controllers or channels are allowed to coherently share the same rings as
applicable. The RINGACC is able to place data which is destined towards
software into cached memory directly.

Supported ring modes:
- Ring Mode
- Messaging Mode
- Credentials Mode
- Queue Manager Mode

TI-SCI integration:

Texas Instrument's System Control Interface (TI-SCI) Message Protocol now
has control over Ringacc module resources management (RM) and Rings
configuration.

The corresponding support of TI-SCI Ringacc module RM protocol
introduced as option through DT parameters:
- ti,sci: phandle on TI-SCI firmware controller DT node
- ti,sci-dev-id: TI-SCI device identifier as per TI-SCI firmware spec

if both parameters present - Ringacc driver will configure/free/reset
Rings
using TI-SCI Message Ringacc RM Protocol.

The Ringacc driver manages Rings allocation by itself now and requests
TI-SCI firmware to allocate and configure specific Rings only. It's done
this way because, Linux driver implements two stage Rings allocation and
configuration (allocate ring and configure ring) while I-SCI Message

I-SCI should be TI-SCI I believe.

Yes, it supposed to be.


Protocol supports only one combined operation (allocate+configure).

Grygorii Strashko <grygorii.strashko@xxxxxx>

Above seems to be missing SoB?

Oh, it is really missing.


Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
---
 drivers/soc/ti/Kconfig | 17 +
 drivers/soc/ti/Makefile | 1 +
 drivers/soc/ti/k3-ringacc.c | 1191 +++++++++++++++++++++++++++++
 include/linux/soc/ti/k3-ringacc.h | 262 +++++++
 4 files changed, 1471 insertions(+)
 create mode 100644 drivers/soc/ti/k3-ringacc.c
 create mode 100644 include/linux/soc/ti/k3-ringacc.h

diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index cf545f428d03..10c76faa503e 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -80,6 +80,23 @@ config TI_SCI_PM_DOMAINS
ÂÂÂÂÂÂÂ called ti_sci_pm_domains. Note this is needed early in boot
before
ÂÂÂÂÂÂÂ rootfs may be available.
 +config TI_K3_RINGACC
+ÂÂÂ tristate "K3 Ring accelerator Sub System"
+ÂÂÂ depends on ARCH_K3 || COMPILE_TEST
+ÂÂÂ depends on TI_SCI_INTA_IRQCHIP
+ÂÂÂ default y
+ÂÂÂ help
+ÂÂÂÂÂ Say y here to support the K3 Ring accelerator module.
+ÂÂÂÂÂ The Ring Accelerator (RINGACC or RA)Â provides hardware
acceleration
+ÂÂÂÂÂ to enable straightforward passing of work between a producer
+ÂÂÂÂÂ and a consumer. There is one RINGACC module per NAVSS on TI
AM65x SoCs
+ÂÂÂÂÂ If unsure, say N.
+
+config TI_K3_RINGACC_DEBUG
+ÂÂÂ tristate "K3 Ring accelerator Sub System tests and debug"
+ÂÂÂ depends on TI_K3_RINGACC
+ÂÂÂ default n
+
 endif # SOC_TI
  config TI_SCI_INTA_MSI_DOMAIN
diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index b3868d392d4f..cc4bc8b08bf5 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_AMX3_PM)ÂÂÂÂÂÂÂÂÂÂÂ += pm33xx.o
 obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
 obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o
 obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN) += ti_sci_inta_msi.o
+obj-$(CONFIG_TI_K3_RINGACC)ÂÂÂÂÂÂÂ += k3-ringacc.o
diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
new file mode 100644
index 000000000000..401dfc963319
--- /dev/null
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -0,0 +1,1191 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI K3 NAVSS Ring Accelerator subsystem driver
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/soc/ti/k3-ringacc.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/soc/ti/ti_sci_inta_msi.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
+
+static LIST_HEAD(k3_ringacc_list);
+static DEFINE_MUTEX(k3_ringacc_list_lock);
+
+#ifdef CONFIG_TI_K3_RINGACC_DEBUG
+#defineÂÂÂ k3_nav_dbg(dev, arg...) dev_err(dev, arg)

dev_err seems exaggeration for debug purposes, maybe just dev_info.

+staticÂÂÂ void dbg_writel(u32 v, void __iomem *reg)
+{
+ÂÂÂ pr_err("WRITEL(32): v(%08X)-->reg(%p)\n", v, reg);

Again, maybe just pr_info.

I think I'll just drop CONFIG_TI_K3_RINGACC_DEBUG altogether along with
dbg_writel/dbg_readl/k3_nav_dbg and use dev_dbg() when appropriate.

Sounds good.



+ÂÂÂ writel(v, reg);
+}
+
+staticÂÂÂ u32 dbg_readl(void __iomem *reg)
+{
+ÂÂÂ u32 v;
+
+ÂÂÂ v = readl(reg);
+ÂÂÂ pr_err("READL(32): v(%08X)<--reg(%p)\n", v, reg);
+ÂÂÂ return v;
+}
+#else
+#defineÂÂÂ k3_nav_dbg(dev, arg...) dev_dbg(dev, arg)
+#define dbg_writel(v, reg) writel(v, reg)

Do you need to use hard writel, writel_relaxed is not enough?

not sure if we really need the barriers, but __raw_writel() should be
fine here imho

xxx_relaxed relaxed versions should be used only when necessary and with
adding appropriate comments why they've been used and what benefits from using
them for each particular case.
So, i do not agree with this blind conversation.


+
+#define dbg_readl(reg) readl(reg)

Same as above but for read?

__raw_readl() could be fine in also.

No. __raw_xxx api should never be used by drivers.



...

+/**
+ * struct k3_ringacc - Rings accelerator descriptor
+ *
+ * @dev - pointer on RA device
+ * @proxy_gcfg - RA proxy global config registers
+ * @proxy_target_base - RA proxy datapath region
+ * @num_rings - number of ring in RA
+ * @rm_gp_range - general purpose rings range from tisci
+ * @dma_ring_reset_quirk - DMA reset w/a enable
+ * @num_proxies - number of RA proxies
+ * @rings - array of rings descriptors (struct @k3_ring)
+ * @list - list of RAs in the system
+ * @tisci - pointer ti-sci handle
+ * @tisci_ring_ops - ti-sci rings ops
+ * @tisci_dev_id - ti-sci device id
+ */

...

+
+#ifdef CONFIG_TI_K3_RINGACC_DEBUG
+void k3_ringacc_ring_dump(struct k3_ring *ring)
+{
+ÂÂÂ struct device *dev = ring->parent->dev;
+
+ÂÂÂ k3_nav_dbg(dev, "dump ring: %d\n", ring->ring_id);
+ÂÂÂ k3_nav_dbg(dev, "dump mem virt %p, dma %pad\n",
+ÂÂÂÂÂÂÂÂÂÂ ring->ring_mem_virt, &ring->ring_mem_dma);
+ÂÂÂ k3_nav_dbg(dev, "dump elmsize %d, size %d, mode %d, proxy_id %d\n",
+ÂÂÂÂÂÂÂÂÂÂ ring->elm_size, ring->size, ring->mode, ring->proxy_id);
+
+ÂÂÂ k3_nav_dbg(dev, "dump ring_rt_regs: db%08x\n",
+ÂÂÂÂÂÂÂÂÂÂ readl(&ring->rt->db));

Why not use readl_relaxed in this func?

__raw_readl() might be enough?

No Raw, but this seems only one place where relaxed version can be used.



+ÂÂÂ k3_nav_dbg(dev, "dump occ%08x\n",
+ÂÂÂÂÂÂÂÂÂÂ readl(&ring->rt->occ));
+ÂÂÂ k3_nav_dbg(dev, "dump indx%08x\n",
+ÂÂÂÂÂÂÂÂÂÂ readl(&ring->rt->indx));
+ÂÂÂ k3_nav_dbg(dev, "dump hwocc%08x\n",
+ÂÂÂÂÂÂÂÂÂÂ readl(&ring->rt->hwocc));
+ÂÂÂ k3_nav_dbg(dev, "dump hwindx%08x\n",
+ÂÂÂÂÂÂÂÂÂÂ readl(&ring->rt->hwindx));
+
+ÂÂÂ if (ring->ring_mem_virt)
+ÂÂÂÂÂÂÂ print_hex_dump(KERN_ERR, "dump ring_mem_virt ",
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ DUMP_PREFIX_NONE, 16, 1,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ring->ring_mem_virt, 16 * 8, false);
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_ring_dump);

Do you really need to export a debug function?

It might come helpful for clients to dump the ring status runtime, but
since we don't have users, I'll move it to static.

Yep. It was exported for debug purposes. But hence there are no active users - cna be removed.


+#endif
+
+struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int id, u32 flags)
+{
+ÂÂÂ int proxy_id = K3_RINGACC_PROXY_NOT_USED;
+
+ÂÂÂ mutex_lock(&ringacc->req_lock);
+
+ÂÂÂ if (id == K3_RINGACC_RING_ID_ANY) {
+ÂÂÂÂÂÂÂ /* Request for any general purpose ring */
+ÂÂÂÂÂÂÂ struct ti_sci_resource_desc *gp_rings =
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &ringacc->rm_gp_range->desc[0];
+ÂÂÂÂÂÂÂ unsigned long size;
+
+ÂÂÂÂÂÂÂ size = gp_rings->start + gp_rings->num;
+ÂÂÂÂÂÂÂ id = find_next_zero_bit(ringacc->rings_inuse, size,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ gp_rings->start);
+ÂÂÂÂÂÂÂ if (id == size)
+ÂÂÂÂÂÂÂÂÂÂÂ goto error;
+ÂÂÂ } else if (id < 0) {
+ÂÂÂÂÂÂÂ goto error;
+ÂÂÂ }
+
+ÂÂÂ if (test_bit(id, ringacc->rings_inuse) &&
+ÂÂÂÂÂÂÂ !(ringacc->rings[id].flags & K3_RING_FLAG_SHARED))
+ÂÂÂÂÂÂÂ goto error;
+ÂÂÂ else if (ringacc->rings[id].flags & K3_RING_FLAG_SHARED)
+ÂÂÂÂÂÂÂ goto out;
+
+ÂÂÂ if (flags & K3_RINGACC_RING_USE_PROXY) {
+ÂÂÂÂÂÂÂ proxy_id = find_next_zero_bit(ringacc->proxy_inuse,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ringacc->num_proxies, 0);
+ÂÂÂÂÂÂÂ if (proxy_id == ringacc->num_proxies)
+ÂÂÂÂÂÂÂÂÂÂÂ goto error;
+ÂÂÂ }
+
+ÂÂÂ if (!try_module_get(ringacc->dev->driver->owner))
+ÂÂÂÂÂÂÂ goto error;
+
+ÂÂÂ if (proxy_id != K3_RINGACC_PROXY_NOT_USED) {
+ÂÂÂÂÂÂÂ set_bit(proxy_id, ringacc->proxy_inuse);
+ÂÂÂÂÂÂÂ ringacc->rings[id].proxy_id = proxy_id;
+ÂÂÂÂÂÂÂ k3_nav_dbg(ringacc->dev, "Giving ring#%d proxy#%d\n",
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ id, proxy_id);
+ÂÂÂ } else {
+ÂÂÂÂÂÂÂ k3_nav_dbg(ringacc->dev, "Giving ring#%d\n", id);
+ÂÂÂ }
+
+ÂÂÂ set_bit(id, ringacc->rings_inuse);
+out:
+ÂÂÂ ringacc->rings[id].use_count++;
+ÂÂÂ mutex_unlock(&ringacc->req_lock);
+ÂÂÂ return &ringacc->rings[id];
+
+error:
+ÂÂÂ mutex_unlock(&ringacc->req_lock);
+ÂÂÂ return NULL;
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
+
+static void k3_ringacc_ring_reset_sci(struct k3_ring *ring)
+{
+ÂÂÂ struct k3_ringacc *ringacc = ring->parent;
+ÂÂÂ int ret;
+
+ÂÂÂ ret = ringacc->tisci_ring_ops->config(
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci,
+ÂÂÂÂÂÂÂÂÂÂÂ TI_SCI_MSG_VALUE_RM_RING_COUNT_VALID,
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci_dev_id,
+ÂÂÂÂÂÂÂÂÂÂÂ ring->ring_id,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ ring->size,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0);
+ÂÂÂ if (ret)
+ÂÂÂÂÂÂÂ dev_err(ringacc->dev, "TISCI reset ring fail (%d) ring_idx
%d\n",
+ÂÂÂÂÂÂÂÂÂÂÂ ret, ring->ring_id);

Return value of sci ops is masked, why not return it and let the caller
handle it properly?

Same comment for anything similar that follows.

Hrm, there is not much a caller can do other than PANIC in case the ring
configuration fails.
I can probagate the error, but not sure what action can be taken, if any.

+}
+
+void k3_ringacc_ring_reset(struct k3_ring *ring)
+{
+ÂÂÂ if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
+ÂÂÂÂÂÂÂ return;
+
+ÂÂÂ ring->occ = 0;
+ÂÂÂ ring->free = 0;
+ÂÂÂ ring->rindex = 0;
+ÂÂÂ ring->windex = 0;
+
+ÂÂÂ k3_ringacc_ring_reset_sci(ring);
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_ring_reset);
+
+static void k3_ringacc_ring_reconfig_qmode_sci(struct k3_ring *ring,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ enum k3_ring_mode mode)
+{
+ÂÂÂ struct k3_ringacc *ringacc = ring->parent;
+ÂÂÂ int ret;
+
+ÂÂÂ ret = ringacc->tisci_ring_ops->config(
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci,
+ÂÂÂÂÂÂÂÂÂÂÂ TI_SCI_MSG_VALUE_RM_RING_MODE_VALID,
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci_dev_id,
+ÂÂÂÂÂÂÂÂÂÂÂ ring->ring_id,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ mode,
+ÂÂÂÂÂÂÂÂÂÂÂ 0,
+ÂÂÂÂÂÂÂÂÂÂÂ 0);
+ÂÂÂ if (ret)
+ÂÂÂÂÂÂÂ dev_err(ringacc->dev, "TISCI reconf qmode fail (%d) ring_idx
%d\n",
+ÂÂÂÂÂÂÂÂÂÂÂ ret, ring->ring_id);
+}
+
+void k3_ringacc_ring_reset_dma(struct k3_ring *ring, u32 occ)
+{
+ÂÂÂ if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
+ÂÂÂÂÂÂÂ return;
+
+ÂÂÂ if (!ring->parent->dma_ring_reset_quirk)
+ÂÂÂÂÂÂÂ return;
+
+ÂÂÂ if (!occ)
+ÂÂÂÂÂÂÂ occ = dbg_readl(&ring->rt->occ);
+
+ÂÂÂ if (occ) {
+ÂÂÂÂÂÂÂ u32 db_ring_cnt, db_ring_cnt_cur;
+
+ÂÂÂÂÂÂÂ k3_nav_dbg(ring->parent->dev, "%s %u occ: %u\n", __func__,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ring->ring_id, occ);
+ÂÂÂÂÂÂÂ /* 2. Reset the ring */

2? Where is 1?

Oh, I'll fix the numbering.

1. is 'Get ring occupancy count"
I think you can just drop numbering



+ÂÂÂÂÂÂÂ k3_ringacc_ring_reset_sci(ring);
+
+ÂÂÂÂÂÂÂ /*
+ÂÂÂÂÂÂÂÂ * 3. Setup the ring in ring/doorbell mode
+ÂÂÂÂÂÂÂÂ * (if not already in this mode)
+ÂÂÂÂÂÂÂÂ */
+ÂÂÂÂÂÂÂ if (ring->mode != K3_RINGACC_RING_MODE_RING)
+ÂÂÂÂÂÂÂÂÂÂÂ k3_ringacc_ring_reconfig_qmode_sci(
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ring, K3_RINGACC_RING_MODE_RING);
+ÂÂÂÂÂÂÂ /*
+ÂÂÂÂÂÂÂÂ * 4. Ring the doorbell 2**22 â ringOcc times.
+ÂÂÂÂÂÂÂÂ * This will wrap the internal UDMAP ring state occupancy
+ÂÂÂÂÂÂÂÂ * counter (which is 21-bits wide) to 0.
+ÂÂÂÂÂÂÂÂ */
+ÂÂÂÂÂÂÂ db_ring_cnt = (1U << 22) - occ;
+
+ÂÂÂÂÂÂÂ while (db_ring_cnt != 0) {
+ÂÂÂÂÂÂÂÂÂÂÂ /*
+ÂÂÂÂÂÂÂÂÂÂÂÂ * Ring the doorbell with the maximum count each
+ÂÂÂÂÂÂÂÂÂÂÂÂ * iteration if possible to minimize the total
+ÂÂÂÂÂÂÂÂÂÂÂÂ * of writes
+ÂÂÂÂÂÂÂÂÂÂÂÂ */
+ÂÂÂÂÂÂÂÂÂÂÂ if (db_ring_cnt > K3_RINGACC_MAX_DB_RING_CNT)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ db_ring_cnt_cur = K3_RINGACC_MAX_DB_RING_CNT;
+ÂÂÂÂÂÂÂÂÂÂÂ else
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ db_ring_cnt_cur = db_ring_cnt;
+
+ÂÂÂÂÂÂÂÂÂÂÂ writel(db_ring_cnt_cur, &ring->rt->db);
+ÂÂÂÂÂÂÂÂÂÂÂ db_ring_cnt -= db_ring_cnt_cur;
+ÂÂÂÂÂÂÂ }
+
+ÂÂÂÂÂÂÂ /* 5. Restore the original ring mode (if not ring mode) */
+ÂÂÂÂÂÂÂ if (ring->mode != K3_RINGACC_RING_MODE_RING)
+ÂÂÂÂÂÂÂÂÂÂÂ k3_ringacc_ring_reconfig_qmode_sci(ring, ring->mode);
+ÂÂÂ }
+
+ÂÂÂ /* 2. Reset the ring */


+
+u32 k3_ringacc_get_tisci_dev_id(struct k3_ring *ring)
+{
+ÂÂÂ if (!ring)
+ÂÂÂÂÂÂÂ return -EINVAL;
+

What if parent is NULL? Can it ever be here?

No, parent can not be NULL as the client would not have the ring in the
first place.


+ÂÂÂ return ring->parent->tisci_dev_id;
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_get_tisci_dev_id);
+
+int k3_ringacc_get_ring_irq_num(struct k3_ring *ring)
+{
+ÂÂÂ int irq_num;
+
+ÂÂÂ if (!ring)
+ÂÂÂÂÂÂÂ return -EINVAL;
+
+ÂÂÂ irq_num = ti_sci_inta_msi_get_virq(ring->parent->dev,
ring->ring_id);
+ÂÂÂ if (irq_num <= 0)
+ÂÂÂÂÂÂÂ irq_num = -EINVAL;
+ÂÂÂ return irq_num;
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_get_ring_irq_num);
+
+static int k3_ringacc_ring_cfg_sci(struct k3_ring *ring)
+{
+ÂÂÂ struct k3_ringacc *ringacc = ring->parent;
+ÂÂÂ u32 ring_idx;
+ÂÂÂ int ret;
+
+ÂÂÂ if (!ringacc->tisci)
+ÂÂÂÂÂÂÂ return -EINVAL;
+
+ÂÂÂ ring_idx = ring->ring_id;
+ÂÂÂ ret = ringacc->tisci_ring_ops->config(
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci,
+ÂÂÂÂÂÂÂÂÂÂÂ TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER,
+ÂÂÂÂÂÂÂÂÂÂÂ ringacc->tisci_dev_id,
+ÂÂÂÂÂÂÂÂÂÂÂ ring_idx,
+ÂÂÂÂÂÂÂÂÂÂÂ lower_32_bits(ring->ring_mem_dma),
+ÂÂÂÂÂÂÂÂÂÂÂ upper_32_bits(ring->ring_mem_dma),
+ÂÂÂÂÂÂÂÂÂÂÂ ring->size,
+ÂÂÂÂÂÂÂÂÂÂÂ ring->mode,
+ÂÂÂÂÂÂÂÂÂÂÂ ring->elm_size,
+ÂÂÂÂÂÂÂÂÂÂÂ 0);
+ÂÂÂ if (ret)
+ÂÂÂÂÂÂÂ dev_err(ringacc->dev, "TISCI config ring fail (%d) ring_idx
%d\n",
+ÂÂÂÂÂÂÂÂÂÂÂ ret, ring_idx);
+
+ÂÂÂ return ret;
+}
+
+int k3_ringacc_ring_cfg(struct k3_ring *ring, struct k3_ring_cfg *cfg)
+{
+ÂÂÂ struct k3_ringacc *ringacc = ring->parent;
+ÂÂÂ int ret = 0;
+
+ÂÂÂ if (!ring || !cfg)
+ÂÂÂÂÂÂÂ return -EINVAL;
+ÂÂÂ if (cfg->elm_size > K3_RINGACC_RING_ELSIZE_256 ||
+ÂÂÂÂÂÂÂ cfg->mode > K3_RINGACC_RING_MODE_QM ||
+ÂÂÂÂÂÂÂ cfg->size & ~K3_RINGACC_CFG_RING_SIZE_ELCNT_MASK ||
+ÂÂÂÂÂÂÂ !test_bit(ring->ring_id, ringacc->rings_inuse))
+ÂÂÂÂÂÂÂ return -EINVAL;
+
+ÂÂÂ if (ring->use_count != 1)

Hmm, isn't this a failure actually?

Yes, it is: -EBUSY

No. This is for shared rings.
0 - should never happens once ring is requested.
1 - only one user - configure ring
1 - shared ring which is configured already - just exit as ring configure already.


+ÂÂÂÂÂÂÂ return 0;
+
+ÂÂÂ ring->size = cfg->size;
+ÂÂÂ ring->elm_size = cfg->elm_size;
+ÂÂÂ ring->mode = cfg->mode;
+ÂÂÂ ring->occ = 0;
+ÂÂÂ ring->free = 0;
+ÂÂÂ ring->rindex = 0;
+ÂÂÂ ring->windex = 0;
+

[...]

--
Best regards,
grygorii