1. Patchew
  2. linux
  3. [PATCH v4 0/5] Introduce PRU platform consumer API
  4. View patch

[PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API

MD Danish Anwar posted 5 patches 2 years, 11 months ago
There is a newer version of this series
[PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by MD Danish Anwar 2 years, 11 months ago 
From: Suman Anna <s-anna@ti.com>

Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
the PRUSS platform driver to read and program respectively a register
within the PRUSS CFG sub-module represented by a syscon driver.

These APIs are internal to PRUSS driver. Various useful registers
and macros for certain register bit-fields and their values have also
been added.

Signed-off-by: Suman Anna <s-anna@ti.com>
Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/soc/ti/pruss.c           | 39 ++++++++++++++
 include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+)

diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
index c8053c0d735f..26d8129b515c 100644
--- a/drivers/soc/ti/pruss.c
+++ b/drivers/soc/ti/pruss.c
@@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
 }
 EXPORT_SYMBOL_GPL(pruss_release_mem_region);
 
+/**
+ * pruss_cfg_read() - read a PRUSS CFG sub-module register
+ * @pruss: the pruss instance handle
+ * @reg: register offset within the CFG sub-module
+ * @val: pointer to return the value in
+ *
+ * Reads a given register within the PRUSS CFG sub-module and
+ * returns it through the passed-in @val pointer
+ *
+ * Return: 0 on success, or an error code otherwise
+ */
+static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
+{
+	if (IS_ERR_OR_NULL(pruss))
+		return -EINVAL;
+
+	return regmap_read(pruss->cfg_regmap, reg, val);
+}
+
+/**
+ * pruss_cfg_update() - configure a PRUSS CFG sub-module register
+ * @pruss: the pruss instance handle
+ * @reg: register offset within the CFG sub-module
+ * @mask: bit mask to use for programming the @val
+ * @val: value to write
+ *
+ * Programs a given register within the PRUSS CFG sub-module
+ *
+ * Return: 0 on success, or an error code otherwise
+ */
+static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
+			    unsigned int mask, unsigned int val)
+{
+	if (IS_ERR_OR_NULL(pruss))
+		return -EINVAL;
+
+	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
+}
+
 static void pruss_of_free_clk_provider(void *data)
 {
 	struct device_node *clk_mux_np = data;
diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
index 33f930e0a0ce..12ef10b9fe9a 100644
--- a/include/linux/remoteproc/pruss.h
+++ b/include/linux/remoteproc/pruss.h
@@ -10,12 +10,99 @@
 #ifndef __LINUX_PRUSS_H
 #define __LINUX_PRUSS_H
 
+#include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/types.h>
 
 #define PRU_RPROC_DRVNAME "pru-rproc"
 
+/*
+ * PRU_ICSS_CFG registers
+ * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
+ */
+#define PRUSS_CFG_REVID		0x00
+#define PRUSS_CFG_SYSCFG	0x04
+#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
+#define PRUSS_CFG_CGR		0x10
+#define PRUSS_CFG_ISRP		0x14
+#define PRUSS_CFG_ISP		0x18
+#define PRUSS_CFG_IESP		0x1C
+#define PRUSS_CFG_IECP		0x20
+#define PRUSS_CFG_SCRP		0x24
+#define PRUSS_CFG_PMAO		0x28
+#define PRUSS_CFG_MII_RT	0x2C
+#define PRUSS_CFG_IEPCLK	0x30
+#define PRUSS_CFG_SPP		0x34
+#define PRUSS_CFG_PIN_MX	0x40
+
+/* PRUSS_GPCFG register bits */
+#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
+
+#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
+#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
+
+#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
+#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
+
+#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
+#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
+#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
+
+#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
+
+#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
+#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
+
+#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
+#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
+
+#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
+#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
+#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
+
+#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
+#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
+
+#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
+#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
+
+/* PRUSS_MII_RT register bits */
+#define PRUSS_MII_RT_EVENT_EN			BIT(0)
+
+/* PRUSS_SPP register bits */
+#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
+#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)
+
+/*
+ * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the
+ * PRUSS_GPCFG0/1 registers
+ *
+ * NOTE: The below defines are the most common values, but there
+ * are some exceptions like on 66AK2G, where the RESERVED and MII2
+ * values are interchanged. Also, this bit-field does not exist on
+ * AM335x SoCs
+ */
+enum pruss_gp_mux_sel {
+	PRUSS_GP_MUX_SEL_GP = 0,
+	PRUSS_GP_MUX_SEL_ENDAT,
+	PRUSS_GP_MUX_SEL_RESERVED,
+	PRUSS_GP_MUX_SEL_SD,
+	PRUSS_GP_MUX_SEL_MII2,
+	PRUSS_GP_MUX_SEL_MAX,
+};
+
+/*
+ * enum pruss_gpi_mode - PRUSS GPI configuration modes, used
+ *			 to program the PRUSS_GPCFG0/1 registers
+ */
+enum pruss_gpi_mode {
+	PRUSS_GPI_MODE_DIRECT = 0,
+	PRUSS_GPI_MODE_PARALLEL,
+	PRUSS_GPI_MODE_28BIT_SHIFT,
+	PRUSS_GPI_MODE_MII,
+};
+
 /**
  * enum pruss_pru_id - PRU core identifiers
  * @PRUSS_PRU0: PRU Core 0.
-- 
2.25.1
Re: [PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by Roger Quadros 2 years, 11 months ago 
Danish,

On 13/03/2023 13:11, MD Danish Anwar wrote:
> From: Suman Anna <s-anna@ti.com>
> 
> Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
> the PRUSS platform driver to read and program respectively a register
> within the PRUSS CFG sub-module represented by a syscon driver.
> 
> These APIs are internal to PRUSS driver. Various useful registers
> and macros for certain register bit-fields and their values have also
> been added.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  drivers/soc/ti/pruss.c           | 39 ++++++++++++++
>  include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
> 
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index c8053c0d735f..26d8129b515c 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
>  }
>  EXPORT_SYMBOL_GPL(pruss_release_mem_region);
>  
> +/**
> + * pruss_cfg_read() - read a PRUSS CFG sub-module register
> + * @pruss: the pruss instance handle
> + * @reg: register offset within the CFG sub-module
> + * @val: pointer to return the value in
> + *
> + * Reads a given register within the PRUSS CFG sub-module and
> + * returns it through the passed-in @val pointer
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
> +{
> +	if (IS_ERR_OR_NULL(pruss))
> +		return -EINVAL;
> +
> +	return regmap_read(pruss->cfg_regmap, reg, val);
> +}
> +
> +/**
> + * pruss_cfg_update() - configure a PRUSS CFG sub-module register
> + * @pruss: the pruss instance handle
> + * @reg: register offset within the CFG sub-module
> + * @mask: bit mask to use for programming the @val
> + * @val: value to write
> + *
> + * Programs a given register within the PRUSS CFG sub-module
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
> +			    unsigned int mask, unsigned int val)
> +{
> +	if (IS_ERR_OR_NULL(pruss))
> +		return -EINVAL;
> +
> +	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
> +}
> +
>  static void pruss_of_free_clk_provider(void *data)
>  {
>  	struct device_node *clk_mux_np = data;
> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
> index 33f930e0a0ce..12ef10b9fe9a 100644
> --- a/include/linux/remoteproc/pruss.h
> +++ b/include/linux/remoteproc/pruss.h
> @@ -10,12 +10,99 @@
>  #ifndef __LINUX_PRUSS_H
>  #define __LINUX_PRUSS_H
>  
> +#include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/types.h>
>  
>  #define PRU_RPROC_DRVNAME "pru-rproc"
>  
> +/*
> + * PRU_ICSS_CFG registers
> + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
> + */
> +#define PRUSS_CFG_REVID		0x00
> +#define PRUSS_CFG_SYSCFG	0x04
> +#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
> +#define PRUSS_CFG_CGR		0x10
> +#define PRUSS_CFG_ISRP		0x14
> +#define PRUSS_CFG_ISP		0x18
> +#define PRUSS_CFG_IESP		0x1C
> +#define PRUSS_CFG_IECP		0x20
> +#define PRUSS_CFG_SCRP		0x24
> +#define PRUSS_CFG_PMAO		0x28
> +#define PRUSS_CFG_MII_RT	0x2C
> +#define PRUSS_CFG_IEPCLK	0x30
> +#define PRUSS_CFG_SPP		0x34
> +#define PRUSS_CFG_PIN_MX	0x40
> +
> +/* PRUSS_GPCFG register bits */
> +#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
> +
> +#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
> +#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
> +
> +#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
> +#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
> +
> +#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
> +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
> +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
> +
> +#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
> +
> +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
> +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
> +
> +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
> +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
> +
> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
> +
> +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
> +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
> +
> +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
> +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
> +
> +/* PRUSS_MII_RT register bits */
> +#define PRUSS_MII_RT_EVENT_EN			BIT(0)
> +
> +/* PRUSS_SPP register bits */
> +#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
> +#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)

Can we please move all the above definitions to private driver/soc/ti/pruss.h?
You can also add pruss_cfg_read and pruss_cfg_update there.

> +
> +/*
> + * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the
> + * PRUSS_GPCFG0/1 registers
> + *
> + * NOTE: The below defines are the most common values, but there
> + * are some exceptions like on 66AK2G, where the RESERVED and MII2
> + * values are interchanged. Also, this bit-field does not exist on
> + * AM335x SoCs
> + */
> +enum pruss_gp_mux_sel {
> +	PRUSS_GP_MUX_SEL_GP = 0,
> +	PRUSS_GP_MUX_SEL_ENDAT,
> +	PRUSS_GP_MUX_SEL_RESERVED,
> +	PRUSS_GP_MUX_SEL_SD,
> +	PRUSS_GP_MUX_SEL_MII2,
> +	PRUSS_GP_MUX_SEL_MAX,
> +};
> +
> +/*
> + * enum pruss_gpi_mode - PRUSS GPI configuration modes, used
> + *			 to program the PRUSS_GPCFG0/1 registers
> + */
> +enum pruss_gpi_mode {
> +	PRUSS_GPI_MODE_DIRECT = 0,
> +	PRUSS_GPI_MODE_PARALLEL,
> +	PRUSS_GPI_MODE_28BIT_SHIFT,
> +	PRUSS_GPI_MODE_MII,
> +};
> +
>  /**
>   * enum pruss_pru_id - PRU core identifiers
>   * @PRUSS_PRU0: PRU Core 0.

cheers,
-roger
Re: [EXTERNAL] Re: [PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by Md Danish Anwar 2 years, 11 months ago 
On 15/03/23 17:37, Roger Quadros wrote:
> Danish,
> 
> On 13/03/2023 13:11, MD Danish Anwar wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
>> the PRUSS platform driver to read and program respectively a register
>> within the PRUSS CFG sub-module represented by a syscon driver.
>>
>> These APIs are internal to PRUSS driver. Various useful registers
>> and macros for certain register bit-fields and their values have also
>> been added.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  drivers/soc/ti/pruss.c           | 39 ++++++++++++++
>>  include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
>>  2 files changed, 126 insertions(+)
>>
>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
>> index c8053c0d735f..26d8129b515c 100644
>> --- a/drivers/soc/ti/pruss.c
>> +++ b/drivers/soc/ti/pruss.c
>> @@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
>>  }
>>  EXPORT_SYMBOL_GPL(pruss_release_mem_region);
>>  
>> +/**
>> + * pruss_cfg_read() - read a PRUSS CFG sub-module register
>> + * @pruss: the pruss instance handle
>> + * @reg: register offset within the CFG sub-module
>> + * @val: pointer to return the value in
>> + *
>> + * Reads a given register within the PRUSS CFG sub-module and
>> + * returns it through the passed-in @val pointer
>> + *
>> + * Return: 0 on success, or an error code otherwise
>> + */
>> +static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
>> +{
>> +	if (IS_ERR_OR_NULL(pruss))
>> +		return -EINVAL;
>> +
>> +	return regmap_read(pruss->cfg_regmap, reg, val);
>> +}
>> +
>> +/**
>> + * pruss_cfg_update() - configure a PRUSS CFG sub-module register
>> + * @pruss: the pruss instance handle
>> + * @reg: register offset within the CFG sub-module
>> + * @mask: bit mask to use for programming the @val
>> + * @val: value to write
>> + *
>> + * Programs a given register within the PRUSS CFG sub-module
>> + *
>> + * Return: 0 on success, or an error code otherwise
>> + */
>> +static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
>> +			    unsigned int mask, unsigned int val)
>> +{
>> +	if (IS_ERR_OR_NULL(pruss))
>> +		return -EINVAL;
>> +
>> +	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
>> +}
>> +
>>  static void pruss_of_free_clk_provider(void *data)
>>  {
>>  	struct device_node *clk_mux_np = data;
>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
>> index 33f930e0a0ce..12ef10b9fe9a 100644
>> --- a/include/linux/remoteproc/pruss.h
>> +++ b/include/linux/remoteproc/pruss.h
>> @@ -10,12 +10,99 @@
>>  #ifndef __LINUX_PRUSS_H
>>  #define __LINUX_PRUSS_H
>>  
>> +#include <linux/bits.h>
>>  #include <linux/device.h>
>>  #include <linux/err.h>
>>  #include <linux/types.h>
>>  
>>  #define PRU_RPROC_DRVNAME "pru-rproc"
>>  
>> +/*
>> + * PRU_ICSS_CFG registers
>> + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
>> + */
>> +#define PRUSS_CFG_REVID		0x00
>> +#define PRUSS_CFG_SYSCFG	0x04
>> +#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
>> +#define PRUSS_CFG_CGR		0x10
>> +#define PRUSS_CFG_ISRP		0x14
>> +#define PRUSS_CFG_ISP		0x18
>> +#define PRUSS_CFG_IESP		0x1C
>> +#define PRUSS_CFG_IECP		0x20
>> +#define PRUSS_CFG_SCRP		0x24
>> +#define PRUSS_CFG_PMAO		0x28
>> +#define PRUSS_CFG_MII_RT	0x2C
>> +#define PRUSS_CFG_IEPCLK	0x30
>> +#define PRUSS_CFG_SPP		0x34
>> +#define PRUSS_CFG_PIN_MX	0x40
>> +
>> +/* PRUSS_GPCFG register bits */
>> +#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
>> +
>> +#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
>> +#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
>> +
>> +#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
>> +#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
>> +
>> +#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
>> +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
>> +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
>> +
>> +#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
>> +
>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
>> +
>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
>> +
>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
>> +
>> +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
>> +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
>> +
>> +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
>> +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
>> +
>> +/* PRUSS_MII_RT register bits */
>> +#define PRUSS_MII_RT_EVENT_EN			BIT(0)
>> +
>> +/* PRUSS_SPP register bits */
>> +#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
>> +#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)
> 
> Can we please move all the above definitions to private driver/soc/ti/pruss.h?
> You can also add pruss_cfg_read and pruss_cfg_update there.
> 

Sure Roger, I'll move all these definitions to pruss.h

>> +
>> +/*
>> + * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the
>> + * PRUSS_GPCFG0/1 registers
>> + *
>> + * NOTE: The below defines are the most common values, but there
>> + * are some exceptions like on 66AK2G, where the RESERVED and MII2
>> + * values are interchanged. Also, this bit-field does not exist on
>> + * AM335x SoCs
>> + */
>> +enum pruss_gp_mux_sel {
>> +	PRUSS_GP_MUX_SEL_GP = 0,
>> +	PRUSS_GP_MUX_SEL_ENDAT,
>> +	PRUSS_GP_MUX_SEL_RESERVED,
>> +	PRUSS_GP_MUX_SEL_SD,
>> +	PRUSS_GP_MUX_SEL_MII2,
>> +	PRUSS_GP_MUX_SEL_MAX,
>> +};
>> +
>> +/*
>> + * enum pruss_gpi_mode - PRUSS GPI configuration modes, used
>> + *			 to program the PRUSS_GPCFG0/1 registers
>> + */
>> +enum pruss_gpi_mode {
>> +	PRUSS_GPI_MODE_DIRECT = 0,
>> +	PRUSS_GPI_MODE_PARALLEL,
>> +	PRUSS_GPI_MODE_28BIT_SHIFT,
>> +	PRUSS_GPI_MODE_MII,
>> +};
>> +
>>  /**
>>   * enum pruss_pru_id - PRU core identifiers
>>   * @PRUSS_PRU0: PRU Core 0.
> 
> cheers,
> -roger

-- 
Thanks and Regards,
Danish.
Re: [EXTERNAL] Re: [PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by Md Danish Anwar 2 years, 11 months ago 
Roger,

On 16/03/23 16:38, Md Danish Anwar wrote:
> 
> On 15/03/23 17:37, Roger Quadros wrote:
>> Danish,
>>
>> On 13/03/2023 13:11, MD Danish Anwar wrote:
>>> From: Suman Anna <s-anna@ti.com>
>>>
>>> Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
>>> the PRUSS platform driver to read and program respectively a register
>>> within the PRUSS CFG sub-module represented by a syscon driver.
>>>
>>> These APIs are internal to PRUSS driver. Various useful registers
>>> and macros for certain register bit-fields and their values have also
>>> been added.
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  drivers/soc/ti/pruss.c           | 39 ++++++++++++++
>>>  include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
>>>  2 files changed, 126 insertions(+)
>>>
>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
>>> index c8053c0d735f..26d8129b515c 100644
>>> --- a/drivers/soc/ti/pruss.c
>>> +++ b/drivers/soc/ti/pruss.c
>>> @@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
>>>  }
>>>  EXPORT_SYMBOL_GPL(pruss_release_mem_region);
>>>  
>>> +/**
>>> + * pruss_cfg_read() - read a PRUSS CFG sub-module register
>>> + * @pruss: the pruss instance handle
>>> + * @reg: register offset within the CFG sub-module
>>> + * @val: pointer to return the value in
>>> + *
>>> + * Reads a given register within the PRUSS CFG sub-module and
>>> + * returns it through the passed-in @val pointer
>>> + *
>>> + * Return: 0 on success, or an error code otherwise
>>> + */
>>> +static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
>>> +{
>>> +	if (IS_ERR_OR_NULL(pruss))
>>> +		return -EINVAL;
>>> +
>>> +	return regmap_read(pruss->cfg_regmap, reg, val);
>>> +}
>>> +
>>> +/**
>>> + * pruss_cfg_update() - configure a PRUSS CFG sub-module register
>>> + * @pruss: the pruss instance handle
>>> + * @reg: register offset within the CFG sub-module
>>> + * @mask: bit mask to use for programming the @val
>>> + * @val: value to write
>>> + *
>>> + * Programs a given register within the PRUSS CFG sub-module
>>> + *
>>> + * Return: 0 on success, or an error code otherwise
>>> + */
>>> +static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
>>> +			    unsigned int mask, unsigned int val)
>>> +{
>>> +	if (IS_ERR_OR_NULL(pruss))
>>> +		return -EINVAL;
>>> +
>>> +	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
>>> +}
>>> +
>>>  static void pruss_of_free_clk_provider(void *data)
>>>  {
>>>  	struct device_node *clk_mux_np = data;
>>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
>>> index 33f930e0a0ce..12ef10b9fe9a 100644
>>> --- a/include/linux/remoteproc/pruss.h
>>> +++ b/include/linux/remoteproc/pruss.h
>>> @@ -10,12 +10,99 @@
>>>  #ifndef __LINUX_PRUSS_H
>>>  #define __LINUX_PRUSS_H
>>>  
>>> +#include <linux/bits.h>
>>>  #include <linux/device.h>
>>>  #include <linux/err.h>
>>>  #include <linux/types.h>
>>>  
>>>  #define PRU_RPROC_DRVNAME "pru-rproc"
>>>  
>>> +/*
>>> + * PRU_ICSS_CFG registers
>>> + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
>>> + */
>>> +#define PRUSS_CFG_REVID		0x00
>>> +#define PRUSS_CFG_SYSCFG	0x04
>>> +#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
>>> +#define PRUSS_CFG_CGR		0x10
>>> +#define PRUSS_CFG_ISRP		0x14
>>> +#define PRUSS_CFG_ISP		0x18
>>> +#define PRUSS_CFG_IESP		0x1C
>>> +#define PRUSS_CFG_IECP		0x20
>>> +#define PRUSS_CFG_SCRP		0x24
>>> +#define PRUSS_CFG_PMAO		0x28
>>> +#define PRUSS_CFG_MII_RT	0x2C
>>> +#define PRUSS_CFG_IEPCLK	0x30
>>> +#define PRUSS_CFG_SPP		0x34
>>> +#define PRUSS_CFG_PIN_MX	0x40
>>> +
>>> +/* PRUSS_GPCFG register bits */
>>> +#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
>>> +
>>> +#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
>>> +#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
>>> +
>>> +#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
>>> +#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
>>> +
>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
>>> +
>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
>>> +
>>> +/* PRUSS_MII_RT register bits */
>>> +#define PRUSS_MII_RT_EVENT_EN			BIT(0)
>>> +
>>> +/* PRUSS_SPP register bits */
>>> +#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
>>> +#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)
>>
>> Can we please move all the above definitions to private driver/soc/ti/pruss.h?
>> You can also add pruss_cfg_read and pruss_cfg_update there.
>>

There is no driver/soc/ti/pruss.h. The pruss.h file is located in
include/linux/remoteproc/pruss.h and there is one pruss_driver.h file which is
located in include/linux/pruss_driver.h

Do you want me to create another header file at driver/soc/ti/pruss.h and place
all these definitions inside that?

Please let me know.

> 
> Sure Roger, I'll move all these definitions to pruss.h
> 
>>> +
>>> +/*
>>> + * enum pruss_gp_mux_sel - PRUSS GPI/O Mux modes for the
>>> + * PRUSS_GPCFG0/1 registers
>>> + *
>>> + * NOTE: The below defines are the most common values, but there
>>> + * are some exceptions like on 66AK2G, where the RESERVED and MII2
>>> + * values are interchanged. Also, this bit-field does not exist on
>>> + * AM335x SoCs
>>> + */
>>> +enum pruss_gp_mux_sel {
>>> +	PRUSS_GP_MUX_SEL_GP = 0,
>>> +	PRUSS_GP_MUX_SEL_ENDAT,
>>> +	PRUSS_GP_MUX_SEL_RESERVED,
>>> +	PRUSS_GP_MUX_SEL_SD,
>>> +	PRUSS_GP_MUX_SEL_MII2,
>>> +	PRUSS_GP_MUX_SEL_MAX,
>>> +};
>>> +
>>> +/*
>>> + * enum pruss_gpi_mode - PRUSS GPI configuration modes, used
>>> + *			 to program the PRUSS_GPCFG0/1 registers
>>> + */
>>> +enum pruss_gpi_mode {
>>> +	PRUSS_GPI_MODE_DIRECT = 0,
>>> +	PRUSS_GPI_MODE_PARALLEL,
>>> +	PRUSS_GPI_MODE_28BIT_SHIFT,
>>> +	PRUSS_GPI_MODE_MII,
>>> +};
>>> +
>>>  /**
>>>   * enum pruss_pru_id - PRU core identifiers
>>>   * @PRUSS_PRU0: PRU Core 0.
>>
>> cheers,
>> -roger
> 

-- 
Thanks and Regards,
Danish.
Re: [EXTERNAL] Re: [PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by Roger Quadros 2 years, 11 months ago 

On 16/03/2023 13:29, Md Danish Anwar wrote:
> Roger,
> 
> On 16/03/23 16:38, Md Danish Anwar wrote:
>>
>> On 15/03/23 17:37, Roger Quadros wrote:
>>> Danish,
>>>
>>> On 13/03/2023 13:11, MD Danish Anwar wrote:
>>>> From: Suman Anna <s-anna@ti.com>
>>>>
>>>> Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
>>>> the PRUSS platform driver to read and program respectively a register
>>>> within the PRUSS CFG sub-module represented by a syscon driver.
>>>>
>>>> These APIs are internal to PRUSS driver. Various useful registers
>>>> and macros for certain register bit-fields and their values have also
>>>> been added.
>>>>
>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>  drivers/soc/ti/pruss.c           | 39 ++++++++++++++
>>>>  include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
>>>>  2 files changed, 126 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
>>>> index c8053c0d735f..26d8129b515c 100644
>>>> --- a/drivers/soc/ti/pruss.c
>>>> +++ b/drivers/soc/ti/pruss.c
>>>> @@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(pruss_release_mem_region);
>>>>  
>>>> +/**
>>>> + * pruss_cfg_read() - read a PRUSS CFG sub-module register
>>>> + * @pruss: the pruss instance handle
>>>> + * @reg: register offset within the CFG sub-module
>>>> + * @val: pointer to return the value in
>>>> + *
>>>> + * Reads a given register within the PRUSS CFG sub-module and
>>>> + * returns it through the passed-in @val pointer
>>>> + *
>>>> + * Return: 0 on success, or an error code otherwise
>>>> + */
>>>> +static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
>>>> +{
>>>> +	if (IS_ERR_OR_NULL(pruss))
>>>> +		return -EINVAL;
>>>> +
>>>> +	return regmap_read(pruss->cfg_regmap, reg, val);
>>>> +}
>>>> +
>>>> +/**
>>>> + * pruss_cfg_update() - configure a PRUSS CFG sub-module register
>>>> + * @pruss: the pruss instance handle
>>>> + * @reg: register offset within the CFG sub-module
>>>> + * @mask: bit mask to use for programming the @val
>>>> + * @val: value to write
>>>> + *
>>>> + * Programs a given register within the PRUSS CFG sub-module
>>>> + *
>>>> + * Return: 0 on success, or an error code otherwise
>>>> + */
>>>> +static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
>>>> +			    unsigned int mask, unsigned int val)
>>>> +{
>>>> +	if (IS_ERR_OR_NULL(pruss))
>>>> +		return -EINVAL;
>>>> +
>>>> +	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
>>>> +}
>>>> +
>>>>  static void pruss_of_free_clk_provider(void *data)
>>>>  {
>>>>  	struct device_node *clk_mux_np = data;
>>>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
>>>> index 33f930e0a0ce..12ef10b9fe9a 100644
>>>> --- a/include/linux/remoteproc/pruss.h
>>>> +++ b/include/linux/remoteproc/pruss.h
>>>> @@ -10,12 +10,99 @@
>>>>  #ifndef __LINUX_PRUSS_H
>>>>  #define __LINUX_PRUSS_H
>>>>  
>>>> +#include <linux/bits.h>
>>>>  #include <linux/device.h>
>>>>  #include <linux/err.h>
>>>>  #include <linux/types.h>
>>>>  
>>>>  #define PRU_RPROC_DRVNAME "pru-rproc"
>>>>  
>>>> +/*
>>>> + * PRU_ICSS_CFG registers
>>>> + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
>>>> + */
>>>> +#define PRUSS_CFG_REVID		0x00
>>>> +#define PRUSS_CFG_SYSCFG	0x04
>>>> +#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
>>>> +#define PRUSS_CFG_CGR		0x10
>>>> +#define PRUSS_CFG_ISRP		0x14
>>>> +#define PRUSS_CFG_ISP		0x18
>>>> +#define PRUSS_CFG_IESP		0x1C
>>>> +#define PRUSS_CFG_IECP		0x20
>>>> +#define PRUSS_CFG_SCRP		0x24
>>>> +#define PRUSS_CFG_PMAO		0x28
>>>> +#define PRUSS_CFG_MII_RT	0x2C
>>>> +#define PRUSS_CFG_IEPCLK	0x30
>>>> +#define PRUSS_CFG_SPP		0x34
>>>> +#define PRUSS_CFG_PIN_MX	0x40
>>>> +
>>>> +/* PRUSS_GPCFG register bits */
>>>> +#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
>>>> +#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
>>>> +#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
>>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
>>>> +
>>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
>>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
>>>> +
>>>> +/* PRUSS_MII_RT register bits */
>>>> +#define PRUSS_MII_RT_EVENT_EN			BIT(0)
>>>> +
>>>> +/* PRUSS_SPP register bits */
>>>> +#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
>>>> +#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)
>>>
>>> Can we please move all the above definitions to private driver/soc/ti/pruss.h?
>>> You can also add pruss_cfg_read and pruss_cfg_update there.
>>>
> 
> There is no driver/soc/ti/pruss.h. The pruss.h file is located in
> include/linux/remoteproc/pruss.h and there is one pruss_driver.h file which is
> located in include/linux/pruss_driver.h
> 
> Do you want me to create another header file at driver/soc/ti/pruss.h and place
> all these definitions inside that?
> 
> Please let me know.

Yes. All private definitions should sit in driver/soc/ti/pruss.h


cheers,
roger
Re: [EXTERNAL] Re: [EXTERNAL] Re: [PATCH v4 3/5] soc: ti: pruss: Add pruss_cfg_read()/update() API
Posted by Md Danish Anwar 2 years, 11 months ago 
On 16/03/23 17:02, Roger Quadros wrote:
> 
> 
> On 16/03/2023 13:29, Md Danish Anwar wrote:
>> Roger,
>>
>> On 16/03/23 16:38, Md Danish Anwar wrote:
>>>
>>> On 15/03/23 17:37, Roger Quadros wrote:
>>>> Danish,
>>>>
>>>> On 13/03/2023 13:11, MD Danish Anwar wrote:
>>>>> From: Suman Anna <s-anna@ti.com>
>>>>>
>>>>> Add two new generic API pruss_cfg_read() and pruss_cfg_update() to
>>>>> the PRUSS platform driver to read and program respectively a register
>>>>> within the PRUSS CFG sub-module represented by a syscon driver.
>>>>>
>>>>> These APIs are internal to PRUSS driver. Various useful registers
>>>>> and macros for certain register bit-fields and their values have also
>>>>> been added.
>>>>>
>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>>>>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>> ---
>>>>>  drivers/soc/ti/pruss.c           | 39 ++++++++++++++
>>>>>  include/linux/remoteproc/pruss.h | 87 ++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 126 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
>>>>> index c8053c0d735f..26d8129b515c 100644
>>>>> --- a/drivers/soc/ti/pruss.c
>>>>> +++ b/drivers/soc/ti/pruss.c
>>>>> @@ -164,6 +164,45 @@ int pruss_release_mem_region(struct pruss *pruss,
>>>>>  }
>>>>>  EXPORT_SYMBOL_GPL(pruss_release_mem_region);
>>>>>  
>>>>> +/**
>>>>> + * pruss_cfg_read() - read a PRUSS CFG sub-module register
>>>>> + * @pruss: the pruss instance handle
>>>>> + * @reg: register offset within the CFG sub-module
>>>>> + * @val: pointer to return the value in
>>>>> + *
>>>>> + * Reads a given register within the PRUSS CFG sub-module and
>>>>> + * returns it through the passed-in @val pointer
>>>>> + *
>>>>> + * Return: 0 on success, or an error code otherwise
>>>>> + */
>>>>> +static int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val)
>>>>> +{
>>>>> +	if (IS_ERR_OR_NULL(pruss))
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	return regmap_read(pruss->cfg_regmap, reg, val);
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * pruss_cfg_update() - configure a PRUSS CFG sub-module register
>>>>> + * @pruss: the pruss instance handle
>>>>> + * @reg: register offset within the CFG sub-module
>>>>> + * @mask: bit mask to use for programming the @val
>>>>> + * @val: value to write
>>>>> + *
>>>>> + * Programs a given register within the PRUSS CFG sub-module
>>>>> + *
>>>>> + * Return: 0 on success, or an error code otherwise
>>>>> + */
>>>>> +static int pruss_cfg_update(struct pruss *pruss, unsigned int reg,
>>>>> +			    unsigned int mask, unsigned int val)
>>>>> +{
>>>>> +	if (IS_ERR_OR_NULL(pruss))
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	return regmap_update_bits(pruss->cfg_regmap, reg, mask, val);
>>>>> +}
>>>>> +
>>>>>  static void pruss_of_free_clk_provider(void *data)
>>>>>  {
>>>>>  	struct device_node *clk_mux_np = data;
>>>>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h
>>>>> index 33f930e0a0ce..12ef10b9fe9a 100644
>>>>> --- a/include/linux/remoteproc/pruss.h
>>>>> +++ b/include/linux/remoteproc/pruss.h
>>>>> @@ -10,12 +10,99 @@
>>>>>  #ifndef __LINUX_PRUSS_H
>>>>>  #define __LINUX_PRUSS_H
>>>>>  
>>>>> +#include <linux/bits.h>
>>>>>  #include <linux/device.h>
>>>>>  #include <linux/err.h>
>>>>>  #include <linux/types.h>
>>>>>  
>>>>>  #define PRU_RPROC_DRVNAME "pru-rproc"
>>>>>  
>>>>> +/*
>>>>> + * PRU_ICSS_CFG registers
>>>>> + * SYSCFG, ISRP, ISP, IESP, IECP, SCRP applicable on AMxxxx devices only
>>>>> + */
>>>>> +#define PRUSS_CFG_REVID		0x00
>>>>> +#define PRUSS_CFG_SYSCFG	0x04
>>>>> +#define PRUSS_CFG_GPCFG(x)	(0x08 + (x) * 4)
>>>>> +#define PRUSS_CFG_CGR		0x10
>>>>> +#define PRUSS_CFG_ISRP		0x14
>>>>> +#define PRUSS_CFG_ISP		0x18
>>>>> +#define PRUSS_CFG_IESP		0x1C
>>>>> +#define PRUSS_CFG_IECP		0x20
>>>>> +#define PRUSS_CFG_SCRP		0x24
>>>>> +#define PRUSS_CFG_PMAO		0x28
>>>>> +#define PRUSS_CFG_MII_RT	0x2C
>>>>> +#define PRUSS_CFG_IEPCLK	0x30
>>>>> +#define PRUSS_CFG_SPP		0x34
>>>>> +#define PRUSS_CFG_PIN_MX	0x40
>>>>> +
>>>>> +/* PRUSS_GPCFG register bits */
>>>>> +#define PRUSS_GPCFG_PRU_GPO_SH_SEL		BIT(25)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_DIV1_SHIFT		20
>>>>> +#define PRUSS_GPCFG_PRU_DIV1_MASK		GENMASK(24, 20)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_DIV0_SHIFT		15
>>>>> +#define PRUSS_GPCFG_PRU_DIV0_MASK		GENMASK(15, 19)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE		BIT(14)
>>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_DIRECT		0
>>>>> +#define PRUSS_GPCFG_PRU_GPO_MODE_SERIAL		BIT(14)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPI_SB			BIT(13)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_SHIFT		8
>>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV1_MASK		GENMASK(12, 8)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_SHIFT		3
>>>>> +#define PRUSS_GPCFG_PRU_GPI_DIV0_MASK		GENMASK(7, 3)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_POSITIVE	0
>>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE_NEGATIVE	BIT(2)
>>>>> +#define PRUSS_GPCFG_PRU_GPI_CLK_MODE		BIT(2)
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_MASK		GENMASK(1, 0)
>>>>> +#define PRUSS_GPCFG_PRU_GPI_MODE_SHIFT		0
>>>>> +
>>>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_SHIFT		26
>>>>> +#define PRUSS_GPCFG_PRU_MUX_SEL_MASK		GENMASK(29, 26)
>>>>> +
>>>>> +/* PRUSS_MII_RT register bits */
>>>>> +#define PRUSS_MII_RT_EVENT_EN			BIT(0)
>>>>> +
>>>>> +/* PRUSS_SPP register bits */
>>>>> +#define PRUSS_SPP_XFER_SHIFT_EN			BIT(1)
>>>>> +#define PRUSS_SPP_PRU1_PAD_HP_EN		BIT(0)
>>>>
>>>> Can we please move all the above definitions to private driver/soc/ti/pruss.h?
>>>> You can also add pruss_cfg_read and pruss_cfg_update there.
>>>>
>>
>> There is no driver/soc/ti/pruss.h. The pruss.h file is located in
>> include/linux/remoteproc/pruss.h and there is one pruss_driver.h file which is
>> located in include/linux/pruss_driver.h
>>
>> Do you want me to create another header file at driver/soc/ti/pruss.h and place
>> all these definitions inside that?
>>
>> Please let me know.
> 
> Yes. All private definitions should sit in driver/soc/ti/pruss.h
> 

Should I keep the change the name to pruss_internal.h? As we already have a
pruss.h inside include/linux/remoteproc, just to avoid any confusion.

> 
> cheers,
> roger

-- 
Thanks and Regards,
Danish.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.