[PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[MD Danish Anwar]

From: Tero Kristo <t-kristo@ti.com>

The GPMUX config value for a PRU device can now be configured by client
by specifying it in the device node ti,pruss-gp-mux-sel.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/remoteproc/pru_rproc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 5b2dc75836fc..327f0c7ee3d6 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -110,6 +110,7 @@ struct pru_private_data {
  * @dbg_single_step: debug state variable to set PRU into single step mode
  * @dbg_continuous: debug state variable to restore PRU execution mode
  * @evt_count: number of mapped events
+ * @gpmux_save: saved value for gpmux config
  */
 struct pru_rproc {
 	int id;
@@ -128,6 +129,7 @@ struct pru_rproc {
 	u32 dbg_single_step;
 	u32 dbg_continuous;
 	u8 evt_count;
+	u8 gpmux_save;
 };
 
 static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int reg)
@@ -229,6 +231,7 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
 	struct device *dev;
 	const char *fw_name;
 	int ret;
+	u32 mux;
 
 	rproc = __pru_rproc_get(np, index);
 	if (IS_ERR(rproc))
@@ -253,6 +256,23 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
 	if (pru_id)
 		*pru_id = pru->id;
 
+	ret = pruss_cfg_get_gpmux(pru->pruss, pru->id, &pru->gpmux_save);
+	if (ret) {
+		dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
+		goto err;
+	}
+
+	/* An error here is acceptable for backward compatibility */
+	ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
+					 &mux);
+	if (!ret) {
+		ret = pruss_cfg_set_gpmux(pru->pruss, pru->id, mux);
+		if (ret) {
+			dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
+			goto err;
+		}
+	}
+
 	ret = of_property_read_string_index(np, "firmware-name", index,
 					    &fw_name);
 	if (!ret) {
@@ -291,6 +311,8 @@ void pru_rproc_put(struct rproc *rproc)
 
 	pru = rproc->priv;
 
+	pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
+
 	pru_rproc_set_firmware(rproc, NULL);
 
 	mutex_lock(&pru->lock);
-- 
2.34.1

Re: [PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[Mathieu Poirier]

On Wed, Aug 02, 2023 at 12:19:25PM +0530, MD Danish Anwar wrote:
> From: Tero Kristo <t-kristo@ti.com>
> 
> The GPMUX config value for a PRU device can now be configured by client
> by specifying it in the device node ti,pruss-gp-mux-sel.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  drivers/remoteproc/pru_rproc.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>

Applied.

Thanks,
Mathieu

> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index 5b2dc75836fc..327f0c7ee3d6 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -110,6 +110,7 @@ struct pru_private_data {
>   * @dbg_single_step: debug state variable to set PRU into single step mode
>   * @dbg_continuous: debug state variable to restore PRU execution mode
>   * @evt_count: number of mapped events
> + * @gpmux_save: saved value for gpmux config
>   */
>  struct pru_rproc {
>  	int id;
> @@ -128,6 +129,7 @@ struct pru_rproc {
>  	u32 dbg_single_step;
>  	u32 dbg_continuous;
>  	u8 evt_count;
> +	u8 gpmux_save;
>  };
>  
>  static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int reg)
> @@ -229,6 +231,7 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
>  	struct device *dev;
>  	const char *fw_name;
>  	int ret;
> +	u32 mux;
>  
>  	rproc = __pru_rproc_get(np, index);
>  	if (IS_ERR(rproc))
> @@ -253,6 +256,23 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
>  	if (pru_id)
>  		*pru_id = pru->id;
>  
> +	ret = pruss_cfg_get_gpmux(pru->pruss, pru->id, &pru->gpmux_save);
> +	if (ret) {
> +		dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
> +		goto err;
> +	}
> +
> +	/* An error here is acceptable for backward compatibility */
> +	ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
> +					 &mux);
> +	if (!ret) {
> +		ret = pruss_cfg_set_gpmux(pru->pruss, pru->id, mux);
> +		if (ret) {
> +			dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
>  	ret = of_property_read_string_index(np, "firmware-name", index,
>  					    &fw_name);
>  	if (!ret) {
> @@ -291,6 +311,8 @@ void pru_rproc_put(struct rproc *rproc)
>  
>  	pru = rproc->priv;
>  
> +	pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
> +
>  	pru_rproc_set_firmware(rproc, NULL);
>  
>  	mutex_lock(&pru->lock);
> -- 
> 2.34.1
> 

[PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[MD Danish Anwar]

From: Tero Kristo <t-kristo@ti.com>

Client device node property ti,pruss-gp-mux-sel can now be used to
configure the GPMUX config value for PRU.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/remoteproc/pru_rproc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
index 2874c8d324f7..29d3a5a930c1 100644
--- a/drivers/remoteproc/pru_rproc.c
+++ b/drivers/remoteproc/pru_rproc.c
@@ -109,6 +109,7 @@ struct pru_private_data {
  * @dbg_single_step: debug state variable to set PRU into single step mode
  * @dbg_continuous: debug state variable to restore PRU execution mode
  * @evt_count: number of mapped events
+ * @gpmux_save: saved value for gpmux config
  */
 struct pru_rproc {
 	int id;
@@ -127,6 +128,7 @@ struct pru_rproc {
 	u32 dbg_single_step;
 	u32 dbg_continuous;
 	u8 evt_count;
+	u8 gpmux_save;
 };
 
 static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int reg)
@@ -228,6 +230,7 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
 	struct device *dev;
 	const char *fw_name;
 	int ret;
+	u32 mux;
 
 	rproc = __pru_rproc_get(np, index);
 	if (IS_ERR(rproc))
@@ -252,6 +255,22 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
 	if (pru_id)
 		*pru_id = pru->id;
 
+	ret = pruss_cfg_get_gpmux(pru->pruss, pru->id, &pru->gpmux_save);
+	if (ret) {
+		dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
+		goto err;
+	}
+
+	ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
+					 &mux);
+	if (!ret) {
+		ret = pruss_cfg_set_gpmux(pru->pruss, pru->id, mux);
+		if (ret) {
+			dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
+			goto err;
+		}
+	}
+
 	ret = of_property_read_string_index(np, "firmware-name", index,
 					    &fw_name);
 	if (!ret) {
@@ -290,6 +309,8 @@ void pru_rproc_put(struct rproc *rproc)
 
 	pru = rproc->priv;
 
+	pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
+
 	pru_rproc_set_firmware(rproc, NULL);
 
 	mutex_lock(&pru->lock);
-- 
2.34.1

Re: [PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[Mathieu Poirier]

Hi MD,

On Thu, Jun 01, 2023 at 04:29:04PM +0530, MD Danish Anwar wrote:
> From: Tero Kristo <t-kristo@ti.com>
> 
> Client device node property ti,pruss-gp-mux-sel can now be used to
> configure the GPMUX config value for PRU.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  drivers/remoteproc/pru_rproc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index 2874c8d324f7..29d3a5a930c1 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -109,6 +109,7 @@ struct pru_private_data {
>   * @dbg_single_step: debug state variable to set PRU into single step mode
>   * @dbg_continuous: debug state variable to restore PRU execution mode
>   * @evt_count: number of mapped events
> + * @gpmux_save: saved value for gpmux config
>   */
>  struct pru_rproc {
>  	int id;
> @@ -127,6 +128,7 @@ struct pru_rproc {
>  	u32 dbg_single_step;
>  	u32 dbg_continuous;
>  	u8 evt_count;
> +	u8 gpmux_save;
>  };
>  
>  static inline u32 pru_control_read_reg(struct pru_rproc *pru, unsigned int reg)
> @@ -228,6 +230,7 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
>  	struct device *dev;
>  	const char *fw_name;
>  	int ret;
> +	u32 mux;
>  
>  	rproc = __pru_rproc_get(np, index);
>  	if (IS_ERR(rproc))
> @@ -252,6 +255,22 @@ struct rproc *pru_rproc_get(struct device_node *np, int index,
>  	if (pru_id)
>  		*pru_id = pru->id;
>  
> +	ret = pruss_cfg_get_gpmux(pru->pruss, pru->id, &pru->gpmux_save);
> +	if (ret) {
> +		dev_err(dev, "failed to get cfg gpmux: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = of_property_read_u32_index(np, "ti,pruss-gp-mux-sel", index,
> +					 &mux);
> +	if (!ret) {
> +		ret = pruss_cfg_set_gpmux(pru->pruss, pru->id, mux);
> +		if (ret) {
> +			dev_err(dev, "failed to set cfg gpmux: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +

It would have been nice to be told in a cover letter that pruss_cfg_get_gpmux()
is in linux-next so that I don't have to go fish for it...

I am fine with the code in this patch, though the changelog is cryptic and could
be enhanced to say "why" this is needed.  The above could use some comments to
make sure people looking at this code understand that an error from
of_property_read_u32_index() is acceptable for backward compatibility.

Here I have to suppose pruss_cfg_get_gpmux() has been added to Nishanth's tree.
As such the only way for me to apply your patch is if Nishanth sends me a pull
request for the patchset that introduced pruss_cfg_get_gpmux().  You can also
resend this in the next cycle.

Thanks,
Mathieu

>  	ret = of_property_read_string_index(np, "firmware-name", index,
>  					    &fw_name);
>  	if (!ret) {
> @@ -290,6 +309,8 @@ void pru_rproc_put(struct rproc *rproc)
>  
>  	pru = rproc->priv;
>  
> +	pruss_cfg_set_gpmux(pru->pruss, pru->id, pru->gpmux_save);
> +
>  	pru_rproc_set_firmware(rproc, NULL);
>  
>  	mutex_lock(&pru->lock);
> -- 
> 2.34.1
> 

Re: [PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[Nishanth Menon]

On 11:26-20230605, Mathieu Poirier wrote:
[...]
> Here I have to suppose pruss_cfg_get_gpmux() has been added to Nishanth's tree.
> As such the only way for me to apply your patch is if Nishanth sends me a pull
> request for the patchset that introduced pruss_cfg_get_gpmux().  You can also
> resend this in the next cycle.

Yes, I had pulled this in [1] along with a few other driver fixes. I
can send you an immutable tag to pull for your tree, if that helps the
process along though, it might have a unrelated driver fixup patch as
part of the series though..

[1] https://lore.kernel.org/all/168434617580.1538524.11482827517408254591.b4-ty@ti.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/log/?h=ti-drivers-soc-next
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH] remoteproc: pru: add support for configuring GPMUX based on client setup

[Mathieu Poirier]

On Mon, 5 Jun 2023 at 14:45, Nishanth Menon <nm@ti.com> wrote:
>
> On 11:26-20230605, Mathieu Poirier wrote:
> [...]
> > Here I have to suppose pruss_cfg_get_gpmux() has been added to Nishanth's tree.
> > As such the only way for me to apply your patch is if Nishanth sends me a pull
> > request for the patchset that introduced pruss_cfg_get_gpmux().  You can also
> > resend this in the next cycle.
>
> Yes, I had pulled this in [1] along with a few other driver fixes. I
> can send you an immutable tag to pull for your tree, if that helps the
> process along though, it might have a unrelated driver fixup patch as
> part of the series though..
>

MD has indicated he would rework his patch and send a new revision in
the next cycle.

> [1] https://lore.kernel.org/all/168434617580.1538524.11482827517408254591.b4-ty@ti.com/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/log/?h=ti-drivers-soc-next
> --
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D