* [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 21:29 ` David Lechner
2018-11-26 7:52 ` [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API Roger Quadros
` (14 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The rproc_da_to_va() API is currently used to perform any device
to kernel address translations to meet the different needs of the
remoteproc core/platform drivers (eg: loading). The function also
invokes the da_to_va ops, if present, to allow the remoteproc
platform drivers to provide address translation. However, not all
platform implementations have linear address spaces, and may need
an additional parameter to be able to perform proper translations.
The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
been expanded to take in an additional flags field enabling some
remoteproc implementations (like the TI PRUSS remoteproc driver)
to use these flags. Also, define some semantics for this flags
argument as this can vary from one implementation to another. A
new flags type is encoded into the upper 16 bits along side the
actual value in the lower 16-bits for the flags argument, to
allow different individual implementations to have better
flexibility in interpreting the flags as per their needs.
Also, update the various remoteproc implementations that use the
.da_to_va() ops for the new signature.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/imx_rproc.c | 2 +-
drivers/remoteproc/keystone_remoteproc.c | 3 ++-
drivers/remoteproc/qcom_q6v5_mss.c | 5 +++--
drivers/remoteproc/qcom_q6v5_pas.c | 2 +-
drivers/remoteproc/qcom_q6v5_wcss.c | 2 +-
drivers/remoteproc/qcom_wcnss.c | 2 +-
drivers/remoteproc/remoteproc_core.c | 15 ++++++++++-----
drivers/remoteproc/remoteproc_elf_loader.c | 6 ++++--
drivers/remoteproc/remoteproc_internal.h | 2 +-
drivers/remoteproc/st_slim_rproc.c | 3 ++-
drivers/remoteproc/wkup_m3_rproc.c | 3 ++-
include/linux/remoteproc.h | 16 +++++++++++++++-
12 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 54c07fd..14d7f15 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -211,7 +211,7 @@ static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
return -ENOENT;
}
-static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct imx_rproc *priv = rproc->priv;
void *va = NULL;
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index aaac311..41c4d5c 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -254,7 +254,8 @@ static void keystone_rproc_kick(struct rproc *rproc, int vqid)
* can be used either by the remoteproc core for loading (when using kernel
* remoteproc loader), or by any rpmsg bus drivers.
*/
-static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len,
+ u32 flags)
{
struct keystone_rproc *ksproc = rproc->priv;
void __iomem *va = NULL;
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 01be731..3b77e12 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -971,7 +971,8 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
int ret = 0;
struct q6v5 *qproc = rproc->priv;
unsigned long mask = BIT((unsigned long)segment->priv);
- void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
+ void *ptr = rproc_da_to_va(rproc, segment->da, segment->size,
+ RPROC_FLAGS_NONE);
/* Unlock mba before copying segments */
if (!qproc->dump_mba_loaded)
@@ -1052,7 +1053,7 @@ static int q6v5_stop(struct rproc *rproc)
return 0;
}
-static void *q6v5_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *q6v5_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct q6v5 *qproc = rproc->priv;
int offset;
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index b1e63fc..46e1c6b 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -167,7 +167,7 @@ static int adsp_stop(struct rproc *rproc)
return ret;
}
-static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
int offset;
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index f93e1e4..5b4d4c6 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -406,7 +406,7 @@ static int q6v5_wcss_stop(struct rproc *rproc)
return 0;
}
-static void *q6v5_wcss_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *q6v5_wcss_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct q6v5_wcss *wcss = rproc->priv;
int offset;
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index b0e07e9..eb7d4b9 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -295,7 +295,7 @@ static int wcnss_stop(struct rproc *rproc)
return ret;
}
-static void *wcnss_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *wcnss_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv;
int offset;
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 54ec38f..39458a7 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -166,6 +166,7 @@ static phys_addr_t rproc_va_to_pa(void *cpu_addr)
* @rproc: handle of a remote processor
* @da: remoteproc device address to translate
* @len: length of the memory region @da is pointing to
+ * @flags: flags to pass onto platform implementations for aiding translations
*
* Some remote processors will ask us to allocate them physically contiguous
* memory regions (which we call "carveouts"), and map them to specific
@@ -181,7 +182,10 @@ static phys_addr_t rproc_va_to_pa(void *cpu_addr)
* carveouts and translate specific device addresses to kernel virtual addresses
* so we can access the referenced memory. This function also allows to perform
* translations on the internal remoteproc memory regions through a platform
- * implementation specific da_to_va ops, if present.
+ * implementation specific da_to_va ops, if present. The @flags field is passed
+ * onto these ops to aid the translation within the ops implementation. The
+ * @flags field is to be passed as a combination of the RPROC_FLAGS_xxx type
+ * and the pertinent flags value for that type.
*
* The function returns a valid kernel address on success or NULL on failure.
*
@@ -190,13 +194,13 @@ static phys_addr_t rproc_va_to_pa(void *cpu_addr)
* here the output of the DMA API for the carveouts, which should be more
* correct.
*/
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
{
struct rproc_mem_entry *carveout;
void *ptr = NULL;
if (rproc->ops->da_to_va) {
- ptr = rproc->ops->da_to_va(rproc, da, len);
+ ptr = rproc->ops->da_to_va(rproc, da, len, flags);
if (ptr)
goto out;
}
@@ -575,7 +579,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
}
/* what's the kernel address of this resource ? */
- ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
+ ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, RPROC_FLAGS_NONE);
if (!ptr) {
dev_err(dev, "erroneous trace resource entry\n");
return -EINVAL;
@@ -1549,7 +1553,8 @@ static void rproc_coredump(struct rproc *rproc)
if (segment->dump) {
segment->dump(rproc, segment, data + offset);
} else {
- ptr = rproc_da_to_va(rproc, segment->da, segment->size);
+ ptr = rproc_da_to_va(rproc, segment->da, segment->size,
+ RPROC_FLAGS_NONE);
if (!ptr) {
dev_err(&rproc->dev,
"invalid coredump segment (%pad, %zu)\n",
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index b17d72e..fff2e0b 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -182,7 +182,8 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
}
/* grab the kernel address for this device address */
- ptr = rproc_da_to_va(rproc, da, memsz);
+ ptr = rproc_da_to_va(rproc, da, memsz,
+ RPROC_FLAGS_ELF_PHDR | phdr->p_flags);
if (!ptr) {
dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
ret = -EINVAL;
@@ -333,6 +334,7 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
if (!shdr)
return NULL;
- return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
+ return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size,
+ RPROC_FLAGS_ELF_SHDR | shdr->sh_flags);
}
EXPORT_SYMBOL(rproc_elf_find_loaded_rsc_table);
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index f6cad24..c62b3e7 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -51,7 +51,7 @@ void rproc_exit_sysfs(void);
void rproc_free_vring(struct rproc_vring *rvring);
int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags);
int rproc_trigger_recovery(struct rproc *rproc);
int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw);
diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index d711d94..c41e58c 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -178,7 +178,8 @@ static int slim_rproc_stop(struct rproc *rproc)
return 0;
}
-static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len,
+ u32 flags)
{
struct st_slim_rproc *slim_rproc = rproc->priv;
void *va = NULL;
diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
index 1ada0e5..eff7497 100644
--- a/drivers/remoteproc/wkup_m3_rproc.c
+++ b/drivers/remoteproc/wkup_m3_rproc.c
@@ -88,7 +88,8 @@ static int wkup_m3_rproc_stop(struct rproc *rproc)
return 0;
}
-static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len,
+ u32 flags)
{
struct wkup_m3_rproc *wkupm3 = rproc->priv;
void *va = NULL;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 68e72f3..9e01a44 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -41,6 +41,7 @@
#include <linux/completion.h>
#include <linux/idr.h>
#include <linux/of.h>
+#include <linux/bitops.h>
/**
* struct resource_table - firmware resource table header
@@ -339,6 +340,19 @@ struct rproc_mem_entry {
struct firmware;
+/*
+ * Macros to use with flags field in rproc_da_to_va API. Use
+ * the upper 16 bits to dictate the flags type and the lower
+ * 16 bits to pass on the value of the flags pertinent to that
+ * type.
+ *
+ * Add any new flags type at a new bit-field position
+ */
+#define RPROC_FLAGS_SHIFT 16
+#define RPROC_FLAGS_NONE 0
+#define RPROC_FLAGS_ELF_PHDR BIT(0 + RPROC_FLAGS_SHIFT)
+#define RPROC_FLAGS_ELF_SHDR BIT(1 + RPROC_FLAGS_SHIFT)
+
/**
* struct rproc_ops - platform-specific device handlers
* @start: power on the device and boot it
@@ -356,7 +370,7 @@ struct rproc_ops {
int (*start)(struct rproc *rproc);
int (*stop)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
- void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
+ void * (*da_to_va)(struct rproc *rproc, u64 da, int len, u32 flags);
int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
struct resource_table *(*find_loaded_rsc_table)(
struct rproc *rproc, const struct firmware *fw);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-11-26 7:52 ` [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter Roger Quadros
@ 2018-11-26 21:29 ` David Lechner
2018-11-29 10:29 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-26 21:29 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/26/18 1:52 AM, Roger Quadros wrote:
> From: Suman Anna <s-anna@ti.com>
>
> The rproc_da_to_va() API is currently used to perform any device
> to kernel address translations to meet the different needs of the
> remoteproc core/platform drivers (eg: loading). The function also
> invokes the da_to_va ops, if present, to allow the remoteproc
> platform drivers to provide address translation. However, not all
> platform implementations have linear address spaces, and may need
> an additional parameter to be able to perform proper translations.
>
> The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
> been expanded to take in an additional flags field enabling some
> remoteproc implementations (like the TI PRUSS remoteproc driver)
> to use these flags. Also, define some semantics for this flags
> argument as this can vary from one implementation to another. A
> new flags type is encoded into the upper 16 bits along side the
> actual value in the lower 16-bits for the flags argument, to
> allow different individual implementations to have better
> flexibility in interpreting the flags as per their needs.
This seems like an overly complex solution for a rather simple
problem. Instead of passing all sorts of flags, could we just add
a parameter named "page" to da_to_va() that indicates the memory
page of the address in the remote processor?
Or perhaps there is some other use for all of these flags that I
am not aware of?
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-11-26 21:29 ` David Lechner
@ 2018-11-29 10:29 ` Roger Quadros
2018-11-29 16:12 ` David Lechner
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-29 10:29 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson, s-anna
Cc: tony, robh+dt, bcousson, ssantosh, nsekhar, t-kristo, nsaulnier,
jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
Bjorn, Suman,
On 26/11/18 23:29, David Lechner wrote:
> On 11/26/18 1:52 AM, Roger Quadros wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> The rproc_da_to_va() API is currently used to perform any device
>> to kernel address translations to meet the different needs of the
>> remoteproc core/platform drivers (eg: loading). The function also
>> invokes the da_to_va ops, if present, to allow the remoteproc
>> platform drivers to provide address translation. However, not all
>> platform implementations have linear address spaces, and may need
>> an additional parameter to be able to perform proper translations.
>>
>> The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
>> been expanded to take in an additional flags field enabling some
>> remoteproc implementations (like the TI PRUSS remoteproc driver)
>> to use these flags. Also, define some semantics for this flags
>> argument as this can vary from one implementation to another. A
>> new flags type is encoded into the upper 16 bits along side the
>> actual value in the lower 16-bits for the flags argument, to
>> allow different individual implementations to have better
>> flexibility in interpreting the flags as per their needs.
>
> This seems like an overly complex solution for a rather simple
> problem. Instead of passing all sorts of flags, could we just add
> a parameter named "page" to da_to_va() that indicates the memory
> page of the address in the remote processor?
>
> Or perhaps there is some other use for all of these flags that I
> am not aware of?
I'm not a big fan of this patch either.
rproc_da_to_va() is used at the following places
2 qcom_q6v5_mss.c qcom_q6v5_dump_segment 974 void *ptr = rproc_da_to_va(rproc, segment->da, segment->size,
3 remoteproc_core.c rproc_da_to_va 197 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
4 remoteproc_core.c rproc_handle_trace 582 ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, RPROC_FLAGS_NONE);
5 remoteproc_core.c rproc_coredump 1592 ptr = rproc_da_to_va(rproc, segment->da, segment->size,
6 remoteproc_elf_loader.c rproc_elf_load_segments 185 ptr = rproc_da_to_va(rproc, da, memsz,
7 remoteproc_elf_loader.c rproc_elf_find_loaded_rsc_table 337 return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size,
At rproc_elf_load_segments() we need to pass enough information so that
the rproc driver can load the segment into proper area (IRAM vs DRAM).
So providing page should suffice.
I want to understand more about rproc_elf_find_loaded_rsc_table() myself.
rproc_elf_find_loaded_rsc_table() is called only in rproc_start() in remoteproc_core.c
with the comment
/*
* The starting device has been given the rproc->cached_table as the
* resource table. The address of the vring along with the other
* allocated resources (carveouts etc) is stored in cached_table.
* In order to pass this information to the remote device we must copy
* this information to device memory. We also update the table_ptr so
* that any subsequent changes will be applied to the loaded version.
*/
loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
Why isn't cached_table sufficient?
Why do we need to call rproc_find_loaded_rsc_table()?
why do we need to load the resource table into remote processor memory at all.
As discussed earlier, some PRU systems have very little memory (512 bytes?)
and we want to avoid unnecessary loading.
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-11-29 10:29 ` Roger Quadros
@ 2018-11-29 16:12 ` David Lechner
2018-12-04 10:03 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-29 16:12 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson, s-anna
Cc: tony, robh+dt, bcousson, ssantosh, nsekhar, t-kristo, nsaulnier,
jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/29/18 4:29 AM, Roger Quadros wrote:
> Bjorn, Suman,
>
> On 26/11/18 23:29, David Lechner wrote:
>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>> From: Suman Anna <s-anna@ti.com>
>>>
>>> The rproc_da_to_va() API is currently used to perform any device
>>> to kernel address translations to meet the different needs of the
>>> remoteproc core/platform drivers (eg: loading). The function also
>>> invokes the da_to_va ops, if present, to allow the remoteproc
>>> platform drivers to provide address translation. However, not all
>>> platform implementations have linear address spaces, and may need
>>> an additional parameter to be able to perform proper translations.
>>>
>>> The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
>>> been expanded to take in an additional flags field enabling some
>>> remoteproc implementations (like the TI PRUSS remoteproc driver)
>>> to use these flags. Also, define some semantics for this flags
>>> argument as this can vary from one implementation to another. A
>>> new flags type is encoded into the upper 16 bits along side the
>>> actual value in the lower 16-bits for the flags argument, to
>>> allow different individual implementations to have better
>>> flexibility in interpreting the flags as per their needs.
>>
>> This seems like an overly complex solution for a rather simple
>> problem. Instead of passing all sorts of flags, could we just add
>> a parameter named "page" to da_to_va() that indicates the memory
>> page of the address in the remote processor?
>>
>> Or perhaps there is some other use for all of these flags that I
>> am not aware of?
>
> I'm not a big fan of this patch either.
>
> rproc_da_to_va() is used at the following places
>
> 2 qcom_q6v5_mss.c qcom_q6v5_dump_segment 974 void *ptr = rproc_da_to_va(rproc, segment->da, segment->size,
> 3 remoteproc_core.c rproc_da_to_va 197 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
> 4 remoteproc_core.c rproc_handle_trace 582 ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, RPROC_FLAGS_NONE);
> 5 remoteproc_core.c rproc_coredump 1592 ptr = rproc_da_to_va(rproc, segment->da, segment->size,
> 6 remoteproc_elf_loader.c rproc_elf_load_segments 185 ptr = rproc_da_to_va(rproc, da, memsz,
> 7 remoteproc_elf_loader.c rproc_elf_find_loaded_rsc_table 337 return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size,
>
> At rproc_elf_load_segments() we need to pass enough information so that
> the rproc driver can load the segment into proper area (IRAM vs DRAM).
> So providing page should suffice.
FYI, the PRU series I sent a while back has some patches to do
something like this so feel free to use them if they are helpful.
https://lore.kernel.org/lkml/20180623210810.21232-2-david@lechnology.com/
https://lore.kernel.org/lkml/20180623210810.21232-3-david@lechnology.com/
>
> I want to understand more about rproc_elf_find_loaded_rsc_table() myself.
> rproc_elf_find_loaded_rsc_table() is called only in rproc_start() in remoteproc_core.c
> with the comment
>
> /*
> * The starting device has been given the rproc->cached_table as the
> * resource table. The address of the vring along with the other
> * allocated resources (carveouts etc) is stored in cached_table.
> * In order to pass this information to the remote device we must copy
> * this information to device memory. We also update the table_ptr so
> * that any subsequent changes will be applied to the loaded version.
> */
> loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
>
> Why isn't cached_table sufficient?
> Why do we need to call rproc_find_loaded_rsc_table()?
>
> why do we need to load the resource table into remote processor memory at all.
> As discussed earlier, some PRU systems have very little memory (512 bytes?)
> and we want to avoid unnecessary loading.
>
> cheers,
> -roger
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-11-29 16:12 ` David Lechner
@ 2018-12-04 10:03 ` Roger Quadros
2019-02-14 3:35 ` Suman Anna
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-12-04 10:03 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson, s-anna
Cc: tony, robh+dt, bcousson, ssantosh, nsekhar, t-kristo, nsaulnier,
jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 29/11/18 18:12, David Lechner wrote:
> On 11/29/18 4:29 AM, Roger Quadros wrote:
>> Bjorn, Suman,
>>
>> On 26/11/18 23:29, David Lechner wrote:
>>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>>> From: Suman Anna <s-anna@ti.com>
>>>>
>>>> The rproc_da_to_va() API is currently used to perform any device
>>>> to kernel address translations to meet the different needs of the
>>>> remoteproc core/platform drivers (eg: loading). The function also
>>>> invokes the da_to_va ops, if present, to allow the remoteproc
>>>> platform drivers to provide address translation. However, not all
>>>> platform implementations have linear address spaces, and may need
>>>> an additional parameter to be able to perform proper translations.
>>>>
>>>> The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
>>>> been expanded to take in an additional flags field enabling some
>>>> remoteproc implementations (like the TI PRUSS remoteproc driver)
>>>> to use these flags. Also, define some semantics for this flags
>>>> argument as this can vary from one implementation to another. A
>>>> new flags type is encoded into the upper 16 bits along side the
>>>> actual value in the lower 16-bits for the flags argument, to
>>>> allow different individual implementations to have better
>>>> flexibility in interpreting the flags as per their needs.
>>>
>>> This seems like an overly complex solution for a rather simple
>>> problem. Instead of passing all sorts of flags, could we just add
>>> a parameter named "page" to da_to_va() that indicates the memory
>>> page of the address in the remote processor?
>>>
>>> Or perhaps there is some other use for all of these flags that I
>>> am not aware of?
>>
>> I'm not a big fan of this patch either.
>>
>> rproc_da_to_va() is used at the following places
>>
>> 2 qcom_q6v5_mss.c qcom_q6v5_dump_segment 974 void *ptr = rproc_da_to_va(rproc, segment->da, segment->size,
>> 3 remoteproc_core.c rproc_da_to_va 197 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
>> 4 remoteproc_core.c rproc_handle_trace 582 ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, RPROC_FLAGS_NONE);
>> 5 remoteproc_core.c rproc_coredump 1592 ptr = rproc_da_to_va(rproc, segment->da, segment->size,
>> 6 remoteproc_elf_loader.c rproc_elf_load_segments 185 ptr = rproc_da_to_va(rproc, da, memsz,
>> 7 remoteproc_elf_loader.c rproc_elf_find_loaded_rsc_table 337 return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size,
>>
>> At rproc_elf_load_segments() we need to pass enough information so that
>> the rproc driver can load the segment into proper area (IRAM vs DRAM).
>> So providing page should suffice.
>
> FYI, the PRU series I sent a while back has some patches to do
> something like this so feel free to use them if they are helpful.
>
> https://lore.kernel.org/lkml/20180623210810.21232-2-david@lechnology.com/
> https://lore.kernel.org/lkml/20180623210810.21232-3-david@lechnology.com/
>
Thanks. I think we need to do something like that. Too bad you had to reverse engineer
the TI specific headers. I'll check if we have this available somewhere internally.
>>
>> I want to understand more about rproc_elf_find_loaded_rsc_table() myself.
>> rproc_elf_find_loaded_rsc_table() is called only in rproc_start() in remoteproc_core.c
>> with the comment
>>
>> /*
>> * The starting device has been given the rproc->cached_table as the
>> * resource table. The address of the vring along with the other
>> * allocated resources (carveouts etc) is stored in cached_table.
>> * In order to pass this information to the remote device we must copy
>> * this information to device memory. We also update the table_ptr so
>> * that any subsequent changes will be applied to the loaded version.
>> */
>> loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
>>
>> Why isn't cached_table sufficient?
>> Why do we need to call rproc_find_loaded_rsc_table()?
>>
>> why do we need to load the resource table into remote processor memory at all.
>> As discussed earlier, some PRU systems have very little memory (512 bytes?)
>> and we want to avoid unnecessary loading.
>>
This question still holds.
Suman?
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter
2018-12-04 10:03 ` Roger Quadros
@ 2019-02-14 3:35 ` Suman Anna
0 siblings, 0 replies; 45+ messages in thread
From: Suman Anna @ 2019-02-14 3:35 UTC (permalink / raw)
To: Roger Quadros, David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, nsekhar, t-kristo, nsaulnier,
jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
Hi Roger,
On 12/4/18 4:03 AM, Roger Quadros wrote:
> On 29/11/18 18:12, David Lechner wrote:
>> On 11/29/18 4:29 AM, Roger Quadros wrote:
>>> Bjorn, Suman,
>>>
>>> On 26/11/18 23:29, David Lechner wrote:
>>>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>>>> From: Suman Anna <s-anna@ti.com>
>>>>>
>>>>> The rproc_da_to_va() API is currently used to perform any device
>>>>> to kernel address translations to meet the different needs of the
>>>>> remoteproc core/platform drivers (eg: loading). The function also
>>>>> invokes the da_to_va ops, if present, to allow the remoteproc
>>>>> platform drivers to provide address translation. However, not all
>>>>> platform implementations have linear address spaces, and may need
>>>>> an additional parameter to be able to perform proper translations.
>>>>>
>>>>> The rproc_da_to_va() API and the rproc .da_to_va ops have therefore
>>>>> been expanded to take in an additional flags field enabling some
>>>>> remoteproc implementations (like the TI PRUSS remoteproc driver)
>>>>> to use these flags. Also, define some semantics for this flags
>>>>> argument as this can vary from one implementation to another. A
>>>>> new flags type is encoded into the upper 16 bits along side the
>>>>> actual value in the lower 16-bits for the flags argument, to
>>>>> allow different individual implementations to have better
>>>>> flexibility in interpreting the flags as per their needs.
>>>>
>>>> This seems like an overly complex solution for a rather simple
>>>> problem. Instead of passing all sorts of flags, could we just add
>>>> a parameter named "page" to da_to_va() that indicates the memory
>>>> page of the address in the remote processor?
>>>>
>>>> Or perhaps there is some other use for all of these flags that I
>>>> am not aware of?
>>>
>>> I'm not a big fan of this patch either.
>>>
>>> rproc_da_to_va() is used at the following places
>>>
>>> 2 qcom_q6v5_mss.c qcom_q6v5_dump_segment 974 void *ptr = rproc_da_to_va(rproc, segment->da, segment->size,
>>> 3 remoteproc_core.c rproc_da_to_va 197 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
>>> 4 remoteproc_core.c rproc_handle_trace 582 ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, RPROC_FLAGS_NONE);
>>> 5 remoteproc_core.c rproc_coredump 1592 ptr = rproc_da_to_va(rproc, segment->da, segment->size,
>>> 6 remoteproc_elf_loader.c rproc_elf_load_segments 185 ptr = rproc_da_to_va(rproc, da, memsz,
>>> 7 remoteproc_elf_loader.c rproc_elf_find_loaded_rsc_table 337 return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size,
>>>
>>> At rproc_elf_load_segments() we need to pass enough information so that
>>> the rproc driver can load the segment into proper area (IRAM vs DRAM).
>>> So providing page should suffice.
>>
>> FYI, the PRU series I sent a while back has some patches to do
>> something like this so feel free to use them if they are helpful.
>>
>> https://lore.kernel.org/lkml/20180623210810.21232-2-david@lechnology.com/
>> https://lore.kernel.org/lkml/20180623210810.21232-3-david@lechnology.com/
>>
>
> Thanks. I think we need to do something like that. Too bad you had to reverse engineer
> the TI specific headers. I'll check if we have this available somewhere internally.
I commented on this in your v2 series, but let's see if we can
incorporate some of this custom logic within the PRU remoteproc driver
else. The remoteproc core does allow you to use your own implementations
for some firmware related ops.
>
>>>
>>> I want to understand more about rproc_elf_find_loaded_rsc_table() myself.
>>> rproc_elf_find_loaded_rsc_table() is called only in rproc_start() in remoteproc_core.c
>>> with the comment
>>>
>>> /*
>>> * The starting device has been given the rproc->cached_table as the
>>> * resource table. The address of the vring along with the other
>>> * allocated resources (carveouts etc) is stored in cached_table.
>>> * In order to pass this information to the remote device we must copy
>>> * this information to device memory. We also update the table_ptr so
>>> * that any subsequent changes will be applied to the loaded version.
>>> */
>>> loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
>>>
>>> Why isn't cached_table sufficient?
>>> Why do we need to call rproc_find_loaded_rsc_table()?
>>>
>>> why do we need to load the resource table into remote processor memory at all.
>>> As discussed earlier, some PRU systems have very little memory (512 bytes?)
>>> and we want to avoid unnecessary loading.
>>>
>
> This question still holds.
> Suman?
The resource table is used for publishing back various allocated
resource values to the remote processor, so this needs to be loaded into
memory. For example, with RSC_CARVEOUTs, we fill in the address Linux
kernel allocated. With VDEVs, the virtio config status is shared and
used for synchronization between the Linux host and the remote processor.
regards
Suman
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
2018-11-26 7:52 ` [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 21:41 ` David Lechner
2018-11-26 7:52 ` [PATCH 03/16] remoteproc: Add support to handle device specific resource types Roger Quadros
` (13 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
A new API, rproc_set_firmware() is added to allow the remoteproc platform
drivers and remoteproc client drivers to be able to configure a custom
firmware name that is different from the default name used during
remoteproc registration. This function is being introduced to provide
a kernel-level equivalent of the current sysfs interface to remoteproc
client drivers. This allows some remoteproc drivers to choose different
firmwares at runtime when the remote processor is not running based on
the functional feature it is providing using that remote processor.
The TI PRU Ethernet driver will be an example of such usage as it
requires to use different firmwares for different supported protocols.
Also, update the firmware_store() function used by the sysfs interface
to reuse this function to avoid code duplication.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 61 +++++++++++++++++++++++++++++++++++
drivers/remoteproc/remoteproc_sysfs.c | 33 ++-----------------
include/linux/remoteproc.h | 1 +
3 files changed, 64 insertions(+), 31 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 39458a7..581e6e8 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2151,6 +2151,67 @@ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type)
}
EXPORT_SYMBOL(rproc_report_crash);
+/**
+ * rproc_set_firmware() - assign a new firmware
+ * @rproc: rproc handle to which the new firmware is being assigned
+ * @fw_name: new firmware name to be assigned
+ *
+ * This function allows remoteproc drivers or clients to configure a custom
+ * firmware name that is different from the default name used during remoteproc
+ * registration. The function does not trigger a remote processor boot,
+ * only sets the firmware name used for a subsequent boot. This function
+ * should also be called only when the remote processor is offline.
+ *
+ * This allows either the userspace to configure a different name through
+ * sysfs or a kernel-level remoteproc or a remoteproc client driver to set
+ * a specific firmware when it is controlling the boot and shutdown of the
+ * remote processor.
+ *
+ * Returns 0 on success or a negative value upon failure
+ */
+int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
+{
+ struct device *dev = rproc->dev.parent;
+ int ret, len;
+ char *p;
+
+ if (!rproc || !fw_name)
+ return -EINVAL;
+
+ ret = mutex_lock_interruptible(&rproc->lock);
+ if (ret) {
+ dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
+ return -EINVAL;
+ }
+
+ if (rproc->state != RPROC_OFFLINE) {
+ dev_err(dev, "can't change firmware while running\n");
+ ret = -EBUSY;
+ goto out;
+ }
+
+ len = strcspn(fw_name, "\n");
+ if (!len) {
+ dev_err(dev, "can't provide a NULL firmware\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ p = kstrndup(fw_name, len, GFP_KERNEL);
+ if (!p) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ kfree(rproc->firmware);
+ rproc->firmware = p;
+
+out:
+ mutex_unlock(&rproc->lock);
+ return ret;
+}
+EXPORT_SYMBOL(rproc_set_firmware);
+
static int __init remoteproc_init(void)
{
rproc_init_sysfs();
diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 3a4c3d7..6cf04a7 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -32,38 +32,9 @@ static ssize_t firmware_store(struct device *dev,
const char *buf, size_t count)
{
struct rproc *rproc = to_rproc(dev);
- char *p;
- int err, len = count;
+ int err;
- err = mutex_lock_interruptible(&rproc->lock);
- if (err) {
- dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
- return -EINVAL;
- }
-
- if (rproc->state != RPROC_OFFLINE) {
- dev_err(dev, "can't change firmware while running\n");
- err = -EBUSY;
- goto out;
- }
-
- len = strcspn(buf, "\n");
- if (!len) {
- dev_err(dev, "can't provide a NULL firmware\n");
- err = -EINVAL;
- goto out;
- }
-
- p = kstrndup(buf, len, GFP_KERNEL);
- if (!p) {
- err = -ENOMEM;
- goto out;
- }
-
- kfree(rproc->firmware);
- rproc->firmware = p;
-out:
- mutex_unlock(&rproc->lock);
+ err = rproc_set_firmware(rproc, buf);
return err ? err : count;
}
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9e01a44..063468b 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -604,6 +604,7 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, int len,
int rproc_boot(struct rproc *rproc);
void rproc_shutdown(struct rproc *rproc);
+int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
int rproc_coredump_add_custom_segment(struct rproc *rproc,
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API
2018-11-26 7:52 ` [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API Roger Quadros
@ 2018-11-26 21:41 ` David Lechner
2018-11-29 8:51 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-26 21:41 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/26/18 1:52 AM, Roger Quadros wrote:
> From: Suman Anna <s-anna@ti.com>
>
> A new API, rproc_set_firmware() is added to allow the remoteproc platform
> drivers and remoteproc client drivers to be able to configure a custom
> firmware name that is different from the default name used during
> remoteproc registration. This function is being introduced to provide
> a kernel-level equivalent of the current sysfs interface to remoteproc
> client drivers. This allows some remoteproc drivers to choose different
> firmwares at runtime when the remote processor is not running based on
> the functional feature it is providing using that remote processor.
> The TI PRU Ethernet driver will be an example of such usage as it
> requires to use different firmwares for different supported protocols.
>
> Also, update the firmware_store() function used by the sysfs interface
> to reuse this function to avoid code duplication.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> drivers/remoteproc/remoteproc_core.c | 61 +++++++++++++++++++++++++++++++++++
> drivers/remoteproc/remoteproc_sysfs.c | 33 ++-----------------
> include/linux/remoteproc.h | 1 +
> 3 files changed, 64 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 39458a7..581e6e8 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2151,6 +2151,67 @@ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type)
...
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
> +{
> + struct device *dev = rproc->dev.parent;
> + int ret, len;
> + char *p;
> +
> + if (!rproc || !fw_name)
> + return -EINVAL;
> +
> + ret = mutex_lock_interruptible(&rproc->lock);
> + if (ret) {
> + dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
> + return -EINVAL;
> + }
> +
> + if (rproc->state != RPROC_OFFLINE) {
> + dev_err(dev, "can't change firmware while running\n");
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + len = strcspn(fw_name, "\n");
> + if (!len) {
> + dev_err(dev, "can't provide a NULL firmware\n");
I realize this was just copied, but technically, this would be an
empty string rather than NULL.
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API
2018-11-26 21:41 ` David Lechner
@ 2018-11-29 8:51 ` Roger Quadros
0 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-29 8:51 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 26/11/18 23:41, David Lechner wrote:
> On 11/26/18 1:52 AM, Roger Quadros wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> A new API, rproc_set_firmware() is added to allow the remoteproc platform
>> drivers and remoteproc client drivers to be able to configure a custom
>> firmware name that is different from the default name used during
>> remoteproc registration. This function is being introduced to provide
>> a kernel-level equivalent of the current sysfs interface to remoteproc
>> client drivers. This allows some remoteproc drivers to choose different
>> firmwares at runtime when the remote processor is not running based on
>> the functional feature it is providing using that remote processor.
>> The TI PRU Ethernet driver will be an example of such usage as it
>> requires to use different firmwares for different supported protocols.
>>
>> Also, update the firmware_store() function used by the sysfs interface
>> to reuse this function to avoid code duplication.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> drivers/remoteproc/remoteproc_core.c | 61 +++++++++++++++++++++++++++++++++++
>> drivers/remoteproc/remoteproc_sysfs.c | 33 ++-----------------
>> include/linux/remoteproc.h | 1 +
>> 3 files changed, 64 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index 39458a7..581e6e8 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -2151,6 +2151,67 @@ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type)
>
> ...
>
>> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + int ret, len;
>> + char *p;
>> +
>> + if (!rproc || !fw_name)
>> + return -EINVAL;
>> +
>> + ret = mutex_lock_interruptible(&rproc->lock);
>> + if (ret) {
>> + dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
>> + return -EINVAL;
>> + }
>> +
>> + if (rproc->state != RPROC_OFFLINE) {
>> + dev_err(dev, "can't change firmware while running\n");
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> +
>> + len = strcspn(fw_name, "\n");
>> + if (!len) {
>> + dev_err(dev, "can't provide a NULL firmware\n");
>
> I realize this was just copied, but technically, this would be an
> empty string rather than NULL.
>
Noted. Thanks.
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 03/16] remoteproc: Add support to handle device specific resource types
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
2018-11-26 7:52 ` [PATCH 01/16] remoteproc: Extend rproc_da_to_va() API with a flags parameter Roger Quadros
2018-11-26 7:52 ` [PATCH 02/16] remoteproc: Add a rproc_set_firmware() API Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver Roger Quadros
` (12 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The remoteproc framework handles a fixed set of resource table entries
today. To make it scalable across multiple platforms, it makes sense
for the framework to provide a way for the device specific implementation
to define and handle vendor specific resource types. These resource types
would be very specific to an implementation instance that it does not
make sense for the framework to handle it. There can be two types of
such resources depending on whether they need to be handled prior to
the loading of the firmware segments or after. A post-loading resource
type is typically needed because it references a loaded segment pointer
for conveying the resource information.
For instance, a remoteproc implementation might want timers information
embedded in the resource table so that the driver could parse the binary
and enable accordingly. Another example would be hwspinlocks that it
is using, to properly share system wide resources. A PRU post-loading
vendor resource handler for interrupts requires the PRU event - interrupt
channel information to be loaded into device memory.
This patch adds a function pointer to the list of rproc_ops for the
driver implementation to handle such custom vendor resources, and
reuses the same handler between pre-loading and post-loading resource
types, with the sub-types in the implementation handling the processing
of those resource types.
Signed-off-by: Suman Anna <s-anna@ti.com>
merge: remoteproc: simplify VENDOR specific resource
We don't need PRE and POST loading as we'll fix the PRU resource
table to be self sufficient.
Earlier it was half baked with channel mapping located elsewhere
in data section thus requiring data section to be loaded before
we can figure out the full INTC map.
Fix vendor resource to be TLV and definition agnostic.
Show in debugfs.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/remoteproc/remoteproc_core.c | 36 +++++++++++++++++++++++++++++++++
drivers/remoteproc/remoteproc_debugfs.c | 3 +++
include/linux/remoteproc.h | 15 +++++++++++++-
3 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 581e6e8..9e387ab 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -898,6 +898,41 @@ void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem)
EXPORT_SYMBOL(rproc_add_carveout);
/**
+ * rproc_handle_vendor_rsc() - provide implementation specific hook
+ * to handle vendor/custom resources
+ * @rproc: the remote processor
+ * @rsc: vendor resource to be handled by remoteproc drivers
+ * @offset: offset of the resource data in resource table
+ * @avail: size of available data
+ *
+ * Remoteproc implementations might want to add resource table entries
+ * that are not generic enough to be handled by the framework. This
+ * provides a hook to handle such custom resources.
+ *
+ * Returns 0 on success, or an appropriate error code otherwise
+ */
+static int rproc_handle_vendor_rsc(struct rproc *rproc,
+ struct fw_rsc_vendor *rsc,
+ int offset, int avail)
+{
+ struct device *dev = &rproc->dev;
+
+ if (sizeof(*rsc) > avail) {
+ dev_err(dev, "vendor rsc is truncated\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "vendor rsc:");
+
+ if (!rproc->ops->handle_vendor_rsc) {
+ dev_err(dev, "no vendor rsc handler! ignoring resource\n");
+ return 0;
+ }
+
+ return rproc->ops->handle_vendor_rsc(rproc, (void *)rsc);
+}
+
+/**
* rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct
* @dev: pointer on device struct
* @va: virtual address
@@ -985,6 +1020,7 @@ static rproc_handle_resource_t rproc_loading_handlers[RSC_LAST] = {
[RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout,
[RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem,
[RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace,
+ [RSC_VENDOR] = (rproc_handle_resource_t)rproc_handle_vendor_rsc,
[RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev,
};
diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
index f330a9a..971c8ba 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -254,6 +254,9 @@ static int rproc_rsc_table_show(struct seq_file *seq, void *p)
v->vring[j].pa);
}
break;
+ case RSC_VENDOR:
+ seq_printf(seq, "Entry %d is of type %s [Vendor specific]\n",
+ i, types[hdr->type]);
default:
seq_printf(seq, "Unknown resource type found: %d [hdr: %pK]\n",
hdr->type, hdr);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 063468b..4881415 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -101,6 +101,7 @@ struct fw_rsc_hdr {
* the remote processor will be writing logs.
* @RSC_VDEV: declare support for a virtio device, and serve as its
* virtio header.
+ * @RSC_VENDOR: vendor specific resource type.
* @RSC_LAST: just keep this one at the end
*
* For more details regarding a specific resource type, please see its
@@ -116,7 +117,8 @@ enum fw_resource_type {
RSC_DEVMEM = 1,
RSC_TRACE = 2,
RSC_VDEV = 3,
- RSC_LAST = 4,
+ RSC_VENDOR = 4,
+ RSC_LAST,
};
#define FW_RSC_ADDR_ANY (-1)
@@ -309,6 +311,14 @@ struct fw_rsc_vdev {
struct rproc;
/**
+ * struct fw_rsc_vendor - vendor specific resource definition
+ * @data: resource data. vendor defined.
+ */
+struct fw_rsc_vendor {
+ u8 data[0];
+} __packed;
+
+/**
* struct rproc_mem_entry - memory entry descriptor
* @va: virtual address
* @dma: dma address
@@ -361,6 +371,7 @@ struct firmware;
* @da_to_va: optional platform hook to perform address translations
* @parse_fw: parse firmware to extract information (e.g. resource table)
* @find_loaded_rsc_table: find the loaded resouce table
+ * @handle_vendor_rsc: hook to handle device specific resource table entries
* @load: load firmware to memory, where the remote processor
* expects to find it
* @sanity_check: sanity check the fw image
@@ -374,6 +385,8 @@ struct rproc_ops {
int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
struct resource_table *(*find_loaded_rsc_table)(
struct rproc *rproc, const struct firmware *fw);
+ int (*handle_vendor_rsc)(struct rproc *rproc,
+ struct fw_rsc_vendor *rsc);
int (*load)(struct rproc *rproc, const struct firmware *fw);
int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (2 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 03/16] remoteproc: Add support to handle device specific resource types Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 22:32 ` David Lechner
2018-11-30 21:39 ` Dimitar Dimitrov
2018-11-26 7:52 ` [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support Roger Quadros
` (11 subsequent siblings)
15 siblings, 2 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The Programmable Real-Time Unit Subsystem (PRUSS) consists of
dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs)
for program execution. This patch adds a remoteproc platform
driver for managing the individual PRU RISC cores life cycle.
The PRU remoteproc driver uses the standard remoteproc core ELF
loader. However, the PRUs do not have a unified address space,
(has an Instruction RAM and a primary Data RAM at both 0x0) and
leverage an added .da_to_va ops to use the standard ELF loader.
This remoteproc driver does not have support for error recovery
and system suspend/resume features. Different compatibles are
used to allow providing scalability for instance-specific device
data if needed. The driver uses a default firmware-name retrieved
from device-tree, and the firmwares are expected to be present
in the standard Linux firmware search paths. They can also be
adjusted by userspace if required through the sysfs interface
provided by the remoteproc core.
The PRU remoteproc driver uses a client-driven boot methodology
- it does _not_ support auto-boot so that the PRU load and boot
is dictated by the corresponding client drivers for achieving
various usecases. This allows flexibility for the client drivers
or applications to set a firmware name (if needed) based on their
desired functionality and boot the PRU. The sysfs bind and unbind
attributes have also been suppressed so that the PRU devices cannot
be unbound and thereby shutdown a PRU from underneath a PRU client
driver.
The driver currently supports the AM335x SoC, and support for other
TI SoCs will be added in subsequent patches.
[rogerq@ti.com] Use request/release_mem_region()
[rogerq@ti.com] Strip INTC handling
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/remoteproc/Kconfig | 14 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/pru_rproc.c | 392 +++++++++++++++++++++++++++++++++++++++++
drivers/remoteproc/pru_rproc.h | 65 +++++++
4 files changed, 472 insertions(+)
create mode 100644 drivers/remoteproc/pru_rproc.c
create mode 100644 drivers/remoteproc/pru_rproc.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index f0abd26..333666e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -197,6 +197,20 @@ config ST_REMOTEPROC
config ST_SLIM_REMOTEPROC
tristate
+config PRUSS_REMOTEPROC
+ tristate "TI PRUSS remoteproc support"
+ depends on TI_PRUSS
+ default n
+ help
+ Support for TI PRU-ICSS remote processors via the remote processor
+ framework.
+
+ Currently supported on AM33xx SoCs.
+
+ Say Y or M here to support the Programmable Realtime Unit (PRU)
+ processors on various TI SoCs. It's safe to say N here if you're
+ not interested in the PRU or if you are unsure.
+
endif # REMOTEPROC
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ce5d061..88a86cc 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -26,3 +26,4 @@ qcom_wcnss_pil-y += qcom_wcnss.o
qcom_wcnss_pil-y += qcom_wcnss_iris.o
obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
+obj-$(CONFIG_PRUSS_REMOTEPROC) += pru_rproc.o
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
new file mode 100644
index 0000000..c35f432
--- /dev/null
+++ b/drivers/remoteproc/pru_rproc.c
@@ -0,0 +1,392 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PRU-ICSS remoteproc driver for various TI SoCs
+ *
+ * Copyright (C) 2014-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Suman Anna <s-anna@ti.com>
+ * Andrew F. Davis <afd@ti.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/remoteproc.h>
+#include <linux/pruss.h>
+
+#include "remoteproc_internal.h"
+#include "pru_rproc.h"
+
+/* PRU_ICSS_PRU_CTRL registers */
+#define PRU_CTRL_CTRL 0x0000
+#define PRU_CTRL_STS 0x0004
+#define PRU_CTRL_WAKEUP_EN 0x0008
+#define PRU_CTRL_CYCLE 0x000C
+#define PRU_CTRL_STALL 0x0010
+#define PRU_CTRL_CTBIR0 0x0020
+#define PRU_CTRL_CTBIR1 0x0024
+#define PRU_CTRL_CTPPR0 0x0028
+#define PRU_CTRL_CTPPR1 0x002C
+
+/* CTRL register bit-fields */
+#define CTRL_CTRL_SOFT_RST_N BIT(0)
+#define CTRL_CTRL_EN BIT(1)
+#define CTRL_CTRL_SLEEPING BIT(2)
+#define CTRL_CTRL_CTR_EN BIT(3)
+#define CTRL_CTRL_SINGLE_STEP BIT(8)
+#define CTRL_CTRL_RUNSTATE BIT(15)
+
+/**
+ * enum pru_mem - PRU core memory range identifiers
+ */
+enum pru_mem {
+ PRU_MEM_IRAM = 0,
+ PRU_MEM_CTRL,
+ PRU_MEM_DEBUG,
+ PRU_MEM_MAX,
+};
+
+/**
+ * struct pru_rproc - PRU remoteproc structure
+ * @id: id of the PRU core within the PRUSS
+ * @pruss: back-reference to parent PRUSS structure
+ * @rproc: remoteproc pointer for this PRU core
+ * @mem_regions: data for each of the PRU memory regions
+ * @dram0: PRUSS DRAM0 region
+ * @dram1: PRUSS DRAM1 region
+ * @shrdram: PRUSS SHARED RAM region
+ * @iram_da: device address of Instruction RAM for this PRU
+ * @pdram_da: device address of primary Data RAM for this PRU
+ * @sdram_da: device address of secondary Data RAM for this PRU
+ * @shrdram_da: device address of shared Data RAM
+ * @fw_name: name of firmware image used during loading
+ */
+struct pru_rproc {
+ int id;
+ struct pruss *pruss;
+ struct rproc *rproc;
+ struct pruss_mem_region mem_regions[PRU_MEM_MAX];
+ struct pruss_mem_region dram0;
+ struct pruss_mem_region dram1;
+ struct pruss_mem_region shrdram;
+ u32 iram_da;
+ u32 pdram_da;
+ u32 sdram_da;
+ u32 shrdram_da;
+ const char *fw_name;
+};
+
+static void *pru_d_da_to_va(struct pru_rproc *pru, u32 da, int len);
+
+static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int reg)
+{
+ return readl_relaxed(pru->mem_regions[PRU_MEM_CTRL].va + reg);
+}
+
+static inline
+void pru_control_write_reg(struct pru_rproc *pru, unsigned int reg, u32 val)
+{
+ writel_relaxed(val, pru->mem_regions[PRU_MEM_CTRL].va + reg);
+}
+
+static inline u32 pru_debug_read_reg(struct pru_rproc *pru, unsigned int reg)
+{
+ return readl_relaxed(pru->mem_regions[PRU_MEM_DEBUG].va + reg);
+}
+
+static inline
+void pru_debug_write_reg(struct pru_rproc *pru, unsigned int reg, u32 val)
+{
+ writel_relaxed(val, pru->mem_regions[PRU_MEM_DEBUG].va + reg);
+}
+
+/* start a PRU core */
+static int pru_rproc_start(struct rproc *rproc)
+{
+ struct device *dev = &rproc->dev;
+ struct pru_rproc *pru = rproc->priv;
+ u32 val;
+
+ dev_dbg(dev, "starting PRU%d: entry-point = 0x%x\n",
+ pru->id, (rproc->bootaddr >> 2));
+
+ val = CTRL_CTRL_EN | ((rproc->bootaddr >> 2) << 16);
+ pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
+
+ /* TODO: INTC setup */
+
+ return 0;
+}
+
+/* stop/disable a PRU core */
+static int pru_rproc_stop(struct rproc *rproc)
+{
+ struct device *dev = &rproc->dev;
+ struct pru_rproc *pru = rproc->priv;
+ u32 val;
+
+ dev_dbg(dev, "stopping PRU%d\n", pru->id);
+
+ val = pru_control_read_reg(pru, PRU_CTRL_CTRL);
+ val &= ~CTRL_CTRL_EN;
+ pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
+
+ /* TODO: INTC cleanup */
+
+ return 0;
+}
+
+/*
+ * Convert PRU device address (data spaces only) to kernel virtual address
+ *
+ * Each PRU has access to all data memories within the PRUSS, accessible at
+ * different ranges. So, look through both its primary and secondary Data
+ * RAMs as well as any shared Data RAM to convert a PRU device address to
+ * kernel virtual address. Data RAM0 is primary Data RAM for PRU0 and Data
+ * RAM1 is primary Data RAM for PRU1.
+ */
+static void *pru_d_da_to_va(struct pru_rproc *pru, u32 da, int len)
+{
+ struct pruss_mem_region dram0, dram1, shrd_ram;
+ u32 offset;
+ void *va = NULL;
+
+ if (len <= 0)
+ return NULL;
+
+ dram0 = pru->dram0;
+ dram1 = pru->dram1;
+ /* PRU1 has its local RAM addresses reversed */
+ if (pru->id == 1)
+ swap(dram0, dram1);
+ shrd_ram = pru->shrdram;
+
+ if (da >= pru->pdram_da && da + len <= pru->pdram_da + dram0.size) {
+ offset = da - pru->pdram_da;
+ va = (__force void *)(dram0.va + offset);
+ } else if (da >= pru->sdram_da &&
+ da + len <= pru->sdram_da + dram1.size) {
+ offset = da - pru->sdram_da;
+ va = (__force void *)(dram1.va + offset);
+ } else if (da >= pru->shrdram_da &&
+ da + len <= pru->shrdram_da + shrd_ram.size) {
+ offset = da - pru->shrdram_da;
+ va = (__force void *)(shrd_ram.va + offset);
+ }
+
+ return va;
+}
+
+/*
+ * Convert PRU device address (instruction space) to kernel virtual address
+ *
+ * A PRU does not have an unified address space. Each PRU has its very own
+ * private Instruction RAM, and its device address is identical to that of
+ * its primary Data RAM device address.
+ */
+static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
+{
+ u32 offset;
+ void *va = NULL;
+
+ if (len <= 0)
+ return NULL;
+
+ if (da >= pru->iram_da &&
+ da + len <= pru->iram_da + pru->mem_regions[PRU_MEM_IRAM].size) {
+ offset = da - pru->iram_da;
+ va = (__force void *)(pru->mem_regions[PRU_MEM_IRAM].va +
+ offset);
+ }
+
+ return va;
+}
+
+/* PRU-specific address translator */
+static void *pru_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
+{
+ struct pru_rproc *pru = rproc->priv;
+ void *va;
+ u32 exec_flag;
+
+ exec_flag = ((flags & RPROC_FLAGS_ELF_SHDR) ? flags & SHF_EXECINSTR :
+ ((flags & RPROC_FLAGS_ELF_PHDR) ? flags & PF_X : 0));
+
+ if (exec_flag)
+ va = pru_i_da_to_va(pru, da, len);
+ else
+ va = pru_d_da_to_va(pru, da, len);
+
+ return va;
+}
+
+static struct rproc_ops pru_rproc_ops = {
+ .start = pru_rproc_start,
+ .stop = pru_rproc_stop,
+ .da_to_va = pru_da_to_va,
+};
+
+static int pru_rproc_set_id(struct pru_rproc *pru)
+{
+ int ret = 0;
+ u32 mask1 = 0x34000;
+ u32 mask2 = 0x38000;
+
+ if ((pru->mem_regions[0].pa & mask1) == mask1)
+ pru->id = 0;
+ else if ((pru->mem_regions[0].pa & mask2) == mask2)
+ pru->id = 1;
+ else
+ ret = -EINVAL;
+
+ return ret;
+}
+
+static int pru_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct platform_device *ppdev = to_platform_device(dev->parent);
+ struct pru_rproc *pru;
+ const char *fw_name;
+ struct rproc *rproc = NULL;
+ struct resource *res;
+ int i, ret;
+ const char *mem_names[PRU_MEM_MAX] = { "iram", "control", "debug" };
+
+ if (!np) {
+ dev_err(dev, "Non-DT platform device not supported\n");
+ return -ENODEV;
+ }
+
+ ret = of_property_read_string(np, "firmware-name", &fw_name);
+ if (ret) {
+ dev_err(dev, "unable to retrieve firmware-name %d\n", ret);
+ return ret;
+ }
+
+ rproc = rproc_alloc(dev, pdev->name, &pru_rproc_ops, fw_name,
+ sizeof(*pru));
+ if (!rproc) {
+ dev_err(dev, "rproc_alloc failed\n");
+ return -ENOMEM;
+ }
+ /* error recovery is not supported for PRUs */
+ rproc->recovery_disabled = true;
+
+ /*
+ * rproc_add will auto-boot the processor normally, but this is
+ * not desired with PRU client driven boot-flow methodology. A PRU
+ * application/client driver will boot the corresponding PRU
+ * remote-processor as part of its state machine either through
+ * the remoteproc sysfs interface or through the equivalent kernel API
+ */
+ rproc->auto_boot = false;
+
+ pru = rproc->priv;
+ pru->pruss = platform_get_drvdata(ppdev);
+ pru->rproc = rproc;
+ pru->fw_name = fw_name;
+
+ ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM0,
+ &pru->dram0);
+ if (ret) {
+ dev_err(dev, "couldn't get PRUSS DRAM0: %d\n", ret);
+ return ret;
+ }
+ pruss_release_mem_region(pru->pruss, &pru->dram0);
+
+ ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM1,
+ &pru->dram1);
+ if (ret) {
+ dev_err(dev, "couldn't get PRUSS DRAM1: %d\n", ret);
+ return ret;
+ }
+ pruss_release_mem_region(pru->pruss, &pru->dram1);
+
+ ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_SHRD_RAM2,
+ &pru->shrdram);
+ if (ret) {
+ dev_err(dev, "couldn't get PRUSS Shared RAM: %d\n", ret);
+ return ret;
+ }
+ pruss_release_mem_region(pru->pruss, &pru->shrdram);
+
+ /* XXX: get this from match data if different in the future */
+ pru->iram_da = 0;
+ pru->pdram_da = 0;
+ pru->sdram_da = 0x2000;
+ pru->shrdram_da = 0x10000;
+
+ for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ mem_names[i]);
+ pru->mem_regions[i].va = devm_ioremap_resource(dev, res);
+ if (IS_ERR(pru->mem_regions[i].va)) {
+ dev_err(dev, "failed to parse and map memory resource %d %s\n",
+ i, mem_names[i]);
+ ret = PTR_ERR(pru->mem_regions[i].va);
+ goto free_rproc;
+ }
+ pru->mem_regions[i].pa = res->start;
+ pru->mem_regions[i].size = resource_size(res);
+
+ dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %p\n",
+ mem_names[i], &pru->mem_regions[i].pa,
+ pru->mem_regions[i].size, pru->mem_regions[i].va);
+ }
+
+ ret = pru_rproc_set_id(pru);
+ if (ret < 0)
+ goto free_rproc;
+
+ platform_set_drvdata(pdev, rproc);
+
+ ret = rproc_add(pru->rproc);
+ if (ret) {
+ dev_err(dev, "rproc_add failed: %d\n", ret);
+ goto free_rproc;
+ }
+
+ dev_info(dev, "PRU rproc node %s probed successfully\n", np->full_name);
+
+ return 0;
+
+free_rproc:
+ rproc_free(rproc);
+ return ret;
+}
+
+static int pru_rproc_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rproc *rproc = platform_get_drvdata(pdev);
+
+ dev_info(dev, "%s: removing rproc %s\n", __func__, rproc->name);
+
+ rproc_del(rproc);
+ rproc_free(rproc);
+
+ return 0;
+}
+
+static const struct of_device_id pru_rproc_match[] = {
+ { .compatible = "ti,am3356-pru", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, pru_rproc_match);
+
+static struct platform_driver pru_rproc_driver = {
+ .driver = {
+ .name = "pru-rproc",
+ .of_match_table = pru_rproc_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = pru_rproc_probe,
+ .remove = pru_rproc_remove,
+};
+module_platform_driver(pru_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_DESCRIPTION("PRU-ICSS Remote Processor Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/pru_rproc.h b/drivers/remoteproc/pru_rproc.h
new file mode 100644
index 0000000..35240e9
--- /dev/null
+++ b/drivers/remoteproc/pru_rproc.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * PRUSS Remote Processor specific types
+ *
+ * Copyright (C) 2014-2018 Texas Instruments Incorporated - http://www.ti.com/
+ * All rights reserved.
+ */
+
+#ifndef _PRU_REMOTEPROC_H_
+#define _PRU_REMOTEPROC_H_
+
+/**
+ * enum pruss_rsc_types - PRU specific resource types
+ *
+ * @PRUSS_RSC_INTRS: Resource holding information on PRU PINTC configuration
+ * @PRUSS_RSC_MAX: Indicates end of known/defined PRU resource types.
+ * This should be the last definition.
+ *
+ * Introduce new custom resource types before PRUSS_RSC_MAX.
+ */
+enum pruss_rsc_types {
+ PRUSS_RSC_INTRS = 1,
+ PRUSS_RSC_MAX = 2,
+};
+
+/**
+ * struct pruss_event_chnl - PRU system events _to_ channel mapping
+ * @event: number of the system event
+ * @chnl: channel number assigned to a given @event
+ *
+ * PRU system events are mapped to channels, and these channels are mapped
+ * to host interrupts. Events can be mapped to channels in a one-to-one or
+ * many-to-one ratio (multiple events per channel), and channels can be
+ * mapped to host interrupts in a one-to-one or many-to-one ratio (multiple
+ * channels per interrupt).
+ *
+ */
+struct pruss_event_chnl {
+ s8 event;
+ s8 chnl;
+};
+
+/**
+ * struct fw_rsc_custom_intrmap - custom resource to define PRU interrupts
+ * @reserved: reserved field providing padding and alignment
+ * @chnl_host_intr_map: array of PRU channels to host interrupt mappings
+ * @event_chnl_map_size: number of event_channel mappings defined in
+ * @event_chnl_map_addr
+ * @event_chnl_map_addr: PRU device address of pointer to array of events to
+ * channel mappings (struct pruss_event_chnl elements)
+ *
+ * PRU system events are mapped to channels, and these channels are mapped
+ * to host interrupts. Events can be mapped to channels in a one-to-one or
+ * many-to-one ratio (multiple events per channel), and channels can be
+ * mapped to host interrupts in a one-to-one or many-to-one ratio (multiple
+ * channels per interrupt).
+ */
+struct fw_rsc_custom_intrmap {
+ u16 reserved;
+ s8 chnl_host_intr_map[10];
+ u32 event_chnl_map_size;
+ u32 event_chnl_map_addr;
+};
+
+#endif /* _PRU_REMOTEPROC_H_ */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-26 7:52 ` [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver Roger Quadros
@ 2018-11-26 22:32 ` David Lechner
2018-11-29 9:26 ` Roger Quadros
2018-11-30 21:39 ` Dimitar Dimitrov
1 sibling, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-26 22:32 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/26/18 1:52 AM, Roger Quadros wrote:
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index ce5d061..88a86cc 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -26,3 +26,4 @@ qcom_wcnss_pil-y += qcom_wcnss.o
> qcom_wcnss_pil-y += qcom_wcnss_iris.o
> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> +obj-$(CONFIG_PRUSS_REMOTEPROC) += pru_rproc.o
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> new file mode 100644
> index 0000000..c35f432
> --- /dev/null
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -0,0 +1,392 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PRU-ICSS remoteproc driver for various TI SoCs
> + *
> + * Copyright (C) 2014-2018 Texas Instruments Incorporated - http://www.ti.com/
> + * Suman Anna <s-anna@ti.com>
> + * Andrew F. Davis <afd@ti.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/remoteproc.h>
alphabetical order?
> +#include <linux/pruss.h>
> +
> +#include "remoteproc_internal.h"
alphabetical order?
> +#include "pru_rproc.h"
> +
> +/* PRU_ICSS_PRU_CTRL registers */
> +#define PRU_CTRL_CTRL 0x0000
> +#define PRU_CTRL_STS 0x0004
> +#define PRU_CTRL_WAKEUP_EN 0x0008
> +#define PRU_CTRL_CYCLE 0x000C
> +#define PRU_CTRL_STALL 0x0010
> +#define PRU_CTRL_CTBIR0 0x0020
> +#define PRU_CTRL_CTBIR1 0x0024
> +#define PRU_CTRL_CTPPR0 0x0028
> +#define PRU_CTRL_CTPPR1 0x002C
> +
> +/* CTRL register bit-fields */
> +#define CTRL_CTRL_SOFT_RST_N BIT(0)
> +#define CTRL_CTRL_EN BIT(1)
> +#define CTRL_CTRL_SLEEPING BIT(2)
> +#define CTRL_CTRL_CTR_EN BIT(3)
> +#define CTRL_CTRL_SINGLE_STEP BIT(8)
> +#define CTRL_CTRL_RUNSTATE BIT(15)
> +
> +/**
> + * enum pru_mem - PRU core memory range identifiers
> + */
> +enum pru_mem {
> + PRU_MEM_IRAM = 0,
> + PRU_MEM_CTRL,
> + PRU_MEM_DEBUG,
> + PRU_MEM_MAX,
> +};
I am finding the name "mem" here to be confusing. I keep thinking
these are just RAM regions instead of memory mapped I/O. Maybe call
it "iomem" instead of "mem"?
...
> +static int pru_rproc_set_id(struct pru_rproc *pru)
> +{
> + int ret = 0;
> + u32 mask1 = 0x34000;
> + u32 mask2 = 0x38000;
These values are non-obvious and could use some comments. Also,
they could be made into constants or macros.
> +
> + if ((pru->mem_regions[0].pa & mask1) == mask1)
how about this instead:
if ((pru->mem_regions[PRU_MEM_IRAM].pa & 0xfffff) == mask1)
The 0xfffff mask will be important on AM18xx where INTC is at 0x34000,
PRU0 IRAM is at 0x38000 and PRU1 IRAM is at 0x3C000.
> + pru->id = 0;
> + else if ((pru->mem_regions[0].pa & mask2) == mask2)
> + pru->id = 1;
> + else
> + ret = -EINVAL;
> +
> + return ret;
> +}
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-26 22:32 ` David Lechner
@ 2018-11-29 9:26 ` Roger Quadros
0 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-29 9:26 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 27/11/18 00:32, David Lechner wrote:
> On 11/26/18 1:52 AM, Roger Quadros wrote:
>
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index ce5d061..88a86cc 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -26,3 +26,4 @@ qcom_wcnss_pil-y += qcom_wcnss.o
>> qcom_wcnss_pil-y += qcom_wcnss_iris.o
>> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
>> obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
>> +obj-$(CONFIG_PRUSS_REMOTEPROC) += pru_rproc.o
>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>> new file mode 100644
>> index 0000000..c35f432
>> --- /dev/null
>> +++ b/drivers/remoteproc/pru_rproc.c
>> @@ -0,0 +1,392 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PRU-ICSS remoteproc driver for various TI SoCs
>> + *
>> + * Copyright (C) 2014-2018 Texas Instruments Incorporated - http://www.ti.com/
>> + * Suman Anna <s-anna@ti.com>
>> + * Andrew F. Davis <afd@ti.com>
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/remoteproc.h>
>
> alphabetical order?
>
>> +#include <linux/pruss.h>
>> +
>> +#include "remoteproc_internal.h"
>
> alphabetical order?
ok for both.
>
>> +#include "pru_rproc.h"
>> +
>> +/* PRU_ICSS_PRU_CTRL registers */
>> +#define PRU_CTRL_CTRL 0x0000
>> +#define PRU_CTRL_STS 0x0004
>> +#define PRU_CTRL_WAKEUP_EN 0x0008
>> +#define PRU_CTRL_CYCLE 0x000C
>> +#define PRU_CTRL_STALL 0x0010
>> +#define PRU_CTRL_CTBIR0 0x0020
>> +#define PRU_CTRL_CTBIR1 0x0024
>> +#define PRU_CTRL_CTPPR0 0x0028
>> +#define PRU_CTRL_CTPPR1 0x002C
>> +
>> +/* CTRL register bit-fields */
>> +#define CTRL_CTRL_SOFT_RST_N BIT(0)
>> +#define CTRL_CTRL_EN BIT(1)
>> +#define CTRL_CTRL_SLEEPING BIT(2)
>> +#define CTRL_CTRL_CTR_EN BIT(3)
>> +#define CTRL_CTRL_SINGLE_STEP BIT(8)
>> +#define CTRL_CTRL_RUNSTATE BIT(15)
>> +
>> +/**
>> + * enum pru_mem - PRU core memory range identifiers
>> + */
>> +enum pru_mem {
>> + PRU_MEM_IRAM = 0,
>> + PRU_MEM_CTRL,
>> + PRU_MEM_DEBUG,
>> + PRU_MEM_MAX,
>> +};
>
> I am finding the name "mem" here to be confusing. I keep thinking
> these are just RAM regions instead of memory mapped I/O. Maybe call
> it "iomem" instead of "mem"?
>
ok.
> ...
>
>> +static int pru_rproc_set_id(struct pru_rproc *pru)
>> +{
>> + int ret = 0;
>> + u32 mask1 = 0x34000;
>> + u32 mask2 = 0x38000;
>
> These values are non-obvious and could use some comments. Also,
> they could be made into constants or macros.
>
ok.
>> +
>> + if ((pru->mem_regions[0].pa & mask1) == mask1)
>
> how about this instead:
>
> if ((pru->mem_regions[PRU_MEM_IRAM].pa & 0xfffff) == mask1)
>
> The 0xfffff mask will be important on AM18xx where INTC is at 0x34000,
> PRU0 IRAM is at 0x38000 and PRU1 IRAM is at 0x3C000.
>
I think this approach of figuring out id based on IRAM address is not scalable.
At the moment ID is used for these operations
pruss_cfg_gpimode()
pruss_cfg_get_gpmux()
pruss_cfg_set_gpmux()
All of which affect the GPCFG register of the respective PRU.
I think a better approach is to get rid of this ID logic and provide the
GPCFG syscon address to the PRU node and let the pru driver directly affect the register.
e.g. on am335x
pru0: pru@4a334000 {
compatible = "ti,am3356-pru";
...
gpcfg = <&pruss_cfg 8>;
};
So the API changes from
int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id id, u8 *mux)
to
int pru_rproc_cfg_get_gpmux(struct pru_rproc *pru, u8 *mux)
>> + pru->id = 0;
>> + else if ((pru->mem_regions[0].pa & mask2) == mask2)
>> + pru->id = 1;
>> + else
>> + ret = -EINVAL;
>> +
>> + return ret;
>> +}
>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-26 7:52 ` [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver Roger Quadros
2018-11-26 22:32 ` David Lechner
@ 2018-11-30 21:39 ` Dimitar Dimitrov
2018-12-04 8:47 ` Roger Quadros
2018-12-14 9:53 ` Roger Quadros
1 sibling, 2 replies; 45+ messages in thread
From: Dimitar Dimitrov @ 2018-11-30 21:39 UTC (permalink / raw)
To: Roger Quadros
Cc: ohad, bjorn.andersson, tony, robh+dt, bcousson, ssantosh, s-anna,
nsekhar, t-kristo, nsaulnier, jreeder, m-karicheri2,
woods.technical, linux-omap, linux-remoteproc, linux-kernel,
devicetree
On Monday, 12/26/2018, 9:52:37 EET Roger Quadros wrote:
> +/*
> + * Convert PRU device address (instruction space) to kernel virtual address
> + *
> + * A PRU does not have an unified address space. Each PRU has its very own
> + * private Instruction RAM, and its device address is identical to that of
> + * its primary Data RAM device address.
> + */
> +static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
> +{
> + u32 offset;
> + void *va = NULL;
> +
> + if (len <= 0)
> + return NULL;
Could you please clear the upper 4 bits the of IRAM device address, in order
to support binutils ELF images? Here is an example line to add here:
+ /* GNU binutils do not support multiple address spaces. The
+ * default linker script from the official GNU pru-ld places
+ * IRAM at an arbitrary high offset, in order to differentiate it
+ * from DRAM. Hence we need to strip the artificial offset
+ * from the IRAM address.
+ */
+ da &= ~0xf0000000u;
+
> +
> + if (da >= pru->iram_da &&
> + da + len <= pru->iram_da + pru->mem_regions[PRU_MEM_IRAM].size {
> + offset = da - pru->iram_da;
> + va = (__force void *)(pru->mem_regions[PRU_MEM_IRAM].va +
> + offset);
> + }
> +
> + return va;
> +}
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-30 21:39 ` Dimitar Dimitrov
@ 2018-12-04 8:47 ` Roger Quadros
2018-12-14 9:53 ` Roger Quadros
1 sibling, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-12-04 8:47 UTC (permalink / raw)
To: Dimitar Dimitrov
Cc: ohad, bjorn.andersson, tony, robh+dt, bcousson, ssantosh, s-anna,
nsekhar, t-kristo, nsaulnier, jreeder, m-karicheri2,
woods.technical, linux-omap, linux-remoteproc, linux-kernel,
devicetree
On 30/11/18 23:39, Dimitar Dimitrov wrote:
> On Monday, 12/26/2018, 9:52:37 EET Roger Quadros wrote:
>> +/*
>> + * Convert PRU device address (instruction space) to kernel virtual address
>> + *
>> + * A PRU does not have an unified address space. Each PRU has its very own
>> + * private Instruction RAM, and its device address is identical to that of
>> + * its primary Data RAM device address.
>> + */
>> +static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
>> +{
>> + u32 offset;
>> + void *va = NULL;
>> +
>> + if (len <= 0)
>> + return NULL;
>
> Could you please clear the upper 4 bits the of IRAM device address, in order
> to support binutils ELF images? Here is an example line to add here:
>
> + /* GNU binutils do not support multiple address spaces. The
> + * default linker script from the official GNU pru-ld places
> + * IRAM at an arbitrary high offset, in order to differentiate it
> + * from DRAM. Hence we need to strip the artificial offset
> + * from the IRAM address.
> + */
> + da &= ~0xf0000000u;
> +
>
OK. Thanks.
>
>> +
>> + if (da >= pru->iram_da &&
>> + da + len <= pru->iram_da + pru->mem_regions[PRU_MEM_IRAM].size {
>> + offset = da - pru->iram_da;
>> + va = (__force void *)(pru->mem_regions[PRU_MEM_IRAM].va +
>> + offset);
>> + }
>> +
>> + return va;
>> +}
>
>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-11-30 21:39 ` Dimitar Dimitrov
2018-12-04 8:47 ` Roger Quadros
@ 2018-12-14 9:53 ` Roger Quadros
2018-12-15 13:43 ` Dimitar Dimitrov
1 sibling, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-12-14 9:53 UTC (permalink / raw)
To: Dimitar Dimitrov
Cc: ohad, bjorn.andersson, tony, robh+dt, bcousson, ssantosh, s-anna,
nsekhar, t-kristo, nsaulnier, jreeder, m-karicheri2,
woods.technical, linux-omap, linux-remoteproc, linux-kernel,
devicetree, David Lechner
Hi Dimitar,
On 30/11/18 23:39, Dimitar Dimitrov wrote:
> On Monday, 12/26/2018, 9:52:37 EET Roger Quadros wrote:
>> +/*
>> + * Convert PRU device address (instruction space) to kernel virtual address
>> + *
>> + * A PRU does not have an unified address space. Each PRU has its very own
>> + * private Instruction RAM, and its device address is identical to that of
>> + * its primary Data RAM device address.
>> + */
>> +static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
>> +{
>> + u32 offset;
>> + void *va = NULL;
>> +
>> + if (len <= 0)
>> + return NULL;
>
> Could you please clear the upper 4 bits the of IRAM device address, in order
> to support binutils ELF images? Here is an example line to add here:
>
> + /* GNU binutils do not support multiple address spaces. The
> + * default linker script from the official GNU pru-ld places
> + * IRAM at an arbitrary high offset, in order to differentiate it
> + * from DRAM. Hence we need to strip the artificial offset
> + * from the IRAM address.
> + */
> + da &= ~0xf0000000u;
> +
>
After some more thought I'm not very sure how to proceed.
I'll be using the below 2 patches in the next patch spin in place of
patch 1 in the current series.
https://lore.kernel.org/lkml/20180623210810.21232-2-david@lechnology.com/
https://lore.kernel.org/lkml/20180623210810.21232-3-david@lechnology.com/
They figure out the PAGE (IRAM vs DRAM) by looking at TI specific section
attributes.
e.g.
[18] .TI.phattrs LOPROC+f000004 00000000 000e08 000010 00 0 0 4
[19] .TI.section.flags LOPROC+f000005 00000000 000e68 00002a 00 0 0 0
[20] .TI.section.page LOPROC+f000007 00000000 000e92 00002a 00 0 0 0
AFAIK the ELF by GNU pru-ld won't contain these sections.
We need to support ELF generated by both tools (TI clpru and GNU pru-ld).
Is it safe to assume that if the ELF doesn't have the TI specific sections
then it was generated by gnupru?
Is there a more straight forward way of differentiating the two. e.g. by looking
at something in the ELF header?
>
>> +
>> + if (da >= pru->iram_da &&
>> + da + len <= pru->iram_da + pru->mem_regions[PRU_MEM_IRAM].size {
>> + offset = da - pru->iram_da;
>> + va = (__force void *)(pru->mem_regions[PRU_MEM_IRAM].va +
>> + offset);
>> + }
>> +
>> + return va;
>> +}
>
>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver
2018-12-14 9:53 ` Roger Quadros
@ 2018-12-15 13:43 ` Dimitar Dimitrov
0 siblings, 0 replies; 45+ messages in thread
From: Dimitar Dimitrov @ 2018-12-15 13:43 UTC (permalink / raw)
To: Roger Quadros
Cc: ohad, bjorn.andersson, tony, robh+dt, bcousson, ssantosh, s-anna,
nsekhar, t-kristo, nsaulnier, jreeder, m-karicheri2,
woods.technical, linux-omap, linux-remoteproc, linux-kernel,
devicetree, David Lechner
On Fri, 12/14 2018 11:53:56 EET Roger Quadros wrote:
> Hi Dimitar,
>
> On 30/11/18 23:39, Dimitar Dimitrov wrote:
> > On Monday, 12/26/2018, 9:52:37 EET Roger Quadros wrote:
> >> +/*
> >> + * Convert PRU device address (instruction space) to kernel virtual
> >> address + *
> >> + * A PRU does not have an unified address space. Each PRU has its very
> >> own
> >> + * private Instruction RAM, and its device address is identical to that
> >> of
> >> + * its primary Data RAM device address.
> >> + */
> >> +static void *pru_i_da_to_va(struct pru_rproc *pru, u32 da, int len)
> >> +{
> >> + u32 offset;
> >> + void *va = NULL;
> >> +
> >> + if (len <= 0)
> >> + return NULL;
> >
> > Could you please clear the upper 4 bits the of IRAM device address, in
> > order to support binutils ELF images? Here is an example line to add
> > here:
> >
> > + /* GNU binutils do not support multiple address spaces. The
> > + * default linker script from the official GNU pru-ld places
> > + * IRAM at an arbitrary high offset, in order to differentiate it
> > + * from DRAM. Hence we need to strip the artificial offset
> > + * from the IRAM address.
> > + */
> > + da &= ~0xf0000000u;
> > +
>
> After some more thought I'm not very sure how to proceed.
>
> I'll be using the below 2 patches in the next patch spin in place of
> patch 1 in the current series.
> https://lore.kernel.org/lkml/20180623210810.21232-2-david@lechnology.com/
> https://lore.kernel.org/lkml/20180623210810.21232-3-david@lechnology.com/
>
> They figure out the PAGE (IRAM vs DRAM) by looking at TI specific section
> attributes.
>
> e.g.
> [18] .TI.phattrs LOPROC+f000004 00000000 000e08 000010 00 0 0 4
> [19] .TI.section.flags LOPROC+f000005 00000000 000e68 00002a 00 0 0 0
> [20] .TI.section.page LOPROC+f000007 00000000 000e92 00002a 00 0 0 0
>
> AFAIK the ELF by GNU pru-ld won't contain these sections.
These sections are not documented, so pru-ld does not generate them.
>
> We need to support ELF generated by both tools (TI clpru and GNU pru-ld).
> Is it safe to assume that if the ELF doesn't have the TI specific sections
> then it was generated by gnupru?
Right now this is correct. My concern is that in the future TI may decide to
document these sections, and pru-ld default linker script may get updated. On
the other hand, if TI considers these specific to their toolchain and not
applicable to GNU, then go ahead and use this indicator.
Slightly more robust might be to check if MSB byte of the entry address is
non-zero:
Entry point address: 0x20000000
^^
>
> Is there a more straight forward way of differentiating the two. e.g. by
> looking at something in the ELF header?
Is there a reason to differentiate? As long as you zero the MSB byte of IRAM
device addresses, there should be no conflict. Very large IRAM addresses are
unlikely to ever be valid, so it should be safe to zero the MSB for any PRU
ELF image.
Regards,
Dimitar
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (3 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 04/16] remoteproc/pru: Add PRU remoteproc driver Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 22:37 ` David Lechner
2018-11-26 7:52 ` [PATCH 06/16] dt-bindings: remoteproc: ti-pruss: Update bindings for supporting rpmsg Roger Quadros
` (10 subsequent siblings)
15 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The remoteproc core creates certain standard debugfs entries,
that does not give a whole lot of useful information for the
PRUs. The PRU remoteproc driver is enhanced to add additional
debugfs entries for PRU. These will be auto-cleaned up when
the parent rproc debug directory is removed.
The enhanced debugfs support adds two new entries: 'regs' and
'single_step'. The 'regs' dumps out the useful CTRL sub-module
registers as well as each of the 32 GPREGs and CT_REGs registers.
The GPREGs and CT_REGs though are printed only when the PRU is
halted and accessible as per the IP design.
The 'single_step' utilizes the single-step execution of the PRU
cores. Writing a non-zero value performs a single step, and a
zero value restores the PRU to execute in the same mode as the
mode before the first single step. (note: if the PRU is halted
because of a halt instruction, then no change occurs).
Logic for setting the PC and jumping over a halt instruction shall
be added in the future.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 135 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 135 insertions(+)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index c35f432..73a7f13 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -8,6 +8,7 @@
*/
#include <linux/bitops.h>
+#include <linux/debugfs.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/of_device.h>
@@ -36,6 +37,10 @@
#define CTRL_CTRL_SINGLE_STEP BIT(8)
#define CTRL_CTRL_RUNSTATE BIT(15)
+/* PRU_ICSS_PRU_DEBUG registers */
+#define PRU_DEBUG_GPREG(x) (0x0000 + (x) * 4)
+#define PRU_DEBUG_CT_REG(x) (0x0080 + (x) * 4)
+
/**
* enum pru_mem - PRU core memory range identifiers
*/
@@ -60,6 +65,8 @@ enum pru_mem {
* @sdram_da: device address of secondary Data RAM for this PRU
* @shrdram_da: device address of shared Data RAM
* @fw_name: name of firmware image used during loading
+ * @dbg_single_step: debug state variable to set PRU into single step mode
+ * @dbg_continuous: debug state variable to restore PRU execution mode
*/
struct pru_rproc {
int id;
@@ -74,6 +81,8 @@ struct pru_rproc {
u32 sdram_da;
u32 shrdram_da;
const char *fw_name;
+ u32 dbg_single_step;
+ u32 dbg_continuous;
};
static void *pru_d_da_to_va(struct pru_rproc *pru, u32 da, int len);
@@ -100,6 +109,130 @@ void pru_debug_write_reg(struct pru_rproc *pru, unsigned int reg, u32 val)
writel_relaxed(val, pru->mem_regions[PRU_MEM_DEBUG].va + reg);
}
+static int pru_rproc_debug_read_regs(struct seq_file *s, void *data)
+{
+ struct rproc *rproc = s->private;
+ struct pru_rproc *pru = rproc->priv;
+ int i, nregs = 32;
+ u32 pru_sts;
+ int pru_is_running;
+
+ seq_puts(s, "============== Control Registers ==============\n");
+ seq_printf(s, "CTRL := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CTRL));
+ pru_sts = pru_control_read_reg(pru, PRU_CTRL_STS);
+ seq_printf(s, "STS (PC) := 0x%08x (0x%08x)\n", pru_sts, pru_sts << 2);
+ seq_printf(s, "WAKEUP_EN := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_WAKEUP_EN));
+ seq_printf(s, "CYCLE := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CYCLE));
+ seq_printf(s, "STALL := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_STALL));
+ seq_printf(s, "CTBIR0 := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CTBIR0));
+ seq_printf(s, "CTBIR1 := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CTBIR1));
+ seq_printf(s, "CTPPR0 := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CTPPR0));
+ seq_printf(s, "CTPPR1 := 0x%08x\n",
+ pru_control_read_reg(pru, PRU_CTRL_CTPPR1));
+
+ seq_puts(s, "=============== Debug Registers ===============\n");
+ pru_is_running = pru_control_read_reg(pru, PRU_CTRL_CTRL) &
+ CTRL_CTRL_RUNSTATE;
+ if (pru_is_running) {
+ seq_puts(s, "PRU is executing, cannot print/access debug registers.\n");
+ return 0;
+ }
+
+ for (i = 0; i < nregs; i++) {
+ seq_printf(s, "GPREG%-2d := 0x%08x\tCT_REG%-2d := 0x%08x\n",
+ i, pru_debug_read_reg(pru, PRU_DEBUG_GPREG(i)),
+ i, pru_debug_read_reg(pru, PRU_DEBUG_CT_REG(i)));
+ }
+
+ return 0;
+}
+
+static int pru_rproc_debug_regs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pru_rproc_debug_read_regs, inode->i_private);
+}
+
+static const struct file_operations pru_rproc_debug_regs_ops = {
+ .open = pru_rproc_debug_regs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+/*
+ * Control PRU single-step mode
+ *
+ * This is a debug helper function used for controlling the single-step
+ * mode of the PRU. The PRU Debug registers are not accessible when the
+ * PRU is in RUNNING state.
+ *
+ * Writing a non-zero value sets the PRU into single-step mode irrespective
+ * of its previous state. The PRU mode is saved only on the first set into
+ * a single-step mode. Writing a zero value will restore the PRU into its
+ * original mode.
+ */
+static int pru_rproc_debug_ss_set(void *data, u64 val)
+{
+ struct rproc *rproc = data;
+ struct pru_rproc *pru = rproc->priv;
+ u32 reg_val;
+
+ val = val ? 1 : 0;
+ if (!val && !pru->dbg_single_step)
+ return 0;
+
+ reg_val = pru_control_read_reg(pru, PRU_CTRL_CTRL);
+
+ if (val && !pru->dbg_single_step)
+ pru->dbg_continuous = reg_val;
+
+ if (val)
+ reg_val |= CTRL_CTRL_SINGLE_STEP | CTRL_CTRL_EN;
+ else
+ reg_val = pru->dbg_continuous;
+
+ pru->dbg_single_step = val;
+ pru_control_write_reg(pru, PRU_CTRL_CTRL, reg_val);
+
+ return 0;
+}
+
+static int pru_rproc_debug_ss_get(void *data, u64 *val)
+{
+ struct rproc *rproc = data;
+ struct pru_rproc *pru = rproc->priv;
+
+ *val = pru->dbg_single_step;
+
+ return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(pru_rproc_debug_ss_fops, pru_rproc_debug_ss_get,
+ pru_rproc_debug_ss_set, "%llu\n");
+
+/*
+ * Create PRU-specific debugfs entries
+ *
+ * The entries are created only if the parent remoteproc debugfs directory
+ * exists, and will be cleaned up by the remoteproc core.
+ */
+static void pru_rproc_create_debug_entries(struct rproc *rproc)
+{
+ if (!rproc->dbg_dir)
+ return;
+
+ debugfs_create_file("regs", 0400, rproc->dbg_dir,
+ rproc, &pru_rproc_debug_regs_ops);
+ debugfs_create_file("single_step", 0600, rproc->dbg_dir,
+ rproc, &pru_rproc_debug_ss_fops);
+}
+
/* start a PRU core */
static int pru_rproc_start(struct rproc *rproc)
{
@@ -348,6 +481,8 @@ static int pru_rproc_probe(struct platform_device *pdev)
goto free_rproc;
}
+ pru_rproc_create_debug_entries(rproc);
+
dev_info(dev, "PRU rproc node %s probed successfully\n", np->full_name);
return 0;
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-11-26 7:52 ` [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support Roger Quadros
@ 2018-11-26 22:37 ` David Lechner
2018-11-29 10:17 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-26 22:37 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/26/18 1:52 AM, Roger Quadros wrote:
> From: Suman Anna <s-anna@ti.com>
>
> The remoteproc core creates certain standard debugfs entries,
> that does not give a whole lot of useful information for the
> PRUs. The PRU remoteproc driver is enhanced to add additional
> debugfs entries for PRU. These will be auto-cleaned up when
> the parent rproc debug directory is removed.
>
> The enhanced debugfs support adds two new entries: 'regs' and
> 'single_step'. The 'regs' dumps out the useful CTRL sub-module
> registers as well as each of the 32 GPREGs and CT_REGs registers.
> The GPREGs and CT_REGs though are printed only when the PRU is
> halted and accessible as per the IP design.
>
If the driver used regmap to access the CTRL I/O memory, then
'regs' wouldn't be needed since regmap already does debugfs.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-11-26 22:37 ` David Lechner
@ 2018-11-29 10:17 ` Roger Quadros
2018-12-18 15:51 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-29 10:17 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 27/11/18 00:37, David Lechner wrote:
> On 11/26/18 1:52 AM, Roger Quadros wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> The remoteproc core creates certain standard debugfs entries,
>> that does not give a whole lot of useful information for the
>> PRUs. The PRU remoteproc driver is enhanced to add additional
>> debugfs entries for PRU. These will be auto-cleaned up when
>> the parent rproc debug directory is removed.
>>
>> The enhanced debugfs support adds two new entries: 'regs' and
>> 'single_step'. The 'regs' dumps out the useful CTRL sub-module
>> registers as well as each of the 32 GPREGs and CT_REGs registers.
>> The GPREGs and CT_REGs though are printed only when the PRU is
>> halted and accessible as per the IP design.
>>
>
> If the driver used regmap to access the CTRL I/O memory, then
> 'regs' wouldn't be needed since regmap already does debugfs.
>
ok, we could split out CTRL from this and use regmap.
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-11-29 10:17 ` Roger Quadros
@ 2018-12-18 15:51 ` Roger Quadros
2018-12-19 12:38 ` Mark Brown
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-12-18 15:51 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson, Mark Brown
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
+Mark
David,
On 29/11/18 12:17, Roger Quadros wrote:
> On 27/11/18 00:37, David Lechner wrote:
>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>> From: Suman Anna <s-anna@ti.com>
>>>
>>> The remoteproc core creates certain standard debugfs entries,
>>> that does not give a whole lot of useful information for the
>>> PRUs. The PRU remoteproc driver is enhanced to add additional
>>> debugfs entries for PRU. These will be auto-cleaned up when
>>> the parent rproc debug directory is removed.
>>>
>>> The enhanced debugfs support adds two new entries: 'regs' and
>>> 'single_step'. The 'regs' dumps out the useful CTRL sub-module
>>> registers as well as each of the 32 GPREGs and CT_REGs registers.
>>> The GPREGs and CT_REGs though are printed only when the PRU is
>>> halted and accessible as per the IP design.
>>>
>>
>> If the driver used regmap to access the CTRL I/O memory, then
>> 'regs' wouldn't be needed since regmap already does debugfs.
>>
>
> ok, we could split out CTRL from this and use regmap.
>
I was thinking of using regmap for both control and debug
but regmap_mmio only supports one iomap even though it supports
multiple ranges.
What can we do here?
The control and debug regions even though separate are right
next to each other
reg = <0x34000 0x3000>,
<0x22000 0x400>,
<0x22400 0x100>;
reg-names = "iram", "control", "debug";
We could combine control and debug into one iomap and use
2 regmap ranges. But this is really working around the
regmap_mmio limitation of not being able to use more than one ioremaps.
Mark, any suggestions?
Is it meaningful to support __devm_regmap_init_mmio_clk()
with separate iomem regions for the ranges?
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-18 15:51 ` Roger Quadros
@ 2018-12-19 12:38 ` Mark Brown
2018-12-19 15:43 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: Mark Brown @ 2018-12-19 12:38 UTC (permalink / raw)
To: Roger Quadros
Cc: David Lechner, ohad, bjorn.andersson, tony, robh+dt, bcousson,
ssantosh, s-anna, nsekhar, t-kristo, nsaulnier, jreeder,
m-karicheri2, woods.technical, linux-omap, linux-remoteproc,
linux-kernel, devicetree
[-- Attachment #1: Type: text/plain, Size: 352 bytes --]
On Tue, Dec 18, 2018 at 05:51:12PM +0200, Roger Quadros wrote:
> We could combine control and debug into one iomap and use
> 2 regmap ranges. But this is really working around the
> regmap_mmio limitation of not being able to use more than one ioremaps.
> Mark, any suggestions?
If they're separate regions why not create separate regmaps for them?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-19 12:38 ` Mark Brown
@ 2018-12-19 15:43 ` Roger Quadros
2018-12-19 15:48 ` David Lechner
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-12-19 15:43 UTC (permalink / raw)
To: Mark Brown
Cc: David Lechner, ohad, bjorn.andersson, tony, robh+dt, bcousson,
ssantosh, s-anna, nsekhar, t-kristo, nsaulnier, jreeder,
m-karicheri2, woods.technical, linux-omap, linux-remoteproc,
linux-kernel, devicetree
On 19/12/18 14:38, Mark Brown wrote:
> On Tue, Dec 18, 2018 at 05:51:12PM +0200, Roger Quadros wrote:
>
>> We could combine control and debug into one iomap and use
>> 2 regmap ranges. But this is really working around the
>> regmap_mmio limitation of not being able to use more than one ioremaps.
>
>> Mark, any suggestions?
>
> If they're separate regions why not create separate regmaps for them?
>
I tried that but only the first regmap shows up in debugfs.
e.g.
+
+ pru->ctrl_regmap = devm_regmap_init_mmio(dev, pru->iomem_regions[PRU_IOMEM_CTRL].va,
+ &pru_regmap_config);
+ if (IS_ERR(pru->ctrl_regmap)) {
+ ret = PTR_ERR(pru->ctrl_regmap);
+ dev_err(dev, "CTRL regmap init failed: %d\n", ret);
+ goto free_rproc;
+ }
+
+
+ pru->debug_regmap = devm_regmap_init_mmio(dev, pru->iomem_regions[PRU_IOMEM_DEBUG].va,
+ &pru_debug_regmap_config);
+ if (IS_ERR(pru->debug_regmap)) {
+ ret = PTR_ERR(pru->debug_regmap);
+ dev_err(dev, "DEBUG regmap init failed: %d\n", ret);
+ goto free_rproc;
}
Did I do something wrong or we just need to enhance regmap_debugfs.c?
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-19 15:43 ` Roger Quadros
@ 2018-12-19 15:48 ` David Lechner
2018-12-19 17:07 ` Mark Brown
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-12-19 15:48 UTC (permalink / raw)
To: Roger Quadros, Mark Brown
Cc: ohad, bjorn.andersson, tony, robh+dt, bcousson, ssantosh, s-anna,
nsekhar, t-kristo, nsaulnier, jreeder, m-karicheri2,
woods.technical, linux-omap, linux-remoteproc, linux-kernel,
devicetree
On 12/19/18 4:43 PM, Roger Quadros wrote:
> On 19/12/18 14:38, Mark Brown wrote:
>> On Tue, Dec 18, 2018 at 05:51:12PM +0200, Roger Quadros wrote:
>>
>>> We could combine control and debug into one iomap and use
>>> 2 regmap ranges. But this is really working around the
>>> regmap_mmio limitation of not being able to use more than one ioremaps.
>>
>>> Mark, any suggestions?
>>
>> If they're separate regions why not create separate regmaps for them?
>>
>
> I tried that but only the first regmap shows up in debugfs.
>
> e.g.
>
> +
> + pru->ctrl_regmap = devm_regmap_init_mmio(dev, pru->iomem_regions[PRU_IOMEM_CTRL].va,
> + &pru_regmap_config);
> + if (IS_ERR(pru->ctrl_regmap)) {
> + ret = PTR_ERR(pru->ctrl_regmap);
> + dev_err(dev, "CTRL regmap init failed: %d\n", ret);
> + goto free_rproc;
> + }
> +
> +
> + pru->debug_regmap = devm_regmap_init_mmio(dev, pru->iomem_regions[PRU_IOMEM_DEBUG].va,
> + &pru_debug_regmap_config);
> + if (IS_ERR(pru->debug_regmap)) {
> + ret = PTR_ERR(pru->debug_regmap);
> + dev_err(dev, "DEBUG regmap init failed: %d\n", ret);
> + goto free_rproc;
> }
>
>
> Did I do something wrong or we just need to enhance regmap_debugfs.c?
Do you assign the name field in pru_regmap_config and
pru_debug_regmap_config?
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-19 15:48 ` David Lechner
@ 2018-12-19 17:07 ` Mark Brown
2018-12-19 17:18 ` Tony Lindgren
0 siblings, 1 reply; 45+ messages in thread
From: Mark Brown @ 2018-12-19 17:07 UTC (permalink / raw)
To: David Lechner
Cc: Roger Quadros, ohad, bjorn.andersson, tony, robh+dt, bcousson,
ssantosh, s-anna, nsekhar, t-kristo, nsaulnier, jreeder,
m-karicheri2, woods.technical, linux-omap, linux-remoteproc,
linux-kernel, devicetree
[-- Attachment #1: Type: text/plain, Size: 404 bytes --]
On Wed, Dec 19, 2018 at 04:48:52PM +0100, David Lechner wrote:
> On 12/19/18 4:43 PM, Roger Quadros wrote:
> > Did I do something wrong or we just need to enhance regmap_debugfs.c?
> Do you assign the name field in pru_regmap_config and
> pru_debug_regmap_config?
Yeah, you'll at least need to override the name since the default is to
use the name of the device and that'll result in two duplicates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-19 17:07 ` Mark Brown
@ 2018-12-19 17:18 ` Tony Lindgren
2018-12-20 8:45 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: Tony Lindgren @ 2018-12-19 17:18 UTC (permalink / raw)
To: Mark Brown
Cc: David Lechner, Roger Quadros, ohad, bjorn.andersson, robh+dt,
bcousson, ssantosh, s-anna, nsekhar, t-kristo, nsaulnier,
jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
* Mark Brown <broonie@kernel.org> [181219 17:07]:
> On Wed, Dec 19, 2018 at 04:48:52PM +0100, David Lechner wrote:
> > On 12/19/18 4:43 PM, Roger Quadros wrote:
>
> > > Did I do something wrong or we just need to enhance regmap_debugfs.c?
>
> > Do you assign the name field in pru_regmap_config and
> > pru_debug_regmap_config?
>
> Yeah, you'll at least need to override the name since the default is to
> use the name of the device and that'll result in two duplicates.
Also combining IO regions often has a serious issue that the
separate IO regions may not be in the same device (or same
interconnect target module).
This means that flushing a posted write with a read-back for
an IO region will only flush it for a single device and not
the other(s). And this leads into hard to find bugs :)
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support
2018-12-19 17:18 ` Tony Lindgren
@ 2018-12-20 8:45 ` Roger Quadros
0 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-12-20 8:45 UTC (permalink / raw)
To: Tony Lindgren, Mark Brown
Cc: David Lechner, ohad, bjorn.andersson, robh+dt, bcousson,
ssantosh, s-anna, nsekhar, t-kristo, nsaulnier, jreeder,
m-karicheri2, woods.technical, linux-omap, linux-remoteproc,
linux-kernel, devicetree
On 19/12/18 19:18, Tony Lindgren wrote:
> * Mark Brown <broonie@kernel.org> [181219 17:07]:
>> On Wed, Dec 19, 2018 at 04:48:52PM +0100, David Lechner wrote:
>>> On 12/19/18 4:43 PM, Roger Quadros wrote:
>>
>>>> Did I do something wrong or we just need to enhance regmap_debugfs.c?
>>
>>> Do you assign the name field in pru_regmap_config and
>>> pru_debug_regmap_config?
>>
>> Yeah, you'll at least need to override the name since the default is to
>> use the name of the device and that'll result in two duplicates.
I hadn't set the name field and this was the issue. It is resolved now.
Thanks :)
>
> Also combining IO regions often has a serious issue that the
> separate IO regions may not be in the same device (or same
> interconnect target module).
>
> This means that flushing a posted write with a read-back for
> an IO region will only flush it for a single device and not
> the other(s). And this leads into hard to find bugs :)
Agree.
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 06/16] dt-bindings: remoteproc: ti-pruss: Update bindings for supporting rpmsg
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (4 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 05/16] remoteproc/pru: Add pru-specific debugfs support Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 07/16] remoteproc/pru: Add support for virtio rpmsg stack Roger Quadros
` (9 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
Update the PRUSS DT bindings to add the properties required to support the
optional virtio rpmsg stack using the virtio-ring based communication
transport between MPU and a PRU core.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
.../devicetree/bindings/soc/ti/ti,pruss.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
index 24fedad..3e5f32f 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
@@ -175,6 +175,32 @@ Required Properties:
- firmware-name : should contain the name of the default firmware image file
located on the firmware search path
+Optional Properties:
+--------------------
+The virtio based communication between the MPU and a PRU core _requires_
+either the 'mboxes' property, or the set of 'interrupt-parent', 'interrupts'
+and 'interrupt-names' properties to be defined. The latter option is the
+preferred choice. The 'mboxes' property is not applicable for 66AK2G and
+DA850/OMAP-L138 SoCs.
+
+- mboxes : OMAP Mailbox specifier denoting the sub-mailbox, if using
+ a mailbox for IPC signalling between host and a PRU core.
+ The specifier format is as per the bindings,
+ Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
+ This property should match with the sub-mailbox node used
+ in the corresponding firmware image.
+- interrupt-parent : phandle to the PRUSS INTC node. Should be defined if
+ interrupts property is to be used.
+- interrupts : array of interrupt specifiers if using PRU system events
+ for IPC signalling between host and a PRU core. This
+ property should match with the PRU system event used in
+ the corresponding firmware image.
+- interrupt-names : should use one of the following names for each interrupt,
+ the name should match the corresponding PRU system event
+ number,
+ "vring" - for PRU to HOST virtqueue signalling
+ "kick" - for HOST to PRU virtqueue signalling
+
MDIO Child Node
================
@@ -243,6 +269,9 @@ Example:
<0x4a322400 0x100>;
reg-names = "iram", "control", "debug";
firmware-name = "am335x-pru0-fw";
+ interrupt-parent = <&pruss_intc>;
+ interrupts = <16>, <17>;
+ interrupt-names = "vring", "kick";
};
pru1: pru@4a338000 {
@@ -252,6 +281,10 @@ Example:
<0x4a324400 0x100>;
reg-names = "iram", "control", "debug";
firmware-name = "am335x-pru1-fw";
+ interrupt-parent = <&pruss_intc>;
+ interrupts = <18>, <19>;
+ interrupt-names = "vring", "kick";
+ /* mboxes = <&mailbox &mbox_pru1>; */
};
pruss_mdio: mdio@4a332400 {
@@ -329,6 +362,9 @@ Example:
<0x54422400 0x100>;
reg-names = "iram", "control", "debug";
firmware-name = "am437x-pru1_0-fw";
+ interrupt-parent = <&pruss1_intc>;
+ interrupts = <16>, <17>;
+ interrupt-names = "vring", "kick";
};
pru1_1: pru@54438000 {
@@ -338,6 +374,9 @@ Example:
<0x54424400 0x100>;
reg-names = "iram", "control", "debug";
firmware-name = "am437x-pru1_1-fw";
+ interrupt-parent = <&pruss1_intc>;
+ interrupts = <18>, <19>;
+ interrupt-names = "vring", "kick";
};
pruss1_mdio: mdio@54432400 {
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 07/16] remoteproc/pru: Add support for virtio rpmsg stack
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (5 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 06/16] dt-bindings: remoteproc: ti-pruss: Update bindings for supporting rpmsg Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 08/16] remoteproc/pru: Add pru_rproc_set_ctable() function Roger Quadros
` (8 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The PRU remoteproc driver has been enhanced to support the optional
rpmsg stack using the virtio-ring based communication transport
between MPU and a PRU core. This provides support to any firmware
images supporting the virtio devices.
The virtio-ring signalling support is provided either through a OMAP
mailbox or through two PRU system events on OMAP-architecture based
SoCs - one event used in each direction for kicking from one processor
and receiving notification on the other processor. The virtio rpmsg
signalling is enabled only using using PRU system events for interrupts
on the Keystone-architecture based 66AK2G SoCs (it is possible to
implement using an alternate Keystone specific IPCGR registers as well).
The driver supports both signalling options, though the PRU events based
signalling is the recommended option as it avoids an external peripheral
access from the PRU side. It also provides a uniform solution across
both the OMAP, Keystone and Davinci architectures. The PRU events based
signalling takes precedence if both options are mentioned. Either of the
options would require the corresponding firmware support though. A build
dependency against MAILBOX is also added. Note that the OMAP Mailbox
IP is not present on 66AK2G and Davinci SoCs, so it is only selected
for OMAP-based SoCs.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/Kconfig | 2 +
drivers/remoteproc/pru_rproc.c | 169 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 168 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 333666e..b89acb0 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -200,6 +200,8 @@ config ST_SLIM_REMOTEPROC
config PRUSS_REMOTEPROC
tristate "TI PRUSS remoteproc support"
depends on TI_PRUSS
+ select MAILBOX
+ select OMAP2PLUS_MBOX if ARCH_OMAP2PLUS
default n
help
Support for TI PRU-ICSS remote processors via the remote processor
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 73a7f13..e0554b3 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -10,6 +10,7 @@
#include <linux/bitops.h>
#include <linux/debugfs.h>
#include <linux/interrupt.h>
+#include <linux/mailbox_client.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/remoteproc.h>
@@ -56,6 +57,10 @@ enum pru_mem {
* @id: id of the PRU core within the PRUSS
* @pruss: back-reference to parent PRUSS structure
* @rproc: remoteproc pointer for this PRU core
+ * @mbox: mailbox channel handle used for vring signalling with MPU
+ * @client: mailbox client to request the mailbox channel
+ * @irq_ring: IRQ number to use for processing vring buffers
+ * @irq_kick: IRQ number to use to perform virtio kick
* @mem_regions: data for each of the PRU memory regions
* @dram0: PRUSS DRAM0 region
* @dram1: PRUSS DRAM1 region
@@ -72,6 +77,10 @@ struct pru_rproc {
int id;
struct pruss *pruss;
struct rproc *rproc;
+ struct mbox_chan *mbox;
+ struct mbox_client client;
+ int irq_vring;
+ int irq_kick;
struct pruss_mem_region mem_regions[PRU_MEM_MAX];
struct pruss_mem_region dram0;
struct pruss_mem_region dram1;
@@ -233,22 +242,124 @@ static void pru_rproc_create_debug_entries(struct rproc *rproc)
rproc, &pru_rproc_debug_ss_fops);
}
+/**
+ * pru_rproc_mbox_callback() - inbound mailbox message handler
+ * @client: mailbox client pointer used for requesting the mailbox channel
+ * @data: mailbox payload
+ *
+ * This handler is invoked by omap's mailbox driver whenever a mailbox
+ * message is received. Usually, the mailbox payload simply contains
+ * the index of the virtqueue that is kicked by the PRU remote processor,
+ * and we let remoteproc core handle it.
+ *
+ * In addition to virtqueue indices, we might also have some out-of-band
+ * values that indicates different events. Those values are deliberately
+ * very big so they don't coincide with virtqueue indices.
+ */
+static void pru_rproc_mbox_callback(struct mbox_client *client, void *data)
+{
+ struct pru_rproc *pru = container_of(client, struct pru_rproc, client);
+ struct device *dev = &pru->rproc->dev;
+ u32 msg = (u32)data;
+
+ dev_dbg(dev, "mbox msg: 0x%x\n", msg);
+
+ /* msg contains the index of the triggered vring */
+ if (rproc_vq_interrupt(pru->rproc, msg) == IRQ_NONE)
+ dev_dbg(dev, "no message was found in vqid %d\n", msg);
+}
+
+/**
+ * pru_rproc_vring_interrupt() - interrupt handler for processing vrings
+ * @irq: irq number associated with the PRU event MPU is listening on
+ * @data: interrupt handler data, will be a PRU rproc structure
+ *
+ * This handler is used by the PRU remoteproc driver when using PRU system
+ * events for processing the virtqueues. Unlike the mailbox IP, there is
+ * no payload associated with an interrupt, so either a unique event is
+ * used for each virtqueue kick, or a both virtqueues are processed on
+ * a single event. The latter is chosen to conserve the usable PRU system
+ * events.
+ */
+static irqreturn_t pru_rproc_vring_interrupt(int irq, void *data)
+{
+ struct pru_rproc *pru = data;
+
+ dev_dbg(&pru->rproc->dev, "got vring irq\n");
+
+ /* process incoming buffers on both the Rx and Tx vrings */
+ rproc_vq_interrupt(pru->rproc, 0);
+ rproc_vq_interrupt(pru->rproc, 1);
+
+ return IRQ_HANDLED;
+}
+
+/* kick a virtqueue */
+static void pru_rproc_kick(struct rproc *rproc, int vq_id)
+{
+ struct device *dev = &rproc->dev;
+ struct pru_rproc *pru = rproc->priv;
+ int ret;
+
+ dev_dbg(dev, "kicking vqid %d on PRU%d\n", vq_id, pru->id);
+
+ if (pru->irq_kick > 0) {
+ ret = pruss_intc_trigger(pru->irq_kick);
+ if (ret < 0)
+ dev_err(dev, "pruss_intc_trigger failed: %d\n", ret);
+ } else if (pru->mbox) {
+ /*
+ * send the index of the triggered virtqueue in the mailbox
+ * payload
+ */
+ ret = mbox_send_message(pru->mbox, (void *)vq_id);
+ if (ret < 0)
+ dev_err(dev, "mbox_send_message failed: %d\n", ret);
+ }
+}
+
/* start a PRU core */
static int pru_rproc_start(struct rproc *rproc)
{
struct device *dev = &rproc->dev;
struct pru_rproc *pru = rproc->priv;
u32 val;
+ int ret;
dev_dbg(dev, "starting PRU%d: entry-point = 0x%x\n",
pru->id, (rproc->bootaddr >> 2));
+ /* TODO: INTC setup */
+
+ if (!list_empty(&pru->rproc->rvdevs)) {
+ if (!pru->mbox && (pru->irq_vring <= 0 || pru->irq_kick <= 0)) {
+ dev_err(dev, "virtio vring interrupt mechanisms are not provided\n");
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ if (!pru->mbox && pru->irq_vring > 0) {
+ ret = request_threaded_irq(pru->irq_vring, NULL,
+ pru_rproc_vring_interrupt,
+ IRQF_ONESHOT, dev_name(dev),
+ pru);
+ if (ret) {
+ dev_err(dev, "failed to enable vring interrupt, ret = %d\n",
+ ret);
+ goto fail;
+ }
+ }
+ }
+
val = CTRL_CTRL_EN | ((rproc->bootaddr >> 2) << 16);
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
- /* TODO: INTC setup */
-
return 0;
+
+fail:
+ /* TODO: INTC cleanup */
+
+ return ret;
}
/* stop/disable a PRU core */
@@ -264,6 +375,10 @@ static int pru_rproc_stop(struct rproc *rproc)
val &= ~CTRL_CTRL_EN;
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
+ if (!list_empty(&pru->rproc->rvdevs) &&
+ !pru->mbox && pru->irq_vring > 0)
+ free_irq(pru->irq_vring, pru);
+
/* TODO: INTC cleanup */
return 0;
@@ -356,6 +471,7 @@ static void *pru_da_to_va(struct rproc *rproc, u64 da, int len, u32 flags)
static struct rproc_ops pru_rproc_ops = {
.start = pru_rproc_start,
.stop = pru_rproc_stop,
+ .kick = pru_rproc_kick,
.da_to_va = pru_da_to_va,
};
@@ -383,6 +499,7 @@ static int pru_rproc_probe(struct platform_device *pdev)
struct pru_rproc *pru;
const char *fw_name;
struct rproc *rproc = NULL;
+ struct mbox_client *client;
struct resource *res;
int i, ret;
const char *mem_names[PRU_MEM_MAX] = { "iram", "control", "debug" };
@@ -475,10 +592,51 @@ static int pru_rproc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rproc);
+ /* get optional vring and kick interrupts for supporting virtio rpmsg */
+ pru->irq_vring = platform_get_irq_byname(pdev, "vring");
+ if (pru->irq_vring <= 0) {
+ ret = pru->irq_vring;
+ if (ret == -EPROBE_DEFER)
+ goto free_rproc;
+ dev_dbg(dev, "unable to get vring interrupt, status = %d\n",
+ ret);
+ }
+
+ pru->irq_kick = platform_get_irq_byname(pdev, "kick");
+ if (pru->irq_kick <= 0) {
+ ret = pru->irq_kick;
+ if (ret == -EPROBE_DEFER)
+ goto free_rproc;
+ dev_dbg(dev, "unable to get kick interrupt, status = %d\n",
+ ret);
+ }
+
+ /*
+ * get optional mailbox for virtio rpmsg signalling if vring and kick
+ * interrupts are not specified for OMAP architecture based SoCs
+ */
+ if (pru->irq_vring <= 0 && pru->irq_kick <= 0 &&
+ !of_device_is_compatible(np, "ti,k2g-pru") &&
+ !of_device_is_compatible(np, "ti,da850-pru")) {
+ client = &pru->client;
+ client->dev = dev;
+ client->tx_done = NULL;
+ client->rx_callback = pru_rproc_mbox_callback;
+ client->tx_block = false;
+ client->knows_txdone = false;
+ pru->mbox = mbox_request_channel(client, 0);
+ if (IS_ERR(pru->mbox)) {
+ ret = PTR_ERR(pru->mbox);
+ pru->mbox = NULL;
+ dev_dbg(dev, "unable to get mailbox channel, status = %d\n",
+ ret);
+ }
+ }
+
ret = rproc_add(pru->rproc);
if (ret) {
dev_err(dev, "rproc_add failed: %d\n", ret);
- goto free_rproc;
+ goto put_mbox;
}
pru_rproc_create_debug_entries(rproc);
@@ -487,6 +645,8 @@ static int pru_rproc_probe(struct platform_device *pdev)
return 0;
+put_mbox:
+ mbox_free_channel(pru->mbox);
free_rproc:
rproc_free(rproc);
return ret;
@@ -496,9 +656,12 @@ static int pru_rproc_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct rproc *rproc = platform_get_drvdata(pdev);
+ struct pru_rproc *pru = rproc->priv;
dev_info(dev, "%s: removing rproc %s\n", __func__, rproc->name);
+ mbox_free_channel(pru->mbox);
+
rproc_del(rproc);
rproc_free(rproc);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 08/16] remoteproc/pru: Add pru_rproc_set_ctable() function
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (6 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 07/16] remoteproc/pru: Add support for virtio rpmsg stack Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 09/16] remoteproc/pru: add APIs to get and put the PRU cores Roger Quadros
` (7 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
Some firmwares expect the OS drivers to configure the CTABLE
entries publishing dynamically allocated memory regions. For
example, the PRU Ethernet firmwares use the C28 and C30 entries
for retrieving the Shared RAM and System SRAM (OCMC) areas
allocated by the PRU Ethernet client driver.
Provide a way for users to do that through a new API,
pru_rproc_set_ctable(). The API returns 0 on success and
a negative value on error.
NOTE:
The programmable CTABLE entries are typically re-programmed by
the PRU firmwares when dealing with a certain block of memory
during block processing. This API provides an interface to the
PRU client drivers to publish a dynamically allocated memory
block with the PRU firmware using a CTABLE entry instead of a
negotiated address in shared memory. Additional synchronization
may be needed between the PRU client drivers and firmwares if
different addresses needs to be published at run-time reusing
the same CTABLE entry.
Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
[s-anna@ti.com: add the NOTE: on patch description, minor cleanups]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 51 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pruss.h | 29 ++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index e0554b3..fa3559b 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -65,6 +65,7 @@ enum pru_mem {
* @dram0: PRUSS DRAM0 region
* @dram1: PRUSS DRAM1 region
* @shrdram: PRUSS SHARED RAM region
+ * @rmw_lock: lock for read, modify, write operations on registers
* @iram_da: device address of Instruction RAM for this PRU
* @pdram_da: device address of primary Data RAM for this PRU
* @sdram_da: device address of secondary Data RAM for this PRU
@@ -85,6 +86,7 @@ struct pru_rproc {
struct pruss_mem_region dram0;
struct pruss_mem_region dram1;
struct pruss_mem_region shrdram;
+ spinlock_t rmw_lock; /* register access lock */
u32 iram_da;
u32 pdram_da;
u32 sdram_da;
@@ -107,6 +109,54 @@ void pru_control_write_reg(struct pru_rproc *pru, unsigned int reg, u32 val)
writel_relaxed(val, pru->mem_regions[PRU_MEM_CTRL].va + reg);
}
+static inline
+void pru_control_set_reg(struct pru_rproc *pru, unsigned int reg,
+ u32 mask, u32 set)
+{
+ u32 val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pru->rmw_lock, flags);
+
+ val = pru_control_read_reg(pru, reg);
+ val &= ~mask;
+ val |= (set & mask);
+ pru_control_write_reg(pru, reg, val);
+
+ spin_unlock_irqrestore(&pru->rmw_lock, flags);
+}
+
+/**
+ * pru_rproc_set_ctable() - set the constant table index for the PRU
+ * @rproc: the rproc instance of the PRU
+ * @c: constant table index to set
+ * @addr: physical address to set it to
+ */
+int pru_rproc_set_ctable(struct rproc *rproc, enum pru_ctable_idx c, u32 addr)
+{
+ struct pru_rproc *pru = rproc->priv;
+ unsigned int reg;
+ u32 mask, set;
+ u16 idx;
+ u16 idx_mask;
+
+ /* pointer is 16 bit and index is 8-bit so mask out the rest */
+ idx_mask = (c >= PRU_C28) ? 0xFFFF : 0xFF;
+
+ /* ctable uses bit 8 and upwards only */
+ idx = (addr >> 8) & idx_mask;
+
+ /* configurable ctable (i.e. C24) starts at PRU_CTRL_CTBIR0 */
+ reg = PRU_CTRL_CTBIR0 + 4 * (c >> 1);
+ mask = idx_mask << (16 * (c & 1));
+ set = idx << (16 * (c & 1));
+
+ pru_control_set_reg(pru, reg, mask, set);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pru_rproc_set_ctable);
+
static inline u32 pru_debug_read_reg(struct pru_rproc *pru, unsigned int reg)
{
return readl_relaxed(pru->mem_regions[PRU_MEM_DEBUG].va + reg);
@@ -537,6 +587,7 @@ static int pru_rproc_probe(struct platform_device *pdev)
pru->pruss = platform_get_drvdata(ppdev);
pru->rproc = rproc;
pru->fw_name = fw_name;
+ spin_lock_init(&pru->rmw_lock);
ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM0,
&pru->dram0);
diff --git a/include/linux/pruss.h b/include/linux/pruss.h
index c797fb1..af04a1c 100644
--- a/include/linux/pruss.h
+++ b/include/linux/pruss.h
@@ -134,7 +134,22 @@ struct pruss_intc_config {
s8 ch_to_host[MAX_PRU_CHANNELS];
};
+/**
+ * enum pru_ctable_idx - Configurable Constant table index identifiers
+ */
+enum pru_ctable_idx {
+ PRU_C24 = 0,
+ PRU_C25,
+ PRU_C26,
+ PRU_C27,
+ PRU_C28,
+ PRU_C29,
+ PRU_C30,
+ PRU_C31,
+};
+
struct pruss;
+struct rproc;
#if IS_ENABLED(CONFIG_TI_PRUSS)
@@ -232,4 +247,18 @@ int pruss_intc_unconfigure(struct pruss *pruss,
#endif /* CONFIG_TI_PRUSS */
+#if IS_ENABLED(CONFIG_PRUSS_REMOTEPROC)
+
+int pru_rproc_set_ctable(struct rproc *rproc, enum pru_ctable_idx c, u32 addr);
+
+#else
+
+static inline int pru_rproc_set_ctable(struct rproc *rproc,
+ enum pru_ctable_idx c, u32 addr)
+{
+ return -ENOTSUPP;
+}
+
+#endif /* CONFIG_PRUSS_REMOTEPROC */
+
#endif /* __LINUX_PRUSS_H */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 09/16] remoteproc/pru: add APIs to get and put the PRU cores
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (7 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 08/16] remoteproc/pru: Add pru_rproc_set_ctable() function Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 10/16] remoteproc/pru: add pru_rproc_get_id() API to retrieve the PRU id Roger Quadros
` (6 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Tero Kristo <t-kristo@ti.com>
Add two new APIs, pru_rproc_get() and pru_rproc_put(), to the PRU
driver to allow client drivers to acquire and release the remoteproc
device associated with a PRU core. The PRU cores are treated as
resources with only one client owning it at a time.
The pru_rproc_get() function returns the rproc handle corresponding
to a PRU core identified by the device tree "prus" property under
the client node. The pru_rproc_put() is the complementary function
to pru_rproc_get().
Signed-off-by: Tero Kristo <t-kristo@ti.com>
[s-anna@ti.com: improve error checking, various fixes and cleanups]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 112 +++++++++++++++++++++++++++++++++++++++++
include/linux/pruss.h | 9 ++++
2 files changed, 121 insertions(+)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index fa3559b..2aa05b0 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -57,6 +57,7 @@ enum pru_mem {
* @id: id of the PRU core within the PRUSS
* @pruss: back-reference to parent PRUSS structure
* @rproc: remoteproc pointer for this PRU core
+ * @client_np: client device node
* @mbox: mailbox channel handle used for vring signalling with MPU
* @client: mailbox client to request the mailbox channel
* @irq_ring: IRQ number to use for processing vring buffers
@@ -71,6 +72,7 @@ enum pru_mem {
* @sdram_da: device address of secondary Data RAM for this PRU
* @shrdram_da: device address of shared Data RAM
* @fw_name: name of firmware image used during loading
+ * @lock: mutex to protect client usage
* @dbg_single_step: debug state variable to set PRU into single step mode
* @dbg_continuous: debug state variable to restore PRU execution mode
*/
@@ -78,6 +80,7 @@ struct pru_rproc {
int id;
struct pruss *pruss;
struct rproc *rproc;
+ struct device_node *client_np;
struct mbox_chan *mbox;
struct mbox_client client;
int irq_vring;
@@ -92,6 +95,7 @@ struct pru_rproc {
u32 sdram_da;
u32 shrdram_da;
const char *fw_name;
+ struct mutex lock; /* client access lock */
u32 dbg_single_step;
u32 dbg_continuous;
};
@@ -126,6 +130,113 @@ void pru_control_set_reg(struct pru_rproc *pru, unsigned int reg,
spin_unlock_irqrestore(&pru->rmw_lock, flags);
}
+static struct rproc *__pru_rproc_get(struct device_node *np, int index)
+{
+ struct device_node *rproc_np = NULL;
+ struct platform_device *pdev;
+ struct rproc *rproc;
+
+ rproc_np = of_parse_phandle(np, "prus", index);
+ if (!rproc_np || !of_device_is_available(rproc_np))
+ return ERR_PTR(-ENODEV);
+
+ pdev = of_find_device_by_node(rproc_np);
+ of_node_put(rproc_np);
+
+ if (!pdev)
+ /* probably PRU not yet probed */
+ return ERR_PTR(-EPROBE_DEFER);
+
+ /* TODO: replace the crude string based check to make sure it is PRU */
+ if (!strstr(dev_name(&pdev->dev), "pru")) {
+ put_device(&pdev->dev);
+ return ERR_PTR(-ENODEV);
+ }
+
+ rproc = platform_get_drvdata(pdev);
+ put_device(&pdev->dev);
+ if (!rproc)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ get_device(&rproc->dev);
+
+ return rproc;
+}
+
+/**
+ * pru_rproc_get() - get the PRU rproc instance from a device node
+ * @np: the user/client device node
+ * @index: index to use for the prus property
+ *
+ * This function looks through a client device node's "prus" property at index
+ * @index and returns the rproc handle for a valid PRU remote processor if
+ * found. The function allows only one user to own the PRU rproc resource at
+ * a time. Caller must call pru_rproc_put() when done with using the rproc,
+ * not required if the function returns a failure.
+ *
+ * Returns the rproc handle on success, and an ERR_PTR on failure using one
+ * of the following error values
+ * -ENODEV if device is not found
+ * -EBUSY if PRU is already acquired by anyone
+ * -EPROBE_DEFER is PRU device is not probed yet
+ */
+struct rproc *pru_rproc_get(struct device_node *np, int index)
+{
+ struct rproc *rproc;
+ struct pru_rproc *pru;
+
+ rproc = __pru_rproc_get(np, index);
+ if (IS_ERR(rproc))
+ return rproc;
+
+ pru = rproc->priv;
+
+ mutex_lock(&pru->lock);
+
+ if (pru->client_np) {
+ mutex_unlock(&pru->lock);
+ put_device(&rproc->dev);
+ return ERR_PTR(-EBUSY);
+ }
+
+ pru->client_np = np;
+
+ mutex_unlock(&pru->lock);
+
+ return rproc;
+}
+EXPORT_SYMBOL_GPL(pru_rproc_get);
+
+/**
+ * pru_rproc_put() - release the PRU rproc resource
+ * @rproc: the rproc resource to release
+ *
+ * Releases the PRU rproc resource and makes it available to other
+ * users.
+ */
+void pru_rproc_put(struct rproc *rproc)
+{
+ struct pru_rproc *pru;
+
+ if (IS_ERR_OR_NULL(rproc))
+ return;
+
+ /* TODO: replace the crude string based check to make sure it is PRU */
+ if (!strstr(dev_name(rproc->dev.parent), "pru"))
+ return;
+
+ pru = rproc->priv;
+ if (!pru->client_np)
+ return;
+
+ mutex_lock(&pru->lock);
+ pru->client_np = NULL;
+ mutex_unlock(&pru->lock);
+
+ put_device(&rproc->dev);
+}
+EXPORT_SYMBOL_GPL(pru_rproc_put);
+
/**
* pru_rproc_set_ctable() - set the constant table index for the PRU
* @rproc: the rproc instance of the PRU
@@ -588,6 +699,7 @@ static int pru_rproc_probe(struct platform_device *pdev)
pru->rproc = rproc;
pru->fw_name = fw_name;
spin_lock_init(&pru->rmw_lock);
+ mutex_init(&pru->lock);
ret = pruss_request_mem_region(pru->pruss, PRUSS_MEM_DRAM0,
&pru->dram0);
diff --git a/include/linux/pruss.h b/include/linux/pruss.h
index af04a1c..405039a 100644
--- a/include/linux/pruss.h
+++ b/include/linux/pruss.h
@@ -249,10 +249,19 @@ int pruss_intc_unconfigure(struct pruss *pruss,
#if IS_ENABLED(CONFIG_PRUSS_REMOTEPROC)
+struct rproc *pru_rproc_get(struct device_node *node, int index);
+void pru_rproc_put(struct rproc *rproc);
int pru_rproc_set_ctable(struct rproc *rproc, enum pru_ctable_idx c, u32 addr);
#else
+static inline struct rproc *pru_rproc_get(struct device_node *node, int index)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
+static inline void pru_rproc_put(struct rproc *rproc) { }
+
static inline int pru_rproc_set_ctable(struct rproc *rproc,
enum pru_ctable_idx c, u32 addr)
{
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 10/16] remoteproc/pru: add pru_rproc_get_id() API to retrieve the PRU id
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (8 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 09/16] remoteproc/pru: add APIs to get and put the PRU cores Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 11/16] soc: ti: pruss: add helper functions to set GPI mode, MII_RT_event and XFR Roger Quadros
` (5 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
Export an API pru_rproc_get_id() to allow other PRUSS platform
drivers to clients to retrieve the PRU id from a remoteproc handle
associated with a PRU. The new function takes in a struct rproc
pointer as argument.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 27 +++++++++++++++++++++++++--
include/linux/pruss.h | 15 +++++++++++++++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 2aa05b0..d8b823d 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -238,6 +238,29 @@ void pru_rproc_put(struct rproc *rproc)
EXPORT_SYMBOL_GPL(pru_rproc_put);
/**
+ * pru_rproc_get_id() - get PRU id from a previously acquired PRU remoteproc
+ * @rproc: the rproc instance of the PRU
+ *
+ * Returns the PRU id of the PRU remote processor that has been acquired through
+ * a pru_rproc_get(), or a negative value on error
+ */
+enum pruss_pru_id pru_rproc_get_id(struct rproc *rproc)
+{
+ struct pru_rproc *pru;
+
+ if (IS_ERR_OR_NULL(rproc) || !rproc->dev.parent)
+ return -EINVAL;
+
+ /* TODO: replace the crude string based check to make sure it is PRU */
+ if (!strstr(dev_name(rproc->dev.parent), "pru"))
+ return -EINVAL;
+
+ pru = rproc->priv;
+ return pru->id;
+}
+EXPORT_SYMBOL_GPL(pru_rproc_get_id);
+
+/**
* pru_rproc_set_ctable() - set the constant table index for the PRU
* @rproc: the rproc instance of the PRU
* @c: constant table index to set
@@ -643,9 +666,9 @@ static int pru_rproc_set_id(struct pru_rproc *pru)
u32 mask2 = 0x38000;
if ((pru->mem_regions[0].pa & mask1) == mask1)
- pru->id = 0;
+ pru->id = PRUSS_PRU0;
else if ((pru->mem_regions[0].pa & mask2) == mask2)
- pru->id = 1;
+ pru->id = PRUSS_PRU1;
else
ret = -EINVAL;
diff --git a/include/linux/pruss.h b/include/linux/pruss.h
index 405039a..c0a3b3e 100644
--- a/include/linux/pruss.h
+++ b/include/linux/pruss.h
@@ -10,6 +10,15 @@
#ifndef __LINUX_PRUSS_H
#define __LINUX_PRUSS_H
+/**
+ * enum pruss_pru_id - PRU core identifiers
+ */
+enum pruss_pru_id {
+ PRUSS_PRU0 = 0,
+ PRUSS_PRU1,
+ PRUSS_NUM_PRUS,
+};
+
/*
* PRU_ICSS_CFG registers
* SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
@@ -251,6 +260,7 @@ int pruss_intc_unconfigure(struct pruss *pruss,
struct rproc *pru_rproc_get(struct device_node *node, int index);
void pru_rproc_put(struct rproc *rproc);
+enum pruss_pru_id pru_rproc_get_id(struct rproc *rproc);
int pru_rproc_set_ctable(struct rproc *rproc, enum pru_ctable_idx c, u32 addr);
#else
@@ -262,6 +272,11 @@ static inline struct rproc *pru_rproc_get(struct device_node *node, int index)
static inline void pru_rproc_put(struct rproc *rproc) { }
+static inline enum pruss_pru_id pru_rproc_get_id(struct rproc *rproc)
+{
+ return -ENOTSUPP;
+}
+
static inline int pru_rproc_set_ctable(struct rproc *rproc,
enum pru_ctable_idx c, u32 addr)
{
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 11/16] soc: ti: pruss: add helper functions to set GPI mode, MII_RT_event and XFR
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (9 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 10/16] remoteproc/pru: add pru_rproc_get_id() API to retrieve the PRU id Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings Roger Quadros
` (4 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Suman Anna <s-anna@ti.com>
The PRUSS CFG module is represented as a syscon node and is currently
managed by the PRUSS platform driver. Add easy accessor functions to set
GPI mode, MII_RT event enable/disable and XFR (XIN XOUT) enable/disable
to enable the PRUSS Ethernet usecase. These functions reuse the generic
pruss_regmap_update() API function.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
include/linux/pruss.h | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/include/linux/pruss.h b/include/linux/pruss.h
index c0a3b3e..7227aae 100644
--- a/include/linux/pruss.h
+++ b/include/linux/pruss.h
@@ -204,6 +204,69 @@ int pruss_intc_configure(struct pruss *pruss,
int pruss_intc_unconfigure(struct pruss *pruss,
struct pruss_intc_config *intc_config);
+/**
+ * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device
+ * @pruss: pruss instance
+ * @id: PRU identifier (0-1)
+ * @mux: pointer to store the current mux value into
+ */
+static inline int pruss_cfg_get_gpmux(struct pruss *pruss,
+ enum pruss_pru_id id, u8 *mux)
+{
+ int ret = 0;
+ u32 val;
+
+ ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(id), &val);
+ if (!ret)
+ *mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >>
+ PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
+ return ret;
+}
+
+/**
+ * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device
+ * @pruss: pruss instance
+ * @pru_id: PRU identifier (0-1)
+ * @mux: new mux value for PRU
+ */
+static inline int pruss_cfg_set_gpmux(struct pruss *pruss,
+ enum pruss_pru_id id, u8 mux)
+{
+ if (mux >= PRUSS_GP_MUX_SEL_MAX)
+ return -EINVAL;
+
+ return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(id),
+ PRUSS_GPCFG_PRU_MUX_SEL_MASK,
+ (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT);
+}
+
+/**
+ * pruss_cfg_miirt_enable() - Enable/disable MII RT Events
+ * @pruss: the pruss instance
+ * @enable: enable/disable
+ *
+ * Enable/disable the MII RT Events for the PRUSS.
+ */
+static inline int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable)
+{
+ u32 set = enable ? PRUSS_MII_RT_EVENT_EN : 0;
+
+ return pruss_cfg_update(pruss, PRUSS_CFG_MII_RT,
+ PRUSS_MII_RT_EVENT_EN, set);
+}
+
+/**
+ * pruss_cfg_xfr_enable() - Enable/disable XIN XOUT shift functionality
+ * @pruss: the pruss instance
+ * @enable: enable/disable
+ */
+static inline int pruss_cfg_xfr_enable(struct pruss *pruss, bool enable)
+{
+ u32 set = enable ? PRUSS_SPP_XFER_SHIFT_EN : 0;
+
+ return pruss_cfg_update(pruss, PRUSS_CFG_SPP,
+ PRUSS_SPP_XFER_SHIFT_EN, set);
+}
#else
static inline struct pruss *pruss_get(struct rproc *rproc)
@@ -254,6 +317,28 @@ int pruss_intc_unconfigure(struct pruss *pruss,
return -ENOTSUPP;
}
+static inline int pruss_cfg_get_gpmux(struct pruss *pruss,
+ enum pruss_pru_id id, u8 *mux)
+{
+ return -ENOTSUPP;
+}
+
+static inline int pruss_cfg_set_gpmux(struct pruss *pruss,
+ enum pruss_pru_id id, u8 mux)
+{
+ return -ENOTSUPP;
+}
+
+static inline int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable)
+{
+ return -ENOTSUPP;
+}
+
+static inline int pruss_cfg_xfr_enable(struct pruss *pruss, bool enable)
+{
+ return -ENOTSUPP;
+}
+
#endif /* CONFIG_TI_PRUSS */
#if IS_ENABLED(CONFIG_PRUSS_REMOTEPROC)
@@ -263,6 +348,30 @@ void pru_rproc_put(struct rproc *rproc);
enum pruss_pru_id pru_rproc_get_id(struct rproc *rproc);
int pru_rproc_set_ctable(struct rproc *rproc, enum pru_ctable_idx c, u32 addr);
+/**
+ * pruss_cfg_gpimode() - set the GPI mode of the PRU
+ * @pruss: the pruss instance handle
+ * @pru: the rproc instance handle of the PRU
+ * @mode: GPI mode to set
+ *
+ * Sets the GPI mode for a given PRU by programming the
+ * corresponding PRUSS_CFG_GPCFGx register
+ *
+ * Returns 0 on success, or an error code otherwise
+ */
+static inline int pruss_cfg_gpimode(struct pruss *pruss, struct rproc *pru,
+ enum pruss_gpi_mode mode)
+{
+ enum pruss_pru_id id = pru_rproc_get_id(pru);
+
+ if (id < 0)
+ return -EINVAL;
+
+ return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(id),
+ PRUSS_GPCFG_PRU_GPI_MODE_MASK,
+ mode << PRUSS_GPCFG_PRU_GPI_MODE_SHIFT);
+}
+
#else
static inline struct rproc *pru_rproc_get(struct device_node *node, int index)
@@ -283,6 +392,12 @@ static inline int pru_rproc_set_ctable(struct rproc *rproc,
return -ENOTSUPP;
}
+static inline int pruss_cfg_gpimode(struct pruss *pruss, struct rproc *pru,
+ enum pruss_gpi_mode mode)
+{
+ return -ENOTSUPP;
+}
+
#endif /* CONFIG_PRUSS_REMOTEPROC */
#endif /* __LINUX_PRUSS_H */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (10 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 11/16] soc: ti: pruss: add helper functions to set GPI mode, MII_RT_event and XFR Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 23:27 ` David Lechner
2018-12-11 22:06 ` Rob Herring
2018-11-26 7:52 ` [PATCH 13/16] remoteproc/pru: add support for configuring GPMUX based on client setup Roger Quadros
` (3 subsequent siblings)
15 siblings, 2 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Tero Kristo <t-kristo@ti.com>
Add documentation for the Texas Instruments PRU application nodes.
These are used to configure specific user applications for PRU instances.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
[s-anna@ti.com: some binding updates]
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
.../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
index 3e5f32f..94c91ee 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
@@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
+Application/User Nodes
+=======================
+A PRU application/user node typically uses one or more PRU device nodes to
+implement a PRU application/functionality. Each application/client node would
+need a reference to at least a PRU node, and optionally pass some configuration
+parameters.
+
+Required Properties:
+--------------------
+- prus : phandles to the PRU nodes used
+
+Optional Properties:
+--------------------
+- firmware-name : firmwares for the PRU cores, the default firmware
+ for the core from the PRU node will be used if not
+ provided. The firmware names should correspond to
+ the PRU cores listed in the 'prus' property
+- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
+ register for a PRU. This selects the internal muxing
+ scheme for the PRU instance. If not provided, the
+ default out-of-reset value (0) for the PRU core is
+ used. Values should correspond to the PRU cores listed
+ in the 'prus' property
+- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
+ with each entry consisting of 4 cell-values. First one
+ is an index towards the "prus" property to identify the
+ PRU core for the interrupt map, second is the PRU
+ System Event id, third is the PRU interrupt channel id
+ and fourth is the PRU host interrupt id. If provided,
+ this map will supercede any other configuration
+ provided through firmware
+
Example:
========
1. /* AM33xx PRU-ICSS */
@@ -397,3 +429,14 @@ Example:
...
};
};
+
+3: /* PRU application node example */
+ app_node: app_node {
+ prus = <&pru1_0>, <&pru1_1>;
+ firmware-name = "pruss-app-fw", "pruss-app-fw-2";
+ ti,pruss-gp-mux-sel = <2>, <1>;
+ /* setup interrupts for prus:
+ prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
+ prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
+ ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
+ }
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-26 7:52 ` [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings Roger Quadros
@ 2018-11-26 23:27 ` David Lechner
2018-11-29 10:07 ` Roger Quadros
2018-12-11 22:06 ` Rob Herring
1 sibling, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-26 23:27 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/26/18 1:52 AM, Roger Quadros wrote:
> From: Tero Kristo <t-kristo@ti.com>
>
> Add documentation for the Texas Instruments PRU application nodes.
> These are used to configure specific user applications for PRU instances.
Could this be made into a generic remoteproc producer/consumer binding? Or
are there really things that are specific to the TI PRU that need to be
handled?
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> [s-anna@ti.com: some binding updates]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> index 3e5f32f..94c91ee 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>
>
> +Application/User Nodes
> +=======================
Are these supposed to be stand-alone platform devices?
> +A PRU application/user node typically uses one or more PRU device nodes to
> +implement a PRU application/functionality. Each application/client node would
> +need a reference to at least a PRU node, and optionally pass some configuration
> +parameters.
I thought device tree is not supposed to be used for configuration.
> +
> +Required Properties:
> +--------------------
> +- prus : phandles to the PRU nodes used
> +
> +Optional Properties:
> +--------------------
> +- firmware-name : firmwares for the PRU cores, the default firmware
> + for the core from the PRU node will be used if not
> + provided. The firmware names should correspond to
> + the PRU cores listed in the 'prus' property
Perhaps this should be a "compatible" property instead of "firmware-name"? The
driver that matches the compatible string can then set the firmware names.
> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
> + register for a PRU. This selects the internal muxing
> + scheme for the PRU instance. If not provided, the
> + default out-of-reset value (0) for the PRU core is
> + used. Values should correspond to the PRU cores listed
> + in the 'prus' property
Is this supposed to be a pinmux? So maybe we should be using pinmux bindings?
> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
> + with each entry consisting of 4 cell-values. First one
> + is an index towards the "prus" property to identify the
> + PRU core for the interrupt map, second is the PRU
> + System Event id, third is the PRU interrupt channel id
> + and fourth is the PRU host interrupt id. If provided,
> + this map will supercede any other configuration
> + provided through firmware
Could this mapping just be cells of the interrupt consumer nodes instead of an
extra property? As I mentioned in a reply to another patch, unless there is a
compelling reason to do otherwise, the channel to host mapping can be required
to be 1:1 as recommended in the TRMs, so that cell can be omitted. Also, since
the interrupt controller is independent of the PRU cores, the cell specifying the
index of the PRU core is not needed in this case. The #interrupt-cells already
includes a cell for the system event number, so this just leaves one cell, the
host channel, to be added to the #interrupt-cells.
So, instead of:
ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
we could have:
interrupt-parent = <&pruss_intc>;
interrupts = <16 7>, <19 3>;
There are also already alternate interrupt bindings that allow for the case
where there is more than one interrupt-parent.
> +
> Example:
> ========
> 1. /* AM33xx PRU-ICSS */
> @@ -397,3 +429,14 @@ Example:
> ...
> };
> };
> +
> +3: /* PRU application node example */
> + app_node: app_node {
> + prus = <&pru1_0>, <&pru1_1>;
> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
> + ti,pruss-gp-mux-sel = <2>, <1>;
> + /* setup interrupts for prus:
> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
> + }
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-26 23:27 ` David Lechner
@ 2018-11-29 10:07 ` Roger Quadros
2018-11-29 16:33 ` David Lechner
0 siblings, 1 reply; 45+ messages in thread
From: Roger Quadros @ 2018-11-29 10:07 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 27/11/18 01:27, David Lechner wrote:
> On 11/26/18 1:52 AM, Roger Quadros wrote:
>> From: Tero Kristo <t-kristo@ti.com>
>>
>> Add documentation for the Texas Instruments PRU application nodes.
>> These are used to configure specific user applications for PRU instances.
>
> Could this be made into a generic remoteproc producer/consumer binding? Or
> are there really things that are specific to the TI PRU that need to be
> handled?
The remoteproc handle and firmware name sound generic enough.
But there are TI PRU specific properties as well which we'll discuss if
they can be made generic.
>
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> [s-anna@ti.com: some binding updates]
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> index 3e5f32f..94c91ee 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
>> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>> +Application/User Nodes
>> +=======================
>
> Are these supposed to be stand-alone platform devices?
>
Yes. The first use case we're going to address is the Ethernet ports on the IDKs.
(Industrial development Kit) http://www.ti.com/tool/TMDXIDK437X
>> +A PRU application/user node typically uses one or more PRU device nodes to
>> +implement a PRU application/functionality. Each application/client node would
>> +need a reference to at least a PRU node, and optionally pass some configuration
>> +parameters.
>
> I thought device tree is not supposed to be used for configuration.
I think we need to word it properly. It is really a hardware/firmware map.
>
>> +
>> +Required Properties:
>> +--------------------
>> +- prus : phandles to the PRU nodes used
>> +
>> +Optional Properties:
>> +--------------------
>> +- firmware-name : firmwares for the PRU cores, the default firmware
>> + for the core from the PRU node will be used if not
>> + provided. The firmware names should correspond to
>> + the PRU cores listed in the 'prus' property
>
> Perhaps this should be a "compatible" property instead of "firmware-name"? The
> driver that matches the compatible string can then set the firmware names.
Compatible property is there to choose the application driver. Should have mentioned
it in Required properties.
It is tricky for the driver to decipher the firmware-name as it needs to support
the same use case on multiple platforms and the firmware name will be different for each.
The driver itself is platform agnostic.
So providing the firmware-name in the DT is the easiest and scalable solution.
>
>> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>> + register for a PRU. This selects the internal muxing
>> + scheme for the PRU instance. If not provided, the
>> + default out-of-reset value (0) for the PRU core is
>> + used. Values should correspond to the PRU cores listed
>> + in the 'prus' property
>
> Is this supposed to be a pinmux? So maybe we should be using pinmux bindings?
We already have pinmux binding for the PRU pins. This GP mux setting is an odd duck.
It provides a way for a set of signals inside the ICSS to be connected to the PRU pins
on the SOC, which are again multiplexed with other SOC pins via the regular pinmux.
Some of the sets are
GPIO mode (0)
EnDAT mode (1)
SD mode (3)
MII2 mode (4)
The application node needs to decide which set it wants to use.
>
>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>> + with each entry consisting of 4 cell-values. First one
>> + is an index towards the "prus" property to identify the
>> + PRU core for the interrupt map, second is the PRU
>> + System Event id, third is the PRU interrupt channel id
>> + and fourth is the PRU host interrupt id. If provided,
>> + this map will supercede any other configuration
>> + provided through firmware
>
> Could this mapping just be cells of the interrupt consumer nodes instead of an
> extra property? As I mentioned in a reply to another patch, unless there is a
> compelling reason to do otherwise, the channel to host mapping can be required
> to be 1:1 as recommended in the TRMs, so that cell can be omitted. Also, since
> the interrupt controller is independent of the PRU cores, the cell specifying the
> index of the PRU core is not needed in this case. The #interrupt-cells already
> includes a cell for the system event number, so this just leaves one cell, the
> host channel, to be added to the #interrupt-cells.
>
> So, instead of:
>
> ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>
> we could have:
>
> interrupt-parent = <&pruss_intc>;
> interrupts = <16 7>, <19 3>;
>
No, interrupts property will be used to provide the actual sysevent IRQs to the
application driver. Below is how the ethernet application node looks like.
pruss2_eth {
compatible = "ti,am57-prueth";
prus = <&pru2_0>, <&pru2_1>;
firmware-name = "ti-pruss/am57xx-pru0-prueth-fw.elf",
"ti-pruss/am57xx-pru1-prueth-fw.elf";
sram = <&ocmcram1>;
interrupt-parent = <&pruss2_intc>;
mii-rt = <&pruss2_mii_rt>;
iep = <&pruss2_iep>;
pruss2_emac0: ethernet-mii0 {
phy-handle = <&pruss2_eth0_phy>;
phy-mode = "mii";
interrupts = <20>, <22>;
interrupt-names = "rx", "tx";
};
pruss2_emac1: ethernet-mii1 {
phy-handle = <&pruss2_eth1_phy>;
phy-mode = "mii";
interrupts = <21>, <23>;
interrupt-names = "rx", "tx";
};
};
You can see that interrupts is providing the RX and TX sysevents.
There needs to be a different way to provide the internal INTC map.
Currently there are 2 ways of providing the INTC map. One is via the
resource table and other is via DT.
> There are also already alternate interrupt bindings that allow for the case
> where there is more than one interrupt-parent.
>
>> +
>> Example:
>> ========
>> 1. /* AM33xx PRU-ICSS */
>> @@ -397,3 +429,14 @@ Example:
>> ...
>> };
>> };
>> +
>> +3: /* PRU application node example */
>> + app_node: app_node {
>> + prus = <&pru1_0>, <&pru1_1>;
>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>> + ti,pruss-gp-mux-sel = <2>, <1>;
>> + /* setup interrupts for prus:
>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>> + }
>>
>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-29 10:07 ` Roger Quadros
@ 2018-11-29 16:33 ` David Lechner
2018-11-30 11:42 ` Roger Quadros
0 siblings, 1 reply; 45+ messages in thread
From: David Lechner @ 2018-11-29 16:33 UTC (permalink / raw)
To: Roger Quadros, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 11/29/18 4:07 AM, Roger Quadros wrote:
> On 27/11/18 01:27, David Lechner wrote:
>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>> From: Tero Kristo <t-kristo@ti.com>
>>>
>>> Add documentation for the Texas Instruments PRU application nodes.
>>> These are used to configure specific user applications for PRU instances.
>>
>> Could this be made into a generic remoteproc producer/consumer binding? Or
>> are there really things that are specific to the TI PRU that need to be
>> handled?
>
> The remoteproc handle and firmware name sound generic enough.
> But there are TI PRU specific properties as well which we'll discuss if
> they can be made generic.
>
>>
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> [s-anna@ti.com: some binding updates]
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
>>> 1 file changed, 43 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>> index 3e5f32f..94c91ee 100644
>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
>>> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>>> +Application/User Nodes
>>> +=======================
>>
>> Are these supposed to be stand-alone platform devices?
>>
>
> Yes. The first use case we're going to address is the Ethernet ports on the IDKs.
> (Industrial development Kit) http://www.ti.com/tool/TMDXIDK437X
>
>>> +A PRU application/user node typically uses one or more PRU device nodes to
>>> +implement a PRU application/functionality. Each application/client node would
>>> +need a reference to at least a PRU node, and optionally pass some configuration
>>> +parameters.
>>
>> I thought device tree is not supposed to be used for configuration.
>
> I think we need to word it properly. It is really a hardware/firmware map.
>>
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- prus : phandles to the PRU nodes used
>>> +
>>> +Optional Properties:
>>> +--------------------
>>> +- firmware-name : firmwares for the PRU cores, the default firmware
>>> + for the core from the PRU node will be used if not
>>> + provided. The firmware names should correspond to
>>> + the PRU cores listed in the 'prus' property
>>
>> Perhaps this should be a "compatible" property instead of "firmware-name"? The
>> driver that matches the compatible string can then set the firmware names.
>
> Compatible property is there to choose the application driver. Should have mentioned
> it in Required properties.
>
> It is tricky for the driver to decipher the firmware-name as it needs to support
> the same use case on multiple platforms and the firmware name will be different for each.
> The driver itself is platform agnostic.
>
> So providing the firmware-name in the DT is the easiest and scalable solution.
>>
>>> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>>> + register for a PRU. This selects the internal muxing
>>> + scheme for the PRU instance. If not provided, the
>>> + default out-of-reset value (0) for the PRU core is
>>> + used. Values should correspond to the PRU cores listed
>>> + in the 'prus' property
>>
>> Is this supposed to be a pinmux? So maybe we should be using pinmux bindings?
>
> We already have pinmux binding for the PRU pins. This GP mux setting is an odd duck.
>
> It provides a way for a set of signals inside the ICSS to be connected to the PRU pins
> on the SOC, which are again multiplexed with other SOC pins via the regular pinmux.
>
> Some of the sets are
>
> GPIO mode (0)
> EnDAT mode (1)
> SD mode (3)
> MII2 mode (4)
>
> The application node needs to decide which set it wants to use.
>
>>
>>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>>> + with each entry consisting of 4 cell-values. First one
>>> + is an index towards the "prus" property to identify the
>>> + PRU core for the interrupt map, second is the PRU
>>> + System Event id, third is the PRU interrupt channel id
>>> + and fourth is the PRU host interrupt id. If provided,
>>> + this map will supercede any other configuration
>>> + provided through firmware
>>
>> Could this mapping just be cells of the interrupt consumer nodes instead of an
>> extra property? As I mentioned in a reply to another patch, unless there is a
>> compelling reason to do otherwise, the channel to host mapping can be required
>> to be 1:1 as recommended in the TRMs, so that cell can be omitted. Also, since
>> the interrupt controller is independent of the PRU cores, the cell specifying the
>> index of the PRU core is not needed in this case. The #interrupt-cells already
>> includes a cell for the system event number, so this just leaves one cell, the
>> host channel, to be added to the #interrupt-cells.
>>
>> So, instead of:
>>
>> ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>
>> we could have:
>>
>> interrupt-parent = <&pruss_intc>;
>> interrupts = <16 7>, <19 3>;
>>
>
> No, interrupts property will be used to provide the actual sysevent IRQs to the
> application driver. Below is how the ethernet application node looks like.
>
> pruss2_eth {
> compatible = "ti,am57-prueth";
> prus = <&pru2_0>, <&pru2_1>;
> firmware-name = "ti-pruss/am57xx-pru0-prueth-fw.elf",
> "ti-pruss/am57xx-pru1-prueth-fw.elf";
> sram = <&ocmcram1>;
> interrupt-parent = <&pruss2_intc>;
> mii-rt = <&pruss2_mii_rt>;
> iep = <&pruss2_iep>;
>
> pruss2_emac0: ethernet-mii0 {
> phy-handle = <&pruss2_eth0_phy>;
> phy-mode = "mii";
> interrupts = <20>, <22>;
> interrupt-names = "rx", "tx";
> };
>
> pruss2_emac1: ethernet-mii1 {
> phy-handle = <&pruss2_eth1_phy>;
> phy-mode = "mii";
> interrupts = <21>, <23>;
> interrupt-names = "rx", "tx";
> };
> };
>
> You can see that interrupts is providing the RX and TX sysevents.
>
> There needs to be a different way to provide the internal INTC map.
>
> Currently there are 2 ways of providing the INTC map. One is via the
> resource table and other is via DT.
>
>> There are also already alternate interrupt bindings that allow for the case
>> where there is more than one interrupt-parent.
Thanks for the insights. On the example above there is not a
ti,pru-interrupt-map property. Does this mean that the interrupt
mapping table comes from the firmware/resource-table in this case?
>>
>>> +
>>> Example:
>>> ========
>>> 1. /* AM33xx PRU-ICSS */
>>> @@ -397,3 +429,14 @@ Example:
>>> ...
>>> };
>>> };
>>> +
>>> +3: /* PRU application node example */
>>> + app_node: app_node {
>>> + prus = <&pru1_0>, <&pru1_1>;
>>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>> + ti,pruss-gp-mux-sel = <2>, <1>;
>>> + /* setup interrupts for prus:
>>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>> + }
>>>
>>
>
> cheers,
> -roger
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-29 16:33 ` David Lechner
@ 2018-11-30 11:42 ` Roger Quadros
0 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-30 11:42 UTC (permalink / raw)
To: David Lechner, ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree
On 29/11/18 18:33, David Lechner wrote:
> On 11/29/18 4:07 AM, Roger Quadros wrote:
>> On 27/11/18 01:27, David Lechner wrote:
>>> On 11/26/18 1:52 AM, Roger Quadros wrote:
>>>> From: Tero Kristo <t-kristo@ti.com>
>>>>
>>>> Add documentation for the Texas Instruments PRU application nodes.
>>>> These are used to configure specific user applications for PRU instances.
>>>
>>> Could this be made into a generic remoteproc producer/consumer binding? Or
>>> are there really things that are specific to the TI PRU that need to be
>>> handled?
>>
>> The remoteproc handle and firmware name sound generic enough.
>> But there are TI PRU specific properties as well which we'll discuss if
>> they can be made generic.
>>
>>>
>>>>
>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>> [s-anna@ti.com: some binding updates]
>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
>>>> 1 file changed, 43 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>>> index 3e5f32f..94c91ee 100644
>>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>>> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
>>>> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>>>> +Application/User Nodes
>>>> +=======================
>>>
>>> Are these supposed to be stand-alone platform devices?
>>>
>>
>> Yes. The first use case we're going to address is the Ethernet ports on the IDKs.
>> (Industrial development Kit) http://www.ti.com/tool/TMDXIDK437X
>>
>>>> +A PRU application/user node typically uses one or more PRU device nodes to
>>>> +implement a PRU application/functionality. Each application/client node would
>>>> +need a reference to at least a PRU node, and optionally pass some configuration
>>>> +parameters.
>>>
>>> I thought device tree is not supposed to be used for configuration.
>>
>> I think we need to word it properly. It is really a hardware/firmware map.
>>>
>>>> +
>>>> +Required Properties:
>>>> +--------------------
>>>> +- prus : phandles to the PRU nodes used
>>>> +
>>>> +Optional Properties:
>>>> +--------------------
>>>> +- firmware-name : firmwares for the PRU cores, the default firmware
>>>> + for the core from the PRU node will be used if not
>>>> + provided. The firmware names should correspond to
>>>> + the PRU cores listed in the 'prus' property
>>>
>>> Perhaps this should be a "compatible" property instead of "firmware-name"? The
>>> driver that matches the compatible string can then set the firmware names.
>>
>> Compatible property is there to choose the application driver. Should have mentioned
>> it in Required properties.
>>
>> It is tricky for the driver to decipher the firmware-name as it needs to support
>> the same use case on multiple platforms and the firmware name will be different for each.
>> The driver itself is platform agnostic.
>>
>> So providing the firmware-name in the DT is the easiest and scalable solution.
>>>
>>>> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>>>> + register for a PRU. This selects the internal muxing
>>>> + scheme for the PRU instance. If not provided, the
>>>> + default out-of-reset value (0) for the PRU core is
>>>> + used. Values should correspond to the PRU cores listed
>>>> + in the 'prus' property
>>>
>>> Is this supposed to be a pinmux? So maybe we should be using pinmux bindings?
>>
>> We already have pinmux binding for the PRU pins. This GP mux setting is an odd duck.
>>
>> It provides a way for a set of signals inside the ICSS to be connected to the PRU pins
>> on the SOC, which are again multiplexed with other SOC pins via the regular pinmux.
>>
>> Some of the sets are
>>
>> GPIO mode (0)
>> EnDAT mode (1)
>> SD mode (3)
>> MII2 mode (4)
>>
>> The application node needs to decide which set it wants to use.
>>
>>>
>>>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>>>> + with each entry consisting of 4 cell-values. First one
>>>> + is an index towards the "prus" property to identify the
>>>> + PRU core for the interrupt map, second is the PRU
>>>> + System Event id, third is the PRU interrupt channel id
>>>> + and fourth is the PRU host interrupt id. If provided,
>>>> + this map will supercede any other configuration
>>>> + provided through firmware
>>>
>>> Could this mapping just be cells of the interrupt consumer nodes instead of an
>>> extra property? As I mentioned in a reply to another patch, unless there is a
>>> compelling reason to do otherwise, the channel to host mapping can be required
>>> to be 1:1 as recommended in the TRMs, so that cell can be omitted. Also, since
>>> the interrupt controller is independent of the PRU cores, the cell specifying the
>>> index of the PRU core is not needed in this case. The #interrupt-cells already
>>> includes a cell for the system event number, so this just leaves one cell, the
>>> host channel, to be added to the #interrupt-cells.
>>>
>>> So, instead of:
>>>
>>> ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>>
>>> we could have:
>>>
>>> interrupt-parent = <&pruss_intc>;
>>> interrupts = <16 7>, <19 3>;
>>>
>>
>> No, interrupts property will be used to provide the actual sysevent IRQs to the
>> application driver. Below is how the ethernet application node looks like.
>>
>> pruss2_eth {
>> compatible = "ti,am57-prueth";
>> prus = <&pru2_0>, <&pru2_1>;
>> firmware-name = "ti-pruss/am57xx-pru0-prueth-fw.elf",
>> "ti-pruss/am57xx-pru1-prueth-fw.elf";
>> sram = <&ocmcram1>;
>> interrupt-parent = <&pruss2_intc>;
>> mii-rt = <&pruss2_mii_rt>;
>> iep = <&pruss2_iep>;
>>
>> pruss2_emac0: ethernet-mii0 {
>> phy-handle = <&pruss2_eth0_phy>;
>> phy-mode = "mii";
>> interrupts = <20>, <22>;
>> interrupt-names = "rx", "tx";
>> };
>>
>> pruss2_emac1: ethernet-mii1 {
>> phy-handle = <&pruss2_eth1_phy>;
>> phy-mode = "mii";
>> interrupts = <21>, <23>;
>> interrupt-names = "rx", "tx";
>> };
>> };
>>
>> You can see that interrupts is providing the RX and TX sysevents.
>>
>> There needs to be a different way to provide the internal INTC map.
>>
>> Currently there are 2 ways of providing the INTC map. One is via the
>> resource table and other is via DT.
>>
>>> There are also already alternate interrupt bindings that allow for the case
>>> where there is more than one interrupt-parent.
>
> Thanks for the insights. On the example above there is not a
> ti,pru-interrupt-map property. Does this mean that the interrupt
> mapping table comes from the firmware/resource-table in this case?
>
Yes, that's correct.
>>>
>>>> +
>>>> Example:
>>>> ========
>>>> 1. /* AM33xx PRU-ICSS */
>>>> @@ -397,3 +429,14 @@ Example:
>>>> ...
>>>> };
>>>> };
>>>> +
>>>> +3: /* PRU application node example */
>>>> + app_node: app_node {
>>>> + prus = <&pru1_0>, <&pru1_1>;
>>>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>>> + ti,pruss-gp-mux-sel = <2>, <1>;
>>>> + /* setup interrupts for prus:
>>>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>>> + }
>>>>
>>>
>>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-11-26 7:52 ` [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings Roger Quadros
2018-11-26 23:27 ` David Lechner
@ 2018-12-11 22:06 ` Rob Herring
2018-12-17 16:03 ` Roger Quadros
1 sibling, 1 reply; 45+ messages in thread
From: Rob Herring @ 2018-12-11 22:06 UTC (permalink / raw)
To: Roger Quadros
Cc: ohad, bjorn.andersson, tony, bcousson, ssantosh, s-anna, nsekhar,
t-kristo, nsaulnier, jreeder, m-karicheri2, woods.technical,
linux-omap, linux-remoteproc, linux-kernel, devicetree
On Mon, Nov 26, 2018 at 09:52:45AM +0200, Roger Quadros wrote:
> From: Tero Kristo <t-kristo@ti.com>
>
> Add documentation for the Texas Instruments PRU application nodes.
> These are used to configure specific user applications for PRU instances.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> [s-anna@ti.com: some binding updates]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> index 3e5f32f..94c91ee 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>
>
> +Application/User Nodes
> +=======================
> +A PRU application/user node typically uses one or more PRU device nodes to
> +implement a PRU application/functionality. Each application/client node would
> +need a reference to at least a PRU node, and optionally pass some configuration
> +parameters.
> +
> +Required Properties:
> +--------------------
> +- prus : phandles to the PRU nodes used
> +
> +Optional Properties:
> +--------------------
> +- firmware-name : firmwares for the PRU cores, the default firmware
> + for the core from the PRU node will be used if not
> + provided. The firmware names should correspond to
> + the PRU cores listed in the 'prus' property
> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
> + register for a PRU. This selects the internal muxing
> + scheme for the PRU instance. If not provided, the
> + default out-of-reset value (0) for the PRU core is
> + used. Values should correspond to the PRU cores listed
> + in the 'prus' property
> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
> + with each entry consisting of 4 cell-values. First one
> + is an index towards the "prus" property to identify the
> + PRU core for the interrupt map, second is the PRU
> + System Event id, third is the PRU interrupt channel id
> + and fourth is the PRU host interrupt id. If provided,
> + this map will supercede any other configuration
> + provided through firmware
Can't you use 'interrupt-map' or use more cells for the PRU intc cells.
Or use interrupts-extended if you need more than 1 parent.
> +
> Example:
> ========
> 1. /* AM33xx PRU-ICSS */
> @@ -397,3 +429,14 @@ Example:
> ...
> };
> };
> +
> +3: /* PRU application node example */
> + app_node: app_node {
> + prus = <&pru1_0>, <&pru1_1>;
> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
> + ti,pruss-gp-mux-sel = <2>, <1>;
> + /* setup interrupts for prus:
> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
> + }
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings
2018-12-11 22:06 ` Rob Herring
@ 2018-12-17 16:03 ` Roger Quadros
0 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-12-17 16:03 UTC (permalink / raw)
To: Rob Herring
Cc: ohad, bjorn.andersson, tony, bcousson, ssantosh, s-anna, nsekhar,
t-kristo, nsaulnier, jreeder, m-karicheri2, woods.technical,
linux-omap, linux-remoteproc, linux-kernel, devicetree
On 12/12/18 00:06, Rob Herring wrote:
> On Mon, Nov 26, 2018 at 09:52:45AM +0200, Roger Quadros wrote:
>> From: Tero Kristo <t-kristo@ti.com>
>>
>> Add documentation for the Texas Instruments PRU application nodes.
>> These are used to configure specific user applications for PRU instances.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> [s-anna@ti.com: some binding updates]
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> .../devicetree/bindings/soc/ti/ti,pruss.txt | 43 ++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> index 3e5f32f..94c91ee 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> @@ -210,6 +210,38 @@ used in TI Davinci SoCs. Please refer to the corresponding binding document,
>> Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>>
>>
>> +Application/User Nodes
>> +=======================
>> +A PRU application/user node typically uses one or more PRU device nodes to
>> +implement a PRU application/functionality. Each application/client node would
>> +need a reference to at least a PRU node, and optionally pass some configuration
>> +parameters.
>> +
>> +Required Properties:
>> +--------------------
>> +- prus : phandles to the PRU nodes used
>> +
>> +Optional Properties:
>> +--------------------
>> +- firmware-name : firmwares for the PRU cores, the default firmware
>> + for the core from the PRU node will be used if not
>> + provided. The firmware names should correspond to
>> + the PRU cores listed in the 'prus' property
>> +- ti,pruss-gp-mux-sel : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>> + register for a PRU. This selects the internal muxing
>> + scheme for the PRU instance. If not provided, the
>> + default out-of-reset value (0) for the PRU core is
>> + used. Values should correspond to the PRU cores listed
>> + in the 'prus' property
>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>> + with each entry consisting of 4 cell-values. First one
>> + is an index towards the "prus" property to identify the
>> + PRU core for the interrupt map, second is the PRU
>> + System Event id, third is the PRU interrupt channel id
>> + and fourth is the PRU host interrupt id. If provided,
>> + this map will supercede any other configuration
>> + provided through firmware
>
> Can't you use 'interrupt-map' or use more cells for the PRU intc cells.
> Or use interrupts-extended if you need more than 1 parent.
>
We don't need more then one parent.
Using more cells for PRU INTC sounds like doable. Although we should be able to support
2 formats (i.e. 4 cells when mapping is provided in DT and 1 cell when mapping information
comes from firmware via resource table)
I'm not sure how 'interrupt-map' can be used as we're not really translating between
2 interrupt domains.
>> +
>> Example:
>> ========
>> 1. /* AM33xx PRU-ICSS */
>> @@ -397,3 +429,14 @@ Example:
>> ...
>> };
>> };
>> +
>> +3: /* PRU application node example */
>> + app_node: app_node {
>> + prus = <&pru1_0>, <&pru1_1>;
>> + firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>> + ti,pruss-gp-mux-sel = <2>, <1>;
>> + /* setup interrupts for prus:
>> + prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>> + prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>> + ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>> + }
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 13/16] remoteproc/pru: add support for configuring GPMUX based on client setup
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (11 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 12/16] dt-bindings: remoteproc: ti-pruss: Document application node bindings Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 14/16] remoteproc/pru: configure firmware " Roger Quadros
` (2 subsequent siblings)
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Tero Kristo <t-kristo@ti.com>
Client device node property ti,pruss-gp-mux-sel can now be used to
configure the GPMUX config value for PRU.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
[s-anna@ti.com: simplify the pru id usage]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index d8b823d..9a08937 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -72,6 +72,7 @@ enum pru_mem {
* @sdram_da: device address of secondary Data RAM for this PRU
* @shrdram_da: device address of shared Data RAM
* @fw_name: name of firmware image used during loading
+ * @gpmux_save: saved value for gpmux config
* @lock: mutex to protect client usage
* @dbg_single_step: debug state variable to set PRU into single step mode
* @dbg_continuous: debug state variable to restore PRU execution mode
@@ -95,6 +96,7 @@ struct pru_rproc {
u32 sdram_da;
u32 shrdram_da;
const char *fw_name;
+ u8 gpmux_save;
struct mutex lock; /* client access lock */
u32 dbg_single_step;
u32 dbg_continuous;
@@ -184,12 +186,16 @@ struct rproc *pru_rproc_get(struct device_node *np, int index)
{
struct rproc *rproc;
struct pru_rproc *pru;
+ struct device *dev;
+ int ret;
+ u32 mux;
rproc = __pru_rproc_get(np, index);
if (IS_ERR(rproc))
return rproc;
pru = rproc->priv;
+ dev = &rproc->dev;
mutex_lock(&pru->lock);
@@ -203,7 +209,27 @@ struct rproc *pru_rproc_get(struct device_node *np, int index)
mutex_unlock(&pru->lock);
+ ret = pruss_cfg_get_gpmux(pru->pruss, pru->id, &pru->gpmux_save);
+ if (ret) {
+ dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
+ goto err;
+ }
+
+ ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
+ &mux);
+ if (!ret) {
+ ret = pruss_cfg_set_gpmux(pru->pruss, pru->id, mux);
+ if (ret) {
+ dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
+ goto err;
+ }
+ }
+
return rproc;
+
+err:
+ pru_rproc_put(rproc);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(pru_rproc_get);
@@ -229,6 +255,8 @@ void pru_rproc_put(struct rproc *rproc)
if (!pru->client_np)
return;
+ pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
+
mutex_lock(&pru->lock);
pru->client_np = NULL;
mutex_unlock(&pru->lock);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 14/16] remoteproc/pru: configure firmware based on client setup
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (12 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 13/16] remoteproc/pru: add support for configuring GPMUX based on client setup Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 15/16] remoteproc/pru: add support for parsing pru interrupt mapping from DT Roger Quadros
2018-11-26 7:52 ` [PATCH 16/16] remoteproc/pru: Add support for INTC Interrupt map resource Roger Quadros
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Tero Kristo <t-kristo@ti.com>
Client device node property firmware-name is now used to configure
firmware for the PRU instances. The default firmware is also
restored once releasing the PRU resource.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
Reviewed-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/remoteproc/pru_rproc.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 9a08937..84f006b 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -132,6 +132,21 @@ void pru_control_set_reg(struct pru_rproc *pru, unsigned int reg,
spin_unlock_irqrestore(&pru->rmw_lock, flags);
}
+/**
+ * pru_rproc_set_firmware() - set firmware for a pru core
+ * @rproc: the rproc instance of the PRU
+ * @fw_name: the new firmware name, or NULL if default is desired
+ */
+static int pru_rproc_set_firmware(struct rproc *rproc, const char *fw_name)
+{
+ struct pru_rproc *pru = rproc->priv;
+
+ if (!fw_name)
+ fw_name = pru->fw_name;
+
+ return rproc_set_firmware(rproc, fw_name);
+}
+
static struct rproc *__pru_rproc_get(struct device_node *np, int index)
{
struct device_node *rproc_np = NULL;
@@ -189,6 +204,7 @@ struct rproc *pru_rproc_get(struct device_node *np, int index)
struct device *dev;
int ret;
u32 mux;
+ const char *fw_name;
rproc = __pru_rproc_get(np, index);
if (IS_ERR(rproc))
@@ -225,6 +241,16 @@ struct rproc *pru_rproc_get(struct device_node *np, int index)
}
}
+ ret = of_property_read_string_index(np, "firmware-name", index,
+ &fw_name);
+ if (!ret) {
+ ret = pru_rproc_set_firmware(rproc, fw_name);
+ if (ret) {
+ dev_err(dev, "failed to set firmware: %d\n", ret);
+ goto err;
+ }
+ }
+
return rproc;
err:
@@ -255,6 +281,7 @@ void pru_rproc_put(struct rproc *rproc)
if (!pru->client_np)
return;
+ pru_rproc_set_firmware(rproc, NULL);
pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
mutex_lock(&pru->lock);
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 15/16] remoteproc/pru: add support for parsing pru interrupt mapping from DT
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (13 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 14/16] remoteproc/pru: configure firmware " Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
2018-11-26 7:52 ` [PATCH 16/16] remoteproc/pru: Add support for INTC Interrupt map resource Roger Quadros
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
From: Tero Kristo <t-kristo@ti.com>
PRU interrupt mapping can now be parsed from devicetree also, from
ti,pru-interrupt-map property. This is an alternative configuration
method in addition to the legacy resource table config. If both are
provided, the config in DT takes precedence.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
[s-anna@ti.com: various fixes and cleanups]
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/remoteproc/pru_rproc.c | 109 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 106 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 84f006b..540cce3 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -63,6 +63,7 @@ enum pru_mem {
* @irq_ring: IRQ number to use for processing vring buffers
* @irq_kick: IRQ number to use to perform virtio kick
* @mem_regions: data for each of the PRU memory regions
+ * @intc_config: PRU INTC configuration data
* @dram0: PRUSS DRAM0 region
* @dram1: PRUSS DRAM1 region
* @shrdram: PRUSS SHARED RAM region
@@ -73,6 +74,7 @@ enum pru_mem {
* @shrdram_da: device address of shared Data RAM
* @fw_name: name of firmware image used during loading
* @gpmux_save: saved value for gpmux config
+ * @dt_irqs: number of irqs configured from DT
* @lock: mutex to protect client usage
* @dbg_single_step: debug state variable to set PRU into single step mode
* @dbg_continuous: debug state variable to restore PRU execution mode
@@ -87,6 +89,7 @@ struct pru_rproc {
int irq_vring;
int irq_kick;
struct pruss_mem_region mem_regions[PRU_MEM_MAX];
+ struct pruss_intc_config intc_config;
struct pruss_mem_region dram0;
struct pruss_mem_region dram1;
struct pruss_mem_region shrdram;
@@ -97,6 +100,7 @@ struct pru_rproc {
u32 shrdram_da;
const char *fw_name;
u8 gpmux_save;
+ int dt_irqs;
struct mutex lock; /* client access lock */
u32 dbg_single_step;
u32 dbg_continuous;
@@ -180,6 +184,87 @@ static struct rproc *__pru_rproc_get(struct device_node *np, int index)
return rproc;
}
+static int pru_get_intc_dt_config(struct device *dev, const char *propname,
+ int index,
+ struct pruss_intc_config *intc_config)
+{
+ struct device_node *np = dev->of_node;
+ struct property *prop;
+ int ret = 0, entries, i;
+ int dt_irqs = 0;
+ u32 *arr;
+ int max_system_events, max_pru_channels, max_pru_host_ints;
+
+ max_system_events = MAX_PRU_SYS_EVENTS;
+ max_pru_channels = MAX_PRU_CHANNELS;
+ max_pru_host_ints = MAX_PRU_CHANNELS;
+
+ prop = of_find_property(np, propname, NULL);
+ if (!prop)
+ return 0;
+
+ entries = of_property_count_u32_elems(np, propname);
+ if (entries <= 0 || entries % 4)
+ return -EINVAL;
+
+ arr = kmalloc_array(entries, sizeof(u32), GFP_KERNEL);
+ if (!arr)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(np, propname, arr, entries);
+ if (ret)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(intc_config->sysev_to_ch); i++)
+ intc_config->sysev_to_ch[i] = -1;
+
+ for (i = 0; i < ARRAY_SIZE(intc_config->ch_to_host); i++)
+ intc_config->ch_to_host[i] = -1;
+
+ for (i = 0; i < entries; i += 4) {
+ if (arr[i] != index)
+ continue;
+
+ if (arr[i + 1] < 0 ||
+ arr[i + 1] >= max_system_events) {
+ dev_dbg(dev, "bad sys event %d\n", arr[i + 1]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (arr[i + 2] < 0 ||
+ arr[i + 2] >= max_pru_channels) {
+ dev_dbg(dev, "bad channel %d\n", arr[i + 2]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (arr[i + 3] < 0 ||
+ arr[i + 3] >= max_pru_host_ints) {
+ dev_dbg(dev, "bad irq %d\n", arr[i + 3]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ intc_config->sysev_to_ch[arr[i + 1]] = arr[i + 2];
+ dev_dbg(dev, "sysevt-to-ch[%d] -> %d\n", arr[i + 1],
+ arr[i + 2]);
+
+ intc_config->ch_to_host[arr[i + 2]] = arr[i + 3];
+ dev_dbg(dev, "chnl-to-host[%d] -> %d\n", arr[i + 2],
+ arr[i + 3]);
+
+ dt_irqs++;
+ }
+
+ kfree(arr);
+ return dt_irqs;
+
+err:
+ kfree(arr);
+ return ret;
+}
+
/**
* pru_rproc_get() - get the PRU rproc instance from a device node
* @np: the user/client device node
@@ -251,6 +336,15 @@ struct rproc *pru_rproc_get(struct device_node *np, int index)
}
}
+ ret = pru_get_intc_dt_config(dev, "ti,pru-interrupt-map",
+ index, &pru->intc_config);
+ if (ret < 0) {
+ dev_err(dev, "error getting DT interrupt map: %d\n", ret);
+ goto err;
+ }
+
+ pru->dt_irqs = ret;
+
return rproc;
err:
@@ -568,7 +662,13 @@ static int pru_rproc_start(struct rproc *rproc)
dev_dbg(dev, "starting PRU%d: entry-point = 0x%x\n",
pru->id, (rproc->bootaddr >> 2));
- /* TODO: INTC setup */
+ if (pru->dt_irqs) {
+ ret = pruss_intc_configure(pru->pruss, &pru->intc_config);
+ if (ret) {
+ dev_err(dev, "failed to configure intc %d\n", ret);
+ return ret;
+ }
+ }
if (!list_empty(&pru->rproc->rvdevs)) {
if (!pru->mbox && (pru->irq_vring <= 0 || pru->irq_kick <= 0)) {
@@ -596,7 +696,8 @@ static int pru_rproc_start(struct rproc *rproc)
return 0;
fail:
- /* TODO: INTC cleanup */
+ if (pru->dt_irqs)
+ pruss_intc_unconfigure(pru->pruss, &pru->intc_config);
return ret;
}
@@ -618,7 +719,9 @@ static int pru_rproc_stop(struct rproc *rproc)
!pru->mbox && pru->irq_vring > 0)
free_irq(pru->irq_vring, pru);
- /* TODO: INTC cleanup */
+ /* undo INTC config */
+ if (pru->dt_irqs)
+ pruss_intc_unconfigure(pru->pruss, &pru->intc_config);
return 0;
}
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 16/16] remoteproc/pru: Add support for INTC Interrupt map resource
2018-11-26 7:52 [PATCH 00/16] remoteproc: Add support for TI PRU Roger Quadros
` (14 preceding siblings ...)
2018-11-26 7:52 ` [PATCH 15/16] remoteproc/pru: add support for parsing pru interrupt mapping from DT Roger Quadros
@ 2018-11-26 7:52 ` Roger Quadros
15 siblings, 0 replies; 45+ messages in thread
From: Roger Quadros @ 2018-11-26 7:52 UTC (permalink / raw)
To: ohad, bjorn.andersson
Cc: tony, robh+dt, bcousson, ssantosh, s-anna, nsekhar, t-kristo,
nsaulnier, jreeder, m-karicheri2, woods.technical, linux-omap,
linux-remoteproc, linux-kernel, devicetree, rogerq
Use the vendor specific resource mechanism to get the
INTC mapping details from firmware.
pru-software-support-package [1] has been historically using
version 0 for this. However, the data structure is not scaleable
and is not self sufficient.
1) it hard codes number of channel to host mappings so is not
scaleable to newer SoCs than have more of these.
2) it does not contain the event to channel mappings within
itself but relies on a pointer to point to another section
in data memory. This causes a weird complication that the
respective data section must be loaded before we can really
use the INTC map.
With this patch we drop support for version 0 and support
version 1 which is a more robust and scalable data structure.
It should be able to support a sufficiently large number (255) of
sysevents, channels and host interrupts and is self contained
so it can be used without dependency on order of loading sections.
[1] git://git.ti.com/pru-software-support-package/pru-software-support-package.git
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/remoteproc/pru_rproc.c | 110 +++++++++++++++++++++++++++++++++++++++--
drivers/remoteproc/pru_rproc.h | 48 +++++++-----------
2 files changed, 126 insertions(+), 32 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 540cce3..9e22c70 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -75,6 +75,7 @@ enum pru_mem {
* @fw_name: name of firmware image used during loading
* @gpmux_save: saved value for gpmux config
* @dt_irqs: number of irqs configured from DT
+ * @fw_irqs: number of irqs configured from FW
* @lock: mutex to protect client usage
* @dbg_single_step: debug state variable to set PRU into single step mode
* @dbg_continuous: debug state variable to restore PRU execution mode
@@ -101,6 +102,7 @@ struct pru_rproc {
const char *fw_name;
u8 gpmux_save;
int dt_irqs;
+ int fw_irqs;
struct mutex lock; /* client access lock */
u32 dbg_single_step;
u32 dbg_continuous;
@@ -651,6 +653,107 @@ static void pru_rproc_kick(struct rproc *rproc, int vq_id)
}
}
+/*
+ * parse the custom interrupt map resource and save the intc_config
+ * for use when booting the processor.
+ */
+static int pru_handle_vendor_intrmap(struct rproc *rproc,
+ struct fw_rsc_pruss_intrmap *rsc)
+{
+ int fw_irqs = 0, i, ret = 0;
+ u8 *arr;
+ struct device *dev = &rproc->dev;
+ struct pru_rproc *pru = rproc->priv;
+
+ dev_dbg(dev, "vendor rsc intc: version %d\n", rsc->version);
+
+ /*
+ * 0 was prototyping version. Not supported.
+ * 1 is currently supported version.
+ */
+ if (rsc->version == 0 || rsc->version > 1) {
+ dev_err(dev, "Unsupported version %d\n", rsc->version);
+ return -EINVAL;
+ }
+
+ /* DT provided INTC config takes precedence */
+ if (pru->dt_irqs) {
+ dev_info(dev, "INTC config in DT and FW. Using DT config.\n");
+ return 0;
+ }
+
+ arr = rsc->data;
+
+ for (i = 0; i < ARRAY_SIZE(pru->intc_config.sysev_to_ch); i++)
+ pru->intc_config.sysev_to_ch[i] = -1;
+
+ for (i = 0; i < ARRAY_SIZE(pru->intc_config.ch_to_host); i++)
+ pru->intc_config.ch_to_host[i] = -1;
+
+ for (i = 0; i < rsc->num_maps * 3; i += 3) {
+ if (arr[i] < 0 ||
+ arr[i] >= MAX_PRU_SYS_EVENTS) {
+ dev_err(dev, "bad sys event %d\n", arr[i]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (arr[i + 1] < 0 ||
+ arr[i + 1] >= MAX_PRU_CHANNELS) {
+ dev_err(dev, "bad channel %d\n", arr[i + 1]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (arr[i + 2] < 0 ||
+ arr[i + 2] >= MAX_PRU_CHANNELS) {
+ dev_err(dev, "bad host irq %d\n", arr[i + 2]);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ pru->intc_config.sysev_to_ch[arr[i]] = arr[i + 1];
+ dev_dbg(dev, "sysevt-to-ch[%d] -> %d\n", arr[i],
+ arr[i + 1]);
+
+ pru->intc_config.ch_to_host[arr[i + 1]] = arr[i + 2];
+ dev_dbg(dev, "chnl-to-host[%d] -> %d\n", arr[i + 1],
+ arr[i + 2]);
+
+ fw_irqs++;
+ }
+
+ pru->fw_irqs = fw_irqs;
+ return 0;
+
+err:
+ pru->fw_irqs = 0;
+ return ret;
+}
+
+/* PRU-specific vendor resource handler */
+static int pru_rproc_handle_vendor_rsc(struct rproc *rproc,
+ struct fw_rsc_vendor *ven_rsc)
+{
+ struct device *dev = rproc->dev.parent;
+ int ret = -EINVAL;
+
+ struct fw_rsc_pruss_intrmap *rsc;
+
+ rsc = (struct fw_rsc_pruss_intrmap *)ven_rsc->data;
+
+ switch (rsc->type) {
+ case PRUSS_RSC_INTRS:
+ ret = pru_handle_vendor_intrmap(rproc, rsc);
+ break;
+ default:
+ dev_err(dev, "%s: cannot handle unknown type %d\n", __func__,
+ rsc->type);
+ }
+
+ return ret;
+}
+
/* start a PRU core */
static int pru_rproc_start(struct rproc *rproc)
{
@@ -662,7 +765,7 @@ static int pru_rproc_start(struct rproc *rproc)
dev_dbg(dev, "starting PRU%d: entry-point = 0x%x\n",
pru->id, (rproc->bootaddr >> 2));
- if (pru->dt_irqs) {
+ if (pru->dt_irqs || pru->fw_irqs) {
ret = pruss_intc_configure(pru->pruss, &pru->intc_config);
if (ret) {
dev_err(dev, "failed to configure intc %d\n", ret);
@@ -696,7 +799,7 @@ static int pru_rproc_start(struct rproc *rproc)
return 0;
fail:
- if (pru->dt_irqs)
+ if (pru->dt_irqs || pru->fw_irqs)
pruss_intc_unconfigure(pru->pruss, &pru->intc_config);
return ret;
@@ -720,7 +823,7 @@ static int pru_rproc_stop(struct rproc *rproc)
free_irq(pru->irq_vring, pru);
/* undo INTC config */
- if (pru->dt_irqs)
+ if (pru->dt_irqs || pru->fw_irqs)
pruss_intc_unconfigure(pru->pruss, &pru->intc_config);
return 0;
@@ -815,6 +918,7 @@ static struct rproc_ops pru_rproc_ops = {
.stop = pru_rproc_stop,
.kick = pru_rproc_kick,
.da_to_va = pru_da_to_va,
+ .handle_vendor_rsc = pru_rproc_handle_vendor_rsc,
};
static int pru_rproc_set_id(struct pru_rproc *pru)
diff --git a/drivers/remoteproc/pru_rproc.h b/drivers/remoteproc/pru_rproc.h
index 35240e9..a9413f6 100644
--- a/drivers/remoteproc/pru_rproc.h
+++ b/drivers/remoteproc/pru_rproc.h
@@ -23,43 +23,33 @@ enum pruss_rsc_types {
PRUSS_RSC_MAX = 2,
};
-/**
- * struct pruss_event_chnl - PRU system events _to_ channel mapping
- * @event: number of the system event
- * @chnl: channel number assigned to a given @event
- *
- * PRU system events are mapped to channels, and these channels are mapped
- * to host interrupts. Events can be mapped to channels in a one-to-one or
- * many-to-one ratio (multiple events per channel), and channels can be
- * mapped to host interrupts in a one-to-one or many-to-one ratio (multiple
- * channels per interrupt).
- *
- */
-struct pruss_event_chnl {
- s8 event;
- s8 chnl;
-};
/**
- * struct fw_rsc_custom_intrmap - custom resource to define PRU interrupts
- * @reserved: reserved field providing padding and alignment
- * @chnl_host_intr_map: array of PRU channels to host interrupt mappings
- * @event_chnl_map_size: number of event_channel mappings defined in
- * @event_chnl_map_addr
- * @event_chnl_map_addr: PRU device address of pointer to array of events to
- * channel mappings (struct pruss_event_chnl elements)
+ * struct fw_rsc_pruss_intrmap - vendor resource to define PRU interrupts
+ * @type: should be PRUSS_RSC_INTRS
+ * @version: should be 1 or greater. 0 was for prototyping and is not supported
+ * @num_maps: number of interrupt mappings that follow
+ * @data: Array of 'num_maps' mappings.
+ * Each mapping is a triplet {s, c, h}
+ * s - system event id
+ * c - channel id
+ * h - host interrupt id
*
* PRU system events are mapped to channels, and these channels are mapped
* to host interrupts. Events can be mapped to channels in a one-to-one or
* many-to-one ratio (multiple events per channel), and channels can be
* mapped to host interrupts in a one-to-one or many-to-one ratio (multiple
* channels per interrupt).
+ *
+ * This resource is variable length due to the nature of INTC map.
+ * The below data structure is scalable so it can support sufficiently
+ * large number of sysevents and hosts.
*/
-struct fw_rsc_custom_intrmap {
- u16 reserved;
- s8 chnl_host_intr_map[10];
- u32 event_chnl_map_size;
- u32 event_chnl_map_addr;
-};
+struct fw_rsc_pruss_intrmap {
+ u16 type;
+ u16 version;
+ u8 num_maps;
+ u8 data[];
+} __packed;
#endif /* _PRU_REMOTEPROC_H_ */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related [flat|nested] 45+ messages in thread