[PATCH v3 00/11] Add support for MT8195 SCP 2nd core

[PATCH v3 00/11] Add support for MT8195 SCP 2nd core

[Tinghan Shen]

The mediatek remoteproc driver currently only allows bringing up a 
single core SCP, e.g. MT8183. It also only bringing up the 1st 
core in SoCs with a dual-core SCP, e.g. MT8195. This series support 
to bring-up the 2nd core of the dual-core SCP.

v2 -> v3:
1. change the representation of dual-core SCP in dts file and update SCP yaml
2. rewrite SCP driver to reflect the change of dts node
3. add SCP core 1 node to mt8195.dtsi
4. remove redundant call of rproc_boot for SCP
5. refine IPI error message

v1 -> v2:
1. update dt-binding property description
2. remove kconfig for scp dual driver
3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c

Tinghan Shen (11):
  dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
  dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
  arm64: dts: mt8195: Add SCP core 1 node
  remoteproc: mediatek: Remove redundant rproc_boot
  remoteproc: mediatek: Add SCP core 1 register definitions
  remoteproc: mediatek: Add MT8195 SCP core 1 operations
  remoteproc: mediatek: Probe MT8195 SCP core 1
  remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
  remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
  remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
  remoteproc: mediatek: Refine ipi handler error message

 .../bindings/remoteproc/mtk,scp.yaml          | 132 ++++++++--
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |   2 +-
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      |  14 +-
 .../mediatek/vcodec/mtk_vcodec_fw_scp.c       |   2 +-
 drivers/remoteproc/mtk_common.h               |  35 +++
 drivers/remoteproc/mtk_scp.c                  | 241 +++++++++++++++++-
 include/linux/remoteproc/mtk_scp.h            |   1 +
 7 files changed, 397 insertions(+), 30 deletions(-)

-- 
2.18.0

[PATCH v3 01/11] dt-bindings: remoteproc: mediatek: Give the subnode a persistent name

[Tinghan Shen]

The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
Give it a clear persistent node name to simplify scp yaml.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../bindings/remoteproc/mtk,scp.yaml          | 35 ++++++++++---------
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |  2 +-
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index 7e091eaffc18..786bed897916 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -58,6 +58,23 @@ properties:
   memory-region:
     maxItems: 1
 
+  cros-ec-rpmsg:
+    type: object
+    description:
+      This subnode represents the rpmsg device. The names of the devices
+      are not important. The properties of this node are defined by the
+      individual bindings for the rpmsg devices.
+
+    properties:
+      mediatek,rpmsg-name:
+        $ref: /schemas/types.yaml#/definitions/string-array
+        description:
+          Contains the name for the rpmsg device. Used to match
+          the subnode to rpmsg device announced by SCP.
+
+    required:
+      - mediatek,rpmsg-name
+
 required:
   - compatible
   - reg
@@ -89,21 +106,7 @@ allOf:
         reg-names:
           maxItems: 2
 
-additionalProperties:
-  type: object
-  description:
-    Subnodes of the SCP represent rpmsg devices. The names of the devices
-    are not important. The properties of these nodes are defined by the
-    individual bindings for the rpmsg devices.
-  properties:
-    mediatek,rpmsg-name:
-      $ref: /schemas/types.yaml#/definitions/string-array
-      description:
-        Contains the name for the rpmsg device. Used to match
-        the subnode to rpmsg device announced by SCP.
-
-  required:
-    - mediatek,rpmsg-name
+additionalProperties: false
 
 examples:
   - |
@@ -118,7 +121,7 @@ examples:
         clocks = <&infracfg CLK_INFRA_SCPSYS>;
         clock-names = "main";
 
-        cros_ec {
+        cros-ec-rpmsg {
             mediatek,rpmsg-name = "cros-ec-rpmsg";
         };
     };
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index b4b86bb1f1a7..693ad5f2a82e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -816,7 +816,7 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&scp_pins>;
 
