Re: [PATCH] ARM: dts: keystone-k2g: Rename message-manager node
On 4/13/21 7:19 PM, Nishanth Menon wrote:
> Rename message-manager instance node name to be better aligned with
> current style of device tree nodes for mailboxes.
>
> Signed-off-by: Nishanth Menon
Acked-by: Suman Anna
> ---
>
> Santosh:
> - This is'nt critical to queue up for 5.13-rc1 window, but just getting
> it out of the way. I noticed it as I was converting the binding to
> yaml. (patch for yaml conversion for the node follows)
>
> arch/arm/boot/dts/keystone-k2g.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/keystone-k2g.dtsi
> b/arch/arm/boot/dts/keystone-k2g.dtsi
> index 05a75019275e..e5c813b5556d 100644
> --- a/arch/arm/boot/dts/keystone-k2g.dtsi
> +++ b/arch/arm/boot/dts/keystone-k2g.dtsi
> @@ -242,7 +242,7 @@ dsp0: dsp@1080 {
> status = "disabled";
> };
>
> - msgmgr: msgmgr@2a0 {
> + msgmgr: mailbox@2a0 {
> compatible = "ti,k2g-message-manager";
> #mbox-cells = <2>;
> reg-names = "queue_proxy_region",
>
Re: [PATCH v2 1/2] dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs
Hi Rob, On 3/27/21 9:31 AM, Suman Anna wrote: > The K3 AM64x SoCs have two dual-core Arm R5F clusters/subsystems, with > 2 R5F cores each, both in the MAIN voltage domain. > > These clusters are a revised IP version compared to those present on > J721E and J7200 SoCs, and supports a new "Single-CPU" mode instead of > LockStep mode. Update the K3 R5F remoteproc bindings with the compatible > info relevant to these R5F clusters/subsystems on K3 AM64x SoCs. > > Signed-off-by: Suman Anna > --- > v2: No changes > > .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 --- Looks like this patch has fallen through the cracks, can you please review and give your ack for this patch so that Bjorn can pick up the series for 5.13? regards Suman > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml > b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml > index d905d614502b..130fbaacc4b1 100644 > --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml > @@ -14,8 +14,12 @@ description: | >processor subsystems/clusters (R5FSS). The dual core cluster can be used >either in a LockStep mode providing safety/fault tolerance features or in a >Split mode providing two individual compute cores for doubling the compute > - capacity. These are used together with other processors present on the SoC > - to achieve various system level goals. > + capacity on most SoCs. These are used together with other processors > present > + on the SoC to achieve various system level goals. > + > + AM64x SoCs do not support LockStep mode, but rather a new non-safety mode > + called "Single-CPU" mode, where only Core0 is used, but with ability to use > + Core1's TCMs as well. > >Each Dual-Core R5F sub-system is represented as a single DTS node >representing the cluster, with a pair of child DT nodes representing > @@ -33,6 +37,7 @@ properties: >- ti,am654-r5fss >- ti,j721e-r5fss >- ti,j7200-r5fss > + - ti,am64-r5fss > >power-domains: > description: | > @@ -56,11 +61,12 @@ properties: > >ti,cluster-mode: > $ref: /schemas/types.yaml#/definitions/uint32 > -enum: [0, 1] > description: | >Configuration Mode for the Dual R5F cores within the R5F cluster. > - Should be either a value of 1 (LockStep mode) or 0 (Split mode), > - default is LockStep mode if omitted. > + Should be either a value of 1 (LockStep mode) or 0 (Split mode) on > + most SoCs (AM65x, J721E, J7200), default is LockStep mode if omitted; > + and should be either a value of 0 (Split mode) or 2 (Single-CPU mode) > + on AM64x SoCs, default is Split mode if omitted. > > # R5F Processor Child Nodes: > # == > @@ -97,6 +103,7 @@ patternProperties: >- ti,am654-r5f >- ti,j721e-r5f >- ti,j7200-r5f > + - ti,am64-r5f > >reg: > items: > @@ -198,6 +205,20 @@ patternProperties: > > unevaluatedProperties: false > > +if: > + properties: > +compatible: > + enum: > +- ti,am64-r5fss > +then: > + properties: > +ti,cluster-mode: > + enum: [0, 2] > +else: > + properties: > +ti,cluster-mode: > + enum: [0, 1] > + > required: >- compatible >- power-domains >
Re: [PATCH v2 0/3] PRU firmware event/interrupt mapping fixes
Hi Bjorn, On 4/7/21 10:56 AM, Suman Anna wrote: > Hi All, > > The following is a minor revised version of the series [1] that includes fixes > for various different issues associated with the PRU firmware event/interrupt > mapping configuration logic. Please see the v1 cover-letter [1] for additional > details. > > There are currently no in-kernel dts nodes yet in mainline kernel (first > nodes will appear in v5.13-rc1) so these can be picked up for either v5.13 > merge window or the current -rc cycle. > > Changes in v2: > - Picked up Reviewed-by tags on Patches 1 and 2 > - Revised Patch 3 to address additional error cleanup paths as >pointed out by Mathieu. All patches are reviewed now, so can you please pick these up for the next merge window if not already in your queue. regards Suman > > regards > Suman > > [1] > https://patchwork.kernel.org/project/linux-remoteproc/cover/20210323223839.17464-1-s-a...@ti.com/ > > Suman Anna (3): > remoteproc: pru: Fixup interrupt-parent logic for fw events > remoteproc: pru: Fix wrong success return value for fw events > remoteproc: pru: Fix and cleanup firmware interrupt mapping logic > > drivers/remoteproc/pru_rproc.c | 41 ++ > 1 file changed, 32 insertions(+), 9 deletions(-) >
[PATCH v2 2/3] remoteproc: pru: Fix wrong success return value for fw events
The irq_create_fwspec_mapping() returns a proper virq value on success
and 0 upon any failure. The pru_handle_intrmap() treats this as an error
and disposes all firmware event mappings correctly, but is returning
this incorrect value as is, letting the pru_rproc_start() interpret it
as a success and boot the PRU.
Fix this by returning an error value back upon any such failure. While
at this, revise the error trace to print some meaningful info about the
failed event.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Signed-off-by: Suman Anna
Reviewed-by: Mathieu Poirier
---
v2:
- No changes, picked up Reviewed-by tag
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20210323223839.17464-3-s-a...@ti.com/
drivers/remoteproc/pru_rproc.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index a9d07c0751be..87b43976c51b 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -339,8 +339,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
pru->mapped_irq[i] = irq_create_fwspec_mapping();
if (!pru->mapped_irq[i]) {
- dev_err(dev, "failed to get virq\n");
- ret = pru->mapped_irq[i];
+ dev_err(dev, "failed to get virq for fw mapping %d:
event %d chnl %d host %d\n",
+ i, fwspec.param[0], fwspec.param[1],
+ fwspec.param[2]);
+ ret = -EINVAL;
goto map_fail;
}
}
--
2.30.1
[PATCH v2 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
The PRU firmware interrupt mappings are configured and unconfigured in
.start() and .stop() callbacks respectively using the variables 'evt_count'
and a 'mapped_irq' pointer. These variables are modified only during these
callbacks but are not re-initialized/reset properly during unwind or
failure paths. These stale values caused a kernel crash while stopping a
PRU remoteproc running a different firmware with no events on a subsequent
run after a previous run that was running a firmware with events.
Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
variables properly during any failures in the .start() callback. While
at this, the pru_dispose_irq_mapping() callsites are all made to look
the same, moving any conditional logic to inside the function.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Reported-by: Vignesh Raghavendra
Signed-off-by: Suman Anna
---
v2:
- Fixed two additional cleanup paths in pru_handle_intrmap()
addressing Mathieu's review comment
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20210323223839.17464-4-s-a...@ti.com/
drivers/remoteproc/pru_rproc.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 87b43976c51b..04863bf23db8 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -266,12 +266,17 @@ static void pru_rproc_create_debug_entries(struct rproc
*rproc)
static void pru_dispose_irq_mapping(struct pru_rproc *pru)
{
- while (pru->evt_count--) {
+ if (!pru->mapped_irq)
+ return;
+
+ while (pru->evt_count) {
+ pru->evt_count--;
if (pru->mapped_irq[pru->evt_count] > 0)
irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
}
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
}
/*
@@ -307,8 +312,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
pru->evt_count = rsc->num_evts;
pru->mapped_irq = kcalloc(pru->evt_count, sizeof(unsigned int),
GFP_KERNEL);
- if (!pru->mapped_irq)
+ if (!pru->mapped_irq) {
+ pru->evt_count = 0;
return -ENOMEM;
+ }
/*
* parse and fill in system event to interrupt channel and
@@ -317,13 +324,19 @@ static int pru_handle_intrmap(struct rproc *rproc)
* corresponding sibling PRUSS INTC node.
*/
parent = of_get_parent(dev_of_node(pru->dev));
- if (!parent)
+ if (!parent) {
+ kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
+ }
irq_parent = of_get_child_by_name(parent, "interrupt-controller");
of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
}
@@ -398,8 +411,7 @@ static int pru_rproc_stop(struct rproc *rproc)
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
/* dispose irq mapping - new firmware can provide new mapping */
- if (pru->mapped_irq)
- pru_dispose_irq_mapping(pru);
+ pru_dispose_irq_mapping(pru);
return 0;
}
--
2.30.1
[PATCH v2 1/3] remoteproc: pru: Fixup interrupt-parent logic for fw events
The PRU firmware interrupt mapping logic in pru_handle_intrmap() uses
of_irq_find_parent() with PRU device node to get a handle to the PRUSS
Interrupt Controller at present. This logic however requires that the
PRU nodes always define a interrupt-parent property. This property is
neither a required/defined property as per the PRU remoteproc binding,
nor is relevant from a DT node point of view without any associated
interrupts. The current logic finds a wrong interrupt controller and
fails to perform proper mapping without any interrupt-parent property
in the PRU nodes.
Fix this logic to always find and use the sibling interrupt controller.
Also, while at this, fix the acquired interrupt controller device node
reference properly.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Signed-off-by: Suman Anna
Reviewed-by: Mathieu Poirier
---
v2:
- Fixed one minor typo ((%s/curret/current) in patch description
- Picked up Reviewed-by tag
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20210323223839.17464-2-s-a...@ti.com/
drivers/remoteproc/pru_rproc.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 16979c1cd2f4..a9d07c0751be 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -284,7 +284,7 @@ static int pru_handle_intrmap(struct rproc *rproc)
struct pru_rproc *pru = rproc->priv;
struct pru_irq_rsc *rsc = pru->pru_interrupt_map;
struct irq_fwspec fwspec;
- struct device_node *irq_parent;
+ struct device_node *parent, *irq_parent;
int i, ret = 0;
/* not having pru_interrupt_map is not an error */
@@ -312,9 +312,16 @@ static int pru_handle_intrmap(struct rproc *rproc)
/*
* parse and fill in system event to interrupt channel and
-* channel-to-host mapping
+* channel-to-host mapping. The interrupt controller to be used
+* for these mappings for a given PRU remoteproc is always its
+* corresponding sibling PRUSS INTC node.
*/
- irq_parent = of_irq_find_parent(pru->dev->of_node);
+ parent = of_get_parent(dev_of_node(pru->dev));
+ if (!parent)
+ return -ENODEV;
+
+ irq_parent = of_get_child_by_name(parent, "interrupt-controller");
+ of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
return -ENODEV;
@@ -337,11 +344,13 @@ static int pru_handle_intrmap(struct rproc *rproc)
goto map_fail;
}
}
+ of_node_put(irq_parent);
return ret;
map_fail:
pru_dispose_irq_mapping(pru);
+ of_node_put(irq_parent);
return ret;
}
--
2.30.1
[PATCH v2 0/3] PRU firmware event/interrupt mapping fixes
Hi All, The following is a minor revised version of the series [1] that includes fixes for various different issues associated with the PRU firmware event/interrupt mapping configuration logic. Please see the v1 cover-letter [1] for additional details. There are currently no in-kernel dts nodes yet in mainline kernel (first nodes will appear in v5.13-rc1) so these can be picked up for either v5.13 merge window or the current -rc cycle. Changes in v2: - Picked up Reviewed-by tags on Patches 1 and 2 - Revised Patch 3 to address additional error cleanup paths as pointed out by Mathieu. regards Suman [1] https://patchwork.kernel.org/project/linux-remoteproc/cover/20210323223839.17464-1-s-a...@ti.com/ Suman Anna (3): remoteproc: pru: Fixup interrupt-parent logic for fw events remoteproc: pru: Fix wrong success return value for fw events remoteproc: pru: Fix and cleanup firmware interrupt mapping logic drivers/remoteproc/pru_rproc.c | 41 ++ 1 file changed, 32 insertions(+), 9 deletions(-) -- 2.30.1
Re: [PATCH 1/3] remoteproc: pru: Fixup interrupt-parent logic for fw events
On 4/6/21 6:28 PM, Mathieu Poirier wrote:
> On Tue, Mar 23, 2021 at 05:38:37PM -0500, Suman Anna wrote:
>> The PRU firmware interrupt mapping logic in pru_handle_intrmap() uses
>> of_irq_find_parent() with PRU device node to get a handle to the PRUSS
>> Interrupt Controller at present. This logic however requires that the
>> PRU nodes always define a interrupt-parent property. This property is
>> neither a required/defined property as per the PRU remoteproc binding,
>> nor is relevant from a DT node point of view without any associated
>> interrupts. The curret logic finds a wrong interrupt controller and
>> fails to perform proper mapping without any interrupt-parent property
>> in the PRU nodes.
>>
>> Fix this logic to always find and use the sibling interrupt controller.
>> Also, while at this, fix the acquired interrupt controller device node
>> reference properly.
>>
>> Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific
>> interrupt configuration")
>> Signed-off-by: Suman Anna
>> ---
>> drivers/remoteproc/pru_rproc.c | 15 ---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>> index 16979c1cd2f4..a9d07c0751be 100644
>> --- a/drivers/remoteproc/pru_rproc.c
>> +++ b/drivers/remoteproc/pru_rproc.c
>> @@ -284,7 +284,7 @@ static int pru_handle_intrmap(struct rproc *rproc)
>> struct pru_rproc *pru = rproc->priv;
>> struct pru_irq_rsc *rsc = pru->pru_interrupt_map;
>> struct irq_fwspec fwspec;
>> -struct device_node *irq_parent;
>> +struct device_node *parent, *irq_parent;
>> int i, ret = 0;
>>
>> /* not having pru_interrupt_map is not an error */
>> @@ -312,9 +312,16 @@ static int pru_handle_intrmap(struct rproc *rproc)
>>
>> /*
>> * parse and fill in system event to interrupt channel and
>> - * channel-to-host mapping
>> + * channel-to-host mapping. The interrupt controller to be used
>> + * for these mappings for a given PRU remoteproc is always its
>> + * corresponding sibling PRUSS INTC node.
>> */
>> -irq_parent = of_irq_find_parent(pru->dev->of_node);
>
> If I understand correctly when an interrupt controller node wasn't speficied
> in
> the parent this was unwinding until it found one...
Correct if not specified in each PRU node, and ends up finding the complete
wrong interrupt controller (GIC) as it walks up the tree.
>
>> +parent = of_get_parent(dev_of_node(pru->dev));
>> +if (!parent)
>> +return -ENODEV;
>> +
>> +irq_parent = of_get_child_by_name(parent, "interrupt-controller");
>> +of_node_put(parent);
>> if (!irq_parent) {
>> kfree(pru->mapped_irq);
>> return -ENODEV;
>> @@ -337,11 +344,13 @@ static int pru_handle_intrmap(struct rproc *rproc)
>> goto map_fail;
>> }
>> }
>> +of_node_put(irq_parent);
>>
>> return ret;
>>
>> map_fail:
>> pru_dispose_irq_mapping(pru);
>> +of_node_put(irq_parent);
>
> Reviewed-by: Mathieu Poirier
Thanks,
Suman
>
>>
>> return ret;
>> }
>> --
>> 2.30.1
>>
Re: [PATCH 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
Hi Mathieu,
On 4/6/21 6:47 PM, Mathieu Poirier wrote:
> On Tue, Mar 23, 2021 at 05:38:39PM -0500, Suman Anna wrote:
>> The PRU firmware interrupt mappings are configured and unconfigured in
>> .start() and .stop() callbacks respectively using the variables 'evt_count'
>> and a 'mapped_irq' pointer. These variables are modified only during these
>> callbacks but are not re-initialized/reset properly during unwind or
>> failure paths. These stale values caused a kernel crash while stopping a
>> PRU remoteproc running a different firmware with no events on a subsequent
>> run after a previous run that was running a firmware with events.
>>
>> Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
>> pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
>> variables properly during any failures in the .start() callback. While
>> at this, the pru_dispose_irq_mapping() callsites are all made to look
>> the same, moving any conditional logic to inside the function.
>>
>> Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific
>> interrupt configuration")
>> Reported-by: Vignesh Raghavendra
>> Signed-off-by: Suman Anna
>> ---
>> drivers/remoteproc/pru_rproc.c | 12 +---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>> index 87b43976c51b..5df19acb90ed 100644
>> --- a/drivers/remoteproc/pru_rproc.c
>> +++ b/drivers/remoteproc/pru_rproc.c
>> @@ -266,12 +266,17 @@ static void pru_rproc_create_debug_entries(struct
>> rproc *rproc)
>>
>> static void pru_dispose_irq_mapping(struct pru_rproc *pru)
>> {
>> -while (pru->evt_count--) {
>> +if (!pru->mapped_irq)
>> +return;
>> +
>> +while (pru->evt_count) {
>> +pru->evt_count--;
>> if (pru->mapped_irq[pru->evt_count] > 0)
>> irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
>> }
>>
>> kfree(pru->mapped_irq);
>> +pru->mapped_irq = NULL;
>> }
>>
>> /*
>> @@ -324,6 +329,8 @@ static int pru_handle_intrmap(struct rproc *rproc)
>> of_node_put(parent);
>> if (!irq_parent) {
>> kfree(pru->mapped_irq);
>> +pru->mapped_irq = NULL;
>> +pru->evt_count = 0;
>
> Patch 1/3 introduced a check on @parent that doesn't free pru->mapped_irq. I
> would also expect that error path to do the same has what is done here. And
> looking further up I see the error path for !pru->mapped_irq doesn't set
> pru->evt_count to zero.
Good catch, thank you. I will fix these up in v2.
regards
Suman
>
> Thanks,
> Mathieu
>
>> return -ENODEV;
>> }
>>
>> @@ -398,8 +405,7 @@ static int pru_rproc_stop(struct rproc *rproc)
>> pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
>>
>> /* dispose irq mapping - new firmware can provide new mapping */
>> -if (pru->mapped_irq)
>> -pru_dispose_irq_mapping(pru);
>> +pru_dispose_irq_mapping(pru);
>>
>> return 0;
>> }
>> --
>> 2.30.1
>>
[PATCH v2 0/2] TI K3 R5F remoteproc support on AM64x SoCs
Hi All, The following series is a minor revision of the K3 R5F remoteproc AM64x support series [1] addressing one review comment from Mathieu. Please see the v1 cover-letter for details about the revised R5F IP in TI AM64x SoCs. Only change is in Patch 2, binding patch is unchanged from v1. Also, corrected the AM46x typo in the cover-letter title :) regards Suman [1] https://patchwork.kernel.org/project/linux-remoteproc/cover/20210318215842.8196-1-s-a...@ti.com/ Suman Anna (2): dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 +++- drivers/remoteproc/ti_k3_r5_remoteproc.c | 149 ++ 2 files changed, 146 insertions(+), 34 deletions(-) -- 2.30.1
[PATCH v2 1/2] dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs
The K3 AM64x SoCs have two dual-core Arm R5F clusters/subsystems, with 2 R5F cores each, both in the MAIN voltage domain. These clusters are a revised IP version compared to those present on J721E and J7200 SoCs, and supports a new "Single-CPU" mode instead of LockStep mode. Update the K3 R5F remoteproc bindings with the compatible info relevant to these R5F clusters/subsystems on K3 AM64x SoCs. Signed-off-by: Suman Anna --- v2: No changes .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 --- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml index d905d614502b..130fbaacc4b1 100644 --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml @@ -14,8 +14,12 @@ description: | processor subsystems/clusters (R5FSS). The dual core cluster can be used either in a LockStep mode providing safety/fault tolerance features or in a Split mode providing two individual compute cores for doubling the compute - capacity. These are used together with other processors present on the SoC - to achieve various system level goals. + capacity on most SoCs. These are used together with other processors present + on the SoC to achieve various system level goals. + + AM64x SoCs do not support LockStep mode, but rather a new non-safety mode + called "Single-CPU" mode, where only Core0 is used, but with ability to use + Core1's TCMs as well. Each Dual-Core R5F sub-system is represented as a single DTS node representing the cluster, with a pair of child DT nodes representing @@ -33,6 +37,7 @@ properties: - ti,am654-r5fss - ti,j721e-r5fss - ti,j7200-r5fss + - ti,am64-r5fss power-domains: description: | @@ -56,11 +61,12 @@ properties: ti,cluster-mode: $ref: /schemas/types.yaml#/definitions/uint32 -enum: [0, 1] description: | Configuration Mode for the Dual R5F cores within the R5F cluster. - Should be either a value of 1 (LockStep mode) or 0 (Split mode), - default is LockStep mode if omitted. + Should be either a value of 1 (LockStep mode) or 0 (Split mode) on + most SoCs (AM65x, J721E, J7200), default is LockStep mode if omitted; + and should be either a value of 0 (Split mode) or 2 (Single-CPU mode) + on AM64x SoCs, default is Split mode if omitted. # R5F Processor Child Nodes: # == @@ -97,6 +103,7 @@ patternProperties: - ti,am654-r5f - ti,j721e-r5f - ti,j7200-r5f + - ti,am64-r5f reg: items: @@ -198,6 +205,20 @@ patternProperties: unevaluatedProperties: false +if: + properties: +compatible: + enum: +- ti,am64-r5fss +then: + properties: +ti,cluster-mode: + enum: [0, 2] +else: + properties: +ti,cluster-mode: + enum: [0, 1] + required: - compatible - power-domains -- 2.30.1
[PATCH v2 2/2] remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs
The K3 AM64x SoC family has a revised R5F sub-system and contains a
subset of the R5F clusters present on J721E SoCs. The K3 AM64x SoCs
only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
each present within the MAIN voltage domain (MAIN_R5FSS0 & MAIN_R5FSS1).
The revised IP has the following distinct features:
1. The R5FSS IP supports a new "Single-CPU" mode instead of the LockStep
mode on existing SoCs (AM65x, J721E or J7200). This mode is similar
to LockStep-mode on J7200 SoCs in terms of TCM usage without the
fault-tolerant safety feature provided by the LockStep mode.
The Core1 TCMs are combined with the Core0 TCMs effectively doubling
the amount of TCMs available in Single-CPU mode. The LockStep-mode
on previous AM65x and J721E SoCs could only use the Core0 TCMs. These
combined TCMs appear contiguous at the respective Core0 TCM addresses.
The code though is executed only on a single CPU (on Core0), and as
such, requires the halt signal to be programmed only for Core0, while
the resets need to be managed for both the cores.
2. TCMs are auto-initialized during module power-up, and the behavior
is programmable through a MMR bit. This feature is the same as on
the recent J7200 SoCs.
Extend the support to these clusters in the K3 R5F remoteproc driver
using AM64x specific compatibles. New TI-SCI flags and a unique cluster
mode are also needed for the cluster mode detection on these SoCs. The
reset assert and deassert sequence of both the cores in Single-CPU mode
is agnostic of the order, so the same LockStep reset and release sequences
are re-used.
The integration of these clusters is very much similar to existing SoCs
otherwise.
Signed-off-by: Suman Anna
---
v2:
- Address a minor comment from Mathieu to drop the unnecessary hunk
double-checking the dt ti,cluster-mode values in k3_r5_probe()
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20210318215842.8196-3-s-a...@ti.com/
drivers/remoteproc/ti_k3_r5_remoteproc.c | 149 ++-
1 file changed, 120 insertions(+), 29 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 5cf8d030a1f0..2975393ebacd 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -40,6 +40,8 @@
#define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
/* Available from J7200 SoCs onwards */
#define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS 0x4000
+/* Applicable to only AM64x SoCs */
+#define PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE 0x8000
/* R5 TI-SCI Processor Control Flags */
#define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT 0x0001
@@ -49,6 +51,8 @@
#define PROC_BOOT_STATUS_FLAG_R5_WFI 0x0002
#define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED 0x0004
#define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED0x0100
+/* Applicable to only AM64x SoCs */
+#define PROC_BOOT_STATUS_FLAG_R5_SINGLECORE_ONLY 0x0200
/**
* struct k3_r5_mem - internal memory structure
@@ -64,19 +68,29 @@ struct k3_r5_mem {
size_t size;
};
+/*
+ * All cluster mode values are not applicable on all SoCs. The following
+ * are the modes supported on various SoCs:
+ * Split mode : AM65x, J721E, J7200 and AM64x SoCs
+ * LockStep mode : AM65x, J721E and J7200 SoCs
+ * Single-CPU mode : AM64x SoCs only
+ */
enum cluster_mode {
CLUSTER_MODE_SPLIT = 0,
CLUSTER_MODE_LOCKSTEP,
+ CLUSTER_MODE_SINGLECPU,
};
/**
* struct k3_r5_soc_data - match data to handle SoC variations
* @tcm_is_double: flag to denote the larger unified TCMs in certain modes
* @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
+ * @single_cpu_mode: flag to denote if SoC/IP supports Single-CPU mode
*/
struct k3_r5_soc_data {
bool tcm_is_double;
bool tcm_ecc_autoinit;
+ bool single_cpu_mode;
};
/**
@@ -369,6 +383,13 @@ static inline int k3_r5_core_run(struct k3_r5_core *core)
* applicable cores to allow loading into the TCMs. The .prepare() ops is
* invoked by remoteproc core before any firmware loading, and is followed
* by the .start() ops after loading to actually let the R5 cores run.
+ *
+ * The Single-CPU mode on applicable SoCs (eg: AM64x) only uses Core0 to
+ * execute code, but combines the TCMs from both cores. The resets for both
+ * cores need to be released to make this possible, as the TCMs are in general
+ * private to each core. Only Core0 needs to be unhalted for running the
+ * cluster in this mode. The function uses the same reset logic as LockStep
+ * mode for this (though the behavior is agnostic of the reset release order).
*/
static int k3_r5_rproc_prepare(struct rproc *rproc)
{
@@ -386,7 +407,9 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
return ret;
Re: [PATCH 2/2] remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs
On 3/25/21 5:17 PM, Mathieu Poirier wrote:
> On Thu, Mar 25, 2021 at 04:00:55PM -0500, Suman Anna wrote:
>> Hi Mathieu,
>>
>> On 3/25/21 12:30 PM, Mathieu Poirier wrote:
>>> Good morning,
>>>
>>> On Thu, Mar 18, 2021 at 04:58:42PM -0500, Suman Anna wrote:
>>>> The K3 AM64x SoC family has a revised R5F sub-system and contains a
>>>> subset of the R5F clusters present on J721E SoCs. The K3 AM64x SoCs
>>>> only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
>>>> each present within the MAIN voltage domain (MAIN_R5FSS0 & MAIN_R5FSS1).
>>>>
>>>> The revised IP has the following distinct features:
>>>> 1. The R5FSS IP supports a new "Single-CPU" mode instead of the LockStep
>>>> mode on existing SoCs (AM65x, J721E or J7200). This mode is similar
>>>> to LockStep-mode on J7200 SoCs in terms of TCM usage without the
>>>> fault-tolerant safety feature provided by the LockStep mode.
>>>>
>>>> The Core1 TCMs are combined with the Core0 TCMs effectively doubling
>>>> the amount of TCMs available in Single-CPU mode. The LockStep-mode
>>>> on previous AM65x and J721E SoCs could only use the Core0 TCMs. These
>>>> combined TCMs appear contiguous at the respective Core0 TCM addresses.
>>>> The code though is executed only on a single CPU (on Core0), and as
>>>> such, requires the halt signal to be programmed only for Core0, while
>>>> the resets need to be managed for both the cores.
>>>>
>>>> 2. TCMs are auto-initialized during module power-up, and the behavior
>>>> is programmable through a MMR bit. This feature is the same as on
>>>> the recent J7200 SoCs.
>>>>
>>>> Extend the support to these clusters in the K3 R5F remoteproc driver
>>>> using AM64x specific compatibles. New TI-SCI flags and a unique cluster
>>>> mode are also needed for the cluster mode detection on these SoCs. The
>>>> reset assert and deassert sequence of both the cores in Single-CPU mode
>>>> is agnostic of the order, so the same LockStep reset and release sequences
>>>> are re-used.
>>>>
>>>> The integration of these clusters is very much similar to existing SoCs
>>>> otherwise.
>>>>
>>>> Signed-off-by: Suman Anna
>>>> ---
>>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++-
>>>> 1 file changed, 126 insertions(+), 29 deletions(-)
>>>>
>>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>>> index 5cf8d030a1f0..497f0d05b887 100644
>>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>>> @@ -40,6 +40,8 @@
>>>> #define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
>>>> /* Available from J7200 SoCs onwards */
>>>> #define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS0x4000
>>>> +/* Applicable to only AM64x SoCs */
>>>> +#define PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE 0x8000
>>>>
>>>> /* R5 TI-SCI Processor Control Flags */
>>>> #define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT 0x0001
>>>> @@ -49,6 +51,8 @@
>>>> #define PROC_BOOT_STATUS_FLAG_R5_WFI 0x0002
>>>> #define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED0x0004
>>>> #define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED 0x0100
>>>> +/* Applicable to only AM64x SoCs */
>>>> +#define PROC_BOOT_STATUS_FLAG_R5_SINGLECORE_ONLY 0x0200
>>>>
>>>> /**
>>>> * struct k3_r5_mem - internal memory structure
>>>> @@ -64,19 +68,29 @@ struct k3_r5_mem {
>>>>size_t size;
>>>> };
>>>>
>>>> +/*
>>>> + * All cluster mode values are not applicable on all SoCs. The following
>>>> + * are the modes supported on various SoCs:
>>>> + * Split mode : AM65x, J721E, J7200 and AM64x SoCs
>>>> + * LockStep mode : AM65x, J721E and J7200 SoCs
>>>> + * Single-CPU mode : AM64x SoCs only
>>>> + */
>>>> enum cluster_mode {
>>>>CLUSTER_MODE_SPLIT = 0,
>>>>CLUSTER_MODE_LOCKSTEP,
>>>> + CLUSTER_MODE_SINGLECPU,
>>>> };
>>>>
>&g
Re: [PATCH 2/2] remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs
Hi Mathieu,
On 3/25/21 12:30 PM, Mathieu Poirier wrote:
> Good morning,
>
> On Thu, Mar 18, 2021 at 04:58:42PM -0500, Suman Anna wrote:
>> The K3 AM64x SoC family has a revised R5F sub-system and contains a
>> subset of the R5F clusters present on J721E SoCs. The K3 AM64x SoCs
>> only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
>> each present within the MAIN voltage domain (MAIN_R5FSS0 & MAIN_R5FSS1).
>>
>> The revised IP has the following distinct features:
>> 1. The R5FSS IP supports a new "Single-CPU" mode instead of the LockStep
>> mode on existing SoCs (AM65x, J721E or J7200). This mode is similar
>> to LockStep-mode on J7200 SoCs in terms of TCM usage without the
>> fault-tolerant safety feature provided by the LockStep mode.
>>
>> The Core1 TCMs are combined with the Core0 TCMs effectively doubling
>> the amount of TCMs available in Single-CPU mode. The LockStep-mode
>> on previous AM65x and J721E SoCs could only use the Core0 TCMs. These
>> combined TCMs appear contiguous at the respective Core0 TCM addresses.
>> The code though is executed only on a single CPU (on Core0), and as
>> such, requires the halt signal to be programmed only for Core0, while
>> the resets need to be managed for both the cores.
>>
>> 2. TCMs are auto-initialized during module power-up, and the behavior
>> is programmable through a MMR bit. This feature is the same as on
>> the recent J7200 SoCs.
>>
>> Extend the support to these clusters in the K3 R5F remoteproc driver
>> using AM64x specific compatibles. New TI-SCI flags and a unique cluster
>> mode are also needed for the cluster mode detection on these SoCs. The
>> reset assert and deassert sequence of both the cores in Single-CPU mode
>> is agnostic of the order, so the same LockStep reset and release sequences
>> are re-used.
>>
>> The integration of these clusters is very much similar to existing SoCs
>> otherwise.
>>
>> Signed-off-by: Suman Anna
>> ---
>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++-
>> 1 file changed, 126 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> index 5cf8d030a1f0..497f0d05b887 100644
>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> @@ -40,6 +40,8 @@
>> #define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
>> /* Available from J7200 SoCs onwards */
>> #define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS 0x4000
>> +/* Applicable to only AM64x SoCs */
>> +#define PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE 0x8000
>>
>> /* R5 TI-SCI Processor Control Flags */
>> #define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT0x0001
>> @@ -49,6 +51,8 @@
>> #define PROC_BOOT_STATUS_FLAG_R5_WFI0x0002
>> #define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED 0x0004
>> #define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED 0x0100
>> +/* Applicable to only AM64x SoCs */
>> +#define PROC_BOOT_STATUS_FLAG_R5_SINGLECORE_ONLY0x0200
>>
>> /**
>> * struct k3_r5_mem - internal memory structure
>> @@ -64,19 +68,29 @@ struct k3_r5_mem {
>> size_t size;
>> };
>>
>> +/*
>> + * All cluster mode values are not applicable on all SoCs. The following
>> + * are the modes supported on various SoCs:
>> + * Split mode : AM65x, J721E, J7200 and AM64x SoCs
>> + * LockStep mode : AM65x, J721E and J7200 SoCs
>> + * Single-CPU mode : AM64x SoCs only
>> + */
>> enum cluster_mode {
>> CLUSTER_MODE_SPLIT = 0,
>> CLUSTER_MODE_LOCKSTEP,
>> +CLUSTER_MODE_SINGLECPU,
>> };
>>
>> /**
>> * struct k3_r5_soc_data - match data to handle SoC variations
>> * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
>> * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
>> + * @single_cpu_mode: flag to denote if SoC/IP supports Single-CPU mode
>> */
>> struct k3_r5_soc_data {
>> bool tcm_is_double;
>> bool tcm_ecc_autoinit;
>> +bool single_cpu_mode;
>> };
>>
>> /**
>> @@ -369,6 +383,13 @@ static inline int k3_r5_core_run(struct k3_r5_core
>> *core)
>> * applicable cores to allow loading into the TCMs. The .prepare() ops is
>> * invoked by remoteproc core before any firmware
Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs
On 3/25/21 3:09 PM, Suman Anna wrote:
> On 3/25/21 12:36 PM, Mathieu Poirier wrote:
>> On Wed, 24 Mar 2021 at 11:09, Suman Anna wrote:
>>>
>>> On 3/23/21 6:20 PM, Mathieu Poirier wrote:
>>>> On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote:
>>>>> The K3 PRUs are 32-bit processors and in general have some limitations
>>>>> in using the standard ARMv8 memcpy function for loading firmware segments,
>>>>> so the driver already uses a custom memcpy implementation. This added
>>>>> logic however is limited to only IRAMs at the moment, but the loading
>>>>> into Data RAMs is not completely ok either and does generate a kernel
>>>>> crash for unaligned accesses.
>>>>>
>>>>> Fix these crashes by removing the existing IRAM logic limitation and
>>>>> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs.
>>>>>
>>>>> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores
>>>>> on K3 AM65x SoCs")
>>>>> Signed-off-by: Suman Anna
>>>>
>>>> Probably a good idea to CC stable as well...
>>>>
>>>> Reviewed-by: Mathieu Poirier
>>>
>>> Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch
>>> though and part of linux-next since next-20210319. I have posted an
>>> additional
>>> 3-patch series for some more PRU fixes. Do you want me to post a v2 for
>>> those
>>> with stable Cc'd?
>>
>> I didn't notice Bjorn had already picked it up. Since the object is
>> now public there is no need to send a V2 for this one. I haven't
>> looked at your other 3-patch series but if you think it is stable
>> material then yes, please send a new revision that CC stable.
>
> Alright, will do.
On a second thought, we don't have any dts nodes in-kernel yet (5.13-rc1 would
be the first kernel with PRU nodes), so those are not critical for v5.11 kernel.
As long as they get fixed in either the current v5.12-rc's or by v5.13-rc1, we
will be fine.
regards
Suman
>
> regards
> Suman
>
>>
>> Mathieu
>>
>>>
>>> regards
>>> Suman
>>>
>>>>
>>>>> ---
>>>>> drivers/remoteproc/pru_rproc.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/remoteproc/pru_rproc.c
>>>>> b/drivers/remoteproc/pru_rproc.c
>>>>> index 2667919d76b3..16979c1cd2f4 100644
>>>>> --- a/drivers/remoteproc/pru_rproc.c
>>>>> +++ b/drivers/remoteproc/pru_rproc.c
>>>>> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc,
>>>>> const struct firmware *fw)
>>>>> break;
>>>>> }
>>>>>
>>>>> -if (pru->data->is_k3 && is_iram) {
>>>>> +if (pru->data->is_k3) {
>>>>> ret = pru_rproc_memcpy(ptr, elf_data +
>>>>> phdr->p_offset,
>>>>> filesz);
>>>>> if (ret) {
>>>>> --
>>>>> 2.30.1
>>>>>
>>>
>
Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs
On 3/25/21 12:36 PM, Mathieu Poirier wrote:
> On Wed, 24 Mar 2021 at 11:09, Suman Anna wrote:
>>
>> On 3/23/21 6:20 PM, Mathieu Poirier wrote:
>>> On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote:
>>>> The K3 PRUs are 32-bit processors and in general have some limitations
>>>> in using the standard ARMv8 memcpy function for loading firmware segments,
>>>> so the driver already uses a custom memcpy implementation. This added
>>>> logic however is limited to only IRAMs at the moment, but the loading
>>>> into Data RAMs is not completely ok either and does generate a kernel
>>>> crash for unaligned accesses.
>>>>
>>>> Fix these crashes by removing the existing IRAM logic limitation and
>>>> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs.
>>>>
>>>> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores
>>>> on K3 AM65x SoCs")
>>>> Signed-off-by: Suman Anna
>>>
>>> Probably a good idea to CC stable as well...
>>>
>>> Reviewed-by: Mathieu Poirier
>>
>> Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch
>> though and part of linux-next since next-20210319. I have posted an
>> additional
>> 3-patch series for some more PRU fixes. Do you want me to post a v2 for those
>> with stable Cc'd?
>
> I didn't notice Bjorn had already picked it up. Since the object is
> now public there is no need to send a V2 for this one. I haven't
> looked at your other 3-patch series but if you think it is stable
> material then yes, please send a new revision that CC stable.
Alright, will do.
regards
Suman
>
> Mathieu
>
>>
>> regards
>> Suman
>>
>>>
>>>> ---
>>>> drivers/remoteproc/pru_rproc.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/remoteproc/pru_rproc.c
>>>> b/drivers/remoteproc/pru_rproc.c
>>>> index 2667919d76b3..16979c1cd2f4 100644
>>>> --- a/drivers/remoteproc/pru_rproc.c
>>>> +++ b/drivers/remoteproc/pru_rproc.c
>>>> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const
>>>> struct firmware *fw)
>>>> break;
>>>> }
>>>>
>>>> -if (pru->data->is_k3 && is_iram) {
>>>> +if (pru->data->is_k3) {
>>>> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset,
>>>> filesz);
>>>> if (ret) {
>>>> --
>>>> 2.30.1
>>>>
>>
Re: [PATCH 2/2] remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs
Hi Mathieu,
On 3/24/21 11:46 AM, Mathieu Poirier wrote:
> Good day Suman,
>
> On Thu, Mar 18, 2021 at 04:58:42PM -0500, Suman Anna wrote:
>> The K3 AM64x SoC family has a revised R5F sub-system and contains a
>> subset of the R5F clusters present on J721E SoCs. The K3 AM64x SoCs
>> only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
>> each present within the MAIN voltage domain (MAIN_R5FSS0 & MAIN_R5FSS1).
>>
>> The revised IP has the following distinct features:
>> 1. The R5FSS IP supports a new "Single-CPU" mode instead of the LockStep
>> mode on existing SoCs (AM65x, J721E or J7200). This mode is similar
>> to LockStep-mode on J7200 SoCs in terms of TCM usage without the
>> fault-tolerant safety feature provided by the LockStep mode.
>>
>> The Core1 TCMs are combined with the Core0 TCMs effectively doubling
>> the amount of TCMs available in Single-CPU mode. The LockStep-mode
>> on previous AM65x and J721E SoCs could only use the Core0 TCMs. These
>> combined TCMs appear contiguous at the respective Core0 TCM addresses.
>> The code though is executed only on a single CPU (on Core0), and as
>> such, requires the halt signal to be programmed only for Core0, while
>> the resets need to be managed for both the cores.
>>
>> 2. TCMs are auto-initialized during module power-up, and the behavior
>> is programmable through a MMR bit. This feature is the same as on
>> the recent J7200 SoCs.
>>
>> Extend the support to these clusters in the K3 R5F remoteproc driver
>> using AM64x specific compatibles. New TI-SCI flags and a unique cluster
>> mode are also needed for the cluster mode detection on these SoCs. The
>> reset assert and deassert sequence of both the cores in Single-CPU mode
>> is agnostic of the order, so the same LockStep reset and release sequences
>> are re-used.
>>
>> The integration of these clusters is very much similar to existing SoCs
>> otherwise.
>>
>> Signed-off-by: Suman Anna
>> ---
>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++-
>> 1 file changed, 126 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> index 5cf8d030a1f0..497f0d05b887 100644
>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> @@ -40,6 +40,8 @@
>> #define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
>> /* Available from J7200 SoCs onwards */
>> #define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS 0x4000
>> +/* Applicable to only AM64x SoCs */
>> +#define PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE 0x8000
>>
>> /* R5 TI-SCI Processor Control Flags */
>> #define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT0x0001
>> @@ -49,6 +51,8 @@
>> #define PROC_BOOT_STATUS_FLAG_R5_WFI0x0002
>> #define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED 0x0004
>> #define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED 0x0100
>> +/* Applicable to only AM64x SoCs */
>> +#define PROC_BOOT_STATUS_FLAG_R5_SINGLECORE_ONLY0x0200
>>
>> /**
>> * struct k3_r5_mem - internal memory structure
>> @@ -64,19 +68,29 @@ struct k3_r5_mem {
>> size_t size;
>> };
>>
>> +/*
>> + * All cluster mode values are not applicable on all SoCs. The following
>> + * are the modes supported on various SoCs:
>> + * Split mode : AM65x, J721E, J7200 and AM64x SoCs
>> + * LockStep mode : AM65x, J721E and J7200 SoCs
>> + * Single-CPU mode : AM64x SoCs only
>> + */
>> enum cluster_mode {
>> CLUSTER_MODE_SPLIT = 0,
>> CLUSTER_MODE_LOCKSTEP,
>> +CLUSTER_MODE_SINGLECPU,
>> };
>>
>> /**
>> * struct k3_r5_soc_data - match data to handle SoC variations
>> * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
>> * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
>> + * @single_cpu_mode: flag to denote if SoC/IP supports Single-CPU mode
>> */
>> struct k3_r5_soc_data {
>> bool tcm_is_double;
>> bool tcm_ecc_autoinit;
>> +bool single_cpu_mode;
>> };
>>
>> /**
>> @@ -369,6 +383,13 @@ static inline int k3_r5_core_run(struct k3_r5_core
>> *core)
>> * applicable cores to allow loading into the TCMs. The .prepare() ops is
>> * invoked by remoteproc core before any firmware
Re: [PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs
On 3/23/21 6:20 PM, Mathieu Poirier wrote:
> On Mon, Mar 15, 2021 at 03:58:59PM -0500, Suman Anna wrote:
>> The K3 PRUs are 32-bit processors and in general have some limitations
>> in using the standard ARMv8 memcpy function for loading firmware segments,
>> so the driver already uses a custom memcpy implementation. This added
>> logic however is limited to only IRAMs at the moment, but the loading
>> into Data RAMs is not completely ok either and does generate a kernel
>> crash for unaligned accesses.
>>
>> Fix these crashes by removing the existing IRAM logic limitation and
>> extending the custom memcpy usage to Data RAMs as well for all K3 SoCs.
>>
>> Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on
>> K3 AM65x SoCs")
>> Signed-off-by: Suman Anna
>
> Probably a good idea to CC stable as well...
>
> Reviewed-by: Mathieu Poirier
Thanks Mathieu. This patch is already staged on Bjorn's rproc-fixes branch
though and part of linux-next since next-20210319. I have posted an additional
3-patch series for some more PRU fixes. Do you want me to post a v2 for those
with stable Cc'd?
regards
Suman
>
>> ---
>> drivers/remoteproc/pru_rproc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>> index 2667919d76b3..16979c1cd2f4 100644
>> --- a/drivers/remoteproc/pru_rproc.c
>> +++ b/drivers/remoteproc/pru_rproc.c
>> @@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const
>> struct firmware *fw)
>> break;
>> }
>>
>> -if (pru->data->is_k3 && is_iram) {
>> +if (pru->data->is_k3) {
>> ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset,
>> filesz);
>> if (ret) {
>> --
>> 2.30.1
>>
[PATCH 3/3] remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
The PRU firmware interrupt mappings are configured and unconfigured in
.start() and .stop() callbacks respectively using the variables 'evt_count'
and a 'mapped_irq' pointer. These variables are modified only during these
callbacks but are not re-initialized/reset properly during unwind or
failure paths. These stale values caused a kernel crash while stopping a
PRU remoteproc running a different firmware with no events on a subsequent
run after a previous run that was running a firmware with events.
Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
variables properly during any failures in the .start() callback. While
at this, the pru_dispose_irq_mapping() callsites are all made to look
the same, moving any conditional logic to inside the function.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Reported-by: Vignesh Raghavendra
Signed-off-by: Suman Anna
---
drivers/remoteproc/pru_rproc.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 87b43976c51b..5df19acb90ed 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -266,12 +266,17 @@ static void pru_rproc_create_debug_entries(struct rproc
*rproc)
static void pru_dispose_irq_mapping(struct pru_rproc *pru)
{
- while (pru->evt_count--) {
+ if (!pru->mapped_irq)
+ return;
+
+ while (pru->evt_count) {
+ pru->evt_count--;
if (pru->mapped_irq[pru->evt_count] > 0)
irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
}
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
}
/*
@@ -324,6 +329,8 @@ static int pru_handle_intrmap(struct rproc *rproc)
of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
+ pru->mapped_irq = NULL;
+ pru->evt_count = 0;
return -ENODEV;
}
@@ -398,8 +405,7 @@ static int pru_rproc_stop(struct rproc *rproc)
pru_control_write_reg(pru, PRU_CTRL_CTRL, val);
/* dispose irq mapping - new firmware can provide new mapping */
- if (pru->mapped_irq)
- pru_dispose_irq_mapping(pru);
+ pru_dispose_irq_mapping(pru);
return 0;
}
--
2.30.1
[PATCH 0/3] PRU firmware event/interrupt mapping fixes
Hi,
The following series includes fixes for various different issues
associated with the PRU firmware event/interrupt mapping configuration
logic added in the same commit c75c9fdac66e ("remoteproc: pru: Add
support for PRU specific interrupt configuration"). The fixes are
agnostic of SoC family.
Following is the summary of issues and fixes:
- Patch #1 fixes the interrupt node finding logic to always use the
inherent sibling relationship between a PRU/RTU/Tx_PRU node and its
corresponding PRUSS INTC node. This fixes the firmware event mappings
for cases when the PRU nodes do not have an 'interrupt-parent' property
(this is the norm, the property is neither required nor added in the DT
nodes normally).
- Patch #2 fixes a minor issue with returning a success value to the
caller on a fw event mapping failure.
- Patch #3 fixes a kernel crash due to switching of firmwares between
consecutive runs, the first one with events and the second one without
events. There are no issues when the same firmwares are run or if they
are run in reverse order.
Patches should apply cleanly on top of the current rproc-fixes branch
commit 9afeefcf06fc ("remoteproc: pru: Fix firmware loading crashes on K3 SoCs")
regards
Suman
Suman Anna (3):
remoteproc: pru: Fixup interrupt-parent logic for fw events
remoteproc: pru: Fix wrong success return value for fw events
remoteproc: pru: Fix and cleanup firmware interrupt mapping logic
drivers/remoteproc/pru_rproc.c | 33 +
1 file changed, 25 insertions(+), 8 deletions(-)
--
2.30.1
[PATCH 2/3] remoteproc: pru: Fix wrong success return value for fw events
The irq_create_fwspec_mapping() returns a proper virq value on success
and 0 upon any failure. The pru_handle_intrmap() treats this as an error
and disposes all firmware event mappings correctly, but is returning
this incorrect value as is, letting the pru_rproc_start() interpret it
as a success and boot the PRU.
Fix this by returning an error value back upon any such failure. While
at this, revise the error trace to print some meaningful info about the
failed event.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Signed-off-by: Suman Anna
---
drivers/remoteproc/pru_rproc.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index a9d07c0751be..87b43976c51b 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -339,8 +339,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
pru->mapped_irq[i] = irq_create_fwspec_mapping();
if (!pru->mapped_irq[i]) {
- dev_err(dev, "failed to get virq\n");
- ret = pru->mapped_irq[i];
+ dev_err(dev, "failed to get virq for fw mapping %d:
event %d chnl %d host %d\n",
+ i, fwspec.param[0], fwspec.param[1],
+ fwspec.param[2]);
+ ret = -EINVAL;
goto map_fail;
}
}
--
2.30.1
[PATCH 1/3] remoteproc: pru: Fixup interrupt-parent logic for fw events
The PRU firmware interrupt mapping logic in pru_handle_intrmap() uses
of_irq_find_parent() with PRU device node to get a handle to the PRUSS
Interrupt Controller at present. This logic however requires that the
PRU nodes always define a interrupt-parent property. This property is
neither a required/defined property as per the PRU remoteproc binding,
nor is relevant from a DT node point of view without any associated
interrupts. The curret logic finds a wrong interrupt controller and
fails to perform proper mapping without any interrupt-parent property
in the PRU nodes.
Fix this logic to always find and use the sibling interrupt controller.
Also, while at this, fix the acquired interrupt controller device node
reference properly.
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt
configuration")
Signed-off-by: Suman Anna
---
drivers/remoteproc/pru_rproc.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 16979c1cd2f4..a9d07c0751be 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -284,7 +284,7 @@ static int pru_handle_intrmap(struct rproc *rproc)
struct pru_rproc *pru = rproc->priv;
struct pru_irq_rsc *rsc = pru->pru_interrupt_map;
struct irq_fwspec fwspec;
- struct device_node *irq_parent;
+ struct device_node *parent, *irq_parent;
int i, ret = 0;
/* not having pru_interrupt_map is not an error */
@@ -312,9 +312,16 @@ static int pru_handle_intrmap(struct rproc *rproc)
/*
* parse and fill in system event to interrupt channel and
-* channel-to-host mapping
+* channel-to-host mapping. The interrupt controller to be used
+* for these mappings for a given PRU remoteproc is always its
+* corresponding sibling PRUSS INTC node.
*/
- irq_parent = of_irq_find_parent(pru->dev->of_node);
+ parent = of_get_parent(dev_of_node(pru->dev));
+ if (!parent)
+ return -ENODEV;
+
+ irq_parent = of_get_child_by_name(parent, "interrupt-controller");
+ of_node_put(parent);
if (!irq_parent) {
kfree(pru->mapped_irq);
return -ENODEV;
@@ -337,11 +344,13 @@ static int pru_handle_intrmap(struct rproc *rproc)
goto map_fail;
}
}
+ of_node_put(irq_parent);
return ret;
map_fail:
pru_dispose_irq_mapping(pru);
+ of_node_put(irq_parent);
return ret;
}
--
2.30.1
Re: [PATCH 0/2] TI K3 R5F remoteproc support on AM46x SoCs
On 3/18/21 4:58 PM, Suman Anna wrote: > Hi All, > > The following series enhances the K3 R5F remoteproc driver to add support > for the R5F clusters on the newer TI K3 AM64x SoC family. The AM64x SoCs > have 2 R5FSS clusters and no DSPs. Both clusters are capable of supporting > either the conventional Split-mode or a brand new "Single-CPU" mode. > > The revised R5FSS IP has the following unique features: > 1. The new Single-CPU mode allows the Core1 TCMs to be combined with > the Core0 TCMs effectively doubling the amount of TCMs available. > This is same behavior as LockStep-mode on J7200 SoCs, but all other > previous SoCs could only use the Core0 TCMs. This combined TCMs appear > contiguous at the respective Core0 TCM addresses. > 2. TCMs are auto-initialized during module power-up, and the behavior > is programmable through a SEC_MMR register bit. This is same as on > J7200 SoCs, and is not present on earlier AM65x and J721E SoCs. > > The series is based on 5.12-rc2, and can apply on top of the current > rproc-next branch as well. I had a small typo in the cover-letter subject line, should read "AM64x" instead of "AM46x". Patches themselves use the correct term. regards Suman > > regards > Suman > > Suman Anna (2): > dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs > remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs > > .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 +++- > drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++ > 2 files changed, 152 insertions(+), 34 deletions(-) >
[PATCH 2/2] remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs
The K3 AM64x SoC family has a revised R5F sub-system and contains a
subset of the R5F clusters present on J721E SoCs. The K3 AM64x SoCs
only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
each present within the MAIN voltage domain (MAIN_R5FSS0 & MAIN_R5FSS1).
The revised IP has the following distinct features:
1. The R5FSS IP supports a new "Single-CPU" mode instead of the LockStep
mode on existing SoCs (AM65x, J721E or J7200). This mode is similar
to LockStep-mode on J7200 SoCs in terms of TCM usage without the
fault-tolerant safety feature provided by the LockStep mode.
The Core1 TCMs are combined with the Core0 TCMs effectively doubling
the amount of TCMs available in Single-CPU mode. The LockStep-mode
on previous AM65x and J721E SoCs could only use the Core0 TCMs. These
combined TCMs appear contiguous at the respective Core0 TCM addresses.
The code though is executed only on a single CPU (on Core0), and as
such, requires the halt signal to be programmed only for Core0, while
the resets need to be managed for both the cores.
2. TCMs are auto-initialized during module power-up, and the behavior
is programmable through a MMR bit. This feature is the same as on
the recent J7200 SoCs.
Extend the support to these clusters in the K3 R5F remoteproc driver
using AM64x specific compatibles. New TI-SCI flags and a unique cluster
mode are also needed for the cluster mode detection on these SoCs. The
reset assert and deassert sequence of both the cores in Single-CPU mode
is agnostic of the order, so the same LockStep reset and release sequences
are re-used.
The integration of these clusters is very much similar to existing SoCs
otherwise.
Signed-off-by: Suman Anna
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++-
1 file changed, 126 insertions(+), 29 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 5cf8d030a1f0..497f0d05b887 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -40,6 +40,8 @@
#define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
/* Available from J7200 SoCs onwards */
#define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS 0x4000
+/* Applicable to only AM64x SoCs */
+#define PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE 0x8000
/* R5 TI-SCI Processor Control Flags */
#define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT 0x0001
@@ -49,6 +51,8 @@
#define PROC_BOOT_STATUS_FLAG_R5_WFI 0x0002
#define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED 0x0004
#define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED0x0100
+/* Applicable to only AM64x SoCs */
+#define PROC_BOOT_STATUS_FLAG_R5_SINGLECORE_ONLY 0x0200
/**
* struct k3_r5_mem - internal memory structure
@@ -64,19 +68,29 @@ struct k3_r5_mem {
size_t size;
};
+/*
+ * All cluster mode values are not applicable on all SoCs. The following
+ * are the modes supported on various SoCs:
+ * Split mode : AM65x, J721E, J7200 and AM64x SoCs
+ * LockStep mode : AM65x, J721E and J7200 SoCs
+ * Single-CPU mode : AM64x SoCs only
+ */
enum cluster_mode {
CLUSTER_MODE_SPLIT = 0,
CLUSTER_MODE_LOCKSTEP,
+ CLUSTER_MODE_SINGLECPU,
};
/**
* struct k3_r5_soc_data - match data to handle SoC variations
* @tcm_is_double: flag to denote the larger unified TCMs in certain modes
* @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
+ * @single_cpu_mode: flag to denote if SoC/IP supports Single-CPU mode
*/
struct k3_r5_soc_data {
bool tcm_is_double;
bool tcm_ecc_autoinit;
+ bool single_cpu_mode;
};
/**
@@ -369,6 +383,13 @@ static inline int k3_r5_core_run(struct k3_r5_core *core)
* applicable cores to allow loading into the TCMs. The .prepare() ops is
* invoked by remoteproc core before any firmware loading, and is followed
* by the .start() ops after loading to actually let the R5 cores run.
+ *
+ * The Single-CPU mode on applicable SoCs (eg: AM64x) only uses Core0 to
+ * execute code, but combines the TCMs from both cores. The resets for both
+ * cores need to be released to make this possible, as the TCMs are in general
+ * private to each core. Only Core0 needs to be unhalted for running the
+ * cluster in this mode. The function uses the same reset logic as LockStep
+ * mode for this (though the behavior is agnostic of the reset release order).
*/
static int k3_r5_rproc_prepare(struct rproc *rproc)
{
@@ -386,7 +407,9 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
return ret;
mem_init_dis = !!(cfg & PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS);
- ret = (cluster->mode == CLUSTER_MODE_LOCKSTEP) ?
+ /* Re-use LockStep-mode reset logic for Single-CPU mode */
+ ret = (cluster->mode
[PATCH 1/2] dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs
The K3 AM64x SoCs have two dual-core Arm R5F clusters/subsystems, with 2 R5F cores each, both in the MAIN voltage domain. These clusters are a revised IP version compared to those present on J721E and J7200 SoCs, and supports a new "Single-CPU" mode instead of LockStep mode. Update the K3 R5F remoteproc bindings with the compatible info relevant to these R5F clusters/subsystems on K3 AM64x SoCs. Signed-off-by: Suman Anna --- .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 --- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml index d905d614502b..130fbaacc4b1 100644 --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml @@ -14,8 +14,12 @@ description: | processor subsystems/clusters (R5FSS). The dual core cluster can be used either in a LockStep mode providing safety/fault tolerance features or in a Split mode providing two individual compute cores for doubling the compute - capacity. These are used together with other processors present on the SoC - to achieve various system level goals. + capacity on most SoCs. These are used together with other processors present + on the SoC to achieve various system level goals. + + AM64x SoCs do not support LockStep mode, but rather a new non-safety mode + called "Single-CPU" mode, where only Core0 is used, but with ability to use + Core1's TCMs as well. Each Dual-Core R5F sub-system is represented as a single DTS node representing the cluster, with a pair of child DT nodes representing @@ -33,6 +37,7 @@ properties: - ti,am654-r5fss - ti,j721e-r5fss - ti,j7200-r5fss + - ti,am64-r5fss power-domains: description: | @@ -56,11 +61,12 @@ properties: ti,cluster-mode: $ref: /schemas/types.yaml#/definitions/uint32 -enum: [0, 1] description: | Configuration Mode for the Dual R5F cores within the R5F cluster. - Should be either a value of 1 (LockStep mode) or 0 (Split mode), - default is LockStep mode if omitted. + Should be either a value of 1 (LockStep mode) or 0 (Split mode) on + most SoCs (AM65x, J721E, J7200), default is LockStep mode if omitted; + and should be either a value of 0 (Split mode) or 2 (Single-CPU mode) + on AM64x SoCs, default is Split mode if omitted. # R5F Processor Child Nodes: # == @@ -97,6 +103,7 @@ patternProperties: - ti,am654-r5f - ti,j721e-r5f - ti,j7200-r5f + - ti,am64-r5f reg: items: @@ -198,6 +205,20 @@ patternProperties: unevaluatedProperties: false +if: + properties: +compatible: + enum: +- ti,am64-r5fss +then: + properties: +ti,cluster-mode: + enum: [0, 2] +else: + properties: +ti,cluster-mode: + enum: [0, 1] + required: - compatible - power-domains -- 2.30.1
[PATCH 0/2] TI K3 R5F remoteproc support on AM46x SoCs
Hi All, The following series enhances the K3 R5F remoteproc driver to add support for the R5F clusters on the newer TI K3 AM64x SoC family. The AM64x SoCs have 2 R5FSS clusters and no DSPs. Both clusters are capable of supporting either the conventional Split-mode or a brand new "Single-CPU" mode. The revised R5FSS IP has the following unique features: 1. The new Single-CPU mode allows the Core1 TCMs to be combined with the Core0 TCMs effectively doubling the amount of TCMs available. This is same behavior as LockStep-mode on J7200 SoCs, but all other previous SoCs could only use the Core0 TCMs. This combined TCMs appear contiguous at the respective Core0 TCM addresses. 2. TCMs are auto-initialized during module power-up, and the behavior is programmable through a SEC_MMR register bit. This is same as on J7200 SoCs, and is not present on earlier AM65x and J721E SoCs. The series is based on 5.12-rc2, and can apply on top of the current rproc-next branch as well. regards Suman Suman Anna (2): dt-bindings: remoteproc: k3-r5f: Update bindings for AM64x SoCs remoteproc: k3-r5: Extend support to R5F clusters on AM64x SoCs .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 31 +++- drivers/remoteproc/ti_k3_r5_remoteproc.c | 155 ++ 2 files changed, 152 insertions(+), 34 deletions(-) -- 2.30.1
Re: [PATCH] dt-bindings: Drop type references on common properties
On 3/16/21 2:48 PM, Rob Herring wrote: > Users of common properties shouldn't have a type definition as the > common schemas already have one. Drop all the unnecessary type > references in the tree. > > A meta-schema update to catch these is pending. > > Cc: Nicolas Saenz Julienne > Cc: Maxime Ripard > Cc: Linus Walleij > Cc: Bartosz Golaszewski > Cc: Bjorn Andersson > Cc: Krzysztof Kozlowski > Cc: Marc Kleine-Budde > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Srinivas Kandagatla > Cc: Ohad Ben-Cohen > Cc: Mark Brown > Cc: Cheng-Yi Chiang > Cc: Benson Leung > Cc: Zhang Rui > Cc: Daniel Lezcano > Cc: Greg Kroah-Hartman > Cc: Stefan Wahren > Cc: Masahiro Yamada > Cc: Odelu Kukatla > Cc: Alex Elder > Cc: Suman Anna > Cc: Kuninori Morimoto > Cc: Dmitry Baryshkov > Cc: linux-g...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-remotep...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring > --- > .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 5 + > Documentation/devicetree/bindings/arm/cpus.yaml | 2 -- > .../bindings/display/allwinner,sun4i-a10-tcon.yaml | 1 - > .../devicetree/bindings/gpio/socionext,uniphier-gpio.yaml| 3 +-- > .../devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 1 - > .../devicetree/bindings/interconnect/qcom,rpmh.yaml | 1 - > .../bindings/memory-controllers/nvidia,tegra210-emc.yaml | 2 +- > Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml | 1 - > Documentation/devicetree/bindings/net/qcom,ipa.yaml | 1 - > Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml | 2 -- > .../devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml | 2 +- For OMAP remoteproc, Acked-by: Suman Anna regards Suman
[PATCH] remoteproc: pru: Fix firmware loading crashes on K3 SoCs
The K3 PRUs are 32-bit processors and in general have some limitations
in using the standard ARMv8 memcpy function for loading firmware segments,
so the driver already uses a custom memcpy implementation. This added
logic however is limited to only IRAMs at the moment, but the loading
into Data RAMs is not completely ok either and does generate a kernel
crash for unaligned accesses.
Fix these crashes by removing the existing IRAM logic limitation and
extending the custom memcpy usage to Data RAMs as well for all K3 SoCs.
Fixes: 1d39f4d19921 ("remoteproc: pru: Add support for various PRU cores on K3
AM65x SoCs")
Signed-off-by: Suman Anna
---
drivers/remoteproc/pru_rproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 2667919d76b3..16979c1cd2f4 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -585,7 +585,7 @@ pru_rproc_load_elf_segments(struct rproc *rproc, const
struct firmware *fw)
break;
}
- if (pru->data->is_k3 && is_iram) {
+ if (pru->data->is_k3) {
ret = pru_rproc_memcpy(ptr, elf_data + phdr->p_offset,
filesz);
if (ret) {
--
2.30.1
Re: [PATCH 1/2] dt-bindings: mailbox: omap: Update binding for AM64x SoCs
On 2/9/21 1:24 PM, Rob Herring wrote:
> On Wed, Jan 27, 2021 at 01:55:59PM -0600, Suman Anna wrote:
>> Update the existing OMAP Mailbox binding to include the info for
>> AM64x SoCs. There are some minor IP integration differences between
>> the AM64x SoCs and the previous AM65x and J721E SoC families.
>>
>> Signed-off-by: Suman Anna
>> ---
>> .../bindings/mailbox/omap-mailbox.txt | 22 +++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
>> b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
>> index 5fe80c1c19fc..c993d1a5c14a 100644
>> --- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
>> +++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
>> @@ -28,6 +28,9 @@ SoCs has each of these instances form a cluster and
>> combine multiple clusters
>> into a single IP block present within the Main NavSS. The interrupt lines
>> from
>> all these clusters are multiplexed and routed to different processor
>> subsystems
>> over a limited number of common interrupt output lines of an Interrupt
>> Router.
>> +The AM64x SoCS also uses a single IP block comprising of multiple clusters,
>> +but the number of clusters are smaller, and the interrupt output lines are
>> +connected directly to various processors.
>>
>> Mailbox Device Node:
>>
>> @@ -42,6 +45,7 @@ Required properties:
>> "ti,omap4-mailbox" for OMAP44xx, OMAP54xx, AM33xx,
>> AM43xx and DRA7xx SoCs
>> "ti,am654-mailbox" for K3 AM65x and J721E SoCs
>> +"ti,am64-mailbox" for K3 AM64x SoCs
>> - reg: Contains the mailbox register address range
>> (base
>> address and length)
>> - interrupts: Contains the interrupt information for the
>> mailbox
>> @@ -178,3 +182,21 @@ mailbox: mailbox@480c8000 {
>> };
>> };
>> };
>> +
>> +4. /* AM64x */
>> +_main {
>
> Please don't add examples for just a new compatible.
Thanks, will keep this in mind for the future and drop this as well just like on
the HwSpinlock binding update.
regards
Suman
>
>> +mailbox0_cluster2: mailbox@2902 {
>> +compatible = "ti,am64-mailbox";
>> +reg = <0x00 0x2902 0x00 0x200>;
>> +interrupts = ,
>> + ;
>> +#mbox-cells = <1>;
>> +ti,mbox-num-users = <4>;
>> +ti,mbox-num-fifos = <16>;
>> +
>> +mbox_main_r5fss0_core0: mbox-main-r5fss0-core0 {
>> +ti,mbox-rx = <0 0 2>;
>> +ti,mbox-tx = <1 0 2>;
>> +};
>> +};
>> +};
>> --
>> 2.29.2
>>
[PATCH v2 0/2] Add Mailbox support for TI K3 AM64x SoCs
Hi Jassi, The following is a minor revision of the series [1] that extends the OMAP Mailbox driver for the IP on TI K3 AM64x SoCs. Only change is in patch 1 to drop the example in the dt-bindings patch. regards Suman [1] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20210127195600.23501-1-s-a...@ti.com/ Suman Anna (2): dt-bindings: mailbox: omap: Update binding for AM64x SoCs mailbox: omap: Add support for K3 AM64x SoCs Documentation/devicetree/bindings/mailbox/omap-mailbox.txt | 4 drivers/mailbox/omap-mailbox.c | 6 +- 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.29.2
[PATCH v2 2/2] mailbox: omap: Add support for K3 AM64x SoCs
The AM64x SoC contains a Mailbox IP instance with multiple clusters
in the MAIN domain, and is a variant of the IP on current AM65x and
J721E SoCs. The AM64x SoC has only 8 clusters with no interrupts
routed to the A53 core on the first 2 clusters. The interrupt outputs
from the IP do not go through any Interrupt Routers and are hard-wired
to each processor, with only couple of interrupts from each cluster
reaching the A53 core. The IP is also not built with the K3 safety
feature in hardware.
Add the support for this IP through a new compatible.
Signed-off-by: Suman Anna
---
v2: No changes
drivers/mailbox/omap-mailbox.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 93fe08aef3ca..7295e3835e30 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -3,7 +3,7 @@
* OMAP mailbox driver
*
* Copyright (C) 2006-2009 Nokia Corporation. All rights reserved.
- * Copyright (C) 2013-2019 Texas Instruments Incorporated - https://www.ti.com
+ * Copyright (C) 2013-2021 Texas Instruments Incorporated - https://www.ti.com
*
* Contact: Hiroshi DOYU
* Suman Anna
@@ -663,6 +663,10 @@ static const struct of_device_id omap_mailbox_of_match[] =
{
.compatible = "ti,am654-mailbox",
.data = _data,
},
+ {
+ .compatible = "ti,am64-mailbox",
+ .data = _data,
+ },
{
/* end */
},
--
2.29.2
[PATCH v2 1/2] dt-bindings: mailbox: omap: Update binding for AM64x SoCs
Update the existing OMAP Mailbox binding to include the info for AM64x SoCs. There are some minor IP integration differences between the AM64x SoCs and the previous AM65x and J721E SoC families. Signed-off-by: Suman Anna --- v2: Remove AM64x example as per Rob's comments v1: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210127195600.23501-2-s-a...@ti.com/ Documentation/devicetree/bindings/mailbox/omap-mailbox.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt index 5fe80c1c19fc..12371f5c6cd9 100644 --- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt +++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt @@ -28,6 +28,9 @@ SoCs has each of these instances form a cluster and combine multiple clusters into a single IP block present within the Main NavSS. The interrupt lines from all these clusters are multiplexed and routed to different processor subsystems over a limited number of common interrupt output lines of an Interrupt Router. +The AM64x SoCS also uses a single IP block comprising of multiple clusters, +but the number of clusters are smaller, and the interrupt output lines are +connected directly to various processors. Mailbox Device Node: @@ -42,6 +45,7 @@ Required properties: "ti,omap4-mailbox" for OMAP44xx, OMAP54xx, AM33xx, AM43xx and DRA7xx SoCs "ti,am654-mailbox" for K3 AM65x and J721E SoCs + "ti,am64-mailbox" for K3 AM64x SoCs - reg: Contains the mailbox register address range (base address and length) - interrupts: Contains the interrupt information for the mailbox -- 2.29.2
[PATCH] soc: ti: k3-ringacc: Use of_device_get_match_data()
Simplify the retrieval of getting the match data in the probe
function by directly using of_device_get_match_data() instead
of using of_match_node() and getting data.
Signed-off-by: Suman Anna
---
drivers/soc/ti/k3-ringacc.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
index b495b0d5d0fa..312ba0f98ad7 100644
--- a/drivers/soc/ti/k3-ringacc.c
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1517,15 +1518,13 @@ EXPORT_SYMBOL_GPL(k3_ringacc_dmarings_init);
static int k3_ringacc_probe(struct platform_device *pdev)
{
const struct ringacc_match_data *match_data;
- const struct of_device_id *match;
struct device *dev = >dev;
struct k3_ringacc *ringacc;
int ret;
- match = of_match_node(k3_ringacc_of_match, dev->of_node);
- if (!match)
+ match_data = of_device_get_match_data(>dev);
+ if (!match_data)
return -ENODEV;
- match_data = match->data;
ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL);
if (!ringacc)
--
2.29.2
Re: [PATCH] remoteproc: pru: future-proof PRU ID matching
On 1/28/21 5:21 PM, David Lechner wrote:
> On 1/28/21 4:55 PM, Suman Anna wrote:
>> Hi David,
>>
>> On 1/15/21 6:53 PM, Suman Anna wrote:
>>> On 1/4/21 3:18 PM, David Lechner wrote:
>>>> static int pru_rproc_probe(struct platform_device *pdev)
>>>> @@ -825,20 +808,28 @@ static int pru_rproc_remove(struct platform_device
>>>> *pdev)
>>>> static const struct pru_private_data pru_data = {
>>>> .type = PRU_TYPE_PRU,
>>>> + .pru0_iram_offset = 0x4000,
>>>> + .pru1_iram_offset = 0x8000,
>>
>> The offsets for the PRU cores are actually 0x34000 and 0x38000 respectively
>> from
>> the base of the PRUSS on non-Davinci SoCs.
>>
>> If we were to use this static data approach, then we might as well continue
>> to
>> use the current address masking logic with the appropriate masks for Davinci
>> (0x38000 and 0x3C000, not true offsets but as masks they would work). Davinci
>> PRUSS is the only one with its differences being the first PRUSS IP, and I
>> would
>> prefer to keep the logic aligned to the IPs on all the recent SoCs on 3
>> different TI SoC families (OMAP, Keystone 2 and K3).
>>
>> Let me know what you think.
>
> I'm not too picky as long as it works. :-)
>
> If keeping the static data to a minimum is really important, I suppose we
> could
> introduce a new type = PRU_TYPE_PRU_V1 for these PRUSSs instead. It sounds
> like
> this information might be useful elsewhere anyway.
Yeah, let me look into this further and see if there is any other way possible
as well. I had some patches for DA850 lying around on some older kernels, and I
know that I definitely needed to introduce a variable to distinguish the
programmability of certain CONST registers (C26 and C27 are fixed on Davinci
IIRC). Will need that variable with the current PRU consumer series. Perhaps,
the same PRU type variable can be used for that as well.
regards
Suman
Re: [PATCH] remoteproc: pru: future-proof PRU ID matching
Hi David,
On 1/15/21 6:53 PM, Suman Anna wrote:
> On 1/4/21 3:18 PM, David Lechner wrote:
>> Currently, to determine the ID (0 or 1) of a PRU core, the last 19 bits
>> of the physical address of the cores IRAM are compared to known values.
>> However, the PRUs on TI AM18XX have IRAM at 0x01c38000 and 0x01c3c000
>> respectively. The former conflicts with PRU1_IRAM_ADDR_MASK which could
>> cause PRU0 to be detected as PRU1. (The latter also conflicts with
>> TX_PRU1_IRAM_ADDR_MASK but it would still be correctly detected as
>> PRU1.)
>>
>> This fixes the problem by moving the address matching offset values to
>> the device-specific data. This way the compatible string does half of
>> the work of narrowing down the addresses to two possibilities instead
>> of checking the address against all possible PRU types. This also lets
>> us narrow down the scope of the match from 19 bits to 16 bits for all
>> PRU types.
>>
>> After this, the TI AM18XX PRUs will be able to be added without running
>> into the problems stated above.
>>
>> We can also drop the local ret variable while touching this code.
>>
>> Signed-off-by: David Lechner
>
> Will test this patch on Mon/Tue on various platforms.
>
> Bjorn,
> Please wait for my Ack on this before you pick this up.
>
> regards
> Suman
>
>> ---
>> drivers/remoteproc/pru_rproc.c | 49 ++
>> 1 file changed, 20 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>> index 2667919d76b3..94ce48df2f48 100644
>> --- a/drivers/remoteproc/pru_rproc.c
>> +++ b/drivers/remoteproc/pru_rproc.c
>> @@ -46,15 +46,6 @@
>> #define PRU_DEBUG_GPREG(x) (0x + (x) * 4)
>> #define PRU_DEBUG_CT_REG(x) (0x0080 + (x) * 4)
>>
>> -/* PRU/RTU/Tx_PRU Core IRAM address masks */
>> -#define PRU_IRAM_ADDR_MASK 0x3
>> -#define PRU0_IRAM_ADDR_MASK 0x34000
>> -#define PRU1_IRAM_ADDR_MASK 0x38000
>> -#define RTU0_IRAM_ADDR_MASK 0x4000
>> -#define RTU1_IRAM_ADDR_MASK 0x6000
>> -#define TX_PRU0_IRAM_ADDR_MASK 0xa000
>> -#define TX_PRU1_IRAM_ADDR_MASK 0xc000
>> -
>> /* PRU device addresses for various type of PRU RAMs */
>> #define PRU_IRAM_DA 0 /* Instruction RAM */
>> #define PRU_PDRAM_DA0 /* Primary Data RAM */
>> @@ -96,10 +87,14 @@ enum pru_type {
>> /**
>> * struct pru_private_data - device data for a PRU core
>> * @type: type of the PRU core (PRU, RTU, Tx_PRU)
>> + * @pru0_iram_offset: used to identify PRU core 0
>> + * @pru1_iram_offset: used to identify PRU core 1
>> * @is_k3: flag used to identify the need for special load handling
>> */
>> struct pru_private_data {
>> enum pru_type type;
>> +u16 pru0_iram_offset;
>> +u16 pru1_iram_offset;
>> unsigned int is_k3 : 1;
>> };
>>
>> @@ -693,33 +688,21 @@ static int pru_rproc_parse_fw(struct rproc *rproc,
>> const struct firmware *fw)
>> }
>>
>> /*
>> - * Compute PRU id based on the IRAM addresses. The PRU IRAMs are
>> + * Compute PRU id based on the last 16 bits of IRAM addresses. The PRU
>> IRAMs are
>> * always at a particular offset within the PRUSS address space.
>> */
>> static int pru_rproc_set_id(struct pru_rproc *pru)
>> {
>> -int ret = 0;
>> -
>> -switch (pru->mem_regions[PRU_IOMEM_IRAM].pa & PRU_IRAM_ADDR_MASK) {
>> -case TX_PRU0_IRAM_ADDR_MASK:
>> -fallthrough;
>> -case RTU0_IRAM_ADDR_MASK:
>> -fallthrough;
>> -case PRU0_IRAM_ADDR_MASK:
>> +u16 offset = pru->mem_regions[PRU_IOMEM_IRAM].pa;
>> +
>> +if (offset == pru->data->pru0_iram_offset)
>> pru->id = 0;
>> -break;
>> -case TX_PRU1_IRAM_ADDR_MASK:
>> -fallthrough;
>> -case RTU1_IRAM_ADDR_MASK:
>> -fallthrough;
>> -case PRU1_IRAM_ADDR_MASK:
>> +else if (offset == pru->data->pru1_iram_offset)
>> pru->id = 1;
>> -break;
>> -default:
>> -ret = -EINVAL;
>> -}
>> +else
>> +return -EINVAL;
>>
>> -return ret;
>> +return 0;
While this logic does work, it is a bit convoluted IMO for any one reading the
code. The offset here is kind of a misnomer, this logic is relying on only the
least 16-bits of the address.
I have actually used a different logic in our downstream code, and we ch
[PATCH 0/2] Add Mailbox support for TI K3 AM64x SoCs
Hi Jassi, The following series extends the OMAP Mailbox driver to enable the Mailbox IP present on the most recent TI K3 AM64x SoCs [1]. AM64x is a 64-bit system, and the Mailbox IP is a cut-down version of the one present on previous TI K3 SoCs like AM65x and J721E. DT nodes will be posted separately once the binding is acked/merged. regards Suman [1] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20210120202532.9011-1-d-gerl...@ti.com/ Suman Anna (2): dt-bindings: mailbox: omap: Update binding for AM64x SoCs mailbox: omap: Add support for K3 AM64x SoCs .../bindings/mailbox/omap-mailbox.txt | 22 +++ drivers/mailbox/omap-mailbox.c| 6 - 2 files changed, 27 insertions(+), 1 deletion(-) -- 2.29.2
[PATCH 2/2] mailbox: omap: Add support for K3 AM64x SoCs
The AM64x SoC contains a Mailbox IP instance with multiple clusters
in the MAIN domain, and is a variant of the IP on current AM65x and
J721E SoCs. The AM64x SoC has only 8 clusters with no interrupts
routed to the A53 core on the first 2 clusters. The interrupt outputs
from the IP do not go through any Interrupt Routers and are hard-wired
to each processor, with only couple of interrupts from each cluster
reaching the A53 core. The IP is also not built with the K3 safety
feature in hardware.
Add the support for this IP through a new compatible.
Signed-off-by: Suman Anna
---
drivers/mailbox/omap-mailbox.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 93fe08aef3ca..7295e3835e30 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -3,7 +3,7 @@
* OMAP mailbox driver
*
* Copyright (C) 2006-2009 Nokia Corporation. All rights reserved.
- * Copyright (C) 2013-2019 Texas Instruments Incorporated - https://www.ti.com
+ * Copyright (C) 2013-2021 Texas Instruments Incorporated - https://www.ti.com
*
* Contact: Hiroshi DOYU
* Suman Anna
@@ -663,6 +663,10 @@ static const struct of_device_id omap_mailbox_of_match[] =
{
.compatible = "ti,am654-mailbox",
.data = _data,
},
+ {
+ .compatible = "ti,am64-mailbox",
+ .data = _data,
+ },
{
/* end */
},
--
2.29.2
[PATCH 1/2] dt-bindings: mailbox: omap: Update binding for AM64x SoCs
Update the existing OMAP Mailbox binding to include the info for
AM64x SoCs. There are some minor IP integration differences between
the AM64x SoCs and the previous AM65x and J721E SoC families.
Signed-off-by: Suman Anna
---
.../bindings/mailbox/omap-mailbox.txt | 22 +++
1 file changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
index 5fe80c1c19fc..c993d1a5c14a 100644
--- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
+++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
@@ -28,6 +28,9 @@ SoCs has each of these instances form a cluster and combine
multiple clusters
into a single IP block present within the Main NavSS. The interrupt lines from
all these clusters are multiplexed and routed to different processor subsystems
over a limited number of common interrupt output lines of an Interrupt Router.
+The AM64x SoCS also uses a single IP block comprising of multiple clusters,
+but the number of clusters are smaller, and the interrupt output lines are
+connected directly to various processors.
Mailbox Device Node:
@@ -42,6 +45,7 @@ Required properties:
"ti,omap4-mailbox" for OMAP44xx, OMAP54xx, AM33xx,
AM43xx and DRA7xx SoCs
"ti,am654-mailbox" for K3 AM65x and J721E SoCs
+ "ti,am64-mailbox" for K3 AM64x SoCs
- reg: Contains the mailbox register address range (base
address and length)
- interrupts: Contains the interrupt information for the mailbox
@@ -178,3 +182,21 @@ mailbox: mailbox@480c8000 {
};
};
};
+
+4. /* AM64x */
+_main {
+ mailbox0_cluster2: mailbox@2902 {
+ compatible = "ti,am64-mailbox";
+ reg = <0x00 0x2902 0x00 0x200>;
+ interrupts = ,
+;
+ #mbox-cells = <1>;
+ ti,mbox-num-users = <4>;
+ ti,mbox-num-fifos = <16>;
+
+ mbox_main_r5fss0_core0: mbox-main-r5fss0-core0 {
+ ti,mbox-rx = <0 0 2>;
+ ti,mbox-tx = <1 0 2>;
+ };
+ };
+};
--
2.29.2
[PATCH] dt-bindings: irqchip: Add node name to PRUSS INTC
The current PRUSS Interrupt Controller binding doesn't exactly specify the convention for the node name. These interrupt-controllers will always have a unit address. Update the binding with the '$nodename' using the expected generic name, this shall ensure the interrupt-controller.yaml is automatically applied to this binding. Signed-off-by: Suman Anna --- This patch is a result of the previous discussion at https://patchwork.kernel.org/comment/23926133/ .../bindings/interrupt-controller/ti,pruss-intc.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml index c2ce215501a5..9731dd4421a1 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml @@ -33,6 +33,9 @@ description: | corresponding PRUSS node. The node should be named "interrupt-controller". properties: + $nodename: +pattern: "^interrupt-controller@[0-9a-f]+$" + compatible: enum: - ti,pruss-intc -- 2.29.2
Re: [PATCH] dt-bindings: irqchip: Add #address-cells to PRUSS INTC
Hi Rob, On 1/25/21 8:47 PM, Rob Herring wrote: > On Mon, Jan 25, 2021 at 6:16 PM Suman Anna wrote: >> >> Hi Rob, >> >> On 1/25/21 6:04 PM, Rob Herring wrote: >>> On Fri, Jan 15, 2021 at 02:58:19PM -0600, Suman Anna wrote: >>>> The '#address-cells' property looks to be a required property for >>>> interrupt controller nodes as indicated by a warning message seen >>>> when building dtbs with W=2. Adding the property to the PRUSS INTC >>>> dts nodes though fails the dtbs_check. Add this property to the >>>> PRUSS INTC binding to make it compliant with both dtbs_check and >>>> building dtbs. >>>> >>>> Signed-off-by: Suman Anna >>>> --- >>>> Hi Rob, >>>> >>>> This patch is also part of our effort to get rid of the warnings seen >>>> around interrupt providers on TI K3 dtbs [1]. I needed this in the PRUSS >>>> INTC bindings to not get a warning with dtbs_check while also ensuring >>>> no warnings while building dtbs with W=2. >>>> >>>> I would have expected the '#address-cells' requirement to be inherited >>>> automatically. And looking through the schema files, I actually do not >>>> see the interrupt-controller.yaml included automatically anywhere. You >>>> had asked us to drop the inclusion in this binding in our first version >>>> with YAML [3]. Am I missing something, and how do we ensure that this >>>> is enforced automatically for everyone? >>> >>> interrupt-controller.yaml is applied to any node named >>> 'interrupt-controller'. More generally, if 'compatible' is not present, >>> then we look at $nodename for the default 'select'. In your case, you >>> didn't name the node appropriately. >> >> Thanks for the clarification. Yeah, I didn't add anything specifically, since >> the expectation is interrupt-controller. Should I be adding that to this >> binding? > > No, either interrupt-controller.yaml needs to learn a new node name or > your node names need to be fixed. I prefer the latter, but if you have > more than 1 and don't have a unit-address (and in turn a 'reg' prop) > we'd have to do the former. How are the interrupts controllers > accessed if there's no way to address them? The PRUSS INTC will always have a unit-address, so we won't have the issues with having to maintain unique names. All my examples already have the nodes in the form 'interrupt-controller@'. Anyway, I will drop this patch, and post a new patch adding the $nodename to the binding. > >> >>> >>> We can't check this in interrupt-controller.yaml because #address-cells >>> is not always 0. GICv3 is one notable exception. >>> >>>> >>>> regards >>>> Suman >>>> >>>> [1] >>>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210115083003.27387-1-lokeshvu...@ti.com/ >>> >>> I've commented on this thread now in regards to #address-cells. >> >> I suppose I still need this patch to be defined to unblock the ICSSG nodes >> getting accepted by our dts maintainer. Care to give your Reviewed-by for the >> change? Or I can spin a v2 with $nodename added as well if that's needed too. > > No, I don't think you have to add #address-cells. We need to fix the > warning in dtc. Thank you for clarifying this. regards Suman
Re: [PATCH] dt-bindings: irqchip: Add #address-cells to PRUSS INTC
<<< No Message Collected >>>
Re: [PATCH v2 0/5] Introduce PRU remoteproc consumer API
Hi Santosh, On 1/24/21 10:34 PM, santosh.shilim...@oracle.com wrote: > Hi Suman, Mathieu, > > On 1/7/21 2:49 PM, Suman Anna wrote: >> On 1/7/21 4:44 PM, Mathieu Poirier wrote: >>> On Wed, Jan 06, 2021 at 06:03:25PM -0600, Suman Anna wrote: >>>> Hi Mathieu, >>>> > [...] >>> I only see input from Andy and Lars in the thread you point out, nothing >>> from >>> Greg. I have also taken a look at the patch [1] that made checkpatch >>> complain >>> about ENOTSUPP. From what I see in that commit log the goal is to prevent >>> new >>> additions of ENOTSUPP to the kernel. >>> >>> Please modify and resend, otherwise I'm sure someone will send another >>> patch to >>> fix it before the end of the cycle. >> >> Yeah ok. I will send out a v3. >> > I haven't seen v3 of this series yet. Please post it > if you would like to include it for 5.12. This series is dependent on couple of patches that would have to come through the remoteproc tree first, and I need to post the next versions of those as well. So, let me sort out those first. You can drop this from your queue for 5.12. regards Suman
[PATCH] soc: ti: pruss: Refactor the CFG sub-module init
The CFG sub-module is not present on some earlier SoCs like the
DA850/OMAPL-138 in the TI Davinci family. Refactor out the CFG
sub-module parse and initialization logic into a separate function
to make it easier to add logic for the PRUSS IP on the above legacy
SoC families.
Signed-off-by: Suman Anna
---
Hi Santosh,
This patch would apply on top of your for_5.12/drivers-soc branch.
David can build his PRUSS support on Davinci on top of this patch.
regards
Suman
drivers/soc/ti/pruss.c | 91 +++---
1 file changed, 50 insertions(+), 41 deletions(-)
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
index 1d6890134312..f22ac1edbdd0 100644
--- a/drivers/soc/ti/pruss.c
+++ b/drivers/soc/ti/pruss.c
@@ -161,6 +161,53 @@ static struct regmap_config regmap_conf = {
.reg_stride = 4,
};
+static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
+{
+ struct device_node *np = dev_of_node(dev);
+ struct device_node *child;
+ struct resource res;
+ int ret;
+
+ child = of_get_child_by_name(np, "cfg");
+ if (!child) {
+ dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
+ return -ENODEV;
+ }
+
+ if (of_address_to_resource(child, 0, )) {
+ ret = -ENOMEM;
+ goto node_put;
+ }
+
+ pruss->cfg_base = devm_ioremap(dev, res.start, resource_size());
+ if (!pruss->cfg_base) {
+ ret = -ENOMEM;
+ goto node_put;
+ }
+
+ regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
+(u64)res.start);
+ regmap_conf.max_register = resource_size() - 4;
+
+ pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
+ _conf);
+ kfree(regmap_conf.name);
+ if (IS_ERR(pruss->cfg_regmap)) {
+ dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
+ PTR_ERR(pruss->cfg_regmap));
+ ret = PTR_ERR(pruss->cfg_regmap);
+ goto node_put;
+ }
+
+ ret = pruss_clk_init(pruss, child);
+ if (ret)
+ dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
+
+node_put:
+ of_node_put(child);
+ return ret;
+}
+
static int pruss_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
@@ -239,56 +286,18 @@ static int pruss_probe(struct platform_device *pdev)
goto rpm_disable;
}
- child = of_get_child_by_name(np, "cfg");
- if (!child) {
- dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
- ret = -ENODEV;
+ ret = pruss_cfg_of_init(dev, pruss);
+ if (ret < 0)
goto rpm_put;
- }
-
- if (of_address_to_resource(child, 0, )) {
- ret = -ENOMEM;
- goto node_put;
- }
-
- pruss->cfg_base = devm_ioremap(dev, res.start, resource_size());
- if (!pruss->cfg_base) {
- ret = -ENOMEM;
- goto node_put;
- }
-
- regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
-(u64)res.start);
- regmap_conf.max_register = resource_size() - 4;
-
- pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
- _conf);
- kfree(regmap_conf.name);
- if (IS_ERR(pruss->cfg_regmap)) {
- dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
- PTR_ERR(pruss->cfg_regmap));
- ret = PTR_ERR(pruss->cfg_regmap);
- goto node_put;
- }
-
- ret = pruss_clk_init(pruss, child);
- if (ret) {
- dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
- goto node_put;
- }
ret = devm_of_platform_populate(dev);
if (ret) {
dev_err(dev, "failed to register child devices\n");
- goto node_put;
+ goto rpm_put;
}
- of_node_put(child);
-
return 0;
-node_put:
- of_node_put(child);
rpm_put:
pm_runtime_put_sync(dev);
rpm_disable:
--
2.29.2
Re: [PATCH] arm64: dts: ti: k3: mmc: fix dtbs_check warnings
On 1/15/21 1:30 PM, Grygorii Strashko wrote:
> Now the dtbs_check produces below warnings
> sdhci@4f8: clock-names:0: 'clk_ahb' was expected
> sdhci@4f8: clock-names:1: 'clk_xin' was expected
> $nodename:0: 'sdhci@4f8' does not match '^mmc(@.*)?$'
>
> Fix above warnings by updating mmc DT definitions to follow
> sdhci-am654.yaml bindings:
> - rename sdhci dt nodes to 'mmc@'
> - swap clk_xin/clk_ahb clocks, the clk_ahb clock expected to be defined
> first
>
> Signed-off-by: Grygorii Strashko
Thanks for fixing these Grygorii,
Reviewed-by: Suman Anna
On a side note, there are still couple more warnings on J721E dtb
/uhome/projects/opensrc/kernels/linux-next/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dt.yaml:
mmc@4fb: compatible: More than one condition true in oneOf schema:
{'oneOf': [{'additionalItems': False,
'items': [{'const': 'ti,am654-sdhci-5.1'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,j721e-sdhci-8bit'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,j721e-sdhci-4bit'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,j721e-sdhci-4bit'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,am64-sdhci-8bit'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,am64-sdhci-4bit'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,j7200-sdhci-8bit'},
{'const': 'ti,j721e-sdhci-8bit'}],
'maxItems': 2,
'minItems': 2,
'type': 'array'},
{'additionalItems': False,
'items': [{'const': 'ti,j7200-sdhci-4bit'},
{'const': 'ti,j721e-sdhci-4bit'}],
'maxItems': 2,
'minItems': 2,
'type': 'array'}]}
From schema:
/uhome/projects/opensrc/kernels/linux-next/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
Also, the YAML itself can do with some improvements. I see a minItems of 1, so I
am assuming clk_ahb is always the mandatory clock where applicable, can you
confirm?
regards
Suman
> ---
> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 4 ++--
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 8
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 18 +-
> 3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index 12591a854020..ceb579fb427d 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -256,7 +256,7 @@
> #size-cells = <0>;
> };
>
> - sdhci0: sdhci@4f8 {
> + sdhci0: mmc@4f8 {
> compatible = "ti,am654-sdhci-5.1";
> reg = <0x0 0x4f8 0x0 0x260>, <0x0 0x4f9 0x0 0x134>;
> power-domains = <_pds 47 TI_SCI_PD_EXCLUSIVE>;
> @@ -280,7 +280,7 @@
> dma-coherent;
> };
>
> - sdhci1: sdhci@4fa {
> + sdhci1: mmc@4fa {
> compatible = "ti,am654-sdhci-5.1";
> reg = <0x0 0x4fa 0x0 0x260>, <0x0 0x4fb 0x0 0x134>;
> power-domains = <_pds 48 TI_SCI_PD_EXCLUSIVE>;
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> index 4e39f0325c03..3f23b913b498 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> @@ -506,8 +506,8 @@
> reg = <0x00 0x04f8 0x00 0x260>, <0x00 0x4f88000 0x00 0x134>;
> interrupts = ;
> power-domains = <_pds 91 TI_SCI_PD_EXCLUSIVE>;
> - clock-names = "clk_xin", "clk_ahb";
> - clocks = <_clks 91 3>, <_clks 91 0>;
> + clock-names = "clk_ahb"
Re: [PATCH] arm64: dts: ti: k3*: Fixup PMU compatibility to be CPU specific
On 1/20/21 1:51 PM, Nishanth Menon wrote:
> We can use CPU specific pmu configuration to expose the appropriate
> CPU specific events rather than just the basic generic pmuv3 perf
> events.
>
> Reported-by: Sudeep Holla
> Signed-off-by: Nishanth Menon
Tested-by: Suman Anna
regards
Suman
> ---
>
> AM65: https://pastebin.ubuntu.com/p/TF2cCMySkt/
> J721E: https://pastebin.ubuntu.com/p/jgGPNmNgG3/
> J7200: https://pastebin.ubuntu.com/p/Kfc3VHHXNB/
>
> Original report:
> https://lore.kernel.org/linux-arm-kernel/20210119172412.smsjdo2sjzqi5vcn@bogus/
>
> I have'nt split this patch up for fixes tag primarily because the
> basic functionality works and this is an improvement than a critical
> fixup to backport for older kernels.
>
> arch/arm64/boot/dts/ti/k3-am65.dtsi | 2 +-
> arch/arm64/boot/dts/ti/k3-j7200.dtsi | 2 +-
> arch/arm64/boot/dts/ti/k3-j721e.dtsi | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> index d84c0bc05023..a9fc1af03f27 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65.dtsi
> @@ -56,7 +56,7 @@ a53_timer0: timer-cl0-cpu0 {
> };
>
> pmu: pmu {
> - compatible = "arm,armv8-pmuv3";
> + compatible = "arm,cortex-a53-pmu";
> /* Recommendation from GIC500 TRM Table A.3 */
> interrupts = ;
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> index 66169bcf7c9a..b7005b803149 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> @@ -114,7 +114,7 @@ a72_timer0: timer-cl0-cpu0 {
> };
>
> pmu: pmu {
> - compatible = "arm,armv8-pmuv3";
> + compatible = "arm,cortex-a72-pmu";
> interrupts = ;
> };
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> index cc483f7344af..f0587fde147e 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> @@ -115,7 +115,7 @@ a72_timer0: timer-cl0-cpu0 {
> };
>
> pmu: pmu {
> - compatible = "arm,armv8-pmuv3";
> + compatible = "arm,cortex-a72-pmu";
> /* Recommendation from GIC500 TRM Table A.3 */
> interrupts = ;
> };
>
Re: [PATCH] remoteproc: pru: future-proof PRU ID matching
On 1/4/21 3:18 PM, David Lechner wrote:
> Currently, to determine the ID (0 or 1) of a PRU core, the last 19 bits
> of the physical address of the cores IRAM are compared to known values.
> However, the PRUs on TI AM18XX have IRAM at 0x01c38000 and 0x01c3c000
> respectively. The former conflicts with PRU1_IRAM_ADDR_MASK which could
> cause PRU0 to be detected as PRU1. (The latter also conflicts with
> TX_PRU1_IRAM_ADDR_MASK but it would still be correctly detected as
> PRU1.)
>
> This fixes the problem by moving the address matching offset values to
> the device-specific data. This way the compatible string does half of
> the work of narrowing down the addresses to two possibilities instead
> of checking the address against all possible PRU types. This also lets
> us narrow down the scope of the match from 19 bits to 16 bits for all
> PRU types.
>
> After this, the TI AM18XX PRUs will be able to be added without running
> into the problems stated above.
>
> We can also drop the local ret variable while touching this code.
>
> Signed-off-by: David Lechner
Will test this patch on Mon/Tue on various platforms.
Bjorn,
Please wait for my Ack on this before you pick this up.
regards
Suman
> ---
> drivers/remoteproc/pru_rproc.c | 49 ++
> 1 file changed, 20 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index 2667919d76b3..94ce48df2f48 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -46,15 +46,6 @@
> #define PRU_DEBUG_GPREG(x) (0x + (x) * 4)
> #define PRU_DEBUG_CT_REG(x) (0x0080 + (x) * 4)
>
> -/* PRU/RTU/Tx_PRU Core IRAM address masks */
> -#define PRU_IRAM_ADDR_MASK 0x3
> -#define PRU0_IRAM_ADDR_MASK 0x34000
> -#define PRU1_IRAM_ADDR_MASK 0x38000
> -#define RTU0_IRAM_ADDR_MASK 0x4000
> -#define RTU1_IRAM_ADDR_MASK 0x6000
> -#define TX_PRU0_IRAM_ADDR_MASK 0xa000
> -#define TX_PRU1_IRAM_ADDR_MASK 0xc000
> -
> /* PRU device addresses for various type of PRU RAMs */
> #define PRU_IRAM_DA 0 /* Instruction RAM */
> #define PRU_PDRAM_DA 0 /* Primary Data RAM */
> @@ -96,10 +87,14 @@ enum pru_type {
> /**
> * struct pru_private_data - device data for a PRU core
> * @type: type of the PRU core (PRU, RTU, Tx_PRU)
> + * @pru0_iram_offset: used to identify PRU core 0
> + * @pru1_iram_offset: used to identify PRU core 1
> * @is_k3: flag used to identify the need for special load handling
> */
> struct pru_private_data {
> enum pru_type type;
> + u16 pru0_iram_offset;
> + u16 pru1_iram_offset;
> unsigned int is_k3 : 1;
> };
>
> @@ -693,33 +688,21 @@ static int pru_rproc_parse_fw(struct rproc *rproc,
> const struct firmware *fw)
> }
>
> /*
> - * Compute PRU id based on the IRAM addresses. The PRU IRAMs are
> + * Compute PRU id based on the last 16 bits of IRAM addresses. The PRU IRAMs
> are
> * always at a particular offset within the PRUSS address space.
> */
> static int pru_rproc_set_id(struct pru_rproc *pru)
> {
> - int ret = 0;
> -
> - switch (pru->mem_regions[PRU_IOMEM_IRAM].pa & PRU_IRAM_ADDR_MASK) {
> - case TX_PRU0_IRAM_ADDR_MASK:
> - fallthrough;
> - case RTU0_IRAM_ADDR_MASK:
> - fallthrough;
> - case PRU0_IRAM_ADDR_MASK:
> + u16 offset = pru->mem_regions[PRU_IOMEM_IRAM].pa;
> +
> + if (offset == pru->data->pru0_iram_offset)
> pru->id = 0;
> - break;
> - case TX_PRU1_IRAM_ADDR_MASK:
> - fallthrough;
> - case RTU1_IRAM_ADDR_MASK:
> - fallthrough;
> - case PRU1_IRAM_ADDR_MASK:
> + else if (offset == pru->data->pru1_iram_offset)
> pru->id = 1;
> - break;
> - default:
> - ret = -EINVAL;
> - }
> + else
> + return -EINVAL;
>
> - return ret;
> + return 0;
> }
>
> static int pru_rproc_probe(struct platform_device *pdev)
> @@ -825,20 +808,28 @@ static int pru_rproc_remove(struct platform_device
> *pdev)
>
> static const struct pru_private_data pru_data = {
> .type = PRU_TYPE_PRU,
> + .pru0_iram_offset = 0x4000,
> + .pru1_iram_offset = 0x8000,
> };
>
> static const struct pru_private_data k3_pru_data = {
> .type = PRU_TYPE_PRU,
> + .pru0_iram_offset = 0x4000,
> + .pru1_iram_offset = 0x8000,
> .is_k3 = 1,
> };
>
> static const struct pru_private_data k3_rtu_data = {
> .type = PRU_TYPE_RTU,
> + .pru0_iram_offset = 0x4000,
> + .pru1_iram_offset = 0x6000,
> .is_k3 = 1,
> };
>
> static const struct pru_private_data k3_tx_pru_data = {
> .type = PRU_TYPE_TX_PRU,
> + .pru0_iram_offset = 0xa000,
> + .pru1_iram_offset = 0xc000,
> .is_k3 = 1,
> };
>
>
Re: [PATCH 2/2] soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS
Hi David,
On 1/4/21 12:30 PM, David Lechner wrote:
> This adds support for the PRUSS found in AM18XX/OMAP-L138. This PRUSS
> doesn't have a CFG register, so that is made optional as selected by
> the device tree compatible string.
>
> ARCH_DAVINCI is added in the Kconfig so that the driver can be selected
> on that platform.
>
> Signed-off-by: David Lechner
> ---
> drivers/soc/ti/Kconfig | 2 +-
> drivers/soc/ti/pruss.c | 76 --
> 2 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 7e2fb1c16af1..7a692a21480a 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -85,7 +85,7 @@ config TI_K3_SOCINFO
>
> config TI_PRUSS
> tristate "TI PRU-ICSS Subsystem Platform drivers"
> - depends on SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX || ARCH_KEYSTONE ||
> ARCH_K3
> + depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX ||
> ARCH_KEYSTONE || ARCH_K3
> select MFD_SYSCON
> help
> TI PRU-ICSS Subsystem platform specific support.
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index 5d6e7132a5c4..bfaf3ff74b01 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -24,10 +24,12 @@
> * struct pruss_private_data - PRUSS driver private data
> * @has_no_sharedram: flag to indicate the absence of PRUSS Shared Data RAM
> * @has_core_mux_clock: flag to indicate the presence of PRUSS core clock
> + * @has_cfg: flag to indicate the presence of PRUSS CFG registers
I recommend to change this to a negative flag as the Davinci platforms are the
only ones that don't have CFG (being the very first SoCs with a PRUSS IP)
sub-module.
> */
> struct pruss_private_data {
> bool has_no_sharedram;
> bool has_core_mux_clock;
> + bool has_cfg;
> };
>
> static void pruss_of_free_clk_provider(void *data)
> @@ -239,42 +241,44 @@ static int pruss_probe(struct platform_device *pdev)
> goto rpm_disable;
> }
>
And use it here to skip all the cfg code parsing. All the below delta is just
for the additional indentation for the flag. If you don't like goto's in
non-error paths, then we can refactor the CFG parse code into a separate
function.
regards
Suman
> - child = of_get_child_by_name(np, "cfg");
> - if (!child) {
> - dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
> - ret = -ENODEV;
> - goto rpm_put;
> - }
> + if (data->has_cfg) {
> + child = of_get_child_by_name(np, "cfg");
> + if (!child) {
> + dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
> + ret = -ENODEV;
> + goto rpm_put;
> + }
>
> - if (of_address_to_resource(child, 0, )) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + if (of_address_to_resource(child, 0, )) {
> + ret = -ENOMEM;
> + goto node_put;
> + }
>
> - pruss->cfg_base = devm_ioremap(dev, res.start, resource_size());
> - if (!pruss->cfg_base) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + pruss->cfg_base = devm_ioremap(dev, res.start,
> resource_size());
> + if (!pruss->cfg_base) {
> + ret = -ENOMEM;
> + goto node_put;
> + }
>
> - regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
> - (u64)res.start);
> - regmap_conf.max_register = resource_size() - 4;
> -
> - pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
> - _conf);
> - kfree(regmap_conf.name);
> - if (IS_ERR(pruss->cfg_regmap)) {
> - dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
> - PTR_ERR(pruss->cfg_regmap));
> - ret = PTR_ERR(pruss->cfg_regmap);
> - goto node_put;
> - }
> + regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
> + (u64)res.start);
> + regmap_conf.max_register = resource_size() - 4;
> +
> + pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
> + _conf);
> + kfree(regmap_conf.name);
> + if (IS_ERR(pruss->cfg_regmap)) {
> + dev_err(dev, "regmap_init_mmio failed for cfg, ret =
> %ld\n",
> + PTR_ERR(pruss->cfg_regmap));
> + ret = PTR_ERR(pruss->cfg_regmap);
> + goto node_put;
> + }
>
> - ret = pruss_clk_init(pruss, child);
> - if (ret) {
> - dev_err(dev, "failed to setup coreclk-mux\n");
> - goto node_put;
> +
[PATCH] dt-bindings: irqchip: Add #address-cells to PRUSS INTC
The '#address-cells' property looks to be a required property for interrupt controller nodes as indicated by a warning message seen when building dtbs with W=2. Adding the property to the PRUSS INTC dts nodes though fails the dtbs_check. Add this property to the PRUSS INTC binding to make it compliant with both dtbs_check and building dtbs. Signed-off-by: Suman Anna --- Hi Rob, This patch is also part of our effort to get rid of the warnings seen around interrupt providers on TI K3 dtbs [1]. I needed this in the PRUSS INTC bindings to not get a warning with dtbs_check while also ensuring no warnings while building dtbs with W=2. I would have expected the '#address-cells' requirement to be inherited automatically. And looking through the schema files, I actually do not see the interrupt-controller.yaml included automatically anywhere. You had asked us to drop the inclusion in this binding in our first version with YAML [3]. Am I missing something, and how do we ensure that this is enforced automatically for everyone? regards Suman [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210115083003.27387-1-lokeshvu...@ti.com/ [2] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20210114194805.8231-1-s-a...@ti.com/ [3] https://patchwork.kernel.org/comment/23484523/ .../bindings/interrupt-controller/ti,pruss-intc.yaml| 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml index c2ce215501a5..dcbfe08e997d 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml @@ -79,6 +79,9 @@ properties: mapping and channels to host interrupts so through this property entire mapping is provided. + "#address-cells": +const: 0 + ti,irqs-reserved: $ref: /schemas/types.yaml#/definitions/uint8 description: | @@ -100,6 +103,7 @@ required: - interrupt-names - interrupt-controller - "#interrupt-cells" + - "#address-cells" additionalProperties: false @@ -123,6 +127,7 @@ examples: "host_intr6", "host_intr7"; interrupt-controller; #interrupt-cells = <3>; +#address-cells = <0>; }; }; @@ -142,6 +147,7 @@ examples: reg = <0x2 0x2000>; interrupt-controller; #interrupt-cells = <3>; +#address-cells = <0>; interrupts = , , , -- 2.29.2
Re: [PATCH 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss
+ Sekhar and Bartosz Hi David, On 1/4/21 12:30 PM, David Lechner wrote: > This adds a "ti,am1806-pruss" compatible type for the PRUSS found in > TI AM18xx/OMAP-L138 SoCs. > > Signed-off-by: David Lechner > --- > Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > index 037c51b2f972..a6ed23fdbc00 100644 > --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > @@ -61,6 +61,7 @@ properties: > >compatible: > enum: > + - ti,am1806-pruss # for AM18xx/OMAP-L138 SoC family Almost all the drivers for these SoCs use the prefix "ti,da850-xxx" for the compatibles. Can we switch to using those instead of ti,am1806? regards Suman >- ti,am3356-pruss # for AM335x SoC family >- ti,am4376-pruss0 # for AM437x SoC family and PRUSS unit 0 >- ti,am4376-pruss1 # for AM437x SoC family and PRUSS unit 1 > @@ -321,6 +322,7 @@ if: > compatible: >contains: > enum: > + - ti,am1806-pruss >- ti,k2g-pruss >- ti,am654-icssg >- ti,j721e-icssg >
Re: [PATCH] dt-bindings: soc: ti: Update TI PRUSS bindings about schemas to include
Hi Santosh, On 12/21/20 3:32 PM, Rob Herring wrote: > On Wed, 16 Dec 2020 23:50:27 +0100, Grzegorz Jaszczyk wrote: >> Now after ti,pruss-intc.yaml and ti,pru-rproc.yaml are merged, include >> them in proper property and extend the examples section. >> >> At the occasion extend the allowed property list about dma-ranges. >> >> Signed-off-by: Grzegorz Jaszczyk >> --- >> .../devicetree/bindings/soc/ti/ti,pruss.yaml | 76 +++ >> 1 file changed, 76 insertions(+) >> > > Reviewed-by: Rob Herring > Gentle reminder, I haven't seen this patch yet on linux-next. Can you please pick this up for 5.12. Thanks, Suman
[irqchip: irq/irqchip-next] irqchip/pruss: Simplify the TI_PRUSS_INTC Kconfig
The following commit has been merged into the irq/irqchip-next branch of irqchip: Commit-ID: b8e594fa20d2e33d40c7a8c7c106549a35c38972 Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/b8e594fa20d2e33d40c7a8c7c106549a35c38972 Author:Suman Anna AuthorDate:Fri, 08 Jan 2021 10:29:01 -06:00 Committer: Marc Zyngier CommitterDate: Sun, 10 Jan 2021 10:19:14 irqchip/pruss: Simplify the TI_PRUSS_INTC Kconfig The TI PRUSS INTC irqchip driver handles the local interrupt controller which is a child device of it's parent PRUSS/ICSSG device. The driver was upstreamed in parallel with the PRUSS platform driver, and was configurable independently previously. The PRUSS interrupt controller is an integral part of the overall PRUSS software architecture, and is not useful at all by itself. Simplify the TI_PRUSS_INTC Kconfig dependencies by making it silent and selected automatically when the TI_PRUSS platform driver is enabled. Signed-off-by: Suman Anna Reviewed-by: David Lechner Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210108162901.6003-1-s-a...@ti.com --- drivers/irqchip/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 94920a5..b147f22 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -493,8 +493,9 @@ config TI_SCI_INTA_IRQCHIP TI System Controller, say Y here. Otherwise, say N. config TI_PRUSS_INTC - tristate "TI PRU-ICSS Interrupt Controller" - depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX || ARCH_KEYSTONE || ARCH_K3 + tristate + depends on TI_PRUSS + default TI_PRUSS select IRQ_DOMAIN help This enables support for the PRU-ICSS Local Interrupt Controller
[PATCH] soc: ti: pruss: Correct the pruss_clk_init error trace text
The pruss_clk_init() function can register more than one clock.
Correct the existing misleading error trace upon a failure within
this function.
Signed-off-by: Suman Anna
---
drivers/soc/ti/pruss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
index 5d6e7132a5c4..1d6890134312 100644
--- a/drivers/soc/ti/pruss.c
+++ b/drivers/soc/ti/pruss.c
@@ -273,7 +273,7 @@ static int pruss_probe(struct platform_device *pdev)
ret = pruss_clk_init(pruss, child);
if (ret) {
- dev_err(dev, "failed to setup coreclk-mux\n");
+ dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
goto node_put;
}
--
2.29.2
[PATCH] irqchip: Simplify the TI_PRUSS_INTC Kconfig
The TI PRUSS INTC irqchip driver handles the local interrupt controller which is a child device of it's parent PRUSS/ICSSG device. The driver was upstreamed in parallel with the PRUSS platform driver, and was configurable independently previously. The PRUSS interrupt controller is an integral part of the overall PRUSS software architecture, and is not useful at all by itself. Simplify the TI_PRUSS_INTC Kconfig dependencies by making it silent and selected automatically when the TI_PRUSS platform driver is enabled. Signed-off-by: Suman Anna --- drivers/irqchip/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 94920a51c628..b147f22a78f4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -493,8 +493,9 @@ config TI_SCI_INTA_IRQCHIP TI System Controller, say Y here. Otherwise, say N. config TI_PRUSS_INTC - tristate "TI PRU-ICSS Interrupt Controller" - depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX || ARCH_KEYSTONE || ARCH_K3 + tristate + depends on TI_PRUSS + default TI_PRUSS select IRQ_DOMAIN help This enables support for the PRU-ICSS Local Interrupt Controller -- 2.29.2
Re: [PATCH v2 0/5] Introduce PRU remoteproc consumer API
On 1/7/21 4:44 PM, Mathieu Poirier wrote: > On Wed, Jan 06, 2021 at 06:03:25PM -0600, Suman Anna wrote: >> Hi Mathieu, >> >> On 1/6/21 5:27 PM, Mathieu Poirier wrote: >>> On Wed, Dec 16, 2020 at 05:52:34PM +0100, Grzegorz Jaszczyk wrote: >>>> Hi All, >>>> >>>> The Programmable Real-Time Unit and Industrial Communication Subsystem >>>> (PRU-ICSS or simply PRUSS) on various TI SoCs consists of dual 32-bit >>>> RISC cores (Programmable Real-Time Units, or PRUs) for program execution. >>>> >>>> There are 3 foundation components for PRUSS subsystem: the PRUSS platform >>>> driver, the PRUSS INTC driver and the PRUSS remoteproc driver. All were >>>> already merged and can be found under: >>>> 1) drivers/soc/ti/pruss.c >>>>Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml >>>> 2) drivers/irqchip/irq-pruss-intc.c >>>> >>>> Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml >>>> 3) drivers/remoteproc/pru_rproc.c >>>>Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml >>>> >>>> The programmable nature of the PRUs provide flexibility to implement custom >>>> peripheral interfaces, fast real-time responses, or specialized data >>>> handling. >>>> Example of a PRU consumer drivers will be: >>>> - Software UART over PRUSS >>>> - PRU-ICSS Ethernet EMAC >>>> >>>> In order to make usage of common PRU resources and allow the consumer >>>> drivers to >>>> configure the PRU hardware for specific usage the PRU API is introduced. >>>> >>>> Patch #3 of this series depends on one not merged remteproc related patch >>>> [1]. >>>> >>>> Please see the individual patches for exact changes in each patch, >>>> following is >>>> the only change from v1: >>>> - Change the 'prus' property name to 'ti,prus' as suggested by Rob >>>> Herring, >>>> which influences patch #1 and patch #2 >>>> >>>> [1] >>>> https://patchwork.kernel.org/project/linux-remoteproc/patch/20201121030156.22857-3-s-a...@ti.com/ >>>> >>>> Best regards, >>>> Grzegorz >>>> >>>> Roger Quadros (1): >>>> remoteproc: pru: Add pru_rproc_set_ctable() function >>>> >>>> Suman Anna (2): >>>> dt-bindings: remoteproc: Add PRU consumer bindings >>>> remoteproc: pru: Deny rproc sysfs ops for PRU client driven boots >>>> >>>> Tero Kristo (2): >>>> remoteproc: pru: Add APIs to get and put the PRU cores >>>> remoteproc: pru: Configure firmware based on client setup >>>> >>>> .../bindings/remoteproc/ti,pru-consumer.yaml | 64 + >>>> drivers/remoteproc/pru_rproc.c| 221 +- >>>> include/linux/pruss.h | 78 +++ >>> >>> This patchset is giving checkpatch.pl errors and as such will not go further >>> with this revision. >> >> Yeah, I am aware of those. Greg has intentionally skipped the checkpatch >> warnings around ENOTSUPP, based on some similar discussion on a different >> patch, >> https://lkml.org/lkml/2020/11/10/764. > > I only see input from Andy and Lars in the thread you point out, nothing from > Greg. I have also taken a look at the patch [1] that made checkpatch complain > about ENOTSUPP. From what I see in that commit log the goal is to prevent new > additions of ENOTSUPP to the kernel. > > Please modify and resend, otherwise I'm sure someone will send another patch > to > fix it before the end of the cycle. Yeah ok. I will send out a v3. regards Suman > > Thanks, > Mathieu > > [1]. 6b9ea5ff5abd checkpatch: warn about uses of ENOTSUPP >> >> Let me know if you prefer that we change these to EOPNOTSUPP. >> >> regards >> Suman >> >>> >>>> 3 files changed, 360 insertions(+), 3 deletions(-) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml >>>> create mode 100644 include/linux/pruss.h >>>> >>>> -- >>>> 2.29.0 >>>> >>
Re: [PATCH v2 0/5] Introduce PRU remoteproc consumer API
Hi Mathieu, On 1/6/21 5:27 PM, Mathieu Poirier wrote: > On Wed, Dec 16, 2020 at 05:52:34PM +0100, Grzegorz Jaszczyk wrote: >> Hi All, >> >> The Programmable Real-Time Unit and Industrial Communication Subsystem >> (PRU-ICSS or simply PRUSS) on various TI SoCs consists of dual 32-bit >> RISC cores (Programmable Real-Time Units, or PRUs) for program execution. >> >> There are 3 foundation components for PRUSS subsystem: the PRUSS platform >> driver, the PRUSS INTC driver and the PRUSS remoteproc driver. All were >> already merged and can be found under: >> 1) drivers/soc/ti/pruss.c >>Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml >> 2) drivers/irqchip/irq-pruss-intc.c >>Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml >> 3) drivers/remoteproc/pru_rproc.c >>Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml >> >> The programmable nature of the PRUs provide flexibility to implement custom >> peripheral interfaces, fast real-time responses, or specialized data >> handling. >> Example of a PRU consumer drivers will be: >> - Software UART over PRUSS >> - PRU-ICSS Ethernet EMAC >> >> In order to make usage of common PRU resources and allow the consumer >> drivers to >> configure the PRU hardware for specific usage the PRU API is introduced. >> >> Patch #3 of this series depends on one not merged remteproc related patch >> [1]. >> >> Please see the individual patches for exact changes in each patch, following >> is >> the only change from v1: >> - Change the 'prus' property name to 'ti,prus' as suggested by Rob Herring, >> which influences patch #1 and patch #2 >> >> [1] >> https://patchwork.kernel.org/project/linux-remoteproc/patch/20201121030156.22857-3-s-a...@ti.com/ >> >> Best regards, >> Grzegorz >> >> Roger Quadros (1): >> remoteproc: pru: Add pru_rproc_set_ctable() function >> >> Suman Anna (2): >> dt-bindings: remoteproc: Add PRU consumer bindings >> remoteproc: pru: Deny rproc sysfs ops for PRU client driven boots >> >> Tero Kristo (2): >> remoteproc: pru: Add APIs to get and put the PRU cores >> remoteproc: pru: Configure firmware based on client setup >> >> .../bindings/remoteproc/ti,pru-consumer.yaml | 64 + >> drivers/remoteproc/pru_rproc.c| 221 +- >> include/linux/pruss.h | 78 +++ > > This patchset is giving checkpatch.pl errors and as such will not go further > with this revision. Yeah, I am aware of those. Greg has intentionally skipped the checkpatch warnings around ENOTSUPP, based on some similar discussion on a different patch, https://lkml.org/lkml/2020/11/10/764. Let me know if you prefer that we change these to EOPNOTSUPP. regards Suman > >> 3 files changed, 360 insertions(+), 3 deletions(-) >> create mode 100644 >> Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml >> create mode 100644 include/linux/pruss.h >> >> -- >> 2.29.0 >>
Re: [PATCH v2 0/5] Introduce PRU remoteproc consumer API
Hi David,
On 1/4/21 2:11 PM, David Lechner wrote:
>
>> Please see the individual patches for exact changes in each patch, following
>> is
>> the only change from v1:
>> - Change the 'prus' property name to 'ti,prus' as suggested by Rob Herring,
>> which influences patch #1 and patch #2
>
> It looks like "soc: ti: pruss: Add pruss_{request, release}_mem_region() API"
> was also dropped in v2. Was this intentional?
No, it is not dropped. That patch is part of a different similarly titled
"Introduce PRU platform consumer API" series [1], which is dependent on this
series and is against a different folder (maintainer): drivers/soc/ti.
regards
Suman
[1] https://patchwork.kernel.org/project/linux-remoteproc/list/?series=400787
Re: [PATCH] dt-bindings: Add missing array size constraints
| 3 +-- >> .../pinctrl/aspeed,ast2500-pinctrl.yaml | 4 ++-- >> .../bindings/power/supply/bq25980.yaml| 1 + >> .../bindings/remoteproc/ingenic,vpu.yaml | 2 +- >> .../remoteproc/ti,omap-remoteproc.yaml| 3 +++ >> .../bindings/riscv/sifive-l2-cache.yaml | 1 + >> .../bindings/serial/renesas,hscif.yaml| 2 ++ >> .../bindings/serial/renesas,scif.yaml | 2 ++ >> .../bindings/serial/renesas,scifa.yaml| 2 ++ >> .../bindings/serial/renesas,scifb.yaml| 2 ++ >> .../sound/allwinner,sun4i-a10-codec.yaml | 1 + >> .../bindings/sound/google,sc7180-trogdor.yaml | 1 + >> .../bindings/sound/samsung,aries-wm8994.yaml | 3 +++ >> .../bindings/sound/samsung,midas-audio.yaml | 2 ++ >> .../devicetree/bindings/sound/tas2562.yaml| 2 ++ >> .../devicetree/bindings/sound/tas2770.yaml| 2 ++ >> .../bindings/sound/tlv320adcx140.yaml | 1 + >> .../devicetree/bindings/spi/renesas,rspi.yaml | 2 ++ >> .../devicetree/bindings/sram/sram.yaml| 2 ++ >> .../timer/allwinner,sun4i-a10-timer.yaml | 2 ++ >> .../bindings/timer/intel,ixp4xx-timer.yaml| 2 +- >> .../usb/allwinner,sun4i-a10-musb.yaml | 2 +- >> .../bindings/usb/brcm,usb-pinmap.yaml | 3 +++ >> .../devicetree/bindings/usb/generic-ehci.yaml | 1 + >> .../devicetree/bindings/usb/generic-ohci.yaml | 1 + >> .../devicetree/bindings/usb/ingenic,musb.yaml | 2 +- >> .../bindings/usb/renesas,usbhs.yaml | 1 + >> .../devicetree/bindings/usb/ti,j721e-usb.yaml | 3 ++- >> .../bindings/usb/ti,keystone-dwc3.yaml| 2 ++ >> 74 files changed, 118 insertions(+), 33 deletions(-) >> [snip] >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml >> b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml >> index c019f9fbe916..d0aa91bbf5e5 100644 >> --- a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml >> +++ b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml >> @@ -44,7 +44,7 @@ properties: >>- const: vpu >> >>interrupts: >> -description: VPU hardware interrupt >> +maxItems: 1 >> >> required: >>- compatible >> diff --git >> a/Documentation/devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml >> b/Documentation/devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml >> index 084960a8f17a..1a1159097a2a 100644 >> --- a/Documentation/devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml >> +++ b/Documentation/devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml >> @@ -70,10 +70,13 @@ properties: >>the firmware image. >> >>clocks: >> +maxItems: 1 >> description: | >>Main functional clock for the remote processor >> >>resets: >> +minItems: 1 >> +maxItems: 2 While this works for passing the schema, if we want to be specifically accurate between the different remoteprocs, the DSPs will have 1 items while the IPUs should have 2 items. I can submit an incremental update if that's ok. Otherwise, for this file, Reviewed-by: Suman Anna >> description: | >>Reset handles for the remote processor >> [snip] regards Suman
Re: [PATCH v4 0/6] Add a PRU remoteproc driver
Hi Bjorn, On 12/8/20 8:09 AM, Grzegorz Jaszczyk wrote: > Hi All, > > The Programmable Real-Time Unit and Industrial Communication Subsystem > (PRU-ICSS or simply PRUSS) on various TI SoCs consists of dual 32-bit > RISC cores (Programmable Real-Time Units, or PRUs) for program execution. > > The K3 AM65x and J721E SoCs have the next generation of the PRU-ICSS IP, > commonly called ICSSG. The ICSSG IP on AM65x SoCs has two PRU cores, > two auxiliary custom PRU cores called Real Time Units (RTUs). The K3 > AM65x SR2.0 and J721E SoCs have a revised version of the ICSSG IP, and > include two additional custom auxiliary PRU cores called Transmit PRUs > (Tx_PRUs). > > This series contains the PRUSS remoteproc driver together with relevant > dt-binding. This is the 3rd foundation component for PRUSS subsystem, the > previous two were already merged and can be found under: > 1) drivers/soc/ti/pruss.c >Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > 2) drivers/irqchip/irq-pruss-intc.c >Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml > > The following is a v4 version of the series. There is only one change > from v3 [1]: > - Use sizeof(unsigned int) instead of sizeof(int) for kcalloc in > pru_handle_intrmap() in patch #3. > > [1] > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20201204201807.14716-1-grzegorz.jaszc...@linaro.org/ > > Best regards, > Grzegorz > > Grzegorz Jaszczyk (1): > remoteproc: pru: Add support for PRU specific interrupt configuration > > Suman Anna (5): > dt-bindings: remoteproc: Add binding doc for PRU cores in the PRU-ICSS > remoteproc: pru: Add a PRU remoteproc driver > remoteproc: pru: Add pru-specific debugfs support > remoteproc: pru: Add support for various PRU cores on K3 AM65x SoCs > remoteproc: pru: Add support for various PRU cores on K3 J721E SoCs All patches in this series are Reviewed now and we have got the binding ack as well. Can you please pick this series up for 5.11 if it is not too late? Thank you, Suman > > .../bindings/remoteproc/ti,pru-rproc.yaml | 214 + > drivers/remoteproc/Kconfig| 12 + > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/pru_rproc.c| 875 ++ > drivers/remoteproc/pru_rproc.h| 46 + > 5 files changed, 1148 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml > create mode 100644 drivers/remoteproc/pru_rproc.c > create mode 100644 drivers/remoteproc/pru_rproc.h >
Re: [PATCH v2 0/6] Add a PRU remoteproc driver
Hi Greg, On 11/19/20 8:08 AM, Grzegorz Jaszczyk wrote: > Hi All, > > The Programmable Real-Time Unit and Industrial Communication Subsystem > (PRU-ICSS or simply PRUSS) on various TI SoCs consists of dual 32-bit > RISC cores (Programmable Real-Time Units, or PRUs) for program execution. > > The K3 AM65x amd J721E SoCs have the next generation of the PRU-ICSS IP, > commonly called ICSSG. The ICSSG IP on AM65x SoCs has two PRU cores, > two auxiliary custom PRU cores called Real Time Units (RTUs). The K3 > AM65x SR2.0 and J721E SoCs have a revised version of the ICSSG IP, and > include two additional custom auxiliary PRU cores called Transmit PRUs > (Tx_PRUs). > > This series contains the PRUSS remoteproc driver together with relevant > dt-binding. This is the 3rd foundation component for PRUSS subsystem, the > previous two were already merged and can be found under: > 1) drivers/soc/ti/pruss.c >Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > 2) drivers/irqchip/irq-pruss-intc.c >Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml > > The following is a v2 version of the series. Please see the individual patches > for exact changes in each patch, following are the main changes from v1: > - Patch #1: fix two yamllint warnings. > - Patch #2: address Suman comments: minor style improvements and fix for > optional resource table handling (moved from patch #3). > - Patch #3: address Suman comment: minor style, comments and trace > improvements > (no functional changes). > - Patch #4: No changes. > - Patch #5: Update documentation of pru_rproc_memcpy() function and is_k3 > flag. > - Patch #6: No changes. > > Best regards, > Grzegorz > > Grzegorz Jaszczyk (1): > remoteproc/pru: Add support for PRU specific interrupt configuration > > Suman Anna (5): > dt-bindings: remoteproc: Add binding doc for PRU cores in the PRU-ICSS > remoteproc/pru: Add a PRU remoteproc driver > remoteproc/pru: Add pru-specific debugfs support > remoteproc/pru: Add support for various PRU cores on K3 AM65x SoCs > remoteproc/pru: Add support for various PRU cores on K3 J721E SoCs One minor change for v3 when you repost to address Mathieu's comments, can you please adjust the patch titles to use "remoteproc: pru:" instead following the latest convention. Thanks, Suman > > .../bindings/remoteproc/ti,pru-rproc.yaml | 214 + > drivers/remoteproc/Kconfig| 12 + > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/pru_rproc.c| 877 ++ > drivers/remoteproc/pru_rproc.h| 46 + > 5 files changed, 1150 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/remoteproc/ti,pru-rproc.yaml > create mode 100644 drivers/remoteproc/pru_rproc.c > create mode 100644 drivers/remoteproc/pru_rproc.h >
Re: [PATCH v2 2/6] remoteproc/pru: Add a PRU remoteproc driver
Hi Mathieu, On 12/2/20 3:13 PM, Mathieu Poirier wrote: > On Wed, Dec 02, 2020 at 01:53:36PM -0700, Mathieu Poirier wrote: >> On Tue, Dec 01, 2020 at 03:54:36PM -0700, Mathieu Poirier wrote: >>> Hi Grzeg, >>> >>> I have started to review this set - comments will come over the next few >>> days. >>> >>> See below for a start. >>> >>> On Thu, Nov 19, 2020 at 03:08:46PM +0100, Grzegorz Jaszczyk wrote: >>>> From: Suman Anna >>>> >>>> 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 PRUs do not have a unified address space (have an Instruction >>>> RAM and a primary Data RAM at both 0x0). The PRU remoteproc driver >>>> therefore uses a custom remoteproc core ELF loader ops. The added >>>> .da_to_va ops is only used to provide translations for the PRU >>>> Data RAMs. 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 for each PRU core, 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, AM437x, AM57xx and 66AK2G >>>> SoCs, and support for other TI SoCs will be added in subsequent >>>> patches. >>>> >>>> Co-developed-by: Andrew F. Davis >>>> Signed-off-by: Andrew F. Davis >>>> Signed-off-by: Suman Anna >>>> Co-developed-by: Grzegorz Jaszczyk >>>> Signed-off-by: Grzegorz Jaszczyk >>>> --- >>>> v1->v2: >>>> - Use PRU_IRAM_ADDR_MASK definition instead of raw 0x3. >>>> - Convert 'len' argument from int to size_t type in all *da_to_va. >>>> - Return 0 in case of missing .resource_table for pru_rproc_parse_fw() >>>> (move the logic from patch #3 where it was corrected). >>>> --- >>>> drivers/remoteproc/Kconfig | 12 + >>>> drivers/remoteproc/Makefile| 1 + >>>> drivers/remoteproc/pru_rproc.c | 435 + >>>> 3 files changed, 448 insertions(+) >>>> create mode 100644 drivers/remoteproc/pru_rproc.c >>>> >>>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig >>>> index d99548fb5dde..3e3865a7cd78 100644 >>>> --- a/drivers/remoteproc/Kconfig >>>> +++ b/drivers/remoteproc/Kconfig >>>> @@ -125,6 +125,18 @@ config KEYSTONE_REMOTEPROC >>>> It's safe to say N here if you're not interested in the Keystone >>>> DSPs or just want to use a bare minimum kernel. >>>> >>>> +config PRU_REMOTEPROC >>>> + tristate "TI PRU remoteproc support" >>>> + depends on TI_PRUSS >>>> + default TI_PRUSS >>>> + help >>>> +Support for TI PRU remote processors present within a PRU-ICSS >>>> +subsystem via the remote processor framework. >>>> + >>>> +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. >>>> + >>>> config QCOM_PIL_INFO >>>>tristate >>>> >>>> diff --git a/drivers/remoteproc/M
Re: [PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs
On 11/23/20 6:55 PM, Mathieu Poirier wrote:
> On Mon, 23 Nov 2020 at 16:51, Mathieu Poirier
> wrote:
>>
>> Good afternoon Suman,
>>
>> On Wed, Nov 18, 2020 at 07:05:31PM -0600, Suman Anna wrote:
>>> The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
>>> TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
>>> and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
>>> IP however doubles the total available TCM in LockStep mode by making the
>>> Core1 TCM visible immediately after the corresponding Core0 TCM.
>>>
>>> The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
>>> size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
>>> above. This increased TCM memory is only usable in LockStep-mode, and
>>> has to be adjusted to the normal 32 KB size in Split mode. Enhance the
>>> TI K3 R5F remoteproc for this logic through a new function. The adjustment
>>> is a no-op on prior SoCs and relies on the correct DTS node sizes in
>>> LockStep-mode on applicable SoCs.
>>>
>>> Signed-off-by: Suman Anna
>>> ---
>>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 43
>>> 1 file changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> index 66a32dcdd7d0..62b5a4c29456 100644
>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> @@ -71,9 +71,11 @@ enum cluster_mode {
>>>
>>> /**
>>> * struct k3_r5_soc_data - match data to handle SoC variations
>>> + * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
>>> * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for
>>> ECC
>>> */
>>> struct k3_r5_soc_data {
>>> + bool tcm_is_double;
>>> bool tcm_ecc_autoinit;
>>> };
>>>
>>> @@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc
>>> *kproc)
>>> of_reserved_mem_device_release(kproc->dev);
>>> }
>>>
>>> +/*
>>> + * Each R5F core within a typical R5FSS instance has a total of 64 KB of
>>> TCMs,
>>> + * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from
>>> both
>>> + * cores are usable in Split-mode, but only the Core0 TCMs can be used in
>>> + * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs
>>> by
>>> + * leveraging the Core1 TCMs as well in certain modes where they would have
>>> + * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done
>>> by
>>> + * making a Core1 TCM visible immediately after the corresponding Core0
>>> TCM.
>>> + * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and
>>> the
>>> + * dts representation reflects this increased size on supported SoCs. The
>>> Core0
>>> + * TCM sizes therefore have to be adjusted to only half the original size
>>> in
>>> + * Split mode.
>>> + */
>>> +static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
>>> +{
>>> + struct k3_r5_cluster *cluster = kproc->cluster;
>>> + struct k3_r5_core *core = kproc->core;
>>> + struct device *cdev = core->dev;
>>> + struct k3_r5_core *core0;
>>> +
>>> + if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
>>> + !cluster->soc_data->tcm_is_double)
>>> + return;
>>
>> Shouldn't this be:
>>
>> if (cluster->mode == CLUSTER_MODE_SPLIT ||
>> !cluster->soc_data->tcm_is_double)
>> return;
>>
>> If am wrong then I'm pretty sure other people will be confused and a comment
>> is
>> warranted.
>>
>
> Forget the above, I misread the context. The memories are already set
> to 64KB so there is nothing to do if in lockstep mode.
Yep, thanks for the review. The comments above the function has all the details.
regards
Suman
>
>>> +
>>> + core0 = list_first_entry(>cores, struct k3_r5_core, elem);
>>> + if (core == core0) {
>>> + WARN_ON(core->mem[0].size != SZ_64K);
>>> + WARN_ON(core->mem[1].size != SZ_64K);
>>> +
>>> + core->mem[0].size /= 2;
>>&
Re: [PATCH v2 2/3] remoteproc: Introduce deny_sysfs_ops flag
On 11/21/20 11:33 PM, Bjorn Andersson wrote:
> On Fri 20 Nov 21:44 CST 2020, Suman Anna wrote:
>
>> On 11/20/20 9:38 PM, Bjorn Andersson wrote:
>>> On Fri 20 Nov 21:01 CST 2020, Suman Anna wrote:
>>>
>>>> The remoteproc framework provides sysfs interfaces for changing
>>>> the firmware name and for starting/stopping a remote processor
>>>> through the sysfs files 'state' and 'firmware'. The 'recovery'
>>>> sysfs file can also be used similarly to control the error recovery
>>>> state machine of a remoteproc. These interfaces are currently
>>>> allowed irrespective of how the remoteprocs were booted (like
>>>> remoteproc self auto-boot, remoteproc client-driven boot etc).
>>>> These interfaces can adversely affect a remoteproc and its clients
>>>> especially when a remoteproc is being controlled by a remoteproc
>>>> client driver(s). Also, not all remoteproc drivers may want to
>>>> support the sysfs interfaces by default.
>>>>
>>>> Add support to deny the sysfs state/firmware/recovery change by
>>>> introducing a state flag 'deny_sysfs_ops' that the individual
>>>> remoteproc drivers can set based on their usage needs. The default
>>>> behavior is to allow the sysfs operations as before.
>>>>
>>>
>>> This makes sense, but can't we implement attribute_group->is_visible to
>>> simply hide these entries from userspace instead of leaving them
>>> "broken"?
>>
>> I would have to look into that, but can that be changed dynamically?
>> Also, note that the enforcement is only on the writes/stores which impact
>> the state-machine, but not the reads/shows.
>>
>> For PRU usecases, we will be setting this dynamically.
>>
>
> It looks to be dynamic, but I don't know if there's any "caching"
> involved. Please have a look and let me know.
OK, will do. I can only check the week after though.
regards
Suman
>
> Regards,
> Bjorn
>
>> regards
>> Suman
>>
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> Signed-off-by: Suman Anna
>>>> ---
>>>> v2: revised to account for the 'recovery' sysfs file as well, patch
>>>> description updated accordingly
>>>> v1:
>>>> https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-5-s-a...@ti.com/
>>>>
>>>> drivers/remoteproc/remoteproc_sysfs.c | 12
>>>> include/linux/remoteproc.h| 2 ++
>>>> 2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/remoteproc/remoteproc_sysfs.c
>>>> b/drivers/remoteproc/remoteproc_sysfs.c
>>>> index bd2950a246c9..3fd18a71c188 100644
>>>> --- a/drivers/remoteproc/remoteproc_sysfs.c
>>>> +++ b/drivers/remoteproc/remoteproc_sysfs.c
>>>> @@ -49,6 +49,10 @@ static ssize_t recovery_store(struct device *dev,
>>>> {
>>>>struct rproc *rproc = to_rproc(dev);
>>>>
>>>> + /* restrict sysfs operations if not allowed by remoteproc drivers */
>>>> + if (rproc->deny_sysfs_ops)
>>>> + return -EPERM;
>>>> +
>>>>if (sysfs_streq(buf, "enabled")) {
>>>>/* change the flag and begin the recovery process if needed */
>>>>rproc->recovery_disabled = false;
>>>> @@ -158,6 +162,10 @@ static ssize_t firmware_store(struct device *dev,
>>>>char *p;
>>>>int err, len = count;
>>>>
>>>> + /* restrict sysfs operations if not allowed by remoteproc drivers */
>>>> + if (rproc->deny_sysfs_ops)
>>>> + return -EPERM;
>>>> +
>>>>err = mutex_lock_interruptible(>lock);
>>>>if (err) {
>>>>dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
>>>> @@ -225,6 +233,10 @@ static ssize_t state_store(struct device *dev,
>>>>struct rproc *rproc = to_rproc(dev);
>>>>int ret = 0;
>>>>
>>>> + /* restrict sysfs operations if not allowed by remoteproc drivers */
>>>> + if (rproc->deny_sysfs_ops)
>>>> + return -EPERM;
>>>> +
>>>>if (sysfs_streq(buf, "start")) {
>>>>if (rproc->state == RPROC_RUNNING)
>>>>return -EBUSY;
>>>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>>>> index 3fa3ba6498e8..dbc3767f7d0e 100644
>>>> --- a/include/linux/remoteproc.h
>>>> +++ b/include/linux/remoteproc.h
>>>> @@ -508,6 +508,7 @@ struct rproc_dump_segment {
>>>> * @has_iommu: flag to indicate if remote processor is behind an MMU
>>>> * @auto_boot: flag to indicate if remote processor should be auto-started
>>>> * @autonomous: true if an external entity has booted the remote processor
>>>> + * @deny_sysfs_ops: flag to not permit sysfs operations on state,
>>>> firmware and recovery
>>>> * @dump_segments: list of segments in the firmware
>>>> * @nb_vdev: number of vdev currently handled by rproc
>>>> * @char_dev: character device of the rproc
>>>> @@ -545,6 +546,7 @@ struct rproc {
>>>>bool has_iommu;
>>>>bool auto_boot;
>>>>bool autonomous;
>>>> + bool deny_sysfs_ops;
>>>>struct list_head dump_segments;
>>>>int nb_vdev;
>>>>u8 elf_class;
>>>> --
>>>> 2.28.0
>>>>
>>
Re: [PATCH] remoteproc: Add module parameter 'auto_boot'
Hi Paul, On 11/21/20 12:47 PM, Paul Cercueil wrote: > Hi Suman, > > Le ven. 20 nov. 2020 à 17:06, Suman Anna a écrit : >> Hi Paul, >> >> On 11/20/20 4:37 PM, Mathieu Poirier wrote: >>> Hi Paul, >>> >>> On Sun, Nov 15, 2020 at 11:50:56AM +, Paul Cercueil wrote: >>>> Until now the remoteproc core would always default to trying to boot the >>>> remote processor at startup. The various remoteproc drivers could >>>> however override that setting. >>>> >>>> Whether or not we want the remote processor to boot, really depends on >>>> the nature of the processor itself - a processor built into a WiFi chip >>>> will need to be booted for the WiFi hardware to be usable, for instance, >>>> but a general-purpose co-processor does not have any predeterminated >>>> function, and as such we cannot assume that the OS will want the >>>> processor to be booted - yet alone that we have a single do-it-all >>>> firmware to load. >>>> >>> >>> If I understand correctly you have various remote processors that use the >>> same firmware >>> but are serving different purposes - is this correct? >>> >>>> Add a 'auto_boot' module parameter that instructs the remoteproc whether >>>> or not it should auto-boot the remote processor, which will default to >>>> "true" to respect the previous behaviour. >>>> >>> >>> Given that the core can't be a module I wonder if this isn't something that >>> would be better off in the specific platform driver or the device tree... >>> Other >>> people might have an opinion as well. >> >> I agree. Even it is a module, all it is setting up is default behavior, and >> doesn't buy you much. If you have one or more remoteproc drivers supporting >> different instances, and each one wants different behavior, you would have to >> customize it in the drivers anyway. ST drivers are customizing this using a >> DT >> flag. > > Devicetree is supposed to describe the hardware, not how you're supposed to > use > the hardware... I agree, but it sneaked in somehow. Anyway, I am not proposing that you repeat the same approach. > >> Given that the individual platform drivers have to be modules, is there any >> issue in customizing this in your platform driver? > > No, I can patch the platform driver instead, but to me it clearly is a core > issue. So, remoteproc core is just setting up a default, and it is upto the individual drivers to override it. I actually have two different rproc drivers on the same SoC wanting different behavior for example. I do like the runtime config in general compared to a build-time Kconfig option, but let's say we do add this option, then you would also need every rproc driver to actually set this field specifically to ensure their expected behavior doesn't change if it is installed as false. regards Suman > > Cheers, > -Paul > >> regards >> Suman >> >>> >>> Thanks, >>> Mathieu >>> >>>> Signed-off-by: Paul Cercueil >>>> --- >>>> drivers/remoteproc/remoteproc_core.c | 7 ++- >>>> 1 file changed, 6 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/remoteproc_core.c >>>> b/drivers/remoteproc/remoteproc_core.c >>>> index dab2c0f5caf0..687b1bfd49db 100644 >>>> --- a/drivers/remoteproc/remoteproc_core.c >>>> +++ b/drivers/remoteproc/remoteproc_core.c >>>> @@ -44,6 +44,11 @@ >>>> >>>> #define HIGH_BITS_MASK 0xULL >>>> >>>> +static bool auto_boot = true; >>>> +module_param(auto_boot, bool, 0400); >>>> +MODULE_PARM_DESC(auto_boot, >>>> + "Auto-boot the remote processor [default=true]"); >>>> + >>>> static DEFINE_MUTEX(rproc_list_mutex); >>>> static LIST_HEAD(rproc_list); >>>> static struct notifier_block rproc_panic_nb; >>>> @@ -2176,7 +2181,7 @@ struct rproc *rproc_alloc(struct device *dev, const >>>> char *name, >>>> return NULL; >>>> >>>> rproc->priv = [1]; >>>> - rproc->auto_boot = true; >>>> + rproc->auto_boot = auto_boot; >>>> rproc->elf_class = ELFCLASSNONE; >>>> rproc->elf_machine = EM_NONE; >>>> >>>> -- >>>> 2.29.2 >>>> >> > >
Re: [PATCH v2 2/3] remoteproc: Introduce deny_sysfs_ops flag
On 11/20/20 9:38 PM, Bjorn Andersson wrote:
> On Fri 20 Nov 21:01 CST 2020, Suman Anna wrote:
>
>> The remoteproc framework provides sysfs interfaces for changing
>> the firmware name and for starting/stopping a remote processor
>> through the sysfs files 'state' and 'firmware'. The 'recovery'
>> sysfs file can also be used similarly to control the error recovery
>> state machine of a remoteproc. These interfaces are currently
>> allowed irrespective of how the remoteprocs were booted (like
>> remoteproc self auto-boot, remoteproc client-driven boot etc).
>> These interfaces can adversely affect a remoteproc and its clients
>> especially when a remoteproc is being controlled by a remoteproc
>> client driver(s). Also, not all remoteproc drivers may want to
>> support the sysfs interfaces by default.
>>
>> Add support to deny the sysfs state/firmware/recovery change by
>> introducing a state flag 'deny_sysfs_ops' that the individual
>> remoteproc drivers can set based on their usage needs. The default
>> behavior is to allow the sysfs operations as before.
>>
>
> This makes sense, but can't we implement attribute_group->is_visible to
> simply hide these entries from userspace instead of leaving them
> "broken"?
I would have to look into that, but can that be changed dynamically?
Also, note that the enforcement is only on the writes/stores which impact
the state-machine, but not the reads/shows.
For PRU usecases, we will be setting this dynamically.
regards
Suman
>
> Regards,
> Bjorn
>
>> Signed-off-by: Suman Anna
>> ---
>> v2: revised to account for the 'recovery' sysfs file as well, patch
>> description updated accordingly
>> v1:
>> https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-5-s-a...@ti.com/
>>
>> drivers/remoteproc/remoteproc_sysfs.c | 12
>> include/linux/remoteproc.h| 2 ++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/remoteproc/remoteproc_sysfs.c
>> b/drivers/remoteproc/remoteproc_sysfs.c
>> index bd2950a246c9..3fd18a71c188 100644
>> --- a/drivers/remoteproc/remoteproc_sysfs.c
>> +++ b/drivers/remoteproc/remoteproc_sysfs.c
>> @@ -49,6 +49,10 @@ static ssize_t recovery_store(struct device *dev,
>> {
>> struct rproc *rproc = to_rproc(dev);
>>
>> +/* restrict sysfs operations if not allowed by remoteproc drivers */
>> +if (rproc->deny_sysfs_ops)
>> +return -EPERM;
>> +
>> if (sysfs_streq(buf, "enabled")) {
>> /* change the flag and begin the recovery process if needed */
>> rproc->recovery_disabled = false;
>> @@ -158,6 +162,10 @@ static ssize_t firmware_store(struct device *dev,
>> char *p;
>> int err, len = count;
>>
>> +/* restrict sysfs operations if not allowed by remoteproc drivers */
>> +if (rproc->deny_sysfs_ops)
>> +return -EPERM;
>> +
>> err = mutex_lock_interruptible(>lock);
>> if (err) {
>> dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
>> @@ -225,6 +233,10 @@ static ssize_t state_store(struct device *dev,
>> struct rproc *rproc = to_rproc(dev);
>> int ret = 0;
>>
>> +/* restrict sysfs operations if not allowed by remoteproc drivers */
>> +if (rproc->deny_sysfs_ops)
>> +return -EPERM;
>> +
>> if (sysfs_streq(buf, "start")) {
>> if (rproc->state == RPROC_RUNNING)
>> return -EBUSY;
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 3fa3ba6498e8..dbc3767f7d0e 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -508,6 +508,7 @@ struct rproc_dump_segment {
>> * @has_iommu: flag to indicate if remote processor is behind an MMU
>> * @auto_boot: flag to indicate if remote processor should be auto-started
>> * @autonomous: true if an external entity has booted the remote processor
>> + * @deny_sysfs_ops: flag to not permit sysfs operations on state, firmware
>> and recovery
>> * @dump_segments: list of segments in the firmware
>> * @nb_vdev: number of vdev currently handled by rproc
>> * @char_dev: character device of the rproc
>> @@ -545,6 +546,7 @@ struct rproc {
>> bool has_iommu;
>> bool auto_boot;
>> bool autonomous;
>> +bool deny_sysfs_ops;
>> struct list_head dump_segments;
>> int nb_vdev;
>> u8 elf_class;
>> --
>> 2.28.0
>>
[PATCH] remoteproc: Add a rproc_set_firmware() API
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, and can only change firmwares when the remoteproc is
offline. This allows some remoteproc drivers to choose different firmwares
at runtime based on the functionality the remote processor is providing.
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
---
drivers/remoteproc/remoteproc_core.c | 63 +++
drivers/remoteproc/remoteproc_sysfs.c | 33 +-
include/linux/remoteproc.h| 1 +
3 files changed, 66 insertions(+), 31 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index dab2c0f5caf0..46c2937ebea9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1934,6 +1934,69 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
#endif
EXPORT_SYMBOL(rproc_get_by_phandle);
+/**
+ * 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.
+ *
+ * Return: 0 on success or a negative value upon failure
+ */
+int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
+{
+ struct device *dev;
+ int ret, len;
+ char *p;
+
+ if (!rproc || !fw_name)
+ return -EINVAL;
+
+ dev = rproc->dev.parent;
+
+ ret = mutex_lock_interruptible(>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 empty string for firmware name\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(>lock);
+ return ret;
+}
+EXPORT_SYMBOL(rproc_set_firmware);
+
static int rproc_validate(struct rproc *rproc)
{
switch (rproc->state) {
diff --git a/drivers/remoteproc/remoteproc_sysfs.c
b/drivers/remoteproc/remoteproc_sysfs.c
index 3fd18a71c188..cf846caf2e1a 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -159,42 +159,13 @@ 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;
/* restrict sysfs operations if not allowed by remoteproc drivers */
if (rproc->deny_sysfs_ops)
return -EPERM;
- err = mutex_lock_interruptible(>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:
- mute
[PATCH v2 2/3] remoteproc: Introduce deny_sysfs_ops flag
The remoteproc framework provides sysfs interfaces for changing
the firmware name and for starting/stopping a remote processor
through the sysfs files 'state' and 'firmware'. The 'recovery'
sysfs file can also be used similarly to control the error recovery
state machine of a remoteproc. These interfaces are currently
allowed irrespective of how the remoteprocs were booted (like
remoteproc self auto-boot, remoteproc client-driven boot etc).
These interfaces can adversely affect a remoteproc and its clients
especially when a remoteproc is being controlled by a remoteproc
client driver(s). Also, not all remoteproc drivers may want to
support the sysfs interfaces by default.
Add support to deny the sysfs state/firmware/recovery change by
introducing a state flag 'deny_sysfs_ops' that the individual
remoteproc drivers can set based on their usage needs. The default
behavior is to allow the sysfs operations as before.
Signed-off-by: Suman Anna
---
v2: revised to account for the 'recovery' sysfs file as well, patch
description updated accordingly
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-5-s-a...@ti.com/
drivers/remoteproc/remoteproc_sysfs.c | 12
include/linux/remoteproc.h| 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_sysfs.c
b/drivers/remoteproc/remoteproc_sysfs.c
index bd2950a246c9..3fd18a71c188 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -49,6 +49,10 @@ static ssize_t recovery_store(struct device *dev,
{
struct rproc *rproc = to_rproc(dev);
+ /* restrict sysfs operations if not allowed by remoteproc drivers */
+ if (rproc->deny_sysfs_ops)
+ return -EPERM;
+
if (sysfs_streq(buf, "enabled")) {
/* change the flag and begin the recovery process if needed */
rproc->recovery_disabled = false;
@@ -158,6 +162,10 @@ static ssize_t firmware_store(struct device *dev,
char *p;
int err, len = count;
+ /* restrict sysfs operations if not allowed by remoteproc drivers */
+ if (rproc->deny_sysfs_ops)
+ return -EPERM;
+
err = mutex_lock_interruptible(>lock);
if (err) {
dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
@@ -225,6 +233,10 @@ static ssize_t state_store(struct device *dev,
struct rproc *rproc = to_rproc(dev);
int ret = 0;
+ /* restrict sysfs operations if not allowed by remoteproc drivers */
+ if (rproc->deny_sysfs_ops)
+ return -EPERM;
+
if (sysfs_streq(buf, "start")) {
if (rproc->state == RPROC_RUNNING)
return -EBUSY;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 3fa3ba6498e8..dbc3767f7d0e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -508,6 +508,7 @@ struct rproc_dump_segment {
* @has_iommu: flag to indicate if remote processor is behind an MMU
* @auto_boot: flag to indicate if remote processor should be auto-started
* @autonomous: true if an external entity has booted the remote processor
+ * @deny_sysfs_ops: flag to not permit sysfs operations on state, firmware and
recovery
* @dump_segments: list of segments in the firmware
* @nb_vdev: number of vdev currently handled by rproc
* @char_dev: character device of the rproc
@@ -545,6 +546,7 @@ struct rproc {
bool has_iommu;
bool auto_boot;
bool autonomous;
+ bool deny_sysfs_ops;
struct list_head dump_segments;
int nb_vdev;
u8 elf_class;
--
2.28.0
[PATCH v2 0/3] remoteproc sysfs fixes/improvements
Hi All, This is a refresh of the unaccepted patches from an old series [1]. Patches 2 and 3 from that series were merged and these are rebased and revised versions of the same patches. I had forgotten about these patches, and am resurrecting these again. Patches are on top of latest 5.10-rc4. The features being introduced here will be needed by the recently posted PRU remoteproc driver [2] in addition to the existing Wkup M3 remoteproc driver. Both of these drivers follow a client-driven boot methodology, with the latter strictly booted by another driver in kernel. The PRU remoteproc driver will be supporting both in-kernel clients as well as control from userspace orthogonally. The logic though is applicable and useful to any remoteproc driver not using 'auto-boot' and using an external driver/application to boot the remoteproc. regards Suman [1] https://patchwork.kernel.org/project/linux-remoteproc/cover/20180915003725.17549-1-s-a...@ti.com/ [2] https://patchwork.kernel.org/project/linux-remoteproc/cover/20201119140850.12268-1-grzegorz.jaszc...@linaro.org/ Suman Anna (3): remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs remoteproc: Introduce deny_sysfs_ops flag remoteproc: wkup_m3: Set deny_sysfs_ops flag drivers/remoteproc/remoteproc_sysfs.c | 28 ++- drivers/remoteproc/wkup_m3_rproc.c| 1 + include/linux/remoteproc.h| 2 ++ 3 files changed, 30 insertions(+), 1 deletion(-) -- 2.28.0
[PATCH v2 3/3] remoteproc: wkup_m3: Set deny_sysfs_ops flag
The Wakeup M3 remote processor is controlled by the wkup_m3_ipc client driver, so set the newly introduced 'deny_sysfs_ops' flag to not allow any overriding of the remoteproc firmware or state from userspace. Signed-off-by: Suman Anna --- v2: rebased version, no code changes, patch title adjusted slightly v1: https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-6-s-a...@ti.com/ drivers/remoteproc/wkup_m3_rproc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c index b9349d684258..d92d7f32ba8d 100644 --- a/drivers/remoteproc/wkup_m3_rproc.c +++ b/drivers/remoteproc/wkup_m3_rproc.c @@ -160,6 +160,7 @@ static int wkup_m3_rproc_probe(struct platform_device *pdev) } rproc->auto_boot = false; + rproc->deny_sysfs_ops = true; wkupm3 = rproc->priv; wkupm3->rproc = rproc; -- 2.28.0
[PATCH v2 1/3] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs
The remoteproc core performs automatic boot and shutdown of a remote
processor during rproc_add() and rproc_del() for remote processors
supporting 'auto-boot'. The remoteproc devices not using 'auto-boot'
require either a remoteproc client driver or a userspace client to
use the sysfs 'state' variable to perform the boot and shutdown. The
in-kernel client drivers hold the corresponding remoteproc driver
module's reference count when they acquire a rproc handle through
the rproc_get_by_phandle() API, but there is no such support for
userspace applications performing the boot through sysfs interface.
The shutdown of a remoteproc upon removing a remoteproc platform
driver is automatic only with 'auto-boot' and this can cause a
remoteproc with no auto-boot to stay powered on and never freed
up if booted using the sysfs interface without a matching stop,
and when the remoteproc driver module is removed or unbound from
the device. This will result in a memory leak as well as the
corresponding remoteproc ida being never deallocated. Fix this
by holding a module reference count for the remoteproc's driver
during a sysfs 'start' and releasing it during the sysfs 'stop'
operation.
Signed-off-by: Suman Anna
Acked-by: Arnaud Pouliquen
---
v2: rebased version, no changes
v1:
https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-2-s-a...@ti.com/
drivers/remoteproc/remoteproc_sysfs.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_sysfs.c
b/drivers/remoteproc/remoteproc_sysfs.c
index d1cf7bf277c4..bd2950a246c9 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -3,6 +3,7 @@
* Remote Processor Framework
*/
+#include
#include
#include
@@ -228,14 +229,27 @@ static ssize_t state_store(struct device *dev,
if (rproc->state == RPROC_RUNNING)
return -EBUSY;
+ /*
+* prevent underlying implementation from being removed
+* when remoteproc does not support auto-boot
+*/
+ if (!rproc->auto_boot &&
+ !try_module_get(dev->parent->driver->owner))
+ return -EINVAL;
+
ret = rproc_boot(rproc);
- if (ret)
+ if (ret) {
dev_err(>dev, "Boot failed: %d\n", ret);
+ if (!rproc->auto_boot)
+ module_put(dev->parent->driver->owner);
+ }
} else if (sysfs_streq(buf, "stop")) {
if (rproc->state != RPROC_RUNNING)
return -EINVAL;
rproc_shutdown(rproc);
+ if (!rproc->auto_boot)
+ module_put(dev->parent->driver->owner);
} else {
dev_err(>dev, "Unrecognised option: %s\n", buf);
ret = -EINVAL;
--
2.28.0
Re: [PATCH] remoteproc: Add module parameter 'auto_boot'
Hi Paul, On 11/20/20 4:37 PM, Mathieu Poirier wrote: > Hi Paul, > > On Sun, Nov 15, 2020 at 11:50:56AM +, Paul Cercueil wrote: >> Until now the remoteproc core would always default to trying to boot the >> remote processor at startup. The various remoteproc drivers could >> however override that setting. >> >> Whether or not we want the remote processor to boot, really depends on >> the nature of the processor itself - a processor built into a WiFi chip >> will need to be booted for the WiFi hardware to be usable, for instance, >> but a general-purpose co-processor does not have any predeterminated >> function, and as such we cannot assume that the OS will want the >> processor to be booted - yet alone that we have a single do-it-all >> firmware to load. >> > > If I understand correctly you have various remote processors that use the > same firmware > but are serving different purposes - is this correct? > >> Add a 'auto_boot' module parameter that instructs the remoteproc whether >> or not it should auto-boot the remote processor, which will default to >> "true" to respect the previous behaviour. >> > > Given that the core can't be a module I wonder if this isn't something that > would be better off in the specific platform driver or the device tree... > Other > people might have an opinion as well. I agree. Even it is a module, all it is setting up is default behavior, and doesn't buy you much. If you have one or more remoteproc drivers supporting different instances, and each one wants different behavior, you would have to customize it in the drivers anyway. ST drivers are customizing this using a DT flag. Given that the individual platform drivers have to be modules, is there any issue in customizing this in your platform driver? regards Suman > > Thanks, > Mathieu > >> Signed-off-by: Paul Cercueil >> --- >> drivers/remoteproc/remoteproc_core.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index dab2c0f5caf0..687b1bfd49db 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -44,6 +44,11 @@ >> >> #define HIGH_BITS_MASK 0xULL >> >> +static bool auto_boot = true; >> +module_param(auto_boot, bool, 0400); >> +MODULE_PARM_DESC(auto_boot, >> + "Auto-boot the remote processor [default=true]"); >> + >> static DEFINE_MUTEX(rproc_list_mutex); >> static LIST_HEAD(rproc_list); >> static struct notifier_block rproc_panic_nb; >> @@ -2176,7 +2181,7 @@ struct rproc *rproc_alloc(struct device *dev, const >> char *name, >> return NULL; >> >> rproc->priv = [1]; >> -rproc->auto_boot = true; >> +rproc->auto_boot = auto_boot; >> rproc->elf_class = ELFCLASSNONE; >> rproc->elf_machine = EM_NONE; >> >> -- >> 2.29.2 >>
[PATCH 1/3] dt-bindings: remoteproc: k3-r5f: Update bindings for J7200 SoCs
The TI K3 J7200 SoCs have two dual-core Arm R5F clusters/subsystems, with 2 R5F cores each, one in each of the MCU and MAIN voltage domains. These clusters are a revised IP version compared to those present on J721E SoCs. Update the K3 R5F remoteproc bindings with the compatible info relevant to these R5F clusters/subsystems on K3 J7200 SoCs. Signed-off-by: Suman Anna --- .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml index 4069f0f5e8fa..d905d614502b 100644 --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml @@ -32,6 +32,7 @@ properties: enum: - ti,am654-r5fss - ti,j721e-r5fss + - ti,j7200-r5fss power-domains: description: | @@ -95,6 +96,7 @@ patternProperties: enum: - ti,am654-r5f - ti,j721e-r5f + - ti,j7200-r5f reg: items: -- 2.28.0
[PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs
The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
IP however doubles the total available TCM in LockStep mode by making the
Core1 TCM visible immediately after the corresponding Core0 TCM.
The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
above. This increased TCM memory is only usable in LockStep-mode, and
has to be adjusted to the normal 32 KB size in Split mode. Enhance the
TI K3 R5F remoteproc for this logic through a new function. The adjustment
is a no-op on prior SoCs and relies on the correct DTS node sizes in
LockStep-mode on applicable SoCs.
Signed-off-by: Suman Anna
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 43
1 file changed, 43 insertions(+)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 66a32dcdd7d0..62b5a4c29456 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -71,9 +71,11 @@ enum cluster_mode {
/**
* struct k3_r5_soc_data - match data to handle SoC variations
+ * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
* @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
*/
struct k3_r5_soc_data {
+ bool tcm_is_double;
bool tcm_ecc_autoinit;
};
@@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc
*kproc)
of_reserved_mem_device_release(kproc->dev);
}
+/*
+ * Each R5F core within a typical R5FSS instance has a total of 64 KB of TCMs,
+ * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from both
+ * cores are usable in Split-mode, but only the Core0 TCMs can be used in
+ * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs by
+ * leveraging the Core1 TCMs as well in certain modes where they would have
+ * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done by
+ * making a Core1 TCM visible immediately after the corresponding Core0 TCM.
+ * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and the
+ * dts representation reflects this increased size on supported SoCs. The Core0
+ * TCM sizes therefore have to be adjusted to only half the original size in
+ * Split mode.
+ */
+static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
+{
+ struct k3_r5_cluster *cluster = kproc->cluster;
+ struct k3_r5_core *core = kproc->core;
+ struct device *cdev = core->dev;
+ struct k3_r5_core *core0;
+
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
+ !cluster->soc_data->tcm_is_double)
+ return;
+
+ core0 = list_first_entry(>cores, struct k3_r5_core, elem);
+ if (core == core0) {
+ WARN_ON(core->mem[0].size != SZ_64K);
+ WARN_ON(core->mem[1].size != SZ_64K);
+
+ core->mem[0].size /= 2;
+ core->mem[1].size /= 2;
+
+ dev_dbg(cdev, "adjusted TCM sizes, ATCM = 0x%zx BTCM = 0x%zx\n",
+ core->mem[0].size, core->mem[1].size);
+ }
+}
+
static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
{
struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
@@ -933,6 +972,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device
*pdev)
goto err_config;
}
+ k3_r5_adjust_tcm_sizes(kproc);
+
ret = k3_r5_reserved_mem_init(kproc);
if (ret) {
dev_err(dev, "reserved memory init failed, ret = %d\n",
@@ -1407,10 +1448,12 @@ static int k3_r5_probe(struct platform_device *pdev)
}
static const struct k3_r5_soc_data am65_j721e_soc_data = {
+ .tcm_is_double = false,
.tcm_ecc_autoinit = false,
};
static const struct k3_r5_soc_data j7200_soc_data = {
+ .tcm_is_double = true,
.tcm_ecc_autoinit = true,
};
--
2.28.0
[PATCH 2/3] remoteproc: k3-r5: Extend support to R5F clusters on J7200 SoCs
The K3 J7200 SoC family has a revised R5F sub-system and contains a
subset of the R5F clusters present on J721E SoCs. The K3 J7200 SoCs
only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
each. One cluster is present within the MCU voltage domain (MCU_R5FSS0),
while the other is present in the MAIN voltage domain (MAIN_R5FSS0).
The revised IP has the following two new features:
1. TCMs are auto-initialized during module power-up, and the behavior
is programmable through a MMR bit.
2. The LockStep-mode allows the Core1 TCMs to be combined with the
Core0 TCMs effectively doubling the amount of TCMs available.
The LockStep-mode on previous SoCs could only use the Core0 TCMs.
This combined TCMs appear contiguous at the respective Core0 TCM
addresses.
Extend the support to these clusters in the K3 R5F remoteproc driver
using J7200 specific compatibles. Logic for the second feature is
added in the next patch. The integration of these clusters is very
much similar to J721E SoCs otherwise.
Signed-off-by: Suman Anna
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 52 +++-
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 40fa7a4d2ec8..66a32dcdd7d0 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -38,6 +38,8 @@
#define PROC_BOOT_CFG_FLAG_R5_TCM_RSTBASE 0x0800
#define PROC_BOOT_CFG_FLAG_R5_BTCM_EN 0x1000
#define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000
+/* Available from J7200 SoCs onwards */
+#define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS 0x4000
/* R5 TI-SCI Processor Control Flags */
#define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT 0x0001
@@ -67,16 +69,26 @@ enum cluster_mode {
CLUSTER_MODE_LOCKSTEP,
};
+/**
+ * struct k3_r5_soc_data - match data to handle SoC variations
+ * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
+ */
+struct k3_r5_soc_data {
+ bool tcm_ecc_autoinit;
+};
+
/**
* struct k3_r5_cluster - K3 R5F Cluster structure
* @dev: cached device pointer
* @mode: Mode to configure the Cluster - Split or LockStep
* @cores: list of R5 cores within the cluster
+ * @soc_data: SoC-specific feature data for a R5FSS
*/
struct k3_r5_cluster {
struct device *dev;
enum cluster_mode mode;
struct list_head cores;
+ const struct k3_r5_soc_data *soc_data;
};
/**
@@ -362,8 +374,16 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct k3_r5_core *core = kproc->core;
struct device *dev = kproc->dev;
+ u32 ctrl = 0, cfg = 0, stat = 0;
+ u64 boot_vec = 0;
+ bool mem_init_dis;
int ret;
+ ret = ti_sci_proc_get_status(core->tsp, _vec, , , );
+ if (ret < 0)
+ return ret;
+ mem_init_dis = !!(cfg & PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS);
+
ret = (cluster->mode == CLUSTER_MODE_LOCKSTEP) ?
k3_r5_lockstep_release(cluster) : k3_r5_split_release(core);
if (ret) {
@@ -372,6 +392,17 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
return ret;
}
+ /*
+* Newer IP revisions like on J7200 SoCs support h/w auto-initialization
+* of TCMs, so there is no need to perform the s/w memzero. This bit is
+* configurable through System Firmware, the default value does perform
+* auto-init, but account for it in case it is disabled
+*/
+ if (cluster->soc_data->tcm_ecc_autoinit && !mem_init_dis) {
+ dev_dbg(dev, "leveraging h/w init for TCM memories\n");
+ return 0;
+ }
+
/*
* Zero out both TCMs unconditionally (access from v8 Arm core is not
* affected by ATCM & BTCM enable configuration values) so that ECC
@@ -1309,15 +1340,23 @@ static int k3_r5_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct device_node *np = dev_of_node(dev);
struct k3_r5_cluster *cluster;
+ const struct k3_r5_soc_data *data;
int ret;
int num_cores;
+ data = of_device_get_match_data(>dev);
+ if (!data) {
+ dev_err(dev, "SoC-specific data is not defined\n");
+ return -ENODEV;
+ }
+
cluster = devm_kzalloc(dev, sizeof(*cluster), GFP_KERNEL);
if (!cluster)
return -ENOMEM;
cluster->dev = dev;
cluster->mode = CLUSTER_MODE_LOCKSTEP;
+ cluster->soc_data = data;
INIT_LIST_HEAD(>cores);
ret = of_property_read_u32(np, "ti,cluster-mode", >mode);
@@ -1367,9 +1406,18 @@ static int k3_r5_probe(struct platform_d
[PATCH 0/3] TI K3 R5F remoteproc support on J7200 SoCs
Hi All, The following series enhances the K3 R5F remoteproc driver to add support for the R5F clusters on the newer TI K3 J7200 SoC family. The J7200 SoCs have 2 R5FSS clusters, and both clusters are capable of supporting either the LockStep or Split-modes like on the existing AM65x and J721E SoCs. The R5FSS IP though is revised compared to K3 AM65x and J721E SoCs and has two new features: 1. TCMs are auto-initialized during module power-up, and the behavior is programmable through a SEC_MMR register bit. 2. The LockStep-mode allows the Core1 TCMs to be combined with the Core0 TCMs effectively doubling the amount of TCMs available. The LockStep-mode on previous SoCs could only use the Core0 TCMs. This combined TCMs appear contiguous at the respective Core0 TCM addresses. The series is based on 5.10-rc1, and can apply on top of the current rproc-next branch as well. Following is the patch summary: - Patch 1 updates the dt-bindings - Patch 2 introduces new SoC data logic and handles the TCM auto-init feature - Patch 3 handles the TCM adjustment logic in Split-mode regards Suman Suman Anna (3): dt-bindings: remoteproc: k3-r5f: Update bindings for J7200 SoCs remoteproc: k3-r5: Extend support to R5F clusters on J7200 SoCs remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 2 + drivers/remoteproc/ti_k3_r5_remoteproc.c | 95 ++- 2 files changed, 95 insertions(+), 2 deletions(-) -- 2.28.0
Re: [PATCH 3/6] remoteproc/pru: Add support for PRU specific interrupt configuration
Hi Greg,
On 11/18/20 9:27 AM, Grzegorz Jaszczyk wrote:
> Hi Suman,
>
> On Tue, 17 Nov 2020 at 21:40, Suman Anna wrote:
>>
>> Hi Greg,
>>
>> On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote:
>>> The firmware blob can contain optional ELF sections: .resource_table
>>> section and .pru_irq_map one. The second one contains the PRUSS
>>> interrupt mapping description, which needs to be setup before powering
>>> on the PRU core. To avoid RAM wastage this ELF section is not mapped to
>>> any ELF segment (by the firmware linker) and therefore is not loaded to
>>> PRU memory.
>>>
>>> The PRU interrupt configuration is handled within the PRUSS INTC irqchip
>>> driver and leverages the system events to interrupt channels and host
>>> interrupts mapping configuration. Relevant irq routing information is
>>> passed through a special .pru_irq_map ELF section (for interrupts routed
>>> to and used by PRU cores) or via the PRU application's device tree node
>>> (for interrupts routed to and used by the main CPU). The mappings are
>>> currently programmed during the booting/shutdown of the PRU.
>>>
>>> The interrupt configuration passed through .pru_irq_map ELF section is
>>> optional. It varies on specific firmware functionality and therefore
>>> have to be unwinded during PRU stop and performed again during
>>> PRU start.
>>>
>>> Co-developed-by: Suman Anna
>>> Signed-off-by: Suman Anna
>>> Signed-off-by: Grzegorz Jaszczyk
>>> ---
>>> drivers/remoteproc/pru_rproc.c | 191 -
>>> drivers/remoteproc/pru_rproc.h | 46
>>> 2 files changed, 236 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/remoteproc/pru_rproc.h
>>>
>>> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
>>> index c94c8e965c21..825e9c7e081b 100644
>>> --- a/drivers/remoteproc/pru_rproc.c
>>> +++ b/drivers/remoteproc/pru_rproc.c
>>> @@ -11,13 +11,16 @@
>>> */
>>>
>>> #include
>>> +#include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>>
>>> #include "remoteproc_internal.h"
>>> #include "remoteproc_elf_helpers.h"
>>> +#include "pru_rproc.h"
>>>
>>> /* PRU_ICSS_PRU_CTRL registers */
>>> #define PRU_CTRL_CTRL0x
>>> @@ -41,6 +44,8 @@
>>> #define PRU_SDRAM_DA 0x2000 /* Secondary Data RAM */
>>> #define PRU_SHRDRAM_DA 0x1 /* Shared Data RAM */
>>>
>>> +#define MAX_PRU_SYS_EVENTS 160
>>> +
>>> /**
>>> * enum pru_iomem - PRU core memory/register range identifiers
>>> *
>>> @@ -64,6 +69,10 @@ enum pru_iomem {
>>> * @rproc: remoteproc pointer for this PRU core
>>> * @mem_regions: data for each of the PRU memory regions
>>> * @fw_name: name of firmware image used during loading
>>> + * @mapped_irq: virtual interrupt numbers of created fw specific mapping
>>> + * @pru_interrupt_map: pointer to interrupt mapping description (firmware)
>>> + * @pru_interrupt_map_sz: pru_interrupt_map size
>>> + * @evt_count: number of mapped events
>>> */
>>> struct pru_rproc {
>>> int id;
>>> @@ -72,6 +81,10 @@ struct pru_rproc {
>>> struct rproc *rproc;
>>> struct pruss_mem_region mem_regions[PRU_IOMEM_MAX];
>>> const char *fw_name;
>>> + int *mapped_irq;
>>> + struct pru_irq_rsc *pru_interrupt_map;
>>> + size_t pru_interrupt_map_sz;
>>> + ssize_t evt_count;
>>
>> Do you really need this to be ssize_t type?
>
> You are right, it is not needed. I will use size_t type instead and
> modify relevant while loop.
Hmm, size_t is ok, but perhaps the same type you used struct pru_irq_rsc is
better.
>
>>
>>> };
>>>
>>> static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int
>>> reg)
>>> @@ -85,15 +98,107 @@ void pru_control_write_reg(struct pru_rproc *pru,
>>> unsigned int reg, u32 val)
>>> writel_relaxed(val, pru->mem_regions[PRU_IOMEM_CTRL].va + reg);
>>> }
>>>
>>> +static void pru_dispose_irq_mapping(struct pru_rproc *pru)
>>> +{
>>> + while (--pru->evt_count >= 0) {
>>> + if (pru->mapped_irq[pru->evt_count] > 0)
>>>
Re: [PATCH 3/6] remoteproc/pru: Add support for PRU specific interrupt configuration
Hi Greg,
On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote:
> The firmware blob can contain optional ELF sections: .resource_table
> section and .pru_irq_map one. The second one contains the PRUSS
> interrupt mapping description, which needs to be setup before powering
> on the PRU core. To avoid RAM wastage this ELF section is not mapped to
> any ELF segment (by the firmware linker) and therefore is not loaded to
> PRU memory.
>
> The PRU interrupt configuration is handled within the PRUSS INTC irqchip
> driver and leverages the system events to interrupt channels and host
> interrupts mapping configuration. Relevant irq routing information is
> passed through a special .pru_irq_map ELF section (for interrupts routed
> to and used by PRU cores) or via the PRU application's device tree node
> (for interrupts routed to and used by the main CPU). The mappings are
> currently programmed during the booting/shutdown of the PRU.
>
> The interrupt configuration passed through .pru_irq_map ELF section is
> optional. It varies on specific firmware functionality and therefore
> have to be unwinded during PRU stop and performed again during
> PRU start.
>
> Co-developed-by: Suman Anna
> Signed-off-by: Suman Anna
> Signed-off-by: Grzegorz Jaszczyk
> ---
> drivers/remoteproc/pru_rproc.c | 191 -
> drivers/remoteproc/pru_rproc.h | 46
> 2 files changed, 236 insertions(+), 1 deletion(-)
> create mode 100644 drivers/remoteproc/pru_rproc.h
>
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index c94c8e965c21..825e9c7e081b 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -11,13 +11,16 @@
> */
>
> #include
> +#include
> #include
> #include
> +#include
> #include
> #include
>
> #include "remoteproc_internal.h"
> #include "remoteproc_elf_helpers.h"
> +#include "pru_rproc.h"
>
> /* PRU_ICSS_PRU_CTRL registers */
> #define PRU_CTRL_CTRL0x
> @@ -41,6 +44,8 @@
> #define PRU_SDRAM_DA 0x2000 /* Secondary Data RAM */
> #define PRU_SHRDRAM_DA 0x1 /* Shared Data RAM */
>
> +#define MAX_PRU_SYS_EVENTS 160
> +
> /**
> * enum pru_iomem - PRU core memory/register range identifiers
> *
> @@ -64,6 +69,10 @@ enum pru_iomem {
> * @rproc: remoteproc pointer for this PRU core
> * @mem_regions: data for each of the PRU memory regions
> * @fw_name: name of firmware image used during loading
> + * @mapped_irq: virtual interrupt numbers of created fw specific mapping
> + * @pru_interrupt_map: pointer to interrupt mapping description (firmware)
> + * @pru_interrupt_map_sz: pru_interrupt_map size
> + * @evt_count: number of mapped events
> */
> struct pru_rproc {
> int id;
> @@ -72,6 +81,10 @@ struct pru_rproc {
> struct rproc *rproc;
> struct pruss_mem_region mem_regions[PRU_IOMEM_MAX];
> const char *fw_name;
> + int *mapped_irq;
> + struct pru_irq_rsc *pru_interrupt_map;
> + size_t pru_interrupt_map_sz;
> + ssize_t evt_count;
Do you really need this to be ssize_t type?
> };
>
> static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int
> reg)
> @@ -85,15 +98,107 @@ void pru_control_write_reg(struct pru_rproc *pru,
> unsigned int reg, u32 val)
> writel_relaxed(val, pru->mem_regions[PRU_IOMEM_CTRL].va + reg);
> }
>
> +static void pru_dispose_irq_mapping(struct pru_rproc *pru)
> +{
> + while (--pru->evt_count >= 0) {
> + if (pru->mapped_irq[pru->evt_count] > 0)
> + irq_dispose_mapping(pru->mapped_irq[pru->evt_count]);
> + }
> +
> + kfree(pru->mapped_irq);
> +}
> +
> +/*
> + * Parse the custom PRU interrupt map resource and configure the INTC
> + * appropriately.
> + */
> +static int pru_handle_intrmap(struct rproc *rproc)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct pru_rproc *pru = rproc->priv;
> + struct pru_irq_rsc *rsc = pru->pru_interrupt_map;
> + struct irq_fwspec fwspec;
> + struct device_node *irq_parent;
> + int i, ret = 0;
> +
> + /* not having pru_interrupt_map is not an error */
> + if (!rsc)
> + return 0;
> +
> + /* currently supporting only type 0 */
> + if (rsc->type != 0) {
> + dev_err(dev, "unsupported rsc type: %d\n", rsc->type);
> + return -EINVAL;
> + }
> +
> + if (rsc->num_evts < 0 || rsc->num_evts > MAX_PRU_SYS_EVENTS)
> + return -EINVAL;
> +
> + i
Re: [PATCH 5/6] remoteproc/pru: Add support for various PRU cores on K3 AM65x SoCs
Hi Greg,
On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote:
> From: Suman Anna
>
> The K3 AM65x family of SoCs have the next generation of the PRU-ICSS
> processor subsystem, commonly referred to as ICSSG. Each ICSSG processor
> subsystem on AM65x SR1.0 contains two primary PRU cores and two new
> auxiliary PRU cores called RTUs. The AM65x SR2.0 SoCs have a revised
> ICSSG IP that is based off the subsequent IP revision used on J721E
> SoCs. This IP instance has two new custom auxiliary PRU cores called
> Transmit PRUs (Tx_PRUs) in addition to the existing PRUs and RTUs.
>
> Each RTU and Tx_PRU cores have their own dedicated IRAM (smaller than
> a PRU), Control and debug feature sets, but is different in terms of
> sub-modules integrated around it and does not have the full capabilities
> associated with a PRU core. The RTU core is typically used to aid a
> PRU core in accelerating data transfers, while the Tx_PRU cores is
> normally used to control the TX L2 FIFO if enabled in Ethernet
> applications. Both can also be used to run independent applications.
> The RTU and Tx_PRU cores though share the same Data RAMs as the PRU
> cores, so the memories have to be partitioned carefully between different
> applications. The new cores also support a new sub-module called Task
> Manager to support two different context thread executions.
>
> Enhance the existing PRU remoteproc driver to support these new PRU, RTU
> and Tx PRU cores by using specific compatibles. The initial names for the
> firmware images for each PRU core are retrieved from DT nodes, and can
> be adjusted through sysfs if required.
>
> The PRU remoteproc driver has to be specifically modified to use a
> custom memcpy function within its ELF loader implementation for these
> new cores in order to overcome a limitation with copying data into each
> of the core's IRAM memories. These memory ports support only 4-byte
> writes, and any sub-word order byte writes clear out the remaining
> bytes other than the bytes being written within the containing word.
> The default ARM64 memcpy also cannot be used as it throws an exception
> when the preferred 8-byte copy operation is attempted. This choice is
> made by using a state flag that is set only on K3 SoCs.
>
> Signed-off-by: Suman Anna
> Co-developed-by: Grzegorz Jaszczyk
> Signed-off-by: Grzegorz Jaszczyk
> ---
> drivers/remoteproc/pru_rproc.c | 141 ++---
> 1 file changed, 132 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index 33806ddcbd5d..04c9f07799e2 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -46,9 +46,13 @@
> #define PRU_DEBUG_GPREG(x) (0x + (x) * 4)
> #define PRU_DEBUG_CT_REG(x) (0x0080 + (x) * 4)
>
> -/* PRU Core IRAM address masks */
> +/* PRU/RTU/Tx_PRU Core IRAM address masks */
> #define PRU0_IRAM_ADDR_MASK 0x34000
> #define PRU1_IRAM_ADDR_MASK 0x38000
> +#define RTU0_IRAM_ADDR_MASK 0x4000
> +#define RTU1_IRAM_ADDR_MASK 0x6000
> +#define TX_PRU0_IRAM_ADDR_MASK 0xa000
> +#define TX_PRU1_IRAM_ADDR_MASK 0xc000
>
> /* PRU device addresses for various type of PRU RAMs */
> #define PRU_IRAM_DA 0 /* Instruction RAM */
> @@ -73,12 +77,38 @@ enum pru_iomem {
> PRU_IOMEM_MAX,
> };
>
> +/**
> + * enum pru_type - PRU core type identifier
> + *
> + * @PRU_TYPE_PRU: Programmable Real-time Unit
> + * @PRU_TYPE_RTU: Auxiliary Programmable Real-Time Unit
> + * @PRU_TYPE_TX_PRU: Transmit Programmable Real-Time Unit
> + * @PRU_TYPE_MAX: just keep this one at the end
> + */
> +enum pru_type {
> + PRU_TYPE_PRU = 0,
> + PRU_TYPE_RTU,
> + PRU_TYPE_TX_PRU,
> + PRU_TYPE_MAX,
> +};
> +
> +/**
> + * struct pru_private_data - device data for a PRU core
> + * @type: type of the PRU core (PRU, RTU, Tx_PRU)
> + * @is_k3: flag used to identify the need for special load & event handling
> + */
> +struct pru_private_data {
> + enum pru_type type;
> + unsigned int is_k3 : 1;
> +};
> +
> /**
> * struct pru_rproc - PRU remoteproc structure
> * @id: id of the PRU core within the PRUSS
> * @dev: PRU core device pointer
> * @pruss: back-reference to parent PRUSS structure
> * @rproc: remoteproc pointer for this PRU core
> + * @data: PRU core specific data
> * @mem_regions: data for each of the PRU memory regions
> * @fw_name: name of firmware image used during loading
> * @mapped_irq: virtual interrupt numbers of created fw specific mapping
> @@ -93,6 +123,7 @@ struct pru_rproc {
> struct device *dev;
> struct pruss *pruss;
> struct rproc *rproc;
> + c
Re: [PATCH 2/6] remoteproc/pru: Add a PRU remoteproc driver
Hi Greg, I have a few minor comments below.. On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote: > From: Suman Anna > > 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 PRUs do not have a unified address space (have an Instruction > RAM and a primary Data RAM at both 0x0). The PRU remoteproc driver > therefore uses a custom remoteproc core ELF loader ops. The added > .da_to_va ops is only used to provide translations for the PRU > Data RAMs. 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 for each PRU core, 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, AM437x, AM57xx and 66AK2G > SoCs, and support for other TI SoCs will be added in subsequent > patches. > > Co-developed-by: Andrew F. Davis > Signed-off-by: Andrew F. Davis > Signed-off-by: Suman Anna > Co-developed-by: Grzegorz Jaszczyk > Signed-off-by: Grzegorz Jaszczyk > --- > drivers/remoteproc/Kconfig | 12 + > drivers/remoteproc/Makefile| 1 + > drivers/remoteproc/pru_rproc.c | 428 + > 3 files changed, 441 insertions(+) > create mode 100644 drivers/remoteproc/pru_rproc.c > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index d99548fb5dde..3e3865a7cd78 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -125,6 +125,18 @@ config KEYSTONE_REMOTEPROC > It's safe to say N here if you're not interested in the Keystone > DSPs or just want to use a bare minimum kernel. > > +config PRU_REMOTEPROC > + tristate "TI PRU remoteproc support" > + depends on TI_PRUSS > + default TI_PRUSS > + help > + Support for TI PRU remote processors present within a PRU-ICSS > + subsystem via the remote processor framework. > + > + 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. > + > config QCOM_PIL_INFO > tristate > > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > index da2ace4ec86c..bb26c9e4ef9c 100644 > --- a/drivers/remoteproc/Makefile > +++ b/drivers/remoteproc/Makefile > @@ -18,6 +18,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC) += > omap_remoteproc.o > obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o > obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o > obj-$(CONFIG_KEYSTONE_REMOTEPROC)+= keystone_remoteproc.o > +obj-$(CONFIG_PRU_REMOTEPROC) += pru_rproc.o > obj-$(CONFIG_QCOM_PIL_INFO) += qcom_pil_info.o > obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o > obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o > diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c > new file mode 100644 > index ..c94c8e965c21 > --- /dev/null > +++ b/drivers/remoteproc/pru_rproc.c > @@ -0,0 +1,428 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * PRU-ICSS remoteproc driver for various TI SoCs > + * > + * Copyright (C) 2014-2020 Texas Instruments Incorporated - > https://www.ti.com/ > + * > + * Author(s): > + * Suman Anna > + * Andrew F. Davis > + * Grzegorz Jaszczyk for Texas Instruments > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include "remoteproc_internal.h" > +#include "remoteproc_elf_helpers.h" > + > +/* PRU_ICSS_PRU_CTRL registers */ >
Re: [PATCH v2] soc: ti: pruss: Fix wrong check against *get_match_data return value
On 11/16/20 1:06 PM, santosh.shilim...@oracle.com wrote:
> On 11/16/20 9:31 AM, Suman Anna wrote:
>> Hi Santosh,
>>
>> On 11/16/20 11:22 AM, Grzegorz Jaszczyk wrote:
>>> Since the of_device_get_match_data() doesn't return error code, remove
>>> wrong IS_ERR test. Proper check against NULL pointer is already done
>>> later before usage: if (data && data->...).
>>>
>>> Additionally, proceeding with empty device data is valid (e.g. in case
>>> of "ti,am3356-pruss").
>>>
>>> Fixes: ba59c9b43c86 ("soc: ti: pruss: support CORECLK_MUX and IEPCLK_MUX")
>>> Reported-by: Wei Yongjun
>>> Signed-off-by: Grzegorz Jaszczyk
>>> Acked-by: Suman Anna
>>
>> Can you please pick this up for 5.10-rc's?
>>
> Nothing is broken so I will just add these for next merge window.
Yeah, that is fine as well. Thanks Santosh.
regards
Suman
Re: [PATCH v2] soc: ti: pruss: Fix wrong check against *get_match_data return value
Hi Santosh,
On 11/16/20 11:22 AM, Grzegorz Jaszczyk wrote:
> Since the of_device_get_match_data() doesn't return error code, remove
> wrong IS_ERR test. Proper check against NULL pointer is already done
> later before usage: if (data && data->...).
>
> Additionally, proceeding with empty device data is valid (e.g. in case
> of "ti,am3356-pruss").
>
> Fixes: ba59c9b43c86 ("soc: ti: pruss: support CORECLK_MUX and IEPCLK_MUX")
> Reported-by: Wei Yongjun
> Signed-off-by: Grzegorz Jaszczyk
> Acked-by: Suman Anna
Can you please pick this up for 5.10-rc's?
Thanks,
Suman
> ---
> v1->v2:
> - Add the appropriate Fixes: tag.
> - %s/Remove/Fix/ in patch title.
> - Add Suman's Acked-by.
> ---
> drivers/soc/ti/pruss.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index cc0b4ad7a3d3..5d6e7132a5c4 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -126,8 +126,6 @@ static int pruss_clk_init(struct pruss *pruss, struct
> device_node *cfg_node)
> int ret = 0;
>
> data = of_device_get_match_data(dev);
> - if (IS_ERR(data))
> - return -ENODEV;
>
> clks_np = of_get_child_by_name(cfg_node, "clocks");
> if (!clks_np) {
> @@ -175,10 +173,6 @@ static int pruss_probe(struct platform_device *pdev)
> const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" };
>
> data = of_device_get_match_data(>dev);
> - if (IS_ERR(data)) {
> - dev_err(dev, "missing private data\n");
> - return -ENODEV;
> - }
>
> ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> if (ret) {
>
Re: [PATCH] soc: ti: pruss: Remove wrong check against *get_match_data return value
Hi Greg,
On 10/26/20 9:49 AM, Grzegorz Jaszczyk wrote:
> Since the of_device_get_match_data() doesn't return error code, remove
> wrong IS_ERR test. Proper check against NULL pointer is already done
> later before usage: if (data && data->...).
>
> Additionally, proceeding with empty device data is valid (e.g. in case
> of "ti,am3356-pruss").
>
> Reported-by: Wei Yongjun
Please add the appropriate Fixes: tag.
And prefer %s/Remove/Fix/ in patch title.
With that,
Acked-by: Suman Anna
regards
Suman
> Signed-off-by: Grzegorz Jaszczyk
> ---
> drivers/soc/ti/pruss.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index cc0b4ad7a3d3..5d6e7132a5c4 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -126,8 +126,6 @@ static int pruss_clk_init(struct pruss *pruss, struct
> device_node *cfg_node)
> int ret = 0;
>
> data = of_device_get_match_data(dev);
> - if (IS_ERR(data))
> - return -ENODEV;
>
> clks_np = of_get_child_by_name(cfg_node, "clocks");
> if (!clks_np) {
> @@ -175,10 +173,6 @@ static int pruss_probe(struct platform_device *pdev)
> const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" };
>
> data = of_device_get_match_data(>dev);
> - if (IS_ERR(data)) {
> - dev_err(dev, "missing private data\n");
> - return -ENODEV;
> - }
>
> ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> if (ret) {
>
[tip: irq/core] irqchip/irq-pruss-intc: Add logic for handling reserved interrupts
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 6016f32d1de2798cc88c1a4b703d0ea096c19793
Gitweb:
https://git.kernel.org/tip/6016f32d1de2798cc88c1a4b703d0ea096c19793
Author:Suman Anna
AuthorDate:Wed, 16 Sep 2020 18:36:36 +02:00
Committer: Marc Zyngier
CommitterDate: Thu, 17 Sep 2020 12:20:31 +01:00
irqchip/irq-pruss-intc: Add logic for handling reserved interrupts
The PRUSS INTC has a fixed number of output interrupt lines that are
connected to a number of processors or other PRUSS instances or other
devices (like DMA) on the SoC. The output interrupt lines 2 through 9
are usually connected to the main Arm host processor and are referred
to as host interrupts 0 through 7 from ARM/MPU perspective.
All of these 8 host interrupts are not always exclusively connected
to the Arm interrupt controller. Some SoCs have some interrupt lines
not connected to the Arm interrupt controller at all, while a few others
have the interrupt lines connected to multiple processors in which they
need to be partitioned as per SoC integration needs. For example, AM437x
and 66AK2G SoCs have 2 PRUSS instances each and have the host interrupt 5
connected to the other PRUSS, while AM335x has host interrupt 0 shared
between MPU and TSC_ADC and host interrupts 6 & 7 shared between MPU and
a DMA controller.
Add logic to the PRUSS INTC driver to ignore both these shared and
invalid interrupts.
Co-developed-by: Grzegorz Jaszczyk
Signed-off-by: Suman Anna
Signed-off-by: Grzegorz Jaszczyk
Signed-off-by: Marc Zyngier
---
drivers/irqchip/irq-pruss-intc.c | 29 -
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
index c8bdef4..e7ba358 100644
--- a/drivers/irqchip/irq-pruss-intc.c
+++ b/drivers/irqchip/irq-pruss-intc.c
@@ -484,7 +484,7 @@ static int pruss_intc_probe(struct platform_device *pdev)
struct pruss_intc *intc;
struct pruss_host_irq_data *host_data;
int i, irq, ret;
- u8 max_system_events;
+ u8 max_system_events, irqs_reserved = 0;
data = of_device_get_match_data(dev);
if (!data)
@@ -504,6 +504,16 @@ static int pruss_intc_probe(struct platform_device *pdev)
if (IS_ERR(intc->base))
return PTR_ERR(intc->base);
+ ret = of_property_read_u8(dev->of_node, "ti,irqs-reserved",
+ _reserved);
+
+ /*
+* The irqs-reserved is used only for some SoC's therefore not having
+* this property is still valid
+*/
+ if (ret < 0 && ret != -EINVAL)
+ return ret;
+
pruss_intc_init(intc);
mutex_init(>lock);
@@ -514,6 +524,9 @@ static int pruss_intc_probe(struct platform_device *pdev)
return -ENOMEM;
for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
+ if (irqs_reserved & BIT(i))
+ continue;
+
irq = platform_get_irq_byname(pdev, irq_names[i]);
if (irq <= 0) {
ret = (irq == 0) ? -EINVAL : irq;
@@ -538,8 +551,11 @@ static int pruss_intc_probe(struct platform_device *pdev)
return 0;
fail_irq:
- while (--i >= 0)
- irq_set_chained_handler_and_data(intc->irqs[i], NULL, NULL);
+ while (--i >= 0) {
+ if (intc->irqs[i])
+ irq_set_chained_handler_and_data(intc->irqs[i], NULL,
+NULL);
+ }
irq_domain_remove(intc->domain);
@@ -553,8 +569,11 @@ static int pruss_intc_remove(struct platform_device *pdev)
unsigned int hwirq;
int i;
- for (i = 0; i < MAX_NUM_HOST_IRQS; i++)
- irq_set_chained_handler_and_data(intc->irqs[i], NULL, NULL);
+ for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
+ if (intc->irqs[i])
+ irq_set_chained_handler_and_data(intc->irqs[i], NULL,
+NULL);
+ }
for (hwirq = 0; hwirq < max_system_events; hwirq++)
irq_dispose_mapping(irq_find_mapping(intc->domain, hwirq));
[tip: irq/core] irqchip/irq-pruss-intc: Add support for ICSSG INTC on K3 SoCs
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 7e92dee60cba51f8a5c7637bac815e70c85935f7
Gitweb:
https://git.kernel.org/tip/7e92dee60cba51f8a5c7637bac815e70c85935f7
Author:Suman Anna
AuthorDate:Wed, 16 Sep 2020 18:36:38 +02:00
Committer: Marc Zyngier
CommitterDate: Thu, 17 Sep 2020 12:20:32 +01:00
irqchip/irq-pruss-intc: Add support for ICSSG INTC on K3 SoCs
The K3 AM65x and J721E SoCs have the next generation of the PRU-ICSS IP,
commonly called ICSSG. The PRUSS INTC present within the ICSSG supports
more System Events (160 vs 64), more Interrupt Channels and Host Interrupts
(20 vs 10) compared to the previous generation PRUSS INTC instances. The
first 2 and the last 10 of these host interrupt lines are used by the
PRU and other auxiliary cores and sub-modules within the ICSSG, with 8
host interrupts connected to MPU. The host interrupts 5, 6, 7 are also
connected to the other ICSSG instances within the SoC and can be
partitioned as per system integration through the board dts files.
Enhance the PRUSS INTC driver to add support for this ICSSG INTC
instance.
Co-developed-by: Grzegorz Jaszczyk
Signed-off-by: Suman Anna
Signed-off-by: Grzegorz Jaszczyk
Signed-off-by: Marc Zyngier
---
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/irq-pruss-intc.c | 9 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 733e59f..25c8944 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -495,7 +495,7 @@ config TI_SCI_INTA_IRQCHIP
config TI_PRUSS_INTC
tristate "TI PRU-ICSS Interrupt Controller"
- depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX ||
ARCH_KEYSTONE
+ depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM43XX || SOC_DRA7XX ||
ARCH_KEYSTONE || ARCH_K3
select IRQ_DOMAIN
help
This enables support for the PRU-ICSS Local Interrupt Controller
diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
index bfe529a..92fb578 100644
--- a/drivers/irqchip/irq-pruss-intc.c
+++ b/drivers/irqchip/irq-pruss-intc.c
@@ -628,11 +628,20 @@ static const struct pruss_intc_match_data pruss_intc_data
= {
.num_host_events = 10,
};
+static const struct pruss_intc_match_data icssg_intc_data = {
+ .num_system_events = 160,
+ .num_host_events = 20,
+};
+
static const struct of_device_id pruss_intc_of_match[] = {
{
.compatible = "ti,pruss-intc",
.data = _intc_data,
},
+ {
+ .compatible = "ti,icssg-intc",
+ .data = _intc_data,
+ },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, pruss_intc_of_match);
[tip: irq/core] dt-bindings: irqchip: Add PRU-ICSS interrupt controller bindings
The following commit has been merged into the irq/core branch of tip: Commit-ID: 8a1b09ed4308caf12c231430afb78d3331a85dc2 Gitweb: https://git.kernel.org/tip/8a1b09ed4308caf12c231430afb78d3331a85dc2 Author:Suman Anna AuthorDate:Wed, 16 Sep 2020 18:34:54 +02:00 Committer: Marc Zyngier CommitterDate: Thu, 17 Sep 2020 12:20:30 +01:00 dt-bindings: irqchip: Add PRU-ICSS interrupt controller bindings The Programmable Real-Time Unit and Industrial Communication Subsystem (PRU-ICSS or simply PRUSS) contains an interrupt controller (INTC) that can handle various system input events and post interrupts back to the device-level initiators. The INTC can support up to 64 input events on most SoCs with individual control configuration and h/w prioritization. These events are mapped onto 10 output interrupt lines through two levels of many-to-one mapping support. Different interrupt lines are routed to the individual PRU cores or to the host CPU or to other PRUSS instances. The K3 AM65x and J721E SoCs have the next generation of the PRU-ICSS IP, commonly called ICSSG. The ICSSG interrupt controller on K3 SoCs provide a higher number of host interrupts (20 vs 10) and can handle an increased number of input events (160 vs 64) from various SoC interrupt sources. Add the bindings document for these interrupt controllers on all the applicable SoCs. It covers the OMAP architecture SoCs - AM33xx, AM437x and AM57xx; the Keystone 2 architecture based 66AK2G SoC; the Davinci architecture based OMAPL138 SoCs, and the K3 architecture based AM65x and J721E SoCs. Co-developed-by: Andrew F. Davis Co-developed-by: Roger Quadros Co-developed-by: Grzegorz Jaszczyk Signed-off-by: Andrew F. Davis Signed-off-by: Roger Quadros Signed-off-by: Suman Anna Signed-off-by: Grzegorz Jaszczyk Reviewed-by: Rob Herring Signed-off-by: Marc Zyngier --- Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml | 158 ++- 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml diff --git a/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml new file mode 100644 index 000..bbf79d1 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/ti,pruss-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI PRU-ICSS Local Interrupt Controller + +maintainers: + - Suman Anna + +description: | + Each PRU-ICSS has a single interrupt controller instance that is common + to all the PRU cores. Most interrupt controllers can route 64 input events + which are then mapped to 10 possible output interrupts through two levels + of mapping. The input events can be triggered by either the PRUs and/or + various other PRUSS internal and external peripherals. The first 2 output + interrupts (0, 1) are fed exclusively to the internal PRU cores, with the + remaining 8 (2 through 9) connected to external interrupt controllers + including the MPU and/or other PRUSS instances, DSPs or devices. + + The property "ti,irqs-reserved" is used for denoting the connection + differences on the output interrupts 2 through 9. If this property is not + defined, it implies that all the PRUSS INTC output interrupts 2 through 9 + (host_intr0 through host_intr7) are connected exclusively to the Arm interrupt + controller. + + The K3 family of SoCs can handle 160 input events that can be mapped to 20 + different possible output interrupts. The additional output interrupts (10 + through 19) are connected to new sub-modules within the ICSSG instances. + + This interrupt-controller node should be defined as a child node of the + corresponding PRUSS node. The node should be named "interrupt-controller". + +properties: + compatible: +enum: + - ti,pruss-intc + - ti,icssg-intc +description: | + Use "ti,pruss-intc" for OMAP-L13x/AM18x/DA850 SoCs, + AM335x family of SoCs, + AM437x family of SoCs, + AM57xx family of SoCs + 66AK2G family of SoCs + Use "ti,icssg-intc" for K3 AM65x & J721E family of SoCs + + reg: +maxItems: 1 + + interrupts: +minItems: 1 +maxItems: 8 +description: | + All the interrupts generated towards the main host processor in the SoC. + A shared interrupt can be skipped if the desired destination and usage is + by a different processor/device. + + interrupt-names: +minItems: 1 +maxItems: 8 +items: + pattern: host_intr[0-
[PATCH v5 4/4] remoteproc: k3-r5: Add loading support for on-chip SRAM regions
The K3 SoCs has various internal on-chip SRAM memories like the SRAM
within the MCU domain or the shared MSMC RAM within NavSS that can be
used for multiple purposes. One such purpose is to have the R5F cores
use a portion of such on-chip SRAM for fast-access data or to directly
execute code.
Add support to the K3 R5 remoteproc driver to parse and support
loading into such memories. The SRAM regions need to be mapped as
normal non-cacheable memory to avoid kernel crashes when the remoteproc
loader code uses the Arm64 memset library function (the "DC ZVA"
instruction throws a alignment fault on device type memory).
These SRAM regions are completely optional as not all firmware images
require these memories, and any such memory has to be reserved as such
in the DTS files.
Signed-off-by: Suman Anna
Reviewed-by: Mathieu Poirier
---
v5: No changes
v4: No changes
v3: https://patchwork.kernel.org/patch/11679329/
- No code changes, picked up review tags
v2: https://patchwork.kernel.org/patch/11632991/
v1: https://patchwork.kernel.org/patch/11456373/
drivers/remoteproc/ti_k3_r5_remoteproc.c | 79
1 file changed, 79 insertions(+)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a6b395ab47b6..d9307935441d 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -85,7 +85,9 @@ struct k3_r5_cluster {
* @dev: cached device pointer
* @rproc: rproc handle representing this core
* @mem: internal memory regions data
+ * @sram: on-chip SRAM memory regions data
* @num_mems: number of internal memory regions
+ * @num_sram: number of on-chip SRAM memory regions
* @reset: reset control handle
* @tsp: TI-SCI processor control handle
* @ti_sci: TI-SCI handle
@@ -99,7 +101,9 @@ struct k3_r5_core {
struct device *dev;
struct rproc *rproc;
struct k3_r5_mem *mem;
+ struct k3_r5_mem *sram;
int num_mems;
+ int num_sram;
struct reset_control *reset;
struct ti_sci_proc *tsp;
const struct ti_sci_handle *ti_sci;
@@ -587,6 +591,18 @@ static void *k3_r5_rproc_da_to_va(struct rproc *rproc, u64
da, size_t len)
}
}
+ /* handle any SRAM regions using SoC-view addresses */
+ for (i = 0; i < core->num_sram; i++) {
+ dev_addr = core->sram[i].dev_addr;
+ size = core->sram[i].size;
+
+ if (da >= dev_addr && ((da + len) <= (dev_addr + size))) {
+ offset = da - dev_addr;
+ va = core->sram[i].cpu_addr + offset;
+ return (__force void *)va;
+ }
+ }
+
/* handle static DDR reserved memory regions */
for (i = 0; i < kproc->num_rmems; i++) {
dev_addr = kproc->rmem[i].dev_addr;
@@ -1027,6 +1043,63 @@ static int k3_r5_core_of_get_internal_memories(struct
platform_device *pdev,
return 0;
}
+static int k3_r5_core_of_get_sram_memories(struct platform_device *pdev,
+ struct k3_r5_core *core)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = >dev;
+ struct device_node *sram_np;
+ struct resource res;
+ int num_sram;
+ int i, ret;
+
+ num_sram = of_property_count_elems_of_size(np, "sram", sizeof(phandle));
+ if (num_sram <= 0) {
+ dev_dbg(dev, "device does not use reserved on-chip memories,
num_sram = %d\n",
+ num_sram);
+ return 0;
+ }
+
+ core->sram = devm_kcalloc(dev, num_sram, sizeof(*core->sram),
GFP_KERNEL);
+ if (!core->sram)
+ return -ENOMEM;
+
+ for (i = 0; i < num_sram; i++) {
+ sram_np = of_parse_phandle(np, "sram", i);
+ if (!sram_np)
+ return -EINVAL;
+
+ if (!of_device_is_available(sram_np)) {
+ of_node_put(sram_np);
+ return -EINVAL;
+ }
+
+ ret = of_address_to_resource(sram_np, 0, );
+ of_node_put(sram_np);
+ if (ret)
+ return -EINVAL;
+
+ core->sram[i].bus_addr = res.start;
+ core->sram[i].dev_addr = res.start;
+ core->sram[i].size = resource_size();
+ core->sram[i].cpu_addr = devm_ioremap_wc(dev, res.start,
+resource_size());
+ if (!core->sram[i].cpu_addr) {
+ dev_err(dev, "failed to parse and map sram%d memory at
%pad\n",
+ i, );
+ return -ENOMEM;
+ }
+
+ dev_dbg(dev, "memory sram%d: bus addr
[PATCH v5 0/4] TI K3 R5F remoteproc support
Hi Bjorn, The following is v5 of the TI K3 R5F remoteproc driver series supporting all the R5F processor clusters/subsystems on TI AM65x and J721E SoCs. Please see the v1 cover-letter [1] for the features supported on these R5F processors. This series has only 1 line change w.r.t v4 version [4], the example in dt-bindings is updated to fix couple of dt_binding_check warnings when applied against the latest linux-next version due to a base TI binding conversion to YAML. Rob has left it to your discretion w.r.t the bindings, so appreciate it if you can review the binding and pick up the series for 5.10. regards Suman [1] R5F v1: https://patchwork.kernel.org/cover/11456367/ [2] R5F v2: https://patchwork.kernel.org/cover/11632993/ [3] R5F v3: https://patchwork.kernel.org/cover/11679327/ [4] R5F v4: https://patchwork.kernel.org/cover/11763783/ Suman Anna (4): dt-bindings: remoteproc: Add bindings for R5F subsystem on TI K3 SoCs remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem remoteproc: k3-r5: Initialize TCM memories for ECC remoteproc: k3-r5: Add loading support for on-chip SRAM regions .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 281 drivers/remoteproc/Kconfig| 13 + drivers/remoteproc/Makefile |1 + drivers/remoteproc/ti_k3_r5_remoteproc.c | 1395 + 4 files changed, 1690 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml create mode 100644 drivers/remoteproc/ti_k3_r5_remoteproc.c -- 2.28.0
[PATCH v5 1/4] dt-bindings: remoteproc: Add bindings for R5F subsystem on TI K3 SoCs
The Texas Instruments K3 family of SoCs have one or more dual-core Arm Cortex R5F processor subsystems/clusters (R5FSS). The clusters can be split between multiple voltage domains as well. Add the device tree bindings document for these R5F subsystem devices. These R5F processors do not have an MMU, and so require fixed memory carveout regions matching the firmware image addresses. The nodes require more than one memory region, with the first memory region used for DMA allocations at runtime. The remaining memory regions are reserved and are used for the loading and running of the R5F remote processors. The R5F processors can also optionally use any internal on-chip SRAM memories either for executing code or using it as fast-access data. The added example illustrates the DT nodes for the single R5FSS device present on K3 AM65x family of SoCs. Signed-off-by: Suman Anna --- v5: Updated compatible in example to fix warnings against linux-next v4: https://patchwork.kernel.org/patch/11763805/ - Fixed whitespace indentation (added one extra space) on required list v3: https://patchwork.kernel.org/patch/11679331/ - Replaced ti,k3-sci-proc.yaml references with the new ti,k3-sci-common.yaml - Updated required list to include the three ti,sci properties v2: https://patchwork.kernel.org/patch/11632997/ v1: https://patchwork.kernel.org/patch/11456381/ .../bindings/remoteproc/ti,k3-r5f-rproc.yaml | 281 ++ 1 file changed, 281 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml new file mode 100644 index ..4069f0f5e8fa --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml @@ -0,0 +1,281 @@ +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/ti,k3-r5f-rproc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI K3 R5F processor subsystems + +maintainers: + - Suman Anna + +description: | + The TI K3 family of SoCs usually have one or more dual-core Arm Cortex R5F + processor subsystems/clusters (R5FSS). The dual core cluster can be used + either in a LockStep mode providing safety/fault tolerance features or in a + Split mode providing two individual compute cores for doubling the compute + capacity. These are used together with other processors present on the SoC + to achieve various system level goals. + + Each Dual-Core R5F sub-system is represented as a single DTS node + representing the cluster, with a pair of child DT nodes representing + the individual R5F cores. Each node has a number of required or optional + properties that enable the OS running on the host processor to perform + the device management of the remote processor and to communicate with the + remote processor. + +properties: + $nodename: +pattern: "^r5fss(@.*)?" + + compatible: +enum: + - ti,am654-r5fss + - ti,j721e-r5fss + + power-domains: +description: | + Should contain a phandle to a PM domain provider node and an args + specifier containing the R5FSS device id value. +maxItems: 1 + + "#address-cells": +const: 1 + + "#size-cells": +const: 1 + + ranges: +description: | + Standard ranges definition providing address translations for + local R5F TCM address spaces to bus addresses. + +# Optional properties: +# + + ti,cluster-mode: +$ref: /schemas/types.yaml#/definitions/uint32 +enum: [0, 1] +description: | + Configuration Mode for the Dual R5F cores within the R5F cluster. + Should be either a value of 1 (LockStep mode) or 0 (Split mode), + default is LockStep mode if omitted. + +# R5F Processor Child Nodes: +# == + +patternProperties: + "^r5f@[a-f0-9]+$": +type: object +description: | + The R5F Sub-System device node should define two R5F child nodes, each + node representing a TI instantiation of the Arm Cortex R5F core. There + are some specific integration differences for the IP like the usage of + a Region Address Translator (RAT) for translating the larger SoC bus + addresses into a 32-bit address space for the processor. + + Each R5F core has an associated 64 KB of Tightly-Coupled Memory (TCM) + internal memories split between two banks - TCMA and TCMB (further + interleaved into two banks TCMB0 and TCMB1). These memories (also called + ATCM and BTCM) provide read/write performance on par with the core's L1 + caches. Each of the TCMs can be enabled or disabled independently and + either of them can be configured to appear at that R5F's address 0x0. + + The cores do not use an MMU, but has a Region Address Translater + (RAT) modu
[PATCH v5 3/4] remoteproc: k3-r5: Initialize TCM memories for ECC
The R5F processors on K3 SoCs all have two TCMs (ATCM and BTCM) that
support 32-bit ECC. The TCMs are typically loaded with some boot-up
code to initialize the R5 MPUs to further execute code out of DDR.
The ECC for the TCMs is enabled by default on K3 SoCs due to internal
default tie-off values, but the TCM memories are not initialized on
device power up. Any read access without the corresponding TCM memory
location initialized will generate an ECC error, and any such access
from a A72 or A53 core will trigger a SError.
So, zero initialize both the TCM memories before loading any firmware
onto a R5F in remoteproc mode. Any R5F booted from U-Boot/SPL would
require a similar initialization in the bootloader. Note that both
the TCMs are initialized unconditionally as the TCM enable config bits
only manage the access and visibility from R5.
Signed-off-by: Suman Anna
Reviewed-by: Mathieu Poirier
---
v5: No changes
v4: No changes
v3: https://patchwork.kernel.org/patch/11679335/
- No code changes, picked up tags
v2: https://patchwork.kernel.org/patch/11632989/
v1: https://patchwork.kernel.org/patch/11456371/
drivers/remoteproc/ti_k3_r5_remoteproc.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 15e52ea67bbe..a6b395ab47b6 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -362,11 +362,24 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
ret = (cluster->mode == CLUSTER_MODE_LOCKSTEP) ?
k3_r5_lockstep_release(cluster) : k3_r5_split_release(core);
- if (ret)
+ if (ret) {
dev_err(dev, "unable to enable cores for TCM loading, ret =
%d\n",
ret);
+ return ret;
+ }
- return ret;
+ /*
+* Zero out both TCMs unconditionally (access from v8 Arm core is not
+* affected by ATCM & BTCM enable configuration values) so that ECC
+* can be effective on all TCM addresses.
+*/
+ dev_dbg(dev, "zeroing out ATCM memory\n");
+ memset(core->mem[0].cpu_addr, 0x00, core->mem[0].size);
+
+ dev_dbg(dev, "zeroing out BTCM memory\n");
+ memset(core->mem[1].cpu_addr, 0x00, core->mem[1].size);
+
+ return 0;
}
/*
--
2.28.0
[PATCH v5 2/4] remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem
The TI K3 family of SoCs typically have one or more dual-core Arm Cortex R5F processor clusters/subsystems (R5FSS). This R5F subsystem/cluster can be configured at boot time to be either run in a LockStep mode or in an Asymmetric Multi Processing (AMP) fashion in Split-mode. This subsystem has 64 KB each Tightly-Coupled Memory (TCM) internal memories for each core split between two banks - TCMA and TCMB (further interleaved into two banks). The subsystem does not have an MMU, but has a Region Address Translater (RAT) module that is accessible only from the R5Fs for providing translations between 32-bit CPU addresses into larger system bus addresses. Add a remoteproc driver to support this subsystem to be able to load and boot the R5F cores primarily in LockStep mode. The code also includes the base support for Split mode. Error Recovery and Power Management features are not currently supported. Loading support includes the internal TCMs and DDR. RAT support is left for a future patch, and as such the reserved memory carveout regions are all expected to be using memory regions within the first 2 GB. The R5F remote processors do not have an MMU, and so require fixed memory carveout regions matching the firmware image addresses. Support for this is provided by mandating multiple memory regions to be attached to the remoteproc device. The first memory region will be used to serve as the DMA pool for all dynamic allocations like the vrings and vring buffers. The remaining memory regions are mapped into the kernel at device probe time, and are used to provide address translations for firmware image segments without the need for any RSC_CARVEOUT entries. Any firmware image using memory outside of the supplied reserved memory carveout regions will be errored out. The R5F processors on TI K3 SoCs require a specific sequence for booting and shutting down the processors. This sequence is also dependent on the mode (LockStep or Split) the R5F cluster is configured for. The R5F cores have a Memory Protection Unit (MPU) that has a default configuration that does not allow the cores to run out of DDR out of reset. This is resolved by using the TCMs for boot-strapping code that applies the appropriate executable permissions on desired DDR memory. The loading into the TCMs requires that the resets be released first with the cores in halted state. The Power Sleep Controller (PSC) module on K3 SoCs requires that the cores be in WFI/WFE states with no active bus transactions before the cores can be put back into reset. Support for this is provided by using the newly introduced .prepare() and .unprepare() ops in the remoteproc core. The .prepare() ops is invoked before any loading, and the .unprepare() ops is invoked after the remoteproc resource cleanup. The R5F core resets are deasserted in .prepare() and asserted in .unprepare(), and the cores themselves are started and halted in .start() and .stop() ops. This ensures symmetric usage and allows the R5F cores state machine to be maintained properly between using the sysfs 'state' variable, bind/unbind and regular module load/unload flows. The subsystem is represented as a single remoteproc in LockStep mode, and as two remoteprocs in Split mode. The driver uses various TI-SCI interfaces to talk to the System Controller (DMSC) for managing configuration, power and reset management of these cores. IPC between the A53 cores and the R5 cores is supported through the virtio rpmsg stack using shared memory and OMAP Mailboxes. The AM65x SoCs typically have a single R5FSS in the MCU voltage domain. The J721E SoCs uses a slightly revised IP and typically have three R5FSSs, with one cluster present within the MCU voltage domain (MCU_R5FSS0), and the remaining two clusters present in the MAIN voltage domain (MAIN_R5FSS0 and MAIN_R5FSS1). The integration of these clusters on J721E SoC is also slightly different in that these IPs do support an actual local reset line, while they are a no-op on AM65x SoCs. Signed-off-by: Suman Anna Reviewed-by: Mathieu Poirier --- v5: No changes v4: https://patchwork.kernel.org/patch/11763795/ - Fixed error check and return code around devm_ioremap_wc in k3_r5_core_of_get_internal_memories() function v3: https://patchwork.kernel.org/patch/11679333/ - Addressed the last few minor comments from Mathieu - Removed the ti_sci_protocol.h header inclusion - Fixed the error checks on devm_reset_control_get_exclusive() - Removed the unnecessary cpu_addr check in failure path in k3_r5_reserved_mem_init() - Replaced http with http v2: https://patchwork.kernel.org/patch/11632995/ v1: https://patchwork.kernel.org/patch/11456375/ drivers/remoteproc/Kconfig | 13 + drivers/remoteproc/Makefile |1 + drivers/remoteproc/ti_k3_r5_remoteproc.c | 1303 ++ 3 files changed, 1317 insertions(+) create mode 100644 drivers/remoteproc/ti_k3_r5_remoteproc.c diff --git a/drivers/remoteproc/Kconfig b/drivers
Re: [PATCH] mfd: syscon: Don't free allocated name for regmap_config
On 9/24/20 7:39 AM, Lee Jones wrote:
> On Thu, 03 Sep 2020, Marc Zyngier wrote:
>
>> The name allocated for the regmap_config structure is freed
>> pretty early, right after the registration of the MMIO region.
>>
>> Unfortunately, that doesn't follow the life cycle that debugfs
>> expects, as it can access the name field long after the free
>> has occured.
>>
>> Move the free on the error path, and keep it forever otherwise.
>>
>> Fixes: e15d7f2b81d2 ("mfd: syscon: Use a unique name with regmap_config")
>> Signed-off-by: Marc Zyngier
>> ---
>> drivers/mfd/syscon.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Fixed the spelling mistake and applied, thanks.
>
Marc,
Can you recheck the behavior once on the latest master to see if this patch is
still needed? I see a new patch within the regmap core that is dealing with the
delayed debugfs registration name. This follows the similar design logic I
suggested on your initial submission [1].
Please see commit 94cc89eb8fa5 ("regmap: debugfs: Fix handling of name string
for debugfs init delays") in mainline.
Lee,
I haven't seen this patch in -next yet, so maybe worthwhile to hold it a little
longer.
regards
Suman
[1] https://patchwork.kernel.org/comment/23575471/
Re: [PATCH v4 0/4] arm64: dts: ti: k3-j7200: add dma and mcu cpsw
On 9/23/20 5:09 PM, Grygorii Strashko wrote: > Hi All, > > This series adds DT nodes for TI J7200 SoC > - Ringacc and UDMA nodes for Main and MCU NAVSS, which are compatible > with J721E Soc, to enable DMA support > - MCU CPSW2g DT nodes to enable networking and board data > > Changes in v4: > - fixed comments from Suman Anna > > Changes in v3: > - rebase > - updated dependencies > - added tested-by > > Changes in v2: > - fixed DT build warnings (Nishanth Menon) > > v3: https://lore.kernel.org/patchwork/cover/1308044/ > v2: https://lore.kernel.org/patchwork/cover/1302290/ > v1: https://lore.kernel.org/patchwork/cover/1301067/ > > Grygorii Strashko (3): > arm64: dts: ti: k3-j7200-main: add main navss cpts node > arm64: dts: ti: k3-j7200-mcu: add mcu cpsw nuss node > arm64: dts: ti: k3-j7200-common-proc-board: add mcu cpsw nuss pinmux > and phy defs > > Peter Ujfalusi (1): > arm64: dts: ti: k3-j7200: add DMA support Thank you for fixing my comments. For the series, Reviewed-by: Suman Anna regards Suman > > .../dts/ti/k3-j7200-common-proc-board.dts | 45 +++ > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 48 +++ > .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 118 ++ > 3 files changed, 211 insertions(+) >
Re: [PATCH v4 0/4] TI K3 R5F remoteproc support
Hi Rob, On 9/22/20 2:47 PM, Rob Herring wrote: > On Tue, Sep 08, 2020 at 12:45:52PM -0500, Suman Anna wrote: >> Hi All, >> >> The following is v4 of the TI K3 R5F remoteproc driver series supporting all >> the R5F processor clusters/subsystems on TI AM65x and J721E SoCs. Please >> see the v1 cover-letter [1] for the features supported on these R5F >> processors. >> >> This series is a rebased version on top of the latest v5.9-rc baseline and >> includes very minor fixes w.r.t v3. The previous K3 DSP dependencies are now >> available in mainline kernel. Please see the individual patches for the delta >> differences (Only patches 1 and 2 updated). >> >> Bjorn, >> This series is only waiting on bindings ack and the conclusion on the >> bindings >> discussion from v2 [4] on which I haven't seen any forward progress on this >> despite all the clarifications. I do not expect any changes even w.r.t >> System DT, >> and we can't really have a common binding between TI and Xilinx R5Fs. > First of all, thank you for reviewing this and your response. > Why not? I'm pretty sure lockstep or not is a thing for both and TCMs > seem to be a common thing. The cluster mode is a common theme, and if you have a preference for a common property-name, both I and Ben can use that. The values though might vary between different vendor SoCs. I have given out all the differences and reasons on a v2 thread, the SoC and clock and reset integration aspects make it look very different. Please see the discussion here, https://patchwork.kernel.org/comment/23560321/ There was only one open comment/question I had regarding Core identification w.r.t my binding. Do you prefer a node-name index difference or a separate core-id/cpu-id property identifying which is Core0 and Core1. > > And I don't really think System DT will not impact it. Though it's not > well enough defined to say either way IMO. Yeah agreed. But the current architecture in System DT does allow you to add plugins to generate the proper compliant dts node. In anycase, I doubt TI will ever be using it in general, because we do not have a concept of DT on our firmwares. I have given all these inputs again on v2, but haven't seen any responses on it. So, I do appreciate your feedback. > > But if Bjorn wants to take this, fine. I'm not acking it though nor > worrying about it for any compatibility with system DT. Any specific reasons? For the most part, I am using all standard properties. regards Suman
Re: [PATCH v3 1/4] arm64: dts: ti: k3-j7200: add DMA support
Hi Grygorii,
On 9/18/20 10:38 AM, Grygorii Strashko wrote:
> From: Peter Ujfalusi
>
> Add the intr, inta, ringacc and udmap nodes for main and mcu NAVSS.
Need to update the changelog, intr and inta are not part of this revised series.
>
> Signed-off-by: Peter Ujfalusi
> Signed-off-by: Grygorii Strashko
> Tested-by: Kishon Vijay Abraham I
> ---
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 36 +++
> .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 44 +++
> 2 files changed, 80 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> index 3df49577b06a..c5015df58cd4 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
> @@ -93,6 +93,42 @@
> interrupt-names = "rx_011";
> interrupts = ;
> };
> +
> + main_ringacc: ringacc@3c00 {
> + compatible = "ti,am654-navss-ringacc";
> + reg = <0x0 0x3c00 0x0 0x40>,
> + <0x0 0x3800 0x0 0x40>,
> + <0x0 0x3112 0x0 0x100>,
> + <0x0 0x3300 0x0 0x4>;
> + reg-names = "rt", "fifos", "proxy_gcfg", "proxy_target";
> + ti,num-rings = <1024>;
> + ti,sci-rm-range-gp-rings = <0x1>; /* GP ring range */
> + ti,sci = <>;
> + ti,sci-dev-id = <211>;
> + msi-parent = <_udmass_inta>;
> + };
> +
> + main_udmap: dma-controller@3115 {
> + compatible = "ti,j721e-navss-main-udmap";
> + reg = <0x0 0x3115 0x0 0x100>,
> + <0x0 0x3400 0x0 0x10>,
> + <0x0 0x3500 0x0 0x10>;
> + reg-names = "gcfg", "rchanrt", "tchanrt";
> + msi-parent = <_udmass_inta>;
> + #dma-cells = <1>;
> +
> + ti,sci = <>;
> + ti,sci-dev-id = <212>;
> + ti,ringacc = <_ringacc>;
> +
> + ti,sci-rm-range-tchan = <0x0d>, /* TX_CHAN */
> + <0x0f>, /* TX_HCHAN */
> + <0x10>; /* TX_UHCHAN */
> + ti,sci-rm-range-rchan = <0x0a>, /* RX_CHAN */
> + <0x0b>, /* RX_HCHAN */
> + <0x0c>; /* RX_UHCHAN */
> + ti,sci-rm-range-rflow = <0x00>; /* GP RFLOW */
> + };
> };
>
> main_pmx0: pinctrl@11c000 {
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> index ec2745e0768e..7ecdfdb46436 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -92,4 +92,48 @@
> ti,sci-dev-id = <137>;
> ti,interrupt-ranges = <16 960 16>;
> };
> +
> + cbass_mcu_navss: navss@2838 {
> + compatible = "simple-mfd";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + dma-coherent;
> + dma-ranges;
> + ti,sci-dev-id = <232>;
> +
> + mcu_ringacc: ringacc@2b80 {
> + compatible = "ti,am654-navss-ringacc";
> + reg = <0x0 0x2b80 0x0 0x40>,
> + <0x0 0x2b00 0x0 0x40>,
> + <0x0 0x2859 0x0 0x100>,
> + <0x0 0x2a50 0x0 0x4>;
Please use style consistent with existing dts nodes, not a fan of mismatched
usage. We are using 0x00 for the higher 32-bit addresses and sizes. Comment
applies to all the nodes and all patches in the series.
regards
Suman
> + reg-names = "rt", "fifos", "proxy_gcfg", "proxy_target";
> + ti,num-rings = <286>;
> + ti,sci-rm-range-gp-rings = <0x1>; /* GP ring range */
> + ti,sci = <>;
> + ti,sci-dev-id = <235>;
> + msi-parent = <_udmass_inta>;
> + };
> +
> + mcu_udmap: dma-controller@285c {
> + compatible = "ti,j721e-navss-mcu-udmap";
> + reg = <0x0 0x285c 0x0 0x100>,
> + <0x0 0x2a80 0x0 0x4>,
> + <0x0 0x2aa0 0x0 0x4>;
> + reg-names = "gcfg", "rchanrt", "tchanrt";
> + msi-parent = <_udmass_inta>;
> + #dma-cells = <1>;
> +
> + ti,sci = <>;
> + ti,sci-dev-id =
Re: [PATCH v3 3/4] arm64: dts: ti: k3-j7200-mcu: add mcu cpsw nuss node
On 9/18/20 10:38 AM, Grygorii Strashko wrote:
> Add DT node for The TI j7200 MCU SoC Gigabit Ethernet two ports Switch
> subsystem (MCU CPSW NUSS).
nit, %s/j7200/J7200/ on this patch and the next.
regards
Suman
>
> Signed-off-by: Grygorii Strashko
> Tested-by: Kishon Vijay Abraham I
> ---
> .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 74 +++
> 1 file changed, 74 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> index 7ecdfdb46436..a994276a8b3d 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -34,6 +34,20 @@
> };
> };
>
> + mcu_conf: syscon@40f0 {
> + compatible = "syscon", "simple-mfd";
> + reg = <0x0 0x40f0 0x0 0x2>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x0 0x40f0 0x2>;
> +
> + phy_gmii_sel: phy@4040 {
> + compatible = "ti,am654-phy-gmii-sel";
> + reg = <0x4040 0x4>;
> + #phy-cells = <1>;
> + };
> + };
> +
> chipid@4314 {
> compatible = "ti,am654-chipid";
> reg = <0x00 0x4314 0x00 0x4>;
> @@ -136,4 +150,64 @@
> ti,sci-rm-range-rflow = <0x00>; /* GP RFLOW */
> };
> };
> +
> + mcu_cpsw: ethernet@4600 {
> + compatible = "ti,j721e-cpsw-nuss";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + reg = <0x0 0x4600 0x0 0x20>;
> + reg-names = "cpsw_nuss";
> + ranges = <0x0 0x0 0x0 0x4600 0x0 0x20>;
> + dma-coherent;
> + clocks = <_clks 18 21>;
> + clock-names = "fck";
> + power-domains = <_pds 18 TI_SCI_PD_EXCLUSIVE>;
> +
> + dmas = <_udmap 0xf000>,
> +<_udmap 0xf001>,
> +<_udmap 0xf002>,
> +<_udmap 0xf003>,
> +<_udmap 0xf004>,
> +<_udmap 0xf005>,
> +<_udmap 0xf006>,
> +<_udmap 0xf007>,
> +<_udmap 0x7000>;
> + dma-names = "tx0", "tx1", "tx2", "tx3",
> + "tx4", "tx5", "tx6", "tx7",
> + "rx";
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpsw_port1: port@1 {
> + reg = <1>;
> + ti,mac-only;
> + label = "port1";
> + ti,syscon-efuse = <_conf 0x200>;
> + phys = <_gmii_sel 1>;
> + };
> + };
> +
> + davinci_mdio: mdio@f00 {
> + compatible = "ti,cpsw-mdio","ti,davinci_mdio";
> + reg = <0x0 0xf00 0x0 0x100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + clocks = <_clks 18 21>;
> + clock-names = "fck";
> + bus_freq = <100>;
> + };
> +
> + cpts@3d000 {
> + compatible = "ti,am65-cpts";
> + reg = <0x0 0x3d000 0x0 0x400>;
> + clocks = <_clks 18 2>;
> + clock-names = "cpts";
> + interrupts-extended = < GIC_SPI 858
> IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "cpts";
> + ti,cpts-ext-ts-inputs = <4>;
> + ti,cpts-periodic-outputs = <2>;
> + };
> + };
> };
>
Re: [PATCH] dt-bindings: soc: ti: ti,pruss: fix schema ID
On 9/17/20 3:35 AM, Krzysztof Kozlowski wrote:
> On Thu, 17 Sep 2020 at 10:32, Grzegorz Jaszczyk
> wrote:
>>
>> On Thu, 17 Sep 2020 at 09:05, Krzysztof Kozlowski wrote:
>>>
>>> Add missing '#' to fix schema errors:
>>>
>>> $id: 'http://devicetree.org/schemas/soc/ti/ti,pruss.yaml' does not match
>>> 'http://devicetree.org/schemas/.*\\.yaml#'
>>> $schema: 'http://devicetree.org/meta-schemas/core.yaml' is not one of
>>> ['http://devicetree.org/meta-schemas/core.yaml#',
>>> 'http://devicetree.org/meta-schemas/base.yaml#']
>>> Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml: ignoring, error
>>> in schema: $id
>>>
Thanks for the fix, Krzysztof.
Fixes: bd691ce0ba9d ("dt-bindings: soc: ti: Add TI PRUSS bindings")
>>> Signed-off-by: Krzysztof Kozlowski
With that,
Acked-by: Suman Anna
Santosh,
Can you please pick this up before you send your pull request to arm-soc
maintainers?
You may also want to check your tooling to see what happened.
regards
Suman
>>> ---
>>> Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> index cf7dc83f724f..037c51b2f972 100644
>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> @@ -1,8 +1,8 @@
>>> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> %YAML 1.2
>>> ---
>>> -$id: http://devicetree.org/schemas/soc/ti/ti,pruss.yaml
>>> -$schema: http://devicetree.org/meta-schemas/core.yaml
>>> +$id: http://devicetree.org/schemas/soc/ti/ti,pruss.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> I've double checked and "#" was present in the original patch sent and
>> ack for upstream: https://patchwork.kernel.org/patch/11729649/
>> It seems like something got wrong on linux-next but this is the only
>> diff between original patch and one found in linux-next. Thank you for
>> taking care of it.
>
> Indeed that's weird. It must get lost when applying...
>
> Best regards,
> Krzysztof
>
Re: [v4,3/4] reset-controller: ti: introduce a new reset handler
On 9/11/20 9:26 AM, Philipp Zabel wrote:
> Hi Crystal,
>
> On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote:
> [...]
>> Should I add the SoC-specific data as follows?
>> This may also modify the ti original code, is it OK?
>>
>> + data->reset_data = of_device_get_match_data(>dev);
>> +
>> + list = of_get_property(np, data->reset_data->reset_bits, );
>>
>> +static const struct common_reset_data ti_reset_data = {
>> + .reset_op_available = false,
>> + .reset_bits = "ti, reset-bits",
> ^
> That space doesn't belong there.
>
>> +};
>> +
>> +static const struct common_reset_data mediatek_reset_data = {
>> + .reset_op_available = true,
>> + .reset_bits = "mediatek, reset-bits",
>> +};
>
> I understand Robs comments as meaning "ti,reset-bits" should have been
> called "reset-bits" in the first place, and you shouldn't repeat adding
> the vendor prefix, as that is implied by the compatible. So this should
> probably be just "reset-bits".
Hmm, not sure about that. I think Rob wants the reset data itself to be added in
the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c).
regards
Suman
>
> Otherwise this looks like it should work.
>
> regards
> Philipp
>
Re: [v4,3/4] reset-controller: ti: introduce a new reset handler
On 9/10/20 9:42 PM, Crystal Guo wrote:
> On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote:
>> On 9/8/20 9:57 PM, Crystal Guo wrote:
>>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote:
>>>> Hi Crystal,
>>>>
>>>> On 8/16/20 10:03 PM, Crystal Guo wrote:
>>>>> Introduce ti_syscon_reset() to integrate assert and deassert together.
>>>>> If some modules need do serialized assert and deassert operations
>>>>> to reset itself, reset_control_reset can be called for convenience.
>>>>
>>>> There are multiple changes in this same patch. I think you should split
>>>> this
>>>> functionality away from the change for the regmap_update_bits() to
>>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4.
>>>>
>>>
>>> Thanks for your suggestion.
>>> I will split this patch in the next version.
>>>
>>>>>
>>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together
>>>>> by 'reset' method. MTK Socs also need this method to perform reset.
>>>>>
>>>>> Signed-off-by: Crystal Guo
>>>>> ---
>>>>> drivers/reset/reset-ti-syscon.c | 26 --
>>>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/reset/reset-ti-syscon.c
>>>>> b/drivers/reset/reset-ti-syscon.c
>>>>> index a2635c21db7f..08289342f9af 100644
>>>>> --- a/drivers/reset/reset-ti-syscon.c
>>>>> +++ b/drivers/reset/reset-ti-syscon.c
>>>>> @@ -15,6 +15,7 @@
>>>>> * GNU General Public License for more details.
>>>>> */
>>>>>
>>>>> +#include
>>>>> #include
>>>>> #include
>>>>> #include
>>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data {
>>>>> struct regmap *regmap;
>>>>> struct ti_syscon_reset_control *controls;
>>>>> unsigned int nr_controls;
>>>>> + unsigned int reset_duration_us;
>>>>> };
>>>>>
>>>>> #define to_ti_syscon_reset_data(rcdev) \
>>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct
>>>>> reset_controller_dev *rcdev,
>>>>> mask = BIT(control->assert_bit);
>>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0;
>>>>>
>>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask,
>>>>> value);
>>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask,
>>>>> value);
>>>>> }
>>>>>
>>>>> /**
>>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct
>>>>> reset_controller_dev *rcdev,
>>>>> mask = BIT(control->deassert_bit);
>>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0;
>>>>>
>>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask,
>>>>> value);
>>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask,
>>>>> value);
>>>>> }
>>>>>
>>>>> /**
>>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct
>>>>> reset_controller_dev *rcdev,
>>>>> !(control->flags & STATUS_SET);
>>>>> }
>>>>>
>>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev,
>>>>> + unsigned long id)
>>>>> +{
>>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev);
>>>>> + int ret;
>>>>> +
>>>>> + ret = ti_syscon_reset_assert(rcdev, id);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + if (data->reset_duration_us)
>>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us *
>>>>> 2);
>>>>> +
>>>>> + return ti_syscon_reset_deassert(rcdev, id);
>>>>
>>>> I echo Philipp's comments [1] from your original v1 series about this. We
>>>> don't
>>>> need a property to distinguish this, but you could add a flag using match
>>>>
Re: [v4,3/4] reset-controller: ti: introduce a new reset handler
On 9/8/20 9:57 PM, Crystal Guo wrote:
> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote:
>> Hi Crystal,
>>
>> On 8/16/20 10:03 PM, Crystal Guo wrote:
>>> Introduce ti_syscon_reset() to integrate assert and deassert together.
>>> If some modules need do serialized assert and deassert operations
>>> to reset itself, reset_control_reset can be called for convenience.
>>
>> There are multiple changes in this same patch. I think you should split this
>> functionality away from the change for the regmap_update_bits() to
>> regmap_write_bits(), similar to what you have done in your v2 Patch 4.
>>
>
> Thanks for your suggestion.
> I will split this patch in the next version.
>
>>>
>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together
>>> by 'reset' method. MTK Socs also need this method to perform reset.
>>>
>>> Signed-off-by: Crystal Guo
>>> ---
>>> drivers/reset/reset-ti-syscon.c | 26 --
>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/reset/reset-ti-syscon.c
>>> b/drivers/reset/reset-ti-syscon.c
>>> index a2635c21db7f..08289342f9af 100644
>>> --- a/drivers/reset/reset-ti-syscon.c
>>> +++ b/drivers/reset/reset-ti-syscon.c
>>> @@ -15,6 +15,7 @@
>>> * GNU General Public License for more details.
>>> */
>>>
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data {
>>> struct regmap *regmap;
>>> struct ti_syscon_reset_control *controls;
>>> unsigned int nr_controls;
>>> + unsigned int reset_duration_us;
>>> };
>>>
>>> #define to_ti_syscon_reset_data(rcdev) \
>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct
>>> reset_controller_dev *rcdev,
>>> mask = BIT(control->assert_bit);
>>> value = (control->flags & ASSERT_SET) ? mask : 0x0;
>>>
>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask,
>>> value);
>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask,
>>> value);
>>> }
>>>
>>> /**
>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct
>>> reset_controller_dev *rcdev,
>>> mask = BIT(control->deassert_bit);
>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0;
>>>
>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask,
>>> value);
>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask,
>>> value);
>>> }
>>>
>>> /**
>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct
>>> reset_controller_dev *rcdev,
>>> !(control->flags & STATUS_SET);
>>> }
>>>
>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev,
>>> + unsigned long id)
>>> +{
>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev);
>>> + int ret;
>>> +
>>> + ret = ti_syscon_reset_assert(rcdev, id);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (data->reset_duration_us)
>>> + usleep_range(data->reset_duration_us, data->reset_duration_us *
>>> 2);
>>> +
>>> + return ti_syscon_reset_deassert(rcdev, id);
>>
>> I echo Philipp's comments [1] from your original v1 series about this. We
>> don't
>> need a property to distinguish this, but you could add a flag using match
>> data
>> and Mediatek compatible, and use that within this function, or optionally set
>> this ops based on compatible (whatever is preferred by Philipp).
>>
>> regards
>> Suman
>>
>> [1] https://patchwork.kernel.org/comment/23519193/
>>
> Hi Suman, Philipp
>
> Which method would you recommend more?
> 1. like v2 patch, but assign the flag "data->assert_deassert_together"
> directly (maybe rename "assert_deassert_together" to
> "reset_op_available")
>
> 2. use Mediatek compatible to decide the reset handler available or not.
I would go with this option. Anyway, I think you might have to add the reset SoC
data as well, based on Rob's comment on the binding.
regards
Suman
>
> Thanks
> Crystal
>
>>> +}
>>> +
>>> static const struct reset_control_ops ti_syscon_reset_ops = {
>>> .assert = ti_syscon_reset_assert,
>>> .deassert = ti_syscon_reset_deassert,
>>> + .reset = ti_syscon_reset,
>>> .status = ti_syscon_reset_status,
>>> };
>>>
>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device
>>> *pdev)
>>> controls[i].flags = be32_to_cpup(list++);
>>> }
>>>
>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us",
>>> + >reset_duration_us);
>>> +
>>> data->rcdev.ops = _syscon_reset_ops;
>>> data->rcdev.owner = THIS_MODULE;
>>> data->rcdev.of_node = np;
>>>
>>
>
Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible
On 9/8/20 1:49 PM, Rob Herring wrote: > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna wrote: >> >> Hi Rob, >> >> On 8/26/20 6:09 AM, Crystal Guo wrote: >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>>>> The TI syscon reset controller provides a common reset management, >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>>>> which denotes to use ti reset-controller driver directly. >>>>> >>>>> Signed-off-by: Crystal Guo >>>>> --- >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> index ab041032339b..5a0e9365b51b 100644 >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> @@ -25,6 +25,7 @@ Required properties: >>>>> "ti,k2l-pscrst" >>>>> "ti,k2hk-pscrst" >>>>> "ti,syscon-reset" >>>>> + "mediatek,infra-reset", "ti,syscon-reset" >>>> >>>> You need your own binding doc. If you can use the same driver then fine, >>>> but that's a separate issue. There's also reset-simple driver if you >>>> have just array of 32-bit registers with a bit per reset. >>>> >>>> Don't repeat 'ti,reset-bits' either. >>> >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek >>> reuse the TI reset controller directly? >> >> Hmm, how do you envision not repeating the same bits in a separate binding? > > I mean 'ti,reset-bits' isn't really something that should have been in > DT in the first place, but rather implied by the compatible string. Ok, should I be deprecating this and move this data to driver then? I am assuming that is how you are envisioning the new Mediatek binding to be atleast. regards Suman > >> Does it help if I convert this to YAML first without a ti, prefix in the >> file name? > > No, I don't think this should be a shared binding. The driver may be > able to be shared, but that's independent from the binding. > > Rob >
Re: [PATCH next v2 0/3] soc: ti: k3: ringacc: add am65x sr2.0 support
Hi Santosh,
On 8/31/20 1:34 PM, santosh.shilim...@oracle.com wrote:
> On 8/29/20 11:41 AM, Grygorii Strashko wrote:
>> Hi Santosh,
>>
>> I've rebased on top of linux-next and identified merge conflict of patch 3
>> with commit 6da45875fa17 ("arm64: dts: k3-am65: Update the RM resource
>> types")
>> in -next.
>>
>> ---
>> This series adds support for the TI AM65x SR2.0 SoC Ringacc which has fixed
>> errata i2023 "RINGACC, UDMA: RINGACC and UDMA Ring State Interoperability
>> Issue after Channel Teardown". This errata also fixed for J271E SoC.
>> The SOC bus chipinfo data is used to identify the SoC and configure
>> i2023 errata W/A.
>>
>> This changes made "ti,dma-ring-reset-quirk" DT property obsolete, so it's
>> removed.
>>
>> Changes in v2:
>> - no functional changes
>> - rebased on top of linux-next
>> - added ask from Rob Herring
>>
> Thanks. Can you please followup DT acks for PRUSS series so that I can
> apply PRUSS + $subject series.
PRUSS dt binding is acked now, so can you pick up the PRUSS v2 series for 5.10
merge window.
regards,
Suman
>
>> v1:
>> https://urldefense.com/v3/__https://lore.kernel.org/patchwork/cover/1284233/__;!!GqivPVa7Brio!PCmZ-nZZ6YQak-0T43bPZYI0gHsYL9k6N7S2gZEFbIr8BRKtv2BK01VejQzlBPBBgcfvCQ$
>>
>> Grygorii Strashko (3):
>> soc: ti: k3: ringacc: add am65x sr2.0 support
>> bindings: soc: ti: soc: ringacc: remove ti,dma-ring-reset-quirk
>> arm64: dts: ti: k3-am65: ringacc: drop ti,dma-ring-reset-quirk
>>
>> .../bindings/soc/ti/k3-ringacc.yaml | 6
>> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 -
>> arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi | 1 -
>> drivers/soc/ti/k3-ringacc.c | 33 +--
>> 4 files changed, 30 insertions(+), 11 deletions(-)
>>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
