[PATCH 0/2] Add support for TI AM18XX/OMAP-L138 PRUSS

[PATCH 0/2] Add support for TI AM18XX/OMAP-L138 PRUSS

[David Lechner]

This is the first step for adding support for the PRUSS on TI AM18XX/OMAP-L138
SoCs. This series adds support in the top-level PRUSS driver. (Patches for the
interrupt controller and individual PRUs are independent of this change and
will be submitted separately.)

David Lechner (2):
  dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss
  soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS

 .../devicetree/bindings/soc/ti/ti,pruss.yaml  |  2 +
 drivers/soc/ti/Kconfig                        |  2 +-
 drivers/soc/ti/pruss.c                        | 76 +++++++++++--------
 3 files changed, 47 insertions(+), 33 deletions(-)

-- 
2.25.1

[PATCH 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss

[David Lechner]

This adds a "ti,am1806-pruss" compatible type for the PRUSS found in
TI AM18xx/OMAP-L138 SoCs.

Signed-off-by: David Lechner <david@lechnology.com>
---
 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
       - 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
-- 
2.25.1

Re: [PATCH 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss

[Rob Herring]

On Mon, 04 Jan 2021 12:30:20 -0600, 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 <david@lechnology.com>
> ---
>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss

[Suman Anna]

+ 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 <david@lechnology.com>
> ---
>  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 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss

[David Lechner]

On 1/15/21 10:45 AM, Suman Anna wrote:
> + 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 <david@lechnology.com>
>> ---
>>   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?

I wasn't sure which chips exactly are "DA850". If someone can tell
me, I can look at the docs to see if they have a PRUSS.

Re: [PATCH 1/2] dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss

[Sekhar Nori]

On 17/01/21 1:48 AM, David Lechner wrote:
> On 1/15/21 10:45 AM, Suman Anna wrote:
>> + 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 <david@lechnology.com>
>>> ---
>>>   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?
> 
> I wasn't sure which chips exactly are "DA850". If someone can tell
> me, I can look at the docs to see if they have a PRUSS.

Hi David, you can treat DA850 is same as OMAP-L138 for all purposes in
kernel.

Thanks,
Sekhar

[PATCH 2/2] soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS

[David Lechner]

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 <david@lechnology.com>
---
 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
  */
 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;
 	}
 
-	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, &res)) {
-		ret = -ENOMEM;
-		goto node_put;
-	}
+		if (of_address_to_resource(child, 0, &res)) {
+			ret = -ENOMEM;
+			goto node_put;
+		}
 
-	pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
-	if (!pruss->cfg_base) {
-		ret = -ENOMEM;
-		goto node_put;
-	}
+		pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
+		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(&res) - 4;
-
-	pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
-						  &regmap_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(&res) - 4;
+
+		pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
+							  &regmap_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;
+		ret = pruss_clk_init(pruss, child);
+		if (ret) {
+			dev_err(dev, "failed to setup coreclk-mux\n");
+			goto node_put;
+		}
 	}
 
 	ret = devm_of_platform_populate(dev);
@@ -309,19 +313,27 @@ static int pruss_remove(struct platform_device *pdev)
 }
 
 /* instance-specific driver private data */