-	cros_ec {
+	cros-ec-rpmsg {
 		compatible = "google,cros-ec-rpmsg";
 		mediatek,rpmsg-name = "cros-ec-rpmsg";
 	};
-- 
2.18.0

Re: [PATCH v3 01/11] dt-bindings: remoteproc: mediatek: Give the subnode a persistent name

[Peng Fan]

On 9/27/2022 10:55 AM, Tinghan Shen wrote:
> The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
> Give it a clear persistent node name to simplify scp yaml.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   .../bindings/remoteproc/mtk,scp.yaml          | 35 ++++++++++---------
>   .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |  2 +-
>   2 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 7e091eaffc18..786bed897916 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -58,6 +58,23 @@ properties:
>     memory-region:
>       maxItems: 1
>   
> +  cros-ec-rpmsg:
> +    type: object
> +    description:
> +      This subnode represents the rpmsg device. The names of the devices
> +      are not important. The properties of this node are defined by the
> +      individual bindings for the rpmsg devices.
> +
> +    properties:
> +      mediatek,rpmsg-name:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        description:
> +          Contains the name for the rpmsg device. Used to match
> +          the subnode to rpmsg device announced by SCP.
> +
> +    required:
> +      - mediatek,rpmsg-name
> +
>   required:
>     - compatible
>     - reg
> @@ -89,21 +106,7 @@ allOf:
>           reg-names:
>             maxItems: 2
>   
> -additionalProperties:
> -  type: object
> -  description:
> -    Subnodes of the SCP represent rpmsg devices. The names of the devices
> -    are not important. The properties of these nodes are defined by the
> -    individual bindings for the rpmsg devices.
> -  properties:
> -    mediatek,rpmsg-name:
> -      $ref: /schemas/types.yaml#/definitions/string-array
> -      description:
> -        Contains the name for the rpmsg device. Used to match
> -        the subnode to rpmsg device announced by SCP.
> -
> -  required:
> -    - mediatek,rpmsg-name
> +additionalProperties: false
>   
>   examples:
>     - |
> @@ -118,7 +121,7 @@ examples:
>           clocks = <&infracfg CLK_INFRA_SCPSYS>;
>           clock-names = "main";
>   
> -        cros_ec {
> +        cros-ec-rpmsg {
>               mediatek,rpmsg-name = "cros-ec-rpmsg";
>           };
>       };
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index b4b86bb1f1a7..693ad5f2a82e 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi

DTS changes should be in separate patch.

Regards,
Peng.

> @@ -816,7 +816,7 @@
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&scp_pins>;
>   
> -	cros_ec {
> +	cros-ec-rpmsg {
>   		compatible = "google,cros-ec-rpmsg";
>   		mediatek,rpmsg-name = "cros-ec-rpmsg";
>   	};

Re: [PATCH v3 01/11] dt-bindings: remoteproc: mediatek: Give the subnode a persistent name

[Krzysztof Kozlowski]

On 27/09/2022 04:55, Tinghan Shen wrote:
> The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
> Give it a clear persistent node name to simplify scp yaml.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../bindings/remoteproc/mtk,scp.yaml          | 35 ++++++++++---------
>  .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |  2 +-
>  2 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 7e091eaffc18..786bed897916 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -58,6 +58,23 @@ properties:
>    memory-region:
>      maxItems: 1
>  
> +  cros-ec-rpmsg:
> +    type: object

additionalProperties: false on this level

> +    description:
> +      This subnode represents the rpmsg device. The names of the devices

What are the devices? You wrote that it is one device, not devices.

> +      are not important. The properties of this node are defined by the
> +      individual bindings for the rpmsg devices.

??? No, you need to define the properties of the node, e.g. by a ref.

> +
> +    properties:
> +      mediatek,rpmsg-name:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        description:
> +          Contains the name for the rpmsg device. Used to match
> +          the subnode to rpmsg device announced by SCP.

maxItems... but is it really a string-array?

> +
> +    required:
> +      - mediatek,rpmsg-name
> +
>  required:
>    - compatible
>    - reg
> @@ -89,21 +106,7 @@ allOf:
>          reg-names:
>            maxItems: 2
>  

Best regards,
Krzysztof

[PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[Tinghan Shen]

The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file
to describe the 2nd core as a subnode of the boot core.

The configuration register is shared by MT8195 SCP core 0
and core 1. The core 1 can retrieve the information of configuration
registers from parent node.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../bindings/remoteproc/mtk,scp.yaml          | 97 ++++++++++++++++++-
 1 file changed, 92 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index 786bed897916..c012265be4eb 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -75,6 +75,83 @@ properties:
     required:
       - mediatek,rpmsg-name
 
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+  ranges: true
+
+patternProperties:
+  "^scp-c[0-9]+@[a-f0-9]+$":
+    type: object
+    description:
+      The MediaTek SCP integrated to SoC might be a multi-core version.
+      The other cores are represented as child nodes of the boot core.
+      There are some integration differences for the IP like the usage of
+      address translator for translating SoC bus addresses into address space
+      for the processor.
+
+      Each SCP core has own cache memory. The SRAM and L1TCM are shared by
+      cores. The power of cache, SRAM and L1TCM power should be enabled
+      before booting SCP cores. The size of cache, SRAM, and L1TCM are varied
+      on differnt SoCs.
+
+      The SCP cores do not use an MMU, but has a set of registers to
+      control the translations between 32-bit CPU addresses into system bus
+      addresses. Cache and memory access settings are provided through a
+      Memory Protection Unit (MPU), programmable only from the SCP.
+
+    properties:
+      compatible:
+        enum:
+          - mediatek,mt8195-scp-core
+
+      reg:
+        description: The base address and size of SRAM.
+        maxItems: 1
+
+      reg-names:
+        const: sram
+
+      interrupts:
+        maxItems: 1
+
+      firmware-name:
+        $ref: /schemas/types.yaml#/definitions/string
+        description:
+          If present, name (or relative path) of the file within the
+          firmware search path containing the firmware image used when
+          initializing sub cores of multi-core SCP.
+
+      memory-region:
+        maxItems: 1
+
+      cros-ec-rpmsg:
+        type: object
+        description:
+          This subnode represents the rpmsg device. The namesof the devices
+          are not important. The properties of this node are defined by the
+          individual bindings for the rpmsg devices.
+
+        properties:
+          mediatek,rpmsg-name:
+            $ref: /schemas/types.yaml#/definitions/string-array
+            description:
+              Contains the name for the rpmsg device. Used to match
+              the subnode to rpmsg device announced by SCP.
+
+        required:
+          - mediatek,rpmsg-name
+
+    required:
+      - compatible
+      - reg
+      - reg-names
+
+    additionalProperties: false
+
 required:
   - compatible
   - reg
@@ -110,16 +187,26 @@ additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/clock/mt8192-clk.h>
-
     scp@10500000 {
-        compatible = "mediatek,mt8192-scp";
+        compatible = "mediatek,mt8195-scp";
         reg = <0x10500000 0x80000>,
               <0x10700000 0x8000>,
               <0x10720000 0xe0000>;
         reg-names = "sram", "cfg", "l1tcm";
-        clocks = <&infracfg CLK_INFRA_SCPSYS>;
-        clock-names = "main";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0x105a0000 0x105a0000 0x20000>;
+
+        scp-c1@105a0000 {
+                compatible = "mediatek,mt8195-scp-core";
+                reg = <0x105a0000 0x20000>;
+                reg-names = "sram";
+
+                cros-ec-rpmsg {
+                    mediatek,rpmsg-name = "cros-ec-rpmsg";
+                };
+        };
 
         cros-ec-rpmsg {
             mediatek,rpmsg-name = "cros-ec-rpmsg";
-- 
2.18.0

Re: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[Peng Fan]

On 9/27/2022 10:55 AM, Tinghan Shen wrote:
> The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file
> to describe the 2nd core as a subnode of the boot core.
> 
> The configuration register is shared by MT8195 SCP core 0
> and core 1. The core 1 can retrieve the information of configuration
> registers from parent node.

I think the commit message would not convince people you put
scp 1 as subnode of scp 0.

Regards,
Peng.

> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   .../bindings/remoteproc/mtk,scp.yaml          | 97 ++++++++++++++++++-
>   1 file changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 786bed897916..c012265be4eb 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -75,6 +75,83 @@ properties:
>       required:
>         - mediatek,rpmsg-name
>   
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^scp-c[0-9]+@[a-f0-9]+$":
> +    type: object
> +    description:
> +      The MediaTek SCP integrated to SoC might be a multi-core version.
> +      The other cores are represented as child nodes of the boot core.
> +      There are some integration differences for the IP like the usage of
> +      address translator for translating SoC bus addresses into address space
> +      for the processor.
> +
> +      Each SCP core has own cache memory. The SRAM and L1TCM are shared by
> +      cores. The power of cache, SRAM and L1TCM power should be enabled
> +      before booting SCP cores. The size of cache, SRAM, and L1TCM are varied
> +      on differnt SoCs.
> +
> +      The SCP cores do not use an MMU, but has a set of registers to
> +      control the translations between 32-bit CPU addresses into system bus
> +      addresses. Cache and memory access settings are provided through a
> +      Memory Protection Unit (MPU), programmable only from the SCP.
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - mediatek,mt8195-scp-core
> +
> +      reg:
> +        description: The base address and size of SRAM.
> +        maxItems: 1
> +
> +      reg-names:
> +        const: sram
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      firmware-name:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description:
> +          If present, name (or relative path) of the file within the
> +          firmware search path containing the firmware image used when
> +          initializing sub cores of multi-core SCP.
> +
> +      memory-region:
> +        maxItems: 1
> +
> +      cros-ec-rpmsg:
> +        type: object
> +        description:
> +          This subnode represents the rpmsg device. The namesof the devices
> +          are not important. The properties of this node are defined by the
> +          individual bindings for the rpmsg devices.
> +
> +        properties:
> +          mediatek,rpmsg-name:
> +            $ref: /schemas/types.yaml#/definitions/string-array
> +            description:
> +              Contains the name for the rpmsg device. Used to match
> +              the subnode to rpmsg device announced by SCP.
> +
> +        required:
> +          - mediatek,rpmsg-name
> +
> +    required:
> +      - compatible
> +      - reg
> +      - reg-names
> +
> +    additionalProperties: false
> +
>   required:
>     - compatible
>     - reg
> @@ -110,16 +187,26 @@ additionalProperties: false
>   
>   examples:
>     - |
> -    #include <dt-bindings/clock/mt8192-clk.h>
> -
>       scp@10500000 {
> -        compatible = "mediatek,mt8192-scp";
> +        compatible = "mediatek,mt8195-scp";
>           reg = <0x10500000 0x80000>,
>                 <0x10700000 0x8000>,
>                 <0x10720000 0xe0000>;
>           reg-names = "sram", "cfg", "l1tcm";
> -        clocks = <&infracfg CLK_INFRA_SCPSYS>;
> -        clock-names = "main";
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x105a0000 0x105a0000 0x20000>;
> +
> +        scp-c1@105a0000 {
> +                compatible = "mediatek,mt8195-scp-core";
> +                reg = <0x105a0000 0x20000>;
> +                reg-names = "sram";
> +
> +                cros-ec-rpmsg {
> +                    mediatek,rpmsg-name = "cros-ec-rpmsg";
> +                };
> +        };
>   
>           cros-ec-rpmsg {
>               mediatek,rpmsg-name = "cros-ec-rpmsg";

Re: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[TingHan Shen (沈廷翰)]

On Wed, 2022-09-28 at 15:01 +0800, Peng Fan wrote:
> 
> On 9/27/2022 10:55 AM, Tinghan Shen wrote:
> > The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file
> > to describe the 2nd core as a subnode of the boot core.
> > 
> > The configuration register is shared by MT8195 SCP core 0
> > and core 1. The core 1 can retrieve the information of configuration
> > registers from parent node.
> 
> I think the commit message would not convince people you put
> scp 1 as subnode of scp 0.
> 
> Regards,
> Peng.
> 
Hi Peng,

Thanks for your review. I should give the most reasonable explanation
for why SCP 1 is a subnode.

Adding SCP 1 as a subnode helps to assure finish probing SCP 1
before starting SCP 0 by using of_platform_populate. It's because
that I want to probe SCP 1 as a remoteproc subdevice of SCP 0.
such that when SCP 0 crashed, SCP 0 can reboot SCP 1.

Regards,
TingHan

RE: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[Peng Fan]

> Subject: Re: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support
> MT8195 dual-core SCP
> 
> On Wed, 2022-09-28 at 15:01 +0800, Peng Fan wrote:
> >
> > On 9/27/2022 10:55 AM, Tinghan Shen wrote:
> > > The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file to
> > > describe the 2nd core as a subnode of the boot core.
> > >
> > > The configuration register is shared by MT8195 SCP core 0 and core
> > > 1. The core 1 can retrieve the information of configuration
> > > registers from parent node.
> >
> > I think the commit message would not convince people you put scp 1 as
> > subnode of scp 0.
> >
> > Regards,
> > Peng.
> >
> Hi Peng,
> 
> Thanks for your review. I should give the most reasonable explanation for
> why SCP 1 is a subnode.
> 
> Adding SCP 1 as a subnode helps to assure finish probing SCP 1 before
> starting SCP 0 by using of_platform_populate. It's because that I want to
> probe SCP 1 as a remoteproc subdevice of SCP 0.
> such that when SCP 0 crashed, SCP 0 can reboot SCP 1.

Binding is for describing hardware..

Regards,
Peng.

> 
> Regards,
> TingHan
> 
> 
> 
> 

Re: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[Krzysztof Kozlowski]

On 28/09/2022 11:17, TingHan Shen (沈廷翰) wrote:
> On Wed, 2022-09-28 at 15:01 +0800, Peng Fan wrote:
>>
>> On 9/27/2022 10:55 AM, Tinghan Shen wrote:
>>> The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file
>>> to describe the 2nd core as a subnode of the boot core.
>>>
>>> The configuration register is shared by MT8195 SCP core 0
>>> and core 1. The core 1 can retrieve the information of configuration
>>> registers from parent node.
>>
>> I think the commit message would not convince people you put
>> scp 1 as subnode of scp 0.
>>
>> Regards,
>> Peng.
>>
> Hi Peng,
> 
> Thanks for your review. I should give the most reasonable explanation
> for why SCP 1 is a subnode.
> 
> Adding SCP 1 as a subnode helps to assure finish probing SCP 1
> before starting SCP 0 by using of_platform_populate. It's because
> that I want to probe SCP 1 as a remoteproc subdevice of SCP 0.
> such that when SCP 0 crashed, SCP 0 can reboot SCP 1.

As Peng noted, this is not proper description of hardware. The SCP 0/1
do not have parent-child relationship, so do not model it that way in
Devicetree.

Probe ordering is operating system specific, not related to bindings.

Best regards,
Krzysztof

Re: [PATCH v3 02/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

[Krzysztof Kozlowski]

On 27/09/2022 04:55, Tinghan Shen wrote:
> The MT8195 SCP is a dual-core RISC-V MCU. Extend the yaml file
> to describe the 2nd core as a subnode of the boot core.
> 
> The configuration register is shared by MT8195 SCP core 0
> and core 1. The core 1 can retrieve the information of configuration
> registers from parent node.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../bindings/remoteproc/mtk,scp.yaml          | 97 ++++++++++++++++++-
>  1 file changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 786bed897916..c012265be4eb 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -75,6 +75,83 @@ properties:
>      required:
>        - mediatek,rpmsg-name
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^scp-c[0-9]+@[a-f0-9]+$":

drop c[0-9]. Node names should be generic

> +    type: object
> +    description:
> +      The MediaTek SCP integrated to SoC might be a multi-core version.
> +      The other cores are represented as child nodes of the boot core.
> +      There are some integration differences for the IP like the usage of
> +      address translator for translating SoC bus addresses into address space
> +      for the processor.
> +
> +      Each SCP core has own cache memory. The SRAM and L1TCM are shared by
> +      cores. The power of cache, SRAM and L1TCM power should be enabled
> +      before booting SCP cores. The size of cache, SRAM, and L1TCM are varied
> +      on differnt SoCs.
> +
> +      The SCP cores do not use an MMU, but has a set of registers to
> +      control the translations between 32-bit CPU addresses into system bus
> +      addresses. Cache and memory access settings are provided through a
> +      Memory Protection Unit (MPU), programmable only from the SCP.
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - mediatek,mt8195-scp-core
> +
> +      reg:
> +        description: The base address and size of SRAM.
> +        maxItems: 1
> +
> +      reg-names:
> +        const: sram
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      firmware-name:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description:
> +          If present, name (or relative path) of the file within the
> +          firmware search path containing the firmware image used when
> +          initializing sub cores of multi-core SCP.
> +
> +      memory-region:
> +        maxItems: 1
> +
> +      cros-ec-rpmsg:
> +        type: object
> +        description:
> +          This subnode represents the rpmsg device. The namesof the devices
> +          are not important. The properties of this node are defined by the
> +          individual bindings for the rpmsg devices.

Same comments as with patch #1.

> +
> +        properties:
> +          mediatek,rpmsg-name:
> +            $ref: /schemas/types.yaml#/definitions/string-array
> +            description:
> +              Contains the name for the rpmsg device. Used to match
> +              the subnode to rpmsg device announced by SCP.
> +
> +        required:
> +          - mediatek,rpmsg-name
> +
> +    required:
> +      - compatible
> +      - reg
> +      - reg-names
> +
> +    additionalProperties: false
> +
>  required:
>    - compatible
>    - reg
> @@ -110,16 +187,26 @@ additionalProperties: false
>  
>  examples:
>    - |
> -    #include <dt-bindings/clock/mt8192-clk.h>
> -
>      scp@10500000 {
> -        compatible = "mediatek,mt8192-scp";
> +        compatible = "mediatek,mt8195-scp";
>          reg = <0x10500000 0x80000>,
>                <0x10700000 0x8000>,
>                <0x10720000 0xe0000>;
>          reg-names = "sram", "cfg", "l1tcm";
> -        clocks = <&infracfg CLK_INFRA_SCPSYS>;
> -        clock-names = "main";
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0x105a0000 0x105a0000 0x20000>;
> +
> +        scp-c1@105a0000 {
> +                compatible = "mediatek,mt8195-scp-core";

Messed indentation. Use 4 spaces for example indentation.


Best regards,
Krzysztof

[PATCH v3 03/11] arm64: dts: mt8195: Add SCP core 1 node

[Tinghan Shen]

Add the 2nd core(core 1) of MT8195 dual-core SCP to devicetree file.
Reserve some SRAM spaces for the core 1 image.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 905d1a90b406..48d457bd39b8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -760,12 +760,24 @@
 
 		scp: scp@10500000 {
 			compatible = "mediatek,mt8195-scp";
-			reg = <0 0x10500000 0 0x100000>,
+			reg = <0 0x10500000 0 0xa0000>,
 			      <0 0x10720000 0 0xe0000>,
 			      <0 0x10700000 0 0x8000>;
 			reg-names = "sram", "cfg", "l1tcm";
 			interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
 			status = "disabled";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x105a0000 0 0x105a0000 0x20000>;
+
+			scp_c1: scp-c1@105a0000 {
+				compatible = "mediatek,mt8195-scp-core";
+				reg = <0x105a0000 0x20000>;
+				reg-names = "sram";
+				interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
+				status = "disabled";
+			};
 		};
 
 		scp_adsp: clock-controller@10720000 {
-- 
2.18.0

Re: [PATCH v3 03/11] arm64: dts: mt8195: Add SCP core 1 node

[AngeloGioacchino Del Regno]

Il 27/09/22 04:55, Tinghan Shen ha scritto:
> Add the 2nd core(core 1) of MT8195 dual-core SCP to devicetree file.
> Reserve some SRAM spaces for the core 1 image.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 905d1a90b406..48d457bd39b8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -760,12 +760,24 @@
>   
>   		scp: scp@10500000 {
>   			compatible = "mediatek,mt8195-scp";
> -			reg = <0 0x10500000 0 0x100000>,
> +			reg = <0 0x10500000 0 0xa0000>,
>   			      <0 0x10720000 0 0xe0000>,
>   			      <0 0x10700000 0 0x8000>;
>   			reg-names = "sram", "cfg", "l1tcm";
>   			interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
>   			status = "disabled";
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x105a0000 0 0x105a0000 0x20000>;
> +
> +			scp_c1: scp-c1@105a0000 {
> +				compatible = "mediatek,mt8195-scp-core";
> +				reg = <0x105a0000 0x20000>;
> +				reg-names = "sram";
> +				interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
> +				status = "disabled";
> +			};

I think that the best way of describing a dual-core SCP in devicetree would
be either something like:

scp: scp@10500000 {
	compatible = "mediatek,mt8195-scp";
	reg = <0 0x10500000 0 0xa0000>, <0 0x105a0000 0 0x20000>,
	      <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
	reg-names = "sram", "sram-c1", "cfg", "l1tcm";
	interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>,
		     <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
	status = "disabled";
};

...but that may pose an issue when trying to assign different (or more instances
of the same) subnode(s) to each core... for which, I'd be more for something like:

scp: scp@10500000 {
	compatible = "mediatek,mt8195-scp";
	reg = <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
	reg-names = "cfg", "l1tcm";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0 0x10500000 0x100000>;
	status = "disabled";

	scp_c0: scp-core@0 {
		compatible = "mediatek,mt8195-scp-core";
		reg = <0x0 0xa0000>;
		reg-names = "sram";
		interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
	};

	scp_c1: scp-core@a0000 {
		compatible = "mediatek,mt8195-scp-core";
		reg = <0xa0000 0x20000>;
		reg-names = "sram";
		interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
	};
};

Regards,
Angelo

Re: [PATCH v3 03/11] arm64: dts: mt8195: Add SCP core 1 node

[TingHan Shen (沈廷翰)]

On Tue, 2022-09-27 at 13:01 +0200, AngeloGioacchino Del Regno wrote:
> Il 27/09/22 04:55, Tinghan Shen ha scritto:
> > Add the 2nd core(core 1) of MT8195 dual-core SCP to devicetree file.
> > Reserve some SRAM spaces for the core 1 image.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +++++++++++++-
> >   1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index 905d1a90b406..48d457bd39b8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -760,12 +760,24 @@
> >   
> >   		scp: scp@10500000 {
> >   			compatible = "mediatek,mt8195-scp";
> > -			reg = <0 0x10500000 0 0x100000>,
> > +			reg = <0 0x10500000 0 0xa0000>,
> >   			      <0 0x10720000 0 0xe0000>,
> >   			      <0 0x10700000 0 0x8000>;
> >   			reg-names = "sram", "cfg", "l1tcm";
> >   			interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
> >   			status = "disabled";
> > +
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges = <0x105a0000 0 0x105a0000 0x20000>;
> > +
> > +			scp_c1: scp-c1@105a0000 {
> > +				compatible = "mediatek,mt8195-scp-core";
> > +				reg = <0x105a0000 0x20000>;
> > +				reg-names = "sram";
> > +				interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
> > +				status = "disabled";
> > +			};
> 
> I think that the best way of describing a dual-core SCP in devicetree would
> be either something like:
> 
> scp: scp@10500000 {
> 	compatible = "mediatek,mt8195-scp";
> 	reg = <0 0x10500000 0 0xa0000>, <0 0x105a0000 0 0x20000>,
> 	      <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
> 	reg-names = "sram", "sram-c1", "cfg", "l1tcm";
> 	interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>,
> 		     <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
> 	status = "disabled";
> };
> 
> ...but that may pose an issue when trying to assign different (or more instances
> of the same) subnode(s) to each core... for which, I'd be more for something like:
> 
> scp: scp@10500000 {
> 	compatible = "mediatek,mt8195-scp";
> 	reg = <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
> 	reg-names = "cfg", "l1tcm";
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 	ranges = <0 0 0x10500000 0x100000>;
> 	status = "disabled";
> 
> 	scp_c0: scp-core@0 {
> 		compatible = "mediatek,mt8195-scp-core";
> 		reg = <0x0 0xa0000>;
> 		reg-names = "sram";
> 		interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
> 	};
> 
> 	scp_c1: scp-core@a0000 {
> 		compatible = "mediatek,mt8195-scp-core";
> 		reg = <0xa0000 0x20000>;
> 		reg-names = "sram";
> 		interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
> 	};
> };
> 
> Regards,
> Angelo
> 
> 
Hi Angelo,

I'm thinking about identifying the cores by the order of the sub nodes, 
i.e. core 0 must be the first sub node and core 1 must be the second sub node, 
because the scp cores in the example have the same compatible name.

I'm hesitant to make the sub nodes appear in a certain order. Is it appropriate?
Or, would it be more readable to create a new core id property? Or utilizing 
different compatble strings for cores? I would appreciat it if you could share your opinion.


-- 
Best regards,
TingHan

Re: [PATCH v3 03/11] arm64: dts: mt8195: Add SCP core 1 node

[AngeloGioacchino Del Regno]

Il 17/01/23 09:19, TingHan Shen (沈廷翰) ha scritto:
> On Tue, 2022-09-27 at 13:01 +0200, AngeloGioacchino Del Regno wrote:
>> Il 27/09/22 04:55, Tinghan Shen ha scritto:
>>> Add the 2nd core(core 1) of MT8195 dual-core SCP to devicetree file.
>>> Reserve some SRAM spaces for the core 1 image.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +++++++++++++-
>>>    1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 905d1a90b406..48d457bd39b8 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -760,12 +760,24 @@
>>>    
>>>    		scp: scp@10500000 {
>>>    			compatible = "mediatek,mt8195-scp";
>>> -			reg = <0 0x10500000 0 0x100000>,
>>> +			reg = <0 0x10500000 0 0xa0000>,
>>>    			      <0 0x10720000 0 0xe0000>,
>>>    			      <0 0x10700000 0 0x8000>;
>>>    			reg-names = "sram", "cfg", "l1tcm";
>>>    			interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
>>>    			status = "disabled";
>>> +
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			ranges = <0x105a0000 0 0x105a0000 0x20000>;
>>> +
>>> +			scp_c1: scp-c1@105a0000 {
>>> +				compatible = "mediatek,mt8195-scp-core";
>>> +				reg = <0x105a0000 0x20000>;
>>> +				reg-names = "sram";
>>> +				interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
>>> +				status = "disabled";
>>> +			};
>>
>> I think that the best way of describing a dual-core SCP in devicetree would
>> be either something like:
>>
>> scp: scp@10500000 {
>> 	compatible = "mediatek,mt8195-scp";
>> 	reg = <0 0x10500000 0 0xa0000>, <0 0x105a0000 0 0x20000>,
>> 	      <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
>> 	reg-names = "sram", "sram-c1", "cfg", "l1tcm";
>> 	interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>,
>> 		     <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
>> 	status = "disabled";
>> };
>>
>> ...but that may pose an issue when trying to assign different (or more instances
>> of the same) subnode(s) to each core... for which, I'd be more for something like:
>>
>> scp: scp@10500000 {
>> 	compatible = "mediatek,mt8195-scp";
>> 	reg = <0 0x10720000 0 0xe0000>, <0 0x10700000 0 0x8000>;
>> 	reg-names = "cfg", "l1tcm";
>> 	#address-cells = <1>;
>> 	#size-cells = <1>;
>> 	ranges = <0 0 0x10500000 0x100000>;
>> 	status = "disabled";
>>
>> 	scp_c0: scp-core@0 {
>> 		compatible = "mediatek,mt8195-scp-core";
>> 		reg = <0x0 0xa0000>;
>> 		reg-names = "sram";
>> 		interrupts = <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH 0>;
>> 	};
>>
>> 	scp_c1: scp-core@a0000 {
>> 		compatible = "mediatek,mt8195-scp-core";
>> 		reg = <0xa0000 0x20000>;
>> 		reg-names = "sram";
>> 		interrupts = <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH 0>;
>> 	};
>> };
>>
>> Regards,
>> Angelo
>>
>>
> Hi Angelo,
> 
> I'm thinking about identifying the cores by the order of the sub nodes,
> i.e. core 0 must be the first sub node and core 1 must be the second sub node,
> because the scp cores in the example have the same compatible name.
> 
> I'm hesitant to make the sub nodes appear in a certain order. Is it appropriate?
> Or, would it be more readable to create a new core id property? Or utilizing
> different compatble strings for cores? I would appreciat it if you could share your opinion.
> 
> 

Assuming that in a future >2 cores architecture only the first core, which I will
call "core 0" for commodity, will have "special treatment" and core 1, 2, 3...N
will always be "interchangeable", I think that something like `mediatek,scp-leader`
would work to identify the first core.

Cheers!
Angelo

[PATCH v3 04/11] remoteproc: mediatek: Remove redundant rproc_boot

[Tinghan Shen]

The video codec doesn't need to explicitly boot SCP in its flow
because the SCP remote processor enables auto boot.

The redundant usage of rproc_boot increases the number of rproc.power
over 1 and prevents successfully shutting down SCP by rproc_shutdown.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../mediatek/vcodec/mtk_vcodec_fw_scp.c         |  2 +-
 drivers/remoteproc/mtk_scp.c                    | 17 +++++++++++++++++
 include/linux/remoteproc/mtk_scp.h              |  1 +
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
index d8e66b645bd8..c3194f88ff31 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
@@ -6,7 +6,7 @@
 
 static int mtk_vcodec_scp_load_firmware(struct mtk_vcodec_fw *fw)
 {
-	return rproc_boot(scp_get_rproc(fw->scp));
+	return scp_boot(fw->scp);
 }
 
 static unsigned int mtk_vcodec_scp_get_vdec_capa(struct mtk_vcodec_fw *fw)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index d421a2ccaa1e..bf68bccab78b 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -673,6 +673,23 @@ struct rproc *scp_get_rproc(struct mtk_scp *scp)
 }
 EXPORT_SYMBOL_GPL(scp_get_rproc);
 
+/**
+ * scp_boot() - Boot SCP
+ *
+ * @scp:	mtk_scp structure
+ **/
+int scp_boot(struct mtk_scp *scp)
+{
+	struct rproc *rproc = scp->rproc;
+
+	/* scp already booted when power > 0 */
+	if (atomic_read(&rproc->power) > 0)
+		return 0;
+	else
+		return rproc_boot(scp->rproc);
+}
+EXPORT_SYMBOL_GPL(scp_boot);
+
 /**
  * scp_get_vdec_hw_capa() - get video decoder hardware capability
  *
diff --git a/include/linux/remoteproc/mtk_scp.h b/include/linux/remoteproc/mtk_scp.h
index 7c2b7cc9fe6c..e463105b351c 100644
--- a/include/linux/remoteproc/mtk_scp.h
+++ b/include/linux/remoteproc/mtk_scp.h
@@ -52,6 +52,7 @@ void scp_put(struct mtk_scp *scp);
 
 struct device *scp_get_device(struct mtk_scp *scp);
 struct rproc *scp_get_rproc(struct mtk_scp *scp);
+int scp_boot(struct mtk_scp *scp);
 
 int scp_ipi_register(struct mtk_scp *scp, u32 id, scp_ipi_handler_t handler,
 		     void *priv);
-- 
2.18.0

Re: [PATCH v3 04/11] remoteproc: mediatek: Remove redundant rproc_boot

[AngeloGioacchino Del Regno]

Il 27/09/22 04:55, Tinghan Shen ha scritto:
> The video codec doesn't need to explicitly boot SCP in its flow
> because the SCP remote processor enables auto boot.
> 
> The redundant usage of rproc_boot increases the number of rproc.power
> over 1 and prevents successfully shutting down SCP by rproc_shutdown.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

You should Cc stable on this commit, as it's a quite important fix.

Regards,
Angelo

Re: [PATCH v3 04/11] remoteproc: mediatek: Remove redundant rproc_boot

[TingHan Shen (沈廷翰)]

On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
> Il 27/09/22 04:55, Tinghan Shen ha scritto:
> > The video codec doesn't need to explicitly boot SCP in its flow
> > because the SCP remote processor enables auto boot.
> > 
> > The redundant usage of rproc_boot increases the number of rproc.power
> > over 1 and prevents successfully shutting down SCP by rproc_shutdown.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> 
> You should Cc stable on this commit, as it's a quite important fix.
> 
> Regards,
> Angelo
> 
Hi Angelo,

Ok, I'll add it at next version.

Thanks,
TingHan

Re: [PATCH v3 04/11] remoteproc: mediatek: Remove redundant rproc_boot

[Peng Fan]

On 9/28/2022 4:27 PM, TingHan Shen (沈廷翰) wrote:
> On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
>> Il 27/09/22 04:55, Tinghan Shen ha scritto:
>>> The video codec doesn't need to explicitly boot SCP in its flow
>>> because the SCP remote processor enables auto boot.
>>>
>>> The redundant usage of rproc_boot increases the number of rproc.power
>>> over 1 and prevents successfully shutting down SCP by rproc_shutdown.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>
>> You should Cc stable on this commit, as it's a quite important fix.
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
> 
> Ok, I'll add it at next version.

If this patch is not relevant with the SCP 1 support in this patchset, 
better separate this patch out as a standalone fix.

Regards,
Peng.

> 
> Thanks,
> TingHan

Re: [PATCH v3 04/11] remoteproc: mediatek: Remove redundant rproc_boot

[TingHan Shen (沈廷翰)]

On Wed, 2022-09-28 at 17:40 +0800, Peng Fan wrote:
> 
> On 9/28/2022 4:27 PM, TingHan Shen (沈廷翰) wrote:
> > On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
> > > Il 27/09/22 04:55, Tinghan Shen ha scritto:
> > > > The video codec doesn't need to explicitly boot SCP in its flow
> > > > because the SCP remote processor enables auto boot.
> > > > 
> > > > The redundant usage of rproc_boot increases the number of rproc.power
> > > > over 1 and prevents successfully shutting down SCP by rproc_shutdown.
> > > > 
> > > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > > 
> > > You should Cc stable on this commit, as it's a quite important fix.
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > Hi Angelo,
> > 
> > Ok, I'll add it at next version.
> 
> If this patch is not relevant with the SCP 1 support in this patchset, 
> better separate this patch out as a standalone fix.

Ok, I'll send it separately.

Regards,
TingHan

> 
> Regards,
> Peng.
> 
> > 
> > Thanks,
> > TingHan

[PATCH v3 05/11] remoteproc: mediatek: Add SCP core 1 register definitions

[Tinghan Shen]

Add MT8195 SCP core 1 related register definitions.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/mtk_common.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index ea6fa1100a00..3778894c96f3 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -47,6 +47,7 @@
 #define MT8192_SCP2SPM_IPC_CLR		0x4094
 #define MT8192_GIPC_IN_SET		0x4098
 #define MT8192_HOST_IPC_INT_BIT		BIT(0)
+#define MT8195_CORE1_HOST_IPC_INT_BIT	BIT(4)
 
 #define MT8192_CORE0_SW_RSTN_CLR	0x10000
 #define MT8192_CORE0_SW_RSTN_SET	0x10004
@@ -56,6 +57,26 @@
 
 #define MT8195_L1TCM_SRAM_PDN_RESERVED_RSI_BITS		GENMASK(7, 4)
 
+#define MT8195_CPU1_SRAM_PD			0x1084
+#define MT8195_SSHUB2APMCU_IPC_SET		0x4088
+#define MT8195_SSHUB2APMCU_IPC_CLR		0x408C
+#define MT8195_CORE1_SW_RSTN_CLR		0x20000
+#define MT8195_CORE1_SW_RSTN_SET		0x20004
+#define MT8195_CORE1_MEM_ATT_PREDEF		0x20008
+#define MT8195_CORE1_WDT_IRQ			0x20030
+#define MT8195_CORE1_WDT_CFG			0x20034
+
+#define MT8195_SEC_CTRL				0x85000
+#define MT8195_CORE_OFFSET_ENABLE_D		BIT(13)
+#define MT8195_CORE_OFFSET_ENABLE_I		BIT(12)
+#define MT8195_L2TCM_OFFSET_RANGE_0_LOW		0x850b0
+#define MT8195_L2TCM_OFFSET_RANGE_0_HIGH	0x850b4
+#define MT8195_L2TCM_OFFSET			0x850d0
+#define SCP_SRAM_REMAP_LOW			0
+#define SCP_SRAM_REMAP_HIGH			1
+#define SCP_SRAM_REMAP_OFFSET			2
+#define SCP_SRAM_REMAP_SIZE			3
+
 #define SCP_FW_VER_LEN			32
 #define SCP_SHARE_BUFFER_SIZE		288
 
-- 
2.18.0

Re: [PATCH v3 05/11] remoteproc: mediatek: Add SCP core 1 register definitions

[AngeloGioacchino Del Regno]

Il 27/09/22 04:56, Tinghan Shen ha scritto:
> Add MT8195 SCP core 1 related register definitions.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v3 06/11] remoteproc: mediatek: Add MT8195 SCP core 1 operations

[Tinghan Shen]

The SCP rproc driver has a set of chip dependent callbacks for
boot sequence and IRQ handling. Implement these callbacks for MT8195
SCP core 1.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_scp.c | 57 ++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index bf68bccab78b..1d17d77b8a14 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -176,6 +176,16 @@ static void mt8192_scp_reset_deassert(struct mtk_scp *scp)
 	writel(1, scp->reg_base + MT8192_CORE0_SW_RSTN_CLR);
 }
 
+static void mt8195_scp_c1_reset_assert(struct mtk_scp *scp)
+{
+	writel(1, scp->reg_base + MT8195_CORE1_SW_RSTN_SET);
+}
+
+static void mt8195_scp_c1_reset_deassert(struct mtk_scp *scp)
+{
+	writel(1, scp->reg_base + MT8195_CORE1_SW_RSTN_CLR);
+}
+
 static void mt8183_scp_irq_handler(struct mtk_scp *scp)
 {
 	u32 scp_to_host;
@@ -212,6 +222,18 @@ static void mt8192_scp_irq_handler(struct mtk_scp *scp)
 	}
 }
 
+static void mt8195_scp_c1_irq_handler(struct mtk_scp *scp)
+{
+	u32 scp_to_host;
+
+	scp_to_host = readl(scp->reg_base + MT8195_SSHUB2APMCU_IPC_SET);
+
+	if (scp_to_host & MT8192_SCP_IPC_INT_BIT)
+		scp_ipi_handler(scp);
+
+	writel(scp_to_host, scp->reg_base + MT8195_SSHUB2APMCU_IPC_CLR);
+}
+
 static irqreturn_t scp_irq_handler(int irq, void *priv)
 {
 	struct mtk_scp *scp = priv;
@@ -453,6 +475,19 @@ static int mt8195_scp_before_load(struct mtk_scp *scp)
 	return 0;
 }
 
+static int mt8195_scp_c1_before_load(struct mtk_scp *scp)
+{
+	scp_sram_power_on(scp->reg_base + MT8195_CPU1_SRAM_PD, 0);
+
+	/* hold SCP in reset while loading FW. */
+	scp->data->scp_reset_assert(scp);
+
+	/* enable MPU for all memory regions */
+	writel(0xff, scp->reg_base + MT8195_CORE1_MEM_ATT_PREDEF);
+
+	return 0;
+}
+
 static int scp_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct mtk_scp *scp = rproc->priv;
@@ -625,6 +660,15 @@ static void mt8195_scp_stop(struct mtk_scp *scp)
 	writel(0, scp->reg_base + MT8192_CORE0_WDT_CFG);
 }
 
+static void mt8195_scp_c1_stop(struct mtk_scp *scp)
+{
+	/* Power off CPU SRAM */
+	scp_sram_power_off(scp->reg_base + MT8195_CPU1_SRAM_PD, 0);
+
+	/* Disable SCP watchdog */
+	writel(0, scp->reg_base + MT8195_CORE1_WDT_CFG);
+}
+
 static int scp_stop(struct rproc *rproc)
 {
 	struct mtk_scp *scp = (struct mtk_scp *)rproc->priv;
@@ -1007,12 +1051,25 @@ static const struct mtk_scp_of_data mt8195_of_data = {
 	.host_to_scp_int_bit = MT8192_HOST_IPC_INT_BIT,
 };
 
+static const struct mtk_scp_of_data mt8195_core_of_data = {
+	.scp_clk_get = mt8195_scp_clk_get,
+	.scp_before_load = mt8195_scp_c1_before_load,
+	.scp_irq_handler = mt8195_scp_c1_irq_handler,
+	.scp_reset_assert = mt8195_scp_c1_reset_assert,
+	.scp_reset_deassert = mt8195_scp_c1_reset_deassert,
+	.scp_stop = mt8195_scp_c1_stop,
+	.scp_da_to_va = mt8192_scp_da_to_va,
+	.host_to_scp_reg = MT8192_GIPC_IN_SET,
+	.host_to_scp_int_bit = MT8195_CORE1_HOST_IPC_INT_BIT,
+};
+
 static const struct of_device_id mtk_scp_of_match[] = {
 	{ .compatible = "mediatek,mt8183-scp", .data = &mt8183_of_data },
 	{ .compatible = "mediatek,mt8186-scp", .data = &mt8186_of_data },
 	{ .compatible = "mediatek,mt8188-scp", .data = &mt8188_of_data },
 	{ .compatible = "mediatek,mt8192-scp", .data = &mt8192_of_data },
 	{ .compatible = "mediatek,mt8195-scp", .data = &mt8195_of_data },
+	{ .compatible = "mediatek,mt8195-scp-core", .data = &mt8195_core_of_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_scp_of_match);
-- 
2.18.0

[PATCH v3 07/11] remoteproc: mediatek: Probe MT8195 SCP core 1

[Tinghan Shen]

The MT8195 SCP configuration registers for core 0 and core 1 is the same.
Let SCP core 1 to reuse the mapped address requested by SCP core 0.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_common.h |  2 ++
 drivers/remoteproc/mtk_scp.c    | 27 +++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 3778894c96f3..54265c515315 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -140,6 +140,8 @@ struct mtk_scp {
 	size_t dram_size;
 
 	struct rproc_subdev *rpmsg_subdev;
+
+	struct mtk_scp *main_scp;
 };
 
 /**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 1d17d77b8a14..f7b738743ba9 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -917,10 +917,29 @@ static int scp_probe(struct platform_device *pdev)
 		scp->l1tcm_phys = res->start;
 	}
 
-	scp->reg_base = devm_platform_ioremap_resource_byname(pdev, "cfg");
-	if (IS_ERR(scp->reg_base))
-		return dev_err_probe(dev, PTR_ERR(scp->reg_base),
-				     "Failed to parse and map cfg memory\n");
+	if (of_device_is_compatible(np, "mediatek,mt8195-scp-core")) {
+		struct device_node *pnp;
+		struct platform_device *scp_pdev;
+
+		pnp = of_get_parent(np);
+		if (!pnp)
+			return dev_err_probe(dev, -ENODEV, "Failed to get parent core 0\n");
+
+		scp_pdev = of_find_device_by_node(pnp);
+		of_node_put(pnp);
+		if (!scp_pdev)
+			return dev_err_probe(dev, -ENODEV, "Failed to get scp core 0 pdev\n");
+
+		scp->main_scp = platform_get_drvdata(scp_pdev);
+		scp->reg_base = scp->main_scp->reg_base;
+	} else {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+		scp->reg_base = devm_ioremap_resource(dev, res);
+
+		if (IS_ERR(scp->reg_base))
+			return dev_err_probe(dev, PTR_ERR(scp->reg_base),
+					     "Failed to parse and map cfg memory\n");
+	}
 
 	ret = scp->data->scp_clk_get(scp);
 	if (ret)
-- 
2.18.0

[PATCH v3 08/11] remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice

[Tinghan Shen]

Register SCP core 1 as a subdevice of core 0 to control the boot sequence
and watchdog handling. The core 1 has to boot after core 0 because the
SCP clock and SRAM power is controlled by SCP core 0.
When SCP core 0 reports a watchdog timeout event, the SRAM is emptied
before rebooting the SCP core 0, forcing the reload of the SCP core 1 image.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_common.h |  8 ++++
 drivers/remoteproc/mtk_scp.c    | 71 +++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 54265c515315..dcde25f8bbf9 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -95,6 +95,13 @@ struct scp_ipi_desc {
 	void *priv;
 };
 
+struct scp_subdev_core {
+	struct rproc_subdev subdev;
+	struct mtk_scp *scp;
+};
+
+#define to_subdev_core(d) container_of(d, struct scp_subdev_core, subdev)
+
 struct mtk_scp;
 
 struct mtk_scp_of_data {
@@ -142,6 +149,7 @@ struct mtk_scp {
 	struct rproc_subdev *rpmsg_subdev;
 
 	struct mtk_scp *main_scp;
+	struct rproc_subdev *core_subdev;
 };
 
 /**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index f7b738743ba9..2d43338b96da 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -871,6 +871,54 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
 	}
 }
 
+static int scp_core_subdev_start(struct rproc_subdev *subdev)
+{
+	struct scp_subdev_core *subdev_core = to_subdev_core(subdev);
+	struct mtk_scp *scp = subdev_core->scp;
+
+	rproc_boot(scp->rproc);
+
+	return 0;
+}
+
+static void scp_core_subdev_stop(struct rproc_subdev *subdev, bool crashed)
+{
+	struct scp_subdev_core *subdev_core = to_subdev_core(subdev);
+	struct mtk_scp *scp = subdev_core->scp;
+
+	rproc_shutdown(scp->rproc);
+}
+
+static int scp_core_subdev_register(struct mtk_scp *scp)
+{
+	struct device *dev = scp->dev;
+	struct scp_subdev_core *subdev_core;
+
+	subdev_core = devm_kzalloc(dev, sizeof(*subdev_core), GFP_KERNEL);
+	if (!subdev_core) {
+		scp->core_subdev = NULL;
+		return -ENOMEM;
+	}
+
+	subdev_core->scp = scp;
+	subdev_core->subdev.start = scp_core_subdev_start;
+	subdev_core->subdev.stop = scp_core_subdev_stop;
+
+	scp->core_subdev = &subdev_core->subdev;
+	rproc_add_subdev(scp->main_scp->rproc, scp->core_subdev);
+
+	return 0;
+}
+
+static void scp_core_subdev_unregister(struct mtk_scp *scp)
+{
+	if (scp->core_subdev) {
+		rproc_remove_subdev(scp->main_scp->rproc, scp->core_subdev);
+		devm_kfree(scp->dev, to_subdev_core(scp->core_subdev));
+		scp->core_subdev = NULL;
+	}
+}
+
 static int scp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -974,6 +1022,23 @@ static int scp_probe(struct platform_device *pdev)
 		goto remove_subdev;
 	}
 
+	if (of_device_is_compatible(np, "mediatek,mt8195-scp-core")) {
+		ret = scp_core_subdev_register(scp);
+		if (ret) {
+			dev_err_probe(dev, ret, "Failed to register subdev\n");
+			goto remove_subdev;
+		}
+
+		/* sub cores are booted as subdevices of main core. */
+		rproc->auto_boot = false;
+	} else {
+		ret = devm_of_platform_populate(dev);
+		if (ret) {
+			dev_err_probe(dev, ret, "Failed to probe sub cores\n");
+			goto remove_subdev;
+		}
+	}
+
 	ret = rproc_add(rproc);
 	if (ret)
 		goto remove_subdev;
@@ -981,6 +1046,7 @@ static int scp_probe(struct platform_device *pdev)
 	return 0;
 
 remove_subdev:
+	scp_core_subdev_unregister(scp);
 	scp_remove_rpmsg_subdev(scp);
 	scp_ipi_unregister(scp, SCP_IPI_INIT);
 release_dev_mem:
@@ -997,6 +1063,11 @@ static int scp_remove(struct platform_device *pdev)
 	struct mtk_scp *scp = platform_get_drvdata(pdev);
 	int i;
 
+	if (of_device_is_compatible(scp->dev->of_node, "mediatek,mt8195-scp-core"))
+		scp_core_subdev_unregister(scp);
+	else
+		devm_of_platform_depopulate(scp->dev);
+
 	rproc_del(scp->rproc);
 	scp_remove_rpmsg_subdev(scp);
 	scp_ipi_unregister(scp, SCP_IPI_INIT);
-- 
2.18.0

[PATCH v3 09/11] remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset

[Tinghan Shen]

Because MT8195 SCP core 0 and core 1 both boot from head of SRAM and
have the same viewpoint of SRAM, SCP has a "Core 1 SRAM offset"
to control the viewpoint of SCP core 1 to allow core 1 boot
from different SRAM location.

The "Core 1 SRAM offset" configuration is composed by specifying a range
and an offset. When SCP core 1 accesses a SRAM address located in the
configured range, SCP bus adds the configured offset to the address to
shift the final physical destination on SCP SRAM. This shift is
transparent to the software running on SCP core 1.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_scp.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 2d43338b96da..0f1b587f8502 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -477,6 +477,8 @@ static int mt8195_scp_before_load(struct mtk_scp *scp)
 
 static int mt8195_scp_c1_before_load(struct mtk_scp *scp)
 {
+	u32 sec_ctrl;
+
 	scp_sram_power_on(scp->reg_base + MT8195_CPU1_SRAM_PD, 0);
 
 	/* hold SCP in reset while loading FW. */
@@ -485,6 +487,27 @@ static int mt8195_scp_c1_before_load(struct mtk_scp *scp)
 	/* enable MPU for all memory regions */
 	writel(0xff, scp->reg_base + MT8195_CORE1_MEM_ATT_PREDEF);
 
+	/* The value of SRAM offset range is from the viewpoint of SCP core 1.
+	 * This configuration adds an offset on SCP bus when SCP core 1 accesses SCP SRAM
+	 * to solve the SCP core 0 and core 1 both fetch the 1st instruction from the same
+	 * SRAM address.
+	 *
+	 * Because SCP core 0 and core 1 both boot from the head of sram, this must be
+	 * configured before boot SCP core 1.
+	 *
+	 * Configure the range of SRAM addresses will be added offset.
+	 */
+	writel(0, scp->reg_base + MT8195_L2TCM_OFFSET_RANGE_0_LOW);
+	writel(scp->sram_size, scp->reg_base + MT8195_L2TCM_OFFSET_RANGE_0_HIGH);
+
+	/* configure the offset value */
+	writel(scp->sram_phys - scp->main_scp->sram_phys, scp->reg_base + MT8195_L2TCM_OFFSET);
+
+	/* enable adding sram offset when fetching instruction and data */
+	sec_ctrl = readl(scp->reg_base + MT8195_SEC_CTRL);
+	sec_ctrl |= MT8195_CORE_OFFSET_ENABLE_I | MT8195_CORE_OFFSET_ENABLE_D;
+	writel(sec_ctrl, scp->reg_base + MT8195_SEC_CTRL);
+
 	return 0;
 }
 
-- 
2.18.0

[PATCH v3 10/11] remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout

[Tinghan Shen]

The MT8195 SCP core 1 watchdog timeout needs to be handled in the
SCP core 0 IRQ handler because the MT8195 SCP core 1 watchdog timeout
IRQ is wired on the same IRQ entry for core 0 watchdog timeout.
MT8195 SCP has a watchdog status register to identify the watchdog
timeout source when IRQ triggered.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_common.h |  4 +++
 drivers/remoteproc/mtk_scp.c    | 44 ++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index dcde25f8bbf9..6cd04ca9e681 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -55,6 +55,10 @@
 #define MT8192_CORE0_WDT_IRQ		0x10030
 #define MT8192_CORE0_WDT_CFG		0x10034
 
+#define MT8195_SYS_STATUS		0x4004
+#define MT8195_CORE0_WDT		BIT(16)
+#define MT8195_CORE1_WDT		BIT(17)
+
 #define MT8195_L1TCM_SRAM_PDN_RESERVED_RSI_BITS		GENMASK(7, 4)
 
 #define MT8195_CPU1_SRAM_PD			0x1084
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 0f1b587f8502..159f3c69cd69 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -222,6 +222,48 @@ static void mt8192_scp_irq_handler(struct mtk_scp *scp)
 	}
 }
 
+static void mt8195_scp_irq_handler(struct mtk_scp *scp)
+{
+	u32 scp_to_host;
+
+	scp_to_host = readl(scp->reg_base + MT8192_SCP2APMCU_IPC_SET);
+
+	if (scp_to_host & MT8192_SCP_IPC_INT_BIT) {
+		scp_ipi_handler(scp);
+	} else {
+		if (readl(scp->reg_base + MT8195_SYS_STATUS) & MT8195_CORE1_WDT) {
+			struct device_node *c1_np;
+			struct platform_device *c1_pdev;
+			struct mtk_scp *c1_scp;
+
+			writel(1, scp->reg_base + MT8195_CORE1_WDT_IRQ);
+
+			c1_np = of_get_compatible_child(scp->dev->of_node,
+							"mediatek,mt8195-scp-core");
+			if (!c1_np) {
+				dev_err(scp->dev, "cannot find core 1 node\n");
+				goto clear_irq;
+			}
+
+			c1_pdev = of_find_device_by_node(c1_np);
+			of_node_put(c1_np);
+			if (!c1_pdev) {
+				dev_err(scp->dev, "cannot find core 1 pdev\n");
+				goto clear_irq;
+			}
+
+			c1_scp = platform_get_drvdata(c1_pdev);
+			scp_wdt_handler(c1_scp, scp_to_host);
+		} else {
+			writel(1, scp->reg_base + MT8192_CORE0_WDT_IRQ);
+			scp_wdt_handler(scp, scp_to_host);
+		}
+	}
+
+clear_irq:
+	writel(scp_to_host, scp->reg_base + MT8192_SCP2APMCU_IPC_CLR);
+}
+
 static void mt8195_scp_c1_irq_handler(struct mtk_scp *scp)
 {
 	u32 scp_to_host;
@@ -1155,7 +1197,7 @@ static const struct mtk_scp_of_data mt8192_of_data = {
 static const struct mtk_scp_of_data mt8195_of_data = {
 	.scp_clk_get = mt8195_scp_clk_get,
 	.scp_before_load = mt8195_scp_before_load,
-	.scp_irq_handler = mt8192_scp_irq_handler,
+	.scp_irq_handler = mt8195_scp_irq_handler,
 	.scp_reset_assert = mt8192_scp_reset_assert,
 	.scp_reset_deassert = mt8192_scp_reset_deassert,
 	.scp_stop = mt8195_scp_stop,
-- 
2.18.0

[PATCH v3 11/11] remoteproc: mediatek: Refine ipi handler error message

[Tinghan Shen]

The error message doesn't accurately reflect the cause of
the error. The error is due to a handler not being found,
not an invalid IPI ID.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/remoteproc/mtk_scp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 159f3c69cd69..8b1765c61442 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -106,7 +106,7 @@ static void scp_ipi_handler(struct mtk_scp *scp)
 	scp_ipi_lock(scp, id);
 	handler = ipi_desc[id].handler;
 	if (!handler) {
-		dev_err(scp->dev, "No such ipi id = %d\n", id);
+		dev_err(scp->dev, "No handler for ipi id = %d\n", id);
 		scp_ipi_unlock(scp, id);
 		return;
 	}
-- 
2.18.0

Re: [PATCH v3 11/11] remoteproc: mediatek: Refine ipi handler error message

[AngeloGioacchino Del Regno]

Il 27/09/22 04:56, Tinghan Shen ha scritto:
> The error message doesn't accurately reflect the cause of
> the error. The error is due to a handler not being found,
> not an invalid IPI ID.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 00/11] Add support for MT8195 SCP 2nd core

[Mathieu Poirier]

Good day,

A lot of comments related to the handling of SCP 0 and 1 have already been made
on this patchset, along with my own advice from the previous patchset on how to
move forward.  As such I will wait for a new revision.

Thanks,
Mathieu

On Tue, Sep 27, 2022 at 10:55:55AM +0800, Tinghan Shen wrote:
> The mediatek remoteproc driver currently only allows bringing up a 
> single core SCP, e.g. MT8183. It also only bringing up the 1st 
> core in SoCs with a dual-core SCP, e.g. MT8195. This series support 
> to bring-up the 2nd core of the dual-core SCP.
> 
> v2 -> v3:
> 1. change the representation of dual-core SCP in dts file and update SCP yaml
> 2. rewrite SCP driver to reflect the change of dts node
> 3. add SCP core 1 node to mt8195.dtsi
> 4. remove redundant call of rproc_boot for SCP
> 5. refine IPI error message
> 
> v1 -> v2:
> 1. update dt-binding property description
> 2. remove kconfig for scp dual driver
> 3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c
> 
> Tinghan Shen (11):
>   dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
>   dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
>   arm64: dts: mt8195: Add SCP core 1 node
>   remoteproc: mediatek: Remove redundant rproc_boot
>   remoteproc: mediatek: Add SCP core 1 register definitions
>   remoteproc: mediatek: Add MT8195 SCP core 1 operations
>   remoteproc: mediatek: Probe MT8195 SCP core 1
>   remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
>   remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
>   remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
>   remoteproc: mediatek: Refine ipi handler error message
> 
>  .../bindings/remoteproc/mtk,scp.yaml          | 132 ++++++++--
>  .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |   2 +-
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi      |  14 +-
>  .../mediatek/vcodec/mtk_vcodec_fw_scp.c       |   2 +-
>  drivers/remoteproc/mtk_common.h               |  35 +++
>  drivers/remoteproc/mtk_scp.c                  | 241 +++++++++++++++++-
>  include/linux/remoteproc/mtk_scp.h            |   1 +
>  7 files changed, 397 insertions(+), 30 deletions(-)
> 
> -- 
> 2.18.0
> 

Re: [PATCH v3 00/11] Add support for MT8195 SCP 2nd core

[TingHan Shen (沈廷翰)]

On Tue, 2022-11-01 at 14:40 -0600, Mathieu Poirier wrote:
> Good day,
> 
> A lot of comments related to the handling of SCP 0 and 1 have already been made
> on this patchset, along with my own advice from the previous patchset on how to
> move forward.  As such I will wait for a new revision.
> 
> Thanks,
> Mathieu

Sorry for late response.
I'll update the series based on all of your comments.
Thank you!

> 
> On Tue, Sep 27, 2022 at 10:55:55AM +0800, Tinghan Shen wrote:
> > The mediatek remoteproc driver currently only allows bringing up a 
> > single core SCP, e.g. MT8183. It also only bringing up the 1st 
> > core in SoCs with a dual-core SCP, e.g. MT8195. This series support 
> > to bring-up the 2nd core of the dual-core SCP.
> > 
> > v2 -> v3:
> > 1. change the representation of dual-core SCP in dts file and update SCP yaml
> > 2. rewrite SCP driver to reflect the change of dts node
> > 3. add SCP core 1 node to mt8195.dtsi
> > 4. remove redundant call of rproc_boot for SCP
> > 5. refine IPI error message
> > 
> > v1 -> v2:
> > 1. update dt-binding property description
> > 2. remove kconfig for scp dual driver
> > 3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c
> > 
> > Tinghan Shen (11):
> >   dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
> >   dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
> >   arm64: dts: mt8195: Add SCP core 1 node
> >   remoteproc: mediatek: Remove redundant rproc_boot
> >   remoteproc: mediatek: Add SCP core 1 register definitions
> >   remoteproc: mediatek: Add MT8195 SCP core 1 operations
> >   remoteproc: mediatek: Probe MT8195 SCP core 1
> >   remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
> >   remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
> >   remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
> >   remoteproc: mediatek: Refine ipi handler error message
> > 
> >  .../bindings/remoteproc/mtk,scp.yaml          | 132 ++++++++--
> >  .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |   2 +-
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi      |  14 +-
> >  .../mediatek/vcodec/mtk_vcodec_fw_scp.c       |   2 +-
> >  drivers/remoteproc/mtk_common.h               |  35 +++
> >  drivers/remoteproc/mtk_scp.c                  | 241 +++++++++++++++++-
> >  include/linux/remoteproc/mtk_scp.h            |   1 +
> >  7 files changed, 397 insertions(+), 30 deletions(-)
> > 
> > -- 
> > 2.18.0
> > 

-- 
Best regards,
TingHan