| Message ID | 20230306110934.2736465-6-danishanwar@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introduce PRU platform consumer API | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 06/03/2023 13:09, MD Danish Anwar wrote: > From: Suman Anna <s-anna@ti.com> > > The PRU-ICSS subsystem on OMAP-architecture based SoCS (AM33xx, AM437x > and AM57xx SoCs) has a control bit STANDBY_INIT in the PRUSS_CFG register > to initiate a Standby sequence (when set) and trigger a MStandby request > to the SoC's PRCM module. This same bit is also used to enable the OCP > master ports (when cleared). The clearing of the STANDBY_INIT bit requires > an acknowledgment from PRCM and is done through the monitoring of the > PRUSS_SYSCFG.SUB_MWAIT bit. > > Add a helper function pruss_cfg_ocp_master_ports() to allow the PRU > client drivers to control this bit and enable or disable the firmware > running on PRU cores access to any peripherals or memory to achieve > desired functionality. The access is disabled by default on power-up > and on any suspend (context is not maintained). > > 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> > --- > drivers/soc/ti/pruss.c | 81 +++++++++++++++++++++++++++++++- > include/linux/remoteproc/pruss.h | 6 +++ > 2 files changed, 85 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > index 537a3910ffd8..dc3abda0b8c2 100644 > --- a/drivers/soc/ti/pruss.c > +++ b/drivers/soc/ti/pruss.c > @@ -22,14 +22,19 @@ > #include <linux/remoteproc.h> > #include <linux/slab.h> > > +#define SYSCFG_STANDBY_INIT BIT(4) > +#define SYSCFG_SUB_MWAIT_READY BIT(5) > + > /** > * 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_ocp_syscfg: flag to indicate if OCP SYSCFG is present > */ > struct pruss_private_data { > bool has_no_sharedram; > bool has_core_mux_clock; > + bool has_ocp_syscfg; > }; > > /** > @@ -205,6 +210,72 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > } > EXPORT_SYMBOL_GPL(pruss_cfg_update); > > +/** > + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports > + * @pruss: the pruss instance handle > + * @enable: set to true for enabling or false for disabling the OCP master ports > + * > + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or > + * disable the OCP master ports (applicable only on SoCs using OCP interconnect > + * like the OMAP family). Clearing the bit achieves dual functionalities - one > + * is to deassert the MStandby signal to the device PRCM, and the other is to > + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The > + * function has to wait for the PRCM to acknowledge through the monitoring of > + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit > + * disables the master access, and also signals the PRCM that the PRUSS is ready > + * for Standby. > + * > + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned > + * when the ready-state fails. > + */ > +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) > +{ > + int ret; > + u32 syscfg_val, i; > + const struct pruss_private_data *data; > + > + if (IS_ERR_OR_NULL(pruss)) > + return -EINVAL; > + > + data = of_device_get_match_data(pruss->dev); > + > + /* nothing to do on non OMAP-SoCs */ > + if (!data || !data->has_ocp_syscfg) > + return 0; > + > + /* assert the MStandby signal during disable path */ > + if (!enable) > + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, > + SYSCFG_STANDBY_INIT, > + SYSCFG_STANDBY_INIT); You can omit the above if() if you just encapsulate the below in if (enable) { > + > + /* enable the OCP master ports and disable MStandby */ > + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); > + if (ret) > + return ret; > + > + /* wait till we are ready for transactions - delay is arbitrary */ > + for (i = 0; i < 10; i++) { > + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); > + if (ret) > + goto disable; > + > + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) > + return 0; > + > + udelay(5); > + } > + > + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); > + ret = -ETIMEDOUT; } > + > +disable: > + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, > + SYSCFG_STANDBY_INIT); > + return ret; > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); > + > static void pruss_of_free_clk_provider(void *data) > { > struct device_node *clk_mux_np = data; > @@ -495,10 +566,16 @@ static int pruss_remove(struct platform_device *pdev) > /* instance-specific driver private data */ > static const struct pruss_private_data am437x_pruss1_data = { > .has_no_sharedram = false, > + .has_ocp_syscfg = true, > }; > > static const struct pruss_private_data am437x_pruss0_data = { > .has_no_sharedram = true, > + .has_ocp_syscfg = false, > +}; > + > +static const struct pruss_private_data am33xx_am57xx_data = { > + .has_ocp_syscfg = true, > }; How about keeping platform data for different platforms separate? i.e. am33xx_pruss_data and am57xx_pruss_data > > static const struct pruss_private_data am65x_j721e_pruss_data = { > @@ -506,10 +583,10 @@ static const struct pruss_private_data am65x_j721e_pruss_data = { > }; > > static const struct of_device_id pruss_of_match[] = { > - { .compatible = "ti,am3356-pruss" }, > + { .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_data }, > { .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, }, > { .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, }, > - { .compatible = "ti,am5728-pruss" }, > + { .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_data }, > { .compatible = "ti,k2g-pruss" }, > { .compatible = "ti,am654-icssg", .data = &am65x_j721e_pruss_data, }, > { .compatible = "ti,j721e-icssg", .data = &am65x_j721e_pruss_data, }, > diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h > index 7952f250301a..8cb99d3cad0d 100644 > --- a/include/linux/remoteproc/pruss.h > +++ b/include/linux/remoteproc/pruss.h > @@ -168,6 +168,7 @@ int pruss_release_mem_region(struct pruss *pruss, > int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); > int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > unsigned int mask, unsigned int val); > +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable); > > #else > > @@ -203,6 +204,11 @@ static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > return -EOPNOTSUPP; > } > > +static inline int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) > +{ > + return -EOPNOTSUPP; > +} > + > #endif /* CONFIG_TI_PRUSS */ > > #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) cheers, -roger
Hi Roger, On 08/03/23 14:11, Roger Quadros wrote: > > > On 06/03/2023 13:09, MD Danish Anwar wrote: >> From: Suman Anna <s-anna@ti.com> >> >> The PRU-ICSS subsystem on OMAP-architecture based SoCS (AM33xx, AM437x >> and AM57xx SoCs) has a control bit STANDBY_INIT in the PRUSS_CFG register >> to initiate a Standby sequence (when set) and trigger a MStandby request >> to the SoC's PRCM module. This same bit is also used to enable the OCP >> master ports (when cleared). The clearing of the STANDBY_INIT bit requires >> an acknowledgment from PRCM and is done through the monitoring of the >> PRUSS_SYSCFG.SUB_MWAIT bit. >> >> Add a helper function pruss_cfg_ocp_master_ports() to allow the PRU >> client drivers to control this bit and enable or disable the firmware >> running on PRU cores access to any peripherals or memory to achieve >> desired functionality. The access is disabled by default on power-up >> and on any suspend (context is not maintained). >> >> 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> >> --- >> drivers/soc/ti/pruss.c | 81 +++++++++++++++++++++++++++++++- >> include/linux/remoteproc/pruss.h | 6 +++ >> 2 files changed, 85 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >> index 537a3910ffd8..dc3abda0b8c2 100644 >> --- a/drivers/soc/ti/pruss.c >> +++ b/drivers/soc/ti/pruss.c >> @@ -22,14 +22,19 @@ >> #include <linux/remoteproc.h> >> #include <linux/slab.h> >> >> +#define SYSCFG_STANDBY_INIT BIT(4) >> +#define SYSCFG_SUB_MWAIT_READY BIT(5) >> + >> /** >> * 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_ocp_syscfg: flag to indicate if OCP SYSCFG is present >> */ >> struct pruss_private_data { >> bool has_no_sharedram; >> bool has_core_mux_clock; >> + bool has_ocp_syscfg; >> }; >> >> /** >> @@ -205,6 +210,72 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >> } >> EXPORT_SYMBOL_GPL(pruss_cfg_update); >> >> +/** >> + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports >> + * @pruss: the pruss instance handle >> + * @enable: set to true for enabling or false for disabling the OCP master ports >> + * >> + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or >> + * disable the OCP master ports (applicable only on SoCs using OCP interconnect >> + * like the OMAP family). Clearing the bit achieves dual functionalities - one >> + * is to deassert the MStandby signal to the device PRCM, and the other is to >> + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The >> + * function has to wait for the PRCM to acknowledge through the monitoring of >> + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit >> + * disables the master access, and also signals the PRCM that the PRUSS is ready >> + * for Standby. >> + * >> + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned >> + * when the ready-state fails. >> + */ >> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >> +{ >> + int ret; >> + u32 syscfg_val, i; >> + const struct pruss_private_data *data; >> + >> + if (IS_ERR_OR_NULL(pruss)) >> + return -EINVAL; >> + >> + data = of_device_get_match_data(pruss->dev); >> + >> + /* nothing to do on non OMAP-SoCs */ >> + if (!data || !data->has_ocp_syscfg) >> + return 0; >> + >> + /* assert the MStandby signal during disable path */ >> + if (!enable) >> + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, >> + SYSCFG_STANDBY_INIT, >> + SYSCFG_STANDBY_INIT); > > You can omit the above if() if you just encapsulate the below in > > if (enable) { > > Sure, I can omit the above if() and put the below block inside if (enable) {}. Currently when API pruss_cfg_ocp_master_ports()is called with enable as false i.e. disabling PRUSS OCP master ports is requested, we directly return pruss_cfg_update() where as if we remove the above if() section and encapsulate below block in if (enable) {}, then in disable scenario, call flow will directly reach the label disable. In the label disable, we are updating cfg and then returning "ret", but at this point the variable ret is not assigned. To counter this should I change the label disable to below? disable: return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, SYSCFG_STANDBY_INIT); >> + >> + /* enable the OCP master ports and disable MStandby */ >> + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); >> + if (ret) >> + return ret; >> + >> + /* wait till we are ready for transactions - delay is arbitrary */ >> + for (i = 0; i < 10; i++) { >> + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); >> + if (ret) >> + goto disable; >> + Changing the disable label will also result in losing the return value of pruss_cfg_read() API call here. >> + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) >> + return 0; >> + >> + udelay(5); >> + } >> + >> + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); >> + ret = -ETIMEDOUT; Changing the disable label will also result in losing ret = -ETIMEDOUT here. > > } > >> + >> +disable: >> + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, >> + SYSCFG_STANDBY_INIT); >> + return ret; >> +} So should I do this modification or keep it as it is? >> +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); >> + >> static void pruss_of_free_clk_provider(void *data) >> { >> struct device_node *clk_mux_np = data; >> @@ -495,10 +566,16 @@ static int pruss_remove(struct platform_device *pdev) >> /* instance-specific driver private data */ >> static const struct pruss_private_data am437x_pruss1_data = { >> .has_no_sharedram = false, >> + .has_ocp_syscfg = true, >> }; >> >> static const struct pruss_private_data am437x_pruss0_data = { >> .has_no_sharedram = true, >> + .has_ocp_syscfg = false, >> +}; >> + >> +static const struct pruss_private_data am33xx_am57xx_data = { >> + .has_ocp_syscfg = true, >> }; > > How about keeping platform data for different platforms separate? > > i.e. am33xx_pruss_data and am57xx_pruss_data > Sure. I'll split am33xx_am57xx_data into am33xx_pruss_data and am57xx_pruss_data as well as am65x_j721e_pruss_data into am65x_pruss_data and j721e_pruss_data. >> >> static const struct pruss_private_data am65x_j721e_pruss_data = { >> @@ -506,10 +583,10 @@ static const struct pruss_private_data am65x_j721e_pruss_data = { >> }; >> >> static const struct of_device_id pruss_of_match[] = { >> - { .compatible = "ti,am3356-pruss" }, >> + { .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_data }, >> { .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, }, >> { .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, }, >> - { .compatible = "ti,am5728-pruss" }, >> + { .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_data }, >> { .compatible = "ti,k2g-pruss" }, >> { .compatible = "ti,am654-icssg", .data = &am65x_j721e_pruss_data, }, >> { .compatible = "ti,j721e-icssg", .data = &am65x_j721e_pruss_data, }, >> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h >> index 7952f250301a..8cb99d3cad0d 100644 >> --- a/include/linux/remoteproc/pruss.h >> +++ b/include/linux/remoteproc/pruss.h >> @@ -168,6 +168,7 @@ int pruss_release_mem_region(struct pruss *pruss, >> int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); >> int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >> unsigned int mask, unsigned int val); >> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable); >> >> #else >> >> @@ -203,6 +204,11 @@ static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >> return -EOPNOTSUPP; >> } >> >> +static inline int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >> +{ >> + return -EOPNOTSUPP; >> +} >> + >> #endif /* CONFIG_TI_PRUSS */ >> >> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) > > cheers, > -roger
On 08/03/2023 13:09, Md Danish Anwar wrote: > Hi Roger, > > On 08/03/23 14:11, Roger Quadros wrote: >> >> >> On 06/03/2023 13:09, MD Danish Anwar wrote: >>> From: Suman Anna <s-anna@ti.com> >>> >>> The PRU-ICSS subsystem on OMAP-architecture based SoCS (AM33xx, AM437x >>> and AM57xx SoCs) has a control bit STANDBY_INIT in the PRUSS_CFG register >>> to initiate a Standby sequence (when set) and trigger a MStandby request >>> to the SoC's PRCM module. This same bit is also used to enable the OCP >>> master ports (when cleared). The clearing of the STANDBY_INIT bit requires >>> an acknowledgment from PRCM and is done through the monitoring of the >>> PRUSS_SYSCFG.SUB_MWAIT bit. >>> >>> Add a helper function pruss_cfg_ocp_master_ports() to allow the PRU >>> client drivers to control this bit and enable or disable the firmware >>> running on PRU cores access to any peripherals or memory to achieve >>> desired functionality. The access is disabled by default on power-up >>> and on any suspend (context is not maintained). >>> >>> 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> >>> --- >>> drivers/soc/ti/pruss.c | 81 +++++++++++++++++++++++++++++++- >>> include/linux/remoteproc/pruss.h | 6 +++ >>> 2 files changed, 85 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>> index 537a3910ffd8..dc3abda0b8c2 100644 >>> --- a/drivers/soc/ti/pruss.c >>> +++ b/drivers/soc/ti/pruss.c >>> @@ -22,14 +22,19 @@ >>> #include <linux/remoteproc.h> >>> #include <linux/slab.h> >>> >>> +#define SYSCFG_STANDBY_INIT BIT(4) >>> +#define SYSCFG_SUB_MWAIT_READY BIT(5) >>> + >>> /** >>> * 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_ocp_syscfg: flag to indicate if OCP SYSCFG is present >>> */ >>> struct pruss_private_data { >>> bool has_no_sharedram; >>> bool has_core_mux_clock; >>> + bool has_ocp_syscfg; >>> }; >>> >>> /** >>> @@ -205,6 +210,72 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>> } >>> EXPORT_SYMBOL_GPL(pruss_cfg_update); >>> >>> +/** >>> + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports >>> + * @pruss: the pruss instance handle >>> + * @enable: set to true for enabling or false for disabling the OCP master ports >>> + * >>> + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or >>> + * disable the OCP master ports (applicable only on SoCs using OCP interconnect >>> + * like the OMAP family). Clearing the bit achieves dual functionalities - one >>> + * is to deassert the MStandby signal to the device PRCM, and the other is to >>> + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The >>> + * function has to wait for the PRCM to acknowledge through the monitoring of >>> + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit >>> + * disables the master access, and also signals the PRCM that the PRUSS is ready >>> + * for Standby. >>> + * >>> + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned >>> + * when the ready-state fails. >>> + */ >>> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >>> +{ >>> + int ret; >>> + u32 syscfg_val, i; >>> + const struct pruss_private_data *data; >>> + >>> + if (IS_ERR_OR_NULL(pruss)) >>> + return -EINVAL; >>> + >>> + data = of_device_get_match_data(pruss->dev); >>> + >>> + /* nothing to do on non OMAP-SoCs */ >>> + if (!data || !data->has_ocp_syscfg) >>> + return 0; >>> + >>> + /* assert the MStandby signal during disable path */ >>> + if (!enable) >>> + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, >>> + SYSCFG_STANDBY_INIT, >>> + SYSCFG_STANDBY_INIT); >> >> You can omit the above if() if you just encapsulate the below in >> >> if (enable) { >> >> > Sure, I can omit the above if() and put the below block inside if (enable) {}. > > Currently when API pruss_cfg_ocp_master_ports()is called with enable as false > i.e. disabling PRUSS OCP master ports is requested, we directly return > pruss_cfg_update() where as if we remove the above if() section and encapsulate > below block in if (enable) {}, then in disable scenario, call flow will > directly reach the label disable. In the label disable, we are updating cfg and > then returning "ret", but at this point the variable ret is not assigned. > > To counter this should I change the label disable to below? > > disable: > return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, > SYSCFG_STANDBY_INIT); But you will loose the error code if we came here due to failure in pruss_cfg_read(). It's ok, don't make the change I suggested and leave it as it is. > >>> + >>> + /* enable the OCP master ports and disable MStandby */ >>> + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); >>> + if (ret) >>> + return ret; >>> + >>> + /* wait till we are ready for transactions - delay is arbitrary */ >>> + for (i = 0; i < 10; i++) { >>> + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); >>> + if (ret) >>> + goto disable; >>> + > > Changing the disable label will also result in losing the return value of > pruss_cfg_read() API call here. > >>> + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) >>> + return 0; >>> + >>> + udelay(5); >>> + } >>> + >>> + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); >>> + ret = -ETIMEDOUT; > > Changing the disable label will also result in losing ret = -ETIMEDOUT here. > >> >> } >> >>> + >>> +disable: >>> + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, >>> + SYSCFG_STANDBY_INIT); >>> + return ret; >>> +} > > So should I do this modification or keep it as it is? > >>> +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); >>> + >>> static void pruss_of_free_clk_provider(void *data) >>> { >>> struct device_node *clk_mux_np = data; >>> @@ -495,10 +566,16 @@ static int pruss_remove(struct platform_device *pdev) >>> /* instance-specific driver private data */ >>> static const struct pruss_private_data am437x_pruss1_data = { >>> .has_no_sharedram = false, >>> + .has_ocp_syscfg = true, >>> }; >>> >>> static const struct pruss_private_data am437x_pruss0_data = { >>> .has_no_sharedram = true, >>> + .has_ocp_syscfg = false, >>> +}; >>> + >>> +static const struct pruss_private_data am33xx_am57xx_data = { >>> + .has_ocp_syscfg = true, >>> }; >> >> How about keeping platform data for different platforms separate? >> >> i.e. am33xx_pruss_data and am57xx_pruss_data >> > > Sure. I'll split am33xx_am57xx_data into am33xx_pruss_data and > am57xx_pruss_data as well as am65x_j721e_pruss_data into am65x_pruss_data and > j721e_pruss_data. > >>> >>> static const struct pruss_private_data am65x_j721e_pruss_data = { >>> @@ -506,10 +583,10 @@ static const struct pruss_private_data am65x_j721e_pruss_data = { >>> }; >>> >>> static const struct of_device_id pruss_of_match[] = { >>> - { .compatible = "ti,am3356-pruss" }, >>> + { .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_data }, >>> { .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, }, >>> { .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, }, >>> - { .compatible = "ti,am5728-pruss" }, >>> + { .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_data }, >>> { .compatible = "ti,k2g-pruss" }, >>> { .compatible = "ti,am654-icssg", .data = &am65x_j721e_pruss_data, }, >>> { .compatible = "ti,j721e-icssg", .data = &am65x_j721e_pruss_data, }, >>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h >>> index 7952f250301a..8cb99d3cad0d 100644 >>> --- a/include/linux/remoteproc/pruss.h >>> +++ b/include/linux/remoteproc/pruss.h >>> @@ -168,6 +168,7 @@ int pruss_release_mem_region(struct pruss *pruss, >>> int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); >>> int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>> unsigned int mask, unsigned int val); >>> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable); >>> >>> #else >>> >>> @@ -203,6 +204,11 @@ static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>> return -EOPNOTSUPP; >>> } >>> >>> +static inline int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >>> +{ >>> + return -EOPNOTSUPP; >>> +} >>> + >>> #endif /* CONFIG_TI_PRUSS */ >>> >>> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >> >> cheers, >> -roger > cheers, -roger
On 08/03/23 16:44, Roger Quadros wrote: > > > On 08/03/2023 13:09, Md Danish Anwar wrote: >> Hi Roger, >> >> On 08/03/23 14:11, Roger Quadros wrote: >>> >>> >>> On 06/03/2023 13:09, MD Danish Anwar wrote: >>>> From: Suman Anna <s-anna@ti.com> >>>> >>>> The PRU-ICSS subsystem on OMAP-architecture based SoCS (AM33xx, AM437x >>>> and AM57xx SoCs) has a control bit STANDBY_INIT in the PRUSS_CFG register >>>> to initiate a Standby sequence (when set) and trigger a MStandby request >>>> to the SoC's PRCM module. This same bit is also used to enable the OCP >>>> master ports (when cleared). The clearing of the STANDBY_INIT bit requires >>>> an acknowledgment from PRCM and is done through the monitoring of the >>>> PRUSS_SYSCFG.SUB_MWAIT bit. >>>> >>>> Add a helper function pruss_cfg_ocp_master_ports() to allow the PRU >>>> client drivers to control this bit and enable or disable the firmware >>>> running on PRU cores access to any peripherals or memory to achieve >>>> desired functionality. The access is disabled by default on power-up >>>> and on any suspend (context is not maintained). >>>> >>>> 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> >>>> --- >>>> drivers/soc/ti/pruss.c | 81 +++++++++++++++++++++++++++++++- >>>> include/linux/remoteproc/pruss.h | 6 +++ >>>> 2 files changed, 85 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>> index 537a3910ffd8..dc3abda0b8c2 100644 >>>> --- a/drivers/soc/ti/pruss.c >>>> +++ b/drivers/soc/ti/pruss.c >>>> @@ -22,14 +22,19 @@ >>>> #include <linux/remoteproc.h> >>>> #include <linux/slab.h> >>>> >>>> +#define SYSCFG_STANDBY_INIT BIT(4) >>>> +#define SYSCFG_SUB_MWAIT_READY BIT(5) >>>> + >>>> /** >>>> * 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_ocp_syscfg: flag to indicate if OCP SYSCFG is present >>>> */ >>>> struct pruss_private_data { >>>> bool has_no_sharedram; >>>> bool has_core_mux_clock; >>>> + bool has_ocp_syscfg; >>>> }; >>>> >>>> /** >>>> @@ -205,6 +210,72 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>>> } >>>> EXPORT_SYMBOL_GPL(pruss_cfg_update); >>>> >>>> +/** >>>> + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports >>>> + * @pruss: the pruss instance handle >>>> + * @enable: set to true for enabling or false for disabling the OCP master ports >>>> + * >>>> + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or >>>> + * disable the OCP master ports (applicable only on SoCs using OCP interconnect >>>> + * like the OMAP family). Clearing the bit achieves dual functionalities - one >>>> + * is to deassert the MStandby signal to the device PRCM, and the other is to >>>> + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The >>>> + * function has to wait for the PRCM to acknowledge through the monitoring of >>>> + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit >>>> + * disables the master access, and also signals the PRCM that the PRUSS is ready >>>> + * for Standby. >>>> + * >>>> + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned >>>> + * when the ready-state fails. >>>> + */ >>>> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >>>> +{ >>>> + int ret; >>>> + u32 syscfg_val, i; >>>> + const struct pruss_private_data *data; >>>> + >>>> + if (IS_ERR_OR_NULL(pruss)) >>>> + return -EINVAL; >>>> + >>>> + data = of_device_get_match_data(pruss->dev); >>>> + >>>> + /* nothing to do on non OMAP-SoCs */ >>>> + if (!data || !data->has_ocp_syscfg) >>>> + return 0; >>>> + >>>> + /* assert the MStandby signal during disable path */ >>>> + if (!enable) >>>> + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, >>>> + SYSCFG_STANDBY_INIT, >>>> + SYSCFG_STANDBY_INIT); >>> >>> You can omit the above if() if you just encapsulate the below in >>> >>> if (enable) { >>> >>> >> Sure, I can omit the above if() and put the below block inside if (enable) {}. >> >> Currently when API pruss_cfg_ocp_master_ports()is called with enable as false >> i.e. disabling PRUSS OCP master ports is requested, we directly return >> pruss_cfg_update() where as if we remove the above if() section and encapsulate >> below block in if (enable) {}, then in disable scenario, call flow will >> directly reach the label disable. In the label disable, we are updating cfg and >> then returning "ret", but at this point the variable ret is not assigned. >> >> To counter this should I change the label disable to below? >> >> disable: >> return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, >> SYSCFG_STANDBY_INIT); > > But you will loose the error code if we came here due to failure in pruss_cfg_read(). > Yes that's why I think it's better to leave it as it is. > It's ok, don't make the change I suggested and leave it as it is. > Sure >> >>>> + >>>> + /* enable the OCP master ports and disable MStandby */ >>>> + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* wait till we are ready for transactions - delay is arbitrary */ >>>> + for (i = 0; i < 10; i++) { >>>> + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); >>>> + if (ret) >>>> + goto disable; >>>> + >> >> Changing the disable label will also result in losing the return value of >> pruss_cfg_read() API call here. >> >>>> + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) >>>> + return 0; >>>> + >>>> + udelay(5); >>>> + } >>>> + >>>> + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); >>>> + ret = -ETIMEDOUT; >> >> Changing the disable label will also result in losing ret = -ETIMEDOUT here. >> >>> >>> } >>> >>>> + >>>> +disable: >>>> + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, >>>> + SYSCFG_STANDBY_INIT); >>>> + return ret; >>>> +} >> >> So should I do this modification or keep it as it is? >> >>>> +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); >>>> + >>>> static void pruss_of_free_clk_provider(void *data) >>>> { >>>> struct device_node *clk_mux_np = data; >>>> @@ -495,10 +566,16 @@ static int pruss_remove(struct platform_device *pdev) >>>> /* instance-specific driver private data */ >>>> static const struct pruss_private_data am437x_pruss1_data = { >>>> .has_no_sharedram = false, >>>> + .has_ocp_syscfg = true, >>>> }; >>>> >>>> static const struct pruss_private_data am437x_pruss0_data = { >>>> .has_no_sharedram = true, >>>> + .has_ocp_syscfg = false, >>>> +}; >>>> + >>>> +static const struct pruss_private_data am33xx_am57xx_data = { >>>> + .has_ocp_syscfg = true, >>>> }; >>> >>> How about keeping platform data for different platforms separate? >>> >>> i.e. am33xx_pruss_data and am57xx_pruss_data >>> >> >> Sure. I'll split am33xx_am57xx_data into am33xx_pruss_data and >> am57xx_pruss_data as well as am65x_j721e_pruss_data into am65x_pruss_data and >> j721e_pruss_data. >> >>>> >>>> static const struct pruss_private_data am65x_j721e_pruss_data = { >>>> @@ -506,10 +583,10 @@ static const struct pruss_private_data am65x_j721e_pruss_data = { >>>> }; >>>> >>>> static const struct of_device_id pruss_of_match[] = { >>>> - { .compatible = "ti,am3356-pruss" }, >>>> + { .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_data }, >>>> { .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, }, >>>> { .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, }, >>>> - { .compatible = "ti,am5728-pruss" }, >>>> + { .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_data }, >>>> { .compatible = "ti,k2g-pruss" }, >>>> { .compatible = "ti,am654-icssg", .data = &am65x_j721e_pruss_data, }, >>>> { .compatible = "ti,j721e-icssg", .data = &am65x_j721e_pruss_data, }, >>>> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h >>>> index 7952f250301a..8cb99d3cad0d 100644 >>>> --- a/include/linux/remoteproc/pruss.h >>>> +++ b/include/linux/remoteproc/pruss.h >>>> @@ -168,6 +168,7 @@ int pruss_release_mem_region(struct pruss *pruss, >>>> int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); >>>> int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>>> unsigned int mask, unsigned int val); >>>> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable); >>>> >>>> #else >>>> >>>> @@ -203,6 +204,11 @@ static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>>> return -EOPNOTSUPP; >>>> } >>>> >>>> +static inline int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >>>> +{ >>>> + return -EOPNOTSUPP; >>>> +} >>>> + >>>> #endif /* CONFIG_TI_PRUSS */ >>>> >>>> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >>> >>> cheers, >>> -roger >> > > cheers, > -roger
Hi, * MD Danish Anwar <danishanwar@ti.com> [230306 11:10]: > From: Suman Anna <s-anna@ti.com> > +/** > + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports > + * @pruss: the pruss instance handle > + * @enable: set to true for enabling or false for disabling the OCP master ports > + * > + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or > + * disable the OCP master ports (applicable only on SoCs using OCP interconnect > + * like the OMAP family). Clearing the bit achieves dual functionalities - one > + * is to deassert the MStandby signal to the device PRCM, and the other is to > + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The > + * function has to wait for the PRCM to acknowledge through the monitoring of > + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit > + * disables the master access, and also signals the PRCM that the PRUSS is ready > + * for Standby. > + * > + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned > + * when the ready-state fails. > + */ > +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) > +{ > + int ret; > + u32 syscfg_val, i; > + const struct pruss_private_data *data; > + > + if (IS_ERR_OR_NULL(pruss)) > + return -EINVAL; > + > + data = of_device_get_match_data(pruss->dev); > + > + /* nothing to do on non OMAP-SoCs */ > + if (!data || !data->has_ocp_syscfg) > + return 0; > + > + /* assert the MStandby signal during disable path */ > + if (!enable) > + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, > + SYSCFG_STANDBY_INIT, > + SYSCFG_STANDBY_INIT); > + > + /* enable the OCP master ports and disable MStandby */ > + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); > + if (ret) > + return ret; > + > + /* wait till we are ready for transactions - delay is arbitrary */ > + for (i = 0; i < 10; i++) { > + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); > + if (ret) > + goto disable; > + > + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) > + return 0; > + > + udelay(5); > + } > + > + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); > + ret = -ETIMEDOUT; > + > +disable: > + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, > + SYSCFG_STANDBY_INIT); > + return ret; > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); The above you should no longer be needed, see for example am33xx-l4.dtsi for pruss_tm: target-module@300000. The SYSCFG register is managed by drivers/bus/ti-sysc.c using compatible "ti,sysc-pruss", and the "sysc" reg-names property. So when any of the child devices do pm_runtime_get() related functions, the PRUSS top level interconnect target module is automatically enabled and disabled as needed. If there's something still missing like dts configuration for some SoCs, or quirk handling in ti-sysc.c, let's fix those instead so we can avoid exporting a custom function for pruss_cfg_ocp_master_ports. Regards, Tony
Hi Tony, On 09/03/23 12:34, Tony Lindgren wrote: > Hi, > > * MD Danish Anwar <danishanwar@ti.com> [230306 11:10]: >> From: Suman Anna <s-anna@ti.com> >> +/** >> + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports >> + * @pruss: the pruss instance handle >> + * @enable: set to true for enabling or false for disabling the OCP master ports >> + * >> + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or >> + * disable the OCP master ports (applicable only on SoCs using OCP interconnect >> + * like the OMAP family). Clearing the bit achieves dual functionalities - one >> + * is to deassert the MStandby signal to the device PRCM, and the other is to >> + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The >> + * function has to wait for the PRCM to acknowledge through the monitoring of >> + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit >> + * disables the master access, and also signals the PRCM that the PRUSS is ready >> + * for Standby. >> + * >> + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned >> + * when the ready-state fails. >> + */ >> +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) >> +{ >> + int ret; >> + u32 syscfg_val, i; >> + const struct pruss_private_data *data; >> + >> + if (IS_ERR_OR_NULL(pruss)) >> + return -EINVAL; >> + >> + data = of_device_get_match_data(pruss->dev); >> + >> + /* nothing to do on non OMAP-SoCs */ >> + if (!data || !data->has_ocp_syscfg) >> + return 0; >> + >> + /* assert the MStandby signal during disable path */ >> + if (!enable) >> + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, >> + SYSCFG_STANDBY_INIT, >> + SYSCFG_STANDBY_INIT); >> + >> + /* enable the OCP master ports and disable MStandby */ >> + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); >> + if (ret) >> + return ret; >> + >> + /* wait till we are ready for transactions - delay is arbitrary */ >> + for (i = 0; i < 10; i++) { >> + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); >> + if (ret) >> + goto disable; >> + >> + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) >> + return 0; >> + >> + udelay(5); >> + } >> + >> + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); >> + ret = -ETIMEDOUT; >> + >> +disable: >> + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, >> + SYSCFG_STANDBY_INIT); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); > > The above you should no longer be needed, see for example am33xx-l4.dtsi > for pruss_tm: target-module@300000. The SYSCFG register is managed by > drivers/bus/ti-sysc.c using compatible "ti,sysc-pruss", and the "sysc" > reg-names property. So when any of the child devices do pm_runtime_get() > related functions, the PRUSS top level interconnect target module is > automatically enabled and disabled as needed. > > If there's something still missing like dts configuration for some SoCs, > or quirk handling in ti-sysc.c, let's fix those instead so we can avoid > exporting a custom function for pruss_cfg_ocp_master_ports. > > Regards, > > Tony I will remove this patch in the next revision of this series as it is no longer needed.
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c index 537a3910ffd8..dc3abda0b8c2 100644 --- a/drivers/soc/ti/pruss.c +++ b/drivers/soc/ti/pruss.c @@ -22,14 +22,19 @@ #include <linux/remoteproc.h> #include <linux/slab.h> +#define SYSCFG_STANDBY_INIT BIT(4) +#define SYSCFG_SUB_MWAIT_READY BIT(5) + /** * 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_ocp_syscfg: flag to indicate if OCP SYSCFG is present */ struct pruss_private_data { bool has_no_sharedram; bool has_core_mux_clock; + bool has_ocp_syscfg; }; /** @@ -205,6 +210,72 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, } EXPORT_SYMBOL_GPL(pruss_cfg_update); +/** + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports + * @pruss: the pruss instance handle + * @enable: set to true for enabling or false for disabling the OCP master ports + * + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or + * disable the OCP master ports (applicable only on SoCs using OCP interconnect + * like the OMAP family). Clearing the bit achieves dual functionalities - one + * is to deassert the MStandby signal to the device PRCM, and the other is to + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The + * function has to wait for the PRCM to acknowledge through the monitoring of + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit + * disables the master access, and also signals the PRCM that the PRUSS is ready + * for Standby. + * + * Return: 0 on success, or an error code otherwise. ETIMEDOUT is returned + * when the ready-state fails. + */ +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) +{ + int ret; + u32 syscfg_val, i; + const struct pruss_private_data *data; + + if (IS_ERR_OR_NULL(pruss)) + return -EINVAL; + + data = of_device_get_match_data(pruss->dev); + + /* nothing to do on non OMAP-SoCs */ + if (!data || !data->has_ocp_syscfg) + return 0; + + /* assert the MStandby signal during disable path */ + if (!enable) + return pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, + SYSCFG_STANDBY_INIT, + SYSCFG_STANDBY_INIT); + + /* enable the OCP master ports and disable MStandby */ + ret = pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, 0); + if (ret) + return ret; + + /* wait till we are ready for transactions - delay is arbitrary */ + for (i = 0; i < 10; i++) { + ret = pruss_cfg_read(pruss, PRUSS_CFG_SYSCFG, &syscfg_val); + if (ret) + goto disable; + + if (!(syscfg_val & SYSCFG_SUB_MWAIT_READY)) + return 0; + + udelay(5); + } + + dev_err(pruss->dev, "timeout waiting for SUB_MWAIT_READY\n"); + ret = -ETIMEDOUT; + +disable: + pruss_cfg_update(pruss, PRUSS_CFG_SYSCFG, SYSCFG_STANDBY_INIT, + SYSCFG_STANDBY_INIT); + return ret; +} +EXPORT_SYMBOL_GPL(pruss_cfg_ocp_master_ports); + static void pruss_of_free_clk_provider(void *data) { struct device_node *clk_mux_np = data; @@ -495,10 +566,16 @@ static int pruss_remove(struct platform_device *pdev) /* instance-specific driver private data */ static const struct pruss_private_data am437x_pruss1_data = { .has_no_sharedram = false, + .has_ocp_syscfg = true, }; static const struct pruss_private_data am437x_pruss0_data = { .has_no_sharedram = true, + .has_ocp_syscfg = false, +}; + +static const struct pruss_private_data am33xx_am57xx_data = { + .has_ocp_syscfg = true, }; static const struct pruss_private_data am65x_j721e_pruss_data = { @@ -506,10 +583,10 @@ static const struct pruss_private_data am65x_j721e_pruss_data = { }; static const struct of_device_id pruss_of_match[] = { - { .compatible = "ti,am3356-pruss" }, + { .compatible = "ti,am3356-pruss", .data = &am33xx_am57xx_data }, { .compatible = "ti,am4376-pruss0", .data = &am437x_pruss0_data, }, { .compatible = "ti,am4376-pruss1", .data = &am437x_pruss1_data, }, - { .compatible = "ti,am5728-pruss" }, + { .compatible = "ti,am5728-pruss", .data = &am33xx_am57xx_data }, { .compatible = "ti,k2g-pruss" }, { .compatible = "ti,am654-icssg", .data = &am65x_j721e_pruss_data, }, { .compatible = "ti,j721e-icssg", .data = &am65x_j721e_pruss_data, }, diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h index 7952f250301a..8cb99d3cad0d 100644 --- a/include/linux/remoteproc/pruss.h +++ b/include/linux/remoteproc/pruss.h @@ -168,6 +168,7 @@ int pruss_release_mem_region(struct pruss *pruss, int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val); int pruss_cfg_update(struct pruss *pruss, unsigned int reg, unsigned int mask, unsigned int val); +int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable); #else @@ -203,6 +204,11 @@ static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, return -EOPNOTSUPP; } +static inline int pruss_cfg_ocp_master_ports(struct pruss *pruss, bool enable) +{ + return -EOPNOTSUPP; +} + #endif /* CONFIG_TI_PRUSS */ #if IS_ENABLED(CONFIG_PRU_REMOTEPROC)