+static const struct pruss_private_data am18xx_pruss_data = {
+	.has_no_sharedram = true,
+};
+
 static const struct pruss_private_data am437x_pruss1_data = {
 	.has_no_sharedram = false,
+	.has_cfg = true,
 };
 
 static const struct pruss_private_data am437x_pruss0_data = {
 	.has_no_sharedram = true,
+	.has_cfg = true,
 };
 
 static const struct pruss_private_data am65x_j721e_pruss_data = {
 	.has_core_mux_clock = true,
+	.has_cfg = true,
 };
 
 static const struct of_device_id pruss_of_match[] = {
+	{ .compatible = "ti,am1806-pruss", .data = &am18xx_pruss_data, },
 	{ .compatible = "ti,am3356-pruss" },
 	{ .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, },
 	{ .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, },
-- 
2.25.1

Re: [PATCH 2/2] soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS

[Suman Anna]

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 <david@lechnology.com>
> ---
>  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, &res)) {
> -		ret = -ENOMEM;
> -		goto node_put;
> -	}
> +		if (of_address_to_resource(child, 0, &res)) {
> +			ret = -ENOMEM;
> +			goto node_put;
> +		}
>  
> -	pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
> -	if (!pruss->cfg_base) {
> -		ret = -ENOMEM;
> -		goto node_put;
> -	}
> +		pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
> +		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(&res) - 4;
> -
> -	pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
> -						  &regmap_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(&res) - 4;
> +
> +		pruss->cfg_regmap = devm_regmap_init_mmio(dev, pruss->cfg_base,
> +							  &regmap_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;
> +		ret = pruss_clk_init(pruss, child);
> +		if (ret) {
> +			dev_err(dev, "failed to setup coreclk-mux\n");
> +			goto node_put;
> +		}
>  	}
>  
>  	ret = devm_of_platform_populate(dev);
> @@ -309,19 +313,27 @@ static int pruss_remove(struct platform_device *pdev)
>  }
>  
>  /* instance-specific driver private data */
> +static const struct pruss_private_data am18xx_pruss_data = {
> +	.has_no_sharedram = true,
> +};
> +
>  static const struct pruss_private_data am437x_pruss1_data = {
>  	.has_no_sharedram = false,
> +	.has_cfg = true,
>  };
>  
>  static const struct pruss_private_data am437x_pruss0_data = {
>  	.has_no_sharedram = true,
> +	.has_cfg = true,
>  };
>  
>  static const struct pruss_private_data am65x_j721e_pruss_data = {
>  	.has_core_mux_clock = true,
> +	.has_cfg = true,
>  };
>  
>  static const struct of_device_id pruss_of_match[] = {
> +	{ .compatible = "ti,am1806-pruss", .data = &am18xx_pruss_data, },
>  	{ .compatible = "ti,am3356-pruss" },
>  	{ .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, },
>  	{ .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, },
> 

Re: [PATCH 2/2] soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS

[David Lechner]

On 1/15/21 6:52 PM, Suman Anna wrote:
> 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 <david@lechnology.com>
>> ---
>>   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.

Negative flags hurt my brain. :-)

I was actually thinking about submitting a patch to convert
has_no_sharedram to a positive flag as well. But I understand
the sentiment of only setting the flag true for the odd case
rather than the usual case.

> 
>>    */
>>   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.
> 

Refactoring to a separate function sounds good to me.

Re: [PATCH 0/2] Add support for TI AM18XX/OMAP-L138 PRUSS

[santosh.shilimkar]

On 1/4/21 10:30 AM, David Lechner wrote:
> This is the first step for adding support for the PRUSS on TI AM18XX/OMAP-L138
> SoCs. This series adds support in the top-level PRUSS driver. (Patches for the
> interrupt controller and individual PRUs are independent of this change and
> will be submitted separately.)
> 
> David Lechner (2):
>    dt-bindings: soc: ti: ti,pruss: add ti,am1806-pruss
>    soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS
> 
Please post updated series after fixing patch 2 as discussed with Suman.

Regards,
Santosh

RE: [PATCH 2/2] soc: ti: pruss: add support for AM18XX/OMAP-L138 PRUSS

[Hans-Frieder Vogt]

Hi David,

since you introduced a configuration parameter which was implicitly true before, the config parameter now needs to be explicitly set to true for all SoCs which have cfg registers (specifically AM33XX and AM57XX), see below.

On Mon, Jan 4, 2021, 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 <david@lechnology.com>

... shortened ...

> @@ -309,19 +313,27 @@ static int pruss_remove(struct platform_device *pdev)
>  }
>
>  /* instance-specific driver private data */
> +static const struct pruss_private_data am18xx_pruss_data = {
> +	.has_no_sharedram = true,
> +};
> +
>  static const struct pruss_private_data am437x_pruss1_data = {
>  	.has_no_sharedram = false,
> +	.has_cfg = true,
>  };
>
>  static const struct pruss_private_data am437x_pruss0_data = {
>  	.has_no_sharedram = true,
> +	.has_cfg = true,
>  };
>
>  static const struct pruss_private_data am65x_j721e_pruss_data = {
>  	.has_core_mux_clock = true,
> +	.has_cfg = true,
>  };
>
>  static const struct of_device_id pruss_of_match[] = {
> +	{ .compatible = "ti,am1806-pruss", .data = &am18xx_pruss_data, },
>  	{ .compatible = "ti,am3356-pruss" },

needs something like:
 	{ .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_pruss_data, },
 	{ .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_pruss_data, },
with
 static const struct pruss_private_data am33xx_am57xx_pruss_data = {
	.has_cfg = true,
 };

>  	{ .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, },
>  	{ .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, },

---
Best regards,
Hans-Frieder