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

From: David Lechner
Date: Sat Jan 16 2021 - 15:19:12 EST

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@xxxxxxxxxxxxxx>
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
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)

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.