| Message ID | 20230323062451.2925996-1-danishanwar@ti.com |
|---|---|
| Headers | show |
| Series | Introduce PRU platform consumer API | expand |
On 23/03/2023 08:24, MD Danish Anwar wrote: > From: Suman Anna <s-anna@ti.com> > > The PRUSS CFG module is represented as a syscon node and is currently > managed by the PRUSS platform driver. Add easy accessor functions to set > GPI mode, MII_RT event enable/disable and XFR (XIN XOUT) enable/disable > to enable the PRUSS Ethernet usecase. These functions reuse the generic > pruss_cfg_update() API function. > > 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> Reviewed-by: Roger Quadros <rogerq@kernel.org>
Hi Mathieu, On 23/03/23 11:54, MD Danish Anwar wrote: > Hi All, > The Programmable Real-Time Unit and Industrial Communication Subsystem (PRU-ICSS > or simply PRUSS) on various TI SoCs consists of dual 32-bit RISC cores > (Programmable Real-Time Units, or PRUs) for program execution. > > There are 3 foundation components for TI PRUSS subsystem: the PRUSS platform > driver, the PRUSS INTC driver and the PRUSS remoteproc driver. All of them have > already been merged and can be found under: > 1) drivers/soc/ti/pruss.c > Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml > 2) drivers/irqchip/irq-pruss-intc.c > Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml > 3) drivers/remoteproc/pru_rproc.c > Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml > > The programmable nature of the PRUs provide flexibility to implement custom > peripheral interfaces, fast real-time responses, or specialized data handling. > Example of a PRU consumer drivers will be: > - Software UART over PRUSS > - PRU-ICSS Ethernet EMAC > > In order to make usage of common PRU resources and allow the consumer drivers > to configure the PRU hardware for specific usage the PRU API is introduced. > Roger has given his RBs for all the patches in this series. Tony has also given his RB. Can you please have a look at this series.
On Thu, Mar 23, 2023 at 11:54:48AM +0530, MD Danish Anwar wrote: > From: "Andrew F. Davis" <afd@ti.com> > > Add two new API - pruss_request_mem_region() & pruss_release_mem_region(), > to the PRUSS platform driver to allow client drivers to acquire and release > the common memory resources present within a PRU-ICSS subsystem. This > allows the client drivers to directly manipulate the respective memories, > as per their design contract with the associated firmware. > > Co-developed-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Andrew F. Davis <afd@ti.com> > Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org> > Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org> > Signed-off-by: MD Danish Anwar <danishanwar@ti.com> > Reviewed-by: Roger Quadros <rogerq@kernel.org> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/soc/ti/pruss.c | 77 ++++++++++++++++++++++++++++++++ > include/linux/pruss_internal.h | 27 +++-------- > include/linux/remoteproc/pruss.h | 39 ++++++++++++++++ > 3 files changed, 121 insertions(+), 22 deletions(-) > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > index 6c2bb02a521d..126b672b9b30 100644 > --- a/drivers/soc/ti/pruss.c > +++ b/drivers/soc/ti/pruss.c > @@ -88,6 +88,82 @@ void pruss_put(struct pruss *pruss) > } > EXPORT_SYMBOL_GPL(pruss_put); > > +/** > + * pruss_request_mem_region() - request a memory resource > + * @pruss: the pruss instance > + * @mem_id: the memory resource id > + * @region: pointer to memory region structure to be filled in > + * > + * This function allows a client driver to request a memory resource, > + * and if successful, will let the client driver own the particular > + * memory region until released using the pruss_release_mem_region() > + * API. > + * > + * Return: 0 if requested memory region is available (in such case pointer to > + * memory region is returned via @region), an error otherwise > + */ > +int pruss_request_mem_region(struct pruss *pruss, enum pruss_mem mem_id, > + struct pruss_mem_region *region) > +{ > + if (!pruss || !region || mem_id >= PRUSS_MEM_MAX) > + return -EINVAL; > + > + mutex_lock(&pruss->lock); > + > + if (pruss->mem_in_use[mem_id]) { > + mutex_unlock(&pruss->lock); > + return -EBUSY; > + } > + > + *region = pruss->mem_regions[mem_id]; > + pruss->mem_in_use[mem_id] = region; > + > + mutex_unlock(&pruss->lock); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_request_mem_region); > + > +/** > + * pruss_release_mem_region() - release a memory resource > + * @pruss: the pruss instance > + * @region: the memory region to release > + * > + * This function is the complimentary function to > + * pruss_request_mem_region(), and allows the client drivers to > + * release back a memory resource. > + * > + * Return: 0 on success, an error code otherwise > + */ > +int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region) > +{ > + int id; > + > + if (!pruss || !region) > + return -EINVAL; > + > + mutex_lock(&pruss->lock); > + > + /* find out the memory region being released */ > + for (id = 0; id < PRUSS_MEM_MAX; id++) { > + if (pruss->mem_in_use[id] == region) > + break; > + } > + > + if (id == PRUSS_MEM_MAX) { > + mutex_unlock(&pruss->lock); > + return -EINVAL; > + } > + > + pruss->mem_in_use[id] = NULL; > + > + mutex_unlock(&pruss->lock); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pruss_release_mem_region); > + > static void pruss_of_free_clk_provider(void *data) > { > struct device_node *clk_mux_np = data; > @@ -290,6 +366,7 @@ static int pruss_probe(struct platform_device *pdev) > return -ENOMEM; > > pruss->dev = dev; > + mutex_init(&pruss->lock); > > child = of_get_child_by_name(np, "memories"); > if (!child) { > diff --git a/include/linux/pruss_internal.h b/include/linux/pruss_internal.h > index 8f91cb164054..cf5287fa01df 100644 > --- a/include/linux/pruss_internal.h > +++ b/include/linux/pruss_internal.h > @@ -9,37 +9,18 @@ > #ifndef _PRUSS_INTERNAL_H_ > #define _PRUSS_INTERNAL_H_ > > +#include <linux/mutex.h> > #include <linux/remoteproc/pruss.h> > #include <linux/types.h> > > -/* > - * enum pruss_mem - PRUSS memory range identifiers > - */ > -enum pruss_mem { > - PRUSS_MEM_DRAM0 = 0, > - PRUSS_MEM_DRAM1, > - PRUSS_MEM_SHRD_RAM2, > - PRUSS_MEM_MAX, > -}; > - > -/** > - * struct pruss_mem_region - PRUSS memory region structure > - * @va: kernel virtual address of the PRUSS memory region > - * @pa: physical (bus) address of the PRUSS memory region > - * @size: size of the PRUSS memory region > - */ > -struct pruss_mem_region { > - void __iomem *va; > - phys_addr_t pa; > - size_t size; > -}; > - > /** > * struct pruss - PRUSS parent structure > * @dev: pruss device pointer > * @cfg_base: base iomap for CFG region > * @cfg_regmap: regmap for config region > * @mem_regions: data for each of the PRUSS memory regions > + * @mem_in_use: to indicate if memory resource is in use > + * @lock: mutex to serialize access to resources > * @core_clk_mux: clk handle for PRUSS CORE_CLK_MUX > * @iep_clk_mux: clk handle for PRUSS IEP_CLK_MUX > */ > @@ -48,6 +29,8 @@ struct pruss { > void __iomem *cfg_base; > struct regmap *cfg_regmap; > struct pruss_mem_region mem_regions[PRUSS_MEM_MAX]; > + struct pruss_mem_region *mem_in_use[PRUSS_MEM_MAX]; > + struct mutex lock; /* PRU resource lock */ > struct clk *core_clk_mux; > struct clk *iep_clk_mux; > }; > diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h > index 93a98cac7829..33f930e0a0ce 100644 > --- a/include/linux/remoteproc/pruss.h > +++ b/include/linux/remoteproc/pruss.h > @@ -44,6 +44,28 @@ enum pru_ctable_idx { > PRU_C31, > }; > > +/* > + * enum pruss_mem - PRUSS memory range identifiers > + */ > +enum pruss_mem { > + PRUSS_MEM_DRAM0 = 0, > + PRUSS_MEM_DRAM1, > + PRUSS_MEM_SHRD_RAM2, > + PRUSS_MEM_MAX, > +}; > + > +/** > + * struct pruss_mem_region - PRUSS memory region structure > + * @va: kernel virtual address of the PRUSS memory region > + * @pa: physical (bus) address of the PRUSS memory region > + * @size: size of the PRUSS memory region > + */ > +struct pruss_mem_region { > + void __iomem *va; > + phys_addr_t pa; > + size_t size; > +}; > + > struct device_node; > struct rproc; > struct pruss; > @@ -52,6 +74,10 @@ struct pruss; > > struct pruss *pruss_get(struct rproc *rproc); > void pruss_put(struct pruss *pruss); > +int pruss_request_mem_region(struct pruss *pruss, enum pruss_mem mem_id, > + struct pruss_mem_region *region); > +int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region); > > #else > > @@ -62,6 +88,19 @@ static inline struct pruss *pruss_get(struct rproc *rproc) > > static inline void pruss_put(struct pruss *pruss) { } > > +static inline int pruss_request_mem_region(struct pruss *pruss, > + enum pruss_mem mem_id, > + struct pruss_mem_region *region) > +{ > + return -EOPNOTSUPP; > +} > + > +static inline int pruss_release_mem_region(struct pruss *pruss, > + struct pruss_mem_region *region) > +{ > + return -EOPNOTSUPP; > +} > + > #endif /* CONFIG_TI_PRUSS */ > > #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) > -- > 2.25.1 >
On Thu, Mar 23, 2023 at 11:54:51AM +0530, MD Danish Anwar wrote: > From: Tero Kristo <t-kristo@ti.com> > > Add two new helper functions pruss_cfg_get_gpmux() & pruss_cfg_set_gpmux() > to get and set the GP MUX mode for programming the PRUSS internal wrapper > mux functionality as needed by usecases. > > Co-developed-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Suman Anna <s-anna@ti.com> > Signed-off-by: Tero Kristo <t-kristo@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> > Reviewed-by: Roger Quadros <rogerq@kernel.org> > --- > drivers/soc/ti/pruss.c | 44 ++++++++++++++++++++++++++++++++ > include/linux/remoteproc/pruss.h | 30 ++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > index ac415442e85b..3aa3c38c6c79 100644 > --- a/drivers/soc/ti/pruss.c > +++ b/drivers/soc/ti/pruss.c > @@ -239,6 +239,50 @@ int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, > } > EXPORT_SYMBOL_GPL(pruss_cfg_xfr_enable); > > +/** > + * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device > + * @pruss: pruss instance > + * @pru_id: PRU identifier (0-1) > + * @mux: pointer to store the current mux value into > + * > + * Return: 0 on success, or an error code otherwise > + */ > +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) > +{ > + int ret = 0; > + u32 val; > + > + if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) > + return -EINVAL; > + > + ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(pru_id), &val); > + if (!ret) > + *mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >> > + PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); What happens if @mux is NULL? Thanks, Mathieu > + return ret; > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_get_gpmux); > + > +/** > + * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device > + * @pruss: pruss instance > + * @pru_id: PRU identifier (0-1) > + * @mux: new mux value for PRU > + * > + * Return: 0 on success, or an error code otherwise > + */ > +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) > +{ > + if (mux >= PRUSS_GP_MUX_SEL_MAX || > + pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) > + return -EINVAL; > + > + return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id), > + PRUSS_GPCFG_PRU_MUX_SEL_MASK, > + (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); > +} > +EXPORT_SYMBOL_GPL(pruss_cfg_set_gpmux); > + > 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 bb001f712980..42f1586c62ac 100644 > --- a/include/linux/remoteproc/pruss.h > +++ b/include/linux/remoteproc/pruss.h > @@ -16,6 +16,24 @@ > > #define PRU_RPROC_DRVNAME "pru-rproc" > > +/* > + * 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 > @@ -110,6 +128,8 @@ int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id, > int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable); > int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, > bool enable); > +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux); > +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux); > > #else > > @@ -152,6 +172,16 @@ static inline int pruss_cfg_xfr_enable(struct pruss *pruss, > return ERR_PTR(-EOPNOTSUPP); > } > > +static inline int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > +static inline int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > #endif /* CONFIG_TI_PRUSS */ > > #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) > -- > 2.25.1 >
On 28/03/23 02:34, Mathieu Poirier wrote: > On Thu, Mar 23, 2023 at 11:54:51AM +0530, MD Danish Anwar wrote: >> From: Tero Kristo <t-kristo@ti.com> >> >> Add two new helper functions pruss_cfg_get_gpmux() & pruss_cfg_set_gpmux() >> to get and set the GP MUX mode for programming the PRUSS internal wrapper >> mux functionality as needed by usecases. >> >> Co-developed-by: Suman Anna <s-anna@ti.com> >> Signed-off-by: Suman Anna <s-anna@ti.com> >> Signed-off-by: Tero Kristo <t-kristo@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> >> Reviewed-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/soc/ti/pruss.c | 44 ++++++++++++++++++++++++++++++++ >> include/linux/remoteproc/pruss.h | 30 ++++++++++++++++++++++ >> 2 files changed, 74 insertions(+) >> >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >> index ac415442e85b..3aa3c38c6c79 100644 >> --- a/drivers/soc/ti/pruss.c >> +++ b/drivers/soc/ti/pruss.c >> @@ -239,6 +239,50 @@ int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, >> } >> EXPORT_SYMBOL_GPL(pruss_cfg_xfr_enable); >> >> +/** >> + * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device >> + * @pruss: pruss instance >> + * @pru_id: PRU identifier (0-1) >> + * @mux: pointer to store the current mux value into >> + * >> + * Return: 0 on success, or an error code otherwise >> + */ >> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) >> +{ >> + int ret = 0; >> + u32 val; >> + >> + if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) >> + return -EINVAL; >> + >> + ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(pru_id), &val); >> + if (!ret) >> + *mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >> >> + PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); > > What happens if @mux is NULL? @mux being null may result in some error here. I will add NULL check for mux before storing the value in mux. I will modify the above if condition to have NULL check for mux as well. The if condition will look like below. if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS || !mux) return -EINVAL; Please let me know if this looks OK. > > Thanks, > Mathieu > > >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(pruss_cfg_get_gpmux); >> + >> +/** >> + * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device >> + * @pruss: pruss instance >> + * @pru_id: PRU identifier (0-1) >> + * @mux: new mux value for PRU >> + * >> + * Return: 0 on success, or an error code otherwise >> + */ >> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) >> +{ >> + if (mux >= PRUSS_GP_MUX_SEL_MAX || >> + pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) >> + return -EINVAL; >> + >> + return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id), >> + PRUSS_GPCFG_PRU_MUX_SEL_MASK, >> + (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); >> +} >> +EXPORT_SYMBOL_GPL(pruss_cfg_set_gpmux); >> + >> 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 bb001f712980..42f1586c62ac 100644 >> --- a/include/linux/remoteproc/pruss.h >> +++ b/include/linux/remoteproc/pruss.h >> @@ -16,6 +16,24 @@ >> >> #define PRU_RPROC_DRVNAME "pru-rproc" >> >> +/* >> + * 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 >> @@ -110,6 +128,8 @@ int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id, >> int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable); >> int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, >> bool enable); >> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux); >> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux); >> >> #else >> >> @@ -152,6 +172,16 @@ static inline int pruss_cfg_xfr_enable(struct pruss *pruss, >> return ERR_PTR(-EOPNOTSUPP); >> } >> >> +static inline int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) >> +{ >> + return ERR_PTR(-EOPNOTSUPP); >> +} >> + >> +static inline int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) >> +{ >> + return ERR_PTR(-EOPNOTSUPP); >> +} >> + >> #endif /* CONFIG_TI_PRUSS */ >> >> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >> -- >> 2.25.1 >>
On Tue, 28 Mar 2023 at 05:28, Md Danish Anwar <a0501179@ti.com> wrote: > > > > On 28/03/23 02:34, Mathieu Poirier wrote: > > On Thu, Mar 23, 2023 at 11:54:51AM +0530, MD Danish Anwar wrote: > >> From: Tero Kristo <t-kristo@ti.com> > >> > >> Add two new helper functions pruss_cfg_get_gpmux() & pruss_cfg_set_gpmux() > >> to get and set the GP MUX mode for programming the PRUSS internal wrapper > >> mux functionality as needed by usecases. > >> > >> Co-developed-by: Suman Anna <s-anna@ti.com> > >> Signed-off-by: Suman Anna <s-anna@ti.com> > >> Signed-off-by: Tero Kristo <t-kristo@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> > >> Reviewed-by: Roger Quadros <rogerq@kernel.org> > >> --- > >> drivers/soc/ti/pruss.c | 44 ++++++++++++++++++++++++++++++++ > >> include/linux/remoteproc/pruss.h | 30 ++++++++++++++++++++++ > >> 2 files changed, 74 insertions(+) > >> > >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > >> index ac415442e85b..3aa3c38c6c79 100644 > >> --- a/drivers/soc/ti/pruss.c > >> +++ b/drivers/soc/ti/pruss.c > >> @@ -239,6 +239,50 @@ int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, > >> } > >> EXPORT_SYMBOL_GPL(pruss_cfg_xfr_enable); > >> > >> +/** > >> + * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device > >> + * @pruss: pruss instance > >> + * @pru_id: PRU identifier (0-1) > >> + * @mux: pointer to store the current mux value into > >> + * > >> + * Return: 0 on success, or an error code otherwise > >> + */ > >> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) > >> +{ > >> + int ret = 0; > >> + u32 val; > >> + > >> + if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) > >> + return -EINVAL; > >> + > >> + ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(pru_id), &val); > >> + if (!ret) > >> + *mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >> > >> + PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); > > > > What happens if @mux is NULL? > > @mux being null may result in some error here. I will add NULL check for mux > before storing the value in mux. > It will result in a kernel panic. > I will modify the above if condition to have NULL check for mux as well. > The if condition will look like below. > > if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS || !mux) > return -EINVAL; > That will be fine. > Please let me know if this looks OK. > > > > > Thanks, > > Mathieu > > > > > >> + return ret; > >> +} > >> +EXPORT_SYMBOL_GPL(pruss_cfg_get_gpmux); > >> + > >> +/** > >> + * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device > >> + * @pruss: pruss instance > >> + * @pru_id: PRU identifier (0-1) > >> + * @mux: new mux value for PRU > >> + * > >> + * Return: 0 on success, or an error code otherwise > >> + */ > >> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) > >> +{ > >> + if (mux >= PRUSS_GP_MUX_SEL_MAX || > >> + pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) > >> + return -EINVAL; > >> + > >> + return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id), > >> + PRUSS_GPCFG_PRU_MUX_SEL_MASK, > >> + (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); > >> +} > >> +EXPORT_SYMBOL_GPL(pruss_cfg_set_gpmux); > >> + > >> 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 bb001f712980..42f1586c62ac 100644 > >> --- a/include/linux/remoteproc/pruss.h > >> +++ b/include/linux/remoteproc/pruss.h > >> @@ -16,6 +16,24 @@ > >> > >> #define PRU_RPROC_DRVNAME "pru-rproc" > >> > >> +/* > >> + * 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 > >> @@ -110,6 +128,8 @@ int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id, > >> int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable); > >> int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, > >> bool enable); > >> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux); > >> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux); > >> > >> #else > >> > >> @@ -152,6 +172,16 @@ static inline int pruss_cfg_xfr_enable(struct pruss *pruss, > >> return ERR_PTR(-EOPNOTSUPP); > >> } > >> > >> +static inline int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) > >> +{ > >> + return ERR_PTR(-EOPNOTSUPP); > >> +} > >> + > >> +static inline int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) > >> +{ > >> + return ERR_PTR(-EOPNOTSUPP); > >> +} > >> + > >> #endif /* CONFIG_TI_PRUSS */ > >> > >> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) > >> -- > >> 2.25.1 > >> > > -- > Thanks and Regards, > Danish.
On 29/03/23 23:34, Mathieu Poirier wrote: > On Tue, 28 Mar 2023 at 05:28, Md Danish Anwar <a0501179@ti.com> wrote: >> >> On 28/03/23 02:34, Mathieu Poirier wrote: >>> On Thu, Mar 23, 2023 at 11:54:51AM +0530, MD Danish Anwar wrote: >>>> From: Tero Kristo <t-kristo@ti.com> >>>> >>>> Add two new helper functions pruss_cfg_get_gpmux() & pruss_cfg_set_gpmux() >>>> to get and set the GP MUX mode for programming the PRUSS internal wrapper >>>> mux functionality as needed by usecases. >>>> >>>> Co-developed-by: Suman Anna <s-anna@ti.com> >>>> Signed-off-by: Suman Anna <s-anna@ti.com> >>>> Signed-off-by: Tero Kristo <t-kristo@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> >>>> Reviewed-by: Roger Quadros <rogerq@kernel.org> >>>> --- >>>> drivers/soc/ti/pruss.c | 44 ++++++++++++++++++++++++++++++++ >>>> include/linux/remoteproc/pruss.h | 30 ++++++++++++++++++++++ >>>> 2 files changed, 74 insertions(+) >>>> >>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>> index ac415442e85b..3aa3c38c6c79 100644 >>>> --- a/drivers/soc/ti/pruss.c >>>> +++ b/drivers/soc/ti/pruss.c >>>> @@ -239,6 +239,50 @@ int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, >>>> } >>>> EXPORT_SYMBOL_GPL(pruss_cfg_xfr_enable); >>>> >>>> +/** >>>> + * pruss_cfg_get_gpmux() - get the current GPMUX value for a PRU device >>>> + * @pruss: pruss instance >>>> + * @pru_id: PRU identifier (0-1) >>>> + * @mux: pointer to store the current mux value into >>>> + * >>>> + * Return: 0 on success, or an error code otherwise >>>> + */ >>>> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) >>>> +{ >>>> + int ret = 0; >>>> + u32 val; >>>> + >>>> + if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) >>>> + return -EINVAL; >>>> + >>>> + ret = pruss_cfg_read(pruss, PRUSS_CFG_GPCFG(pru_id), &val); >>>> + if (!ret) >>>> + *mux = (u8)((val & PRUSS_GPCFG_PRU_MUX_SEL_MASK) >> >>>> + PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); >>> >>> What happens if @mux is NULL? >> >> @mux being null may result in some error here. I will add NULL check for mux >> before storing the value in mux. >> > > It will result in a kernel panic. > >> I will modify the above if condition to have NULL check for mux as well. >> The if condition will look like below. >> >> if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS || !mux) >> return -EINVAL; >> > > That will be fine.> Sure, I'll go ahead and make this change. >> Please let me know if this looks OK. >> >>> >>> Thanks, >>> Mathieu >>> >>> >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL_GPL(pruss_cfg_get_gpmux); >>>> + >>>> +/** >>>> + * pruss_cfg_set_gpmux() - set the GPMUX value for a PRU device >>>> + * @pruss: pruss instance >>>> + * @pru_id: PRU identifier (0-1) >>>> + * @mux: new mux value for PRU >>>> + * >>>> + * Return: 0 on success, or an error code otherwise >>>> + */ >>>> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) >>>> +{ >>>> + if (mux >= PRUSS_GP_MUX_SEL_MAX || >>>> + pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) >>>> + return -EINVAL; >>>> + >>>> + return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id), >>>> + PRUSS_GPCFG_PRU_MUX_SEL_MASK, >>>> + (u32)mux << PRUSS_GPCFG_PRU_MUX_SEL_SHIFT); >>>> +} >>>> +EXPORT_SYMBOL_GPL(pruss_cfg_set_gpmux); >>>> + >>>> 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 bb001f712980..42f1586c62ac 100644 >>>> --- a/include/linux/remoteproc/pruss.h >>>> +++ b/include/linux/remoteproc/pruss.h >>>> @@ -16,6 +16,24 @@ >>>> >>>> #define PRU_RPROC_DRVNAME "pru-rproc" >>>> >>>> +/* >>>> + * 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 >>>> @@ -110,6 +128,8 @@ int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id, >>>> int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable); >>>> int pruss_cfg_xfr_enable(struct pruss *pruss, enum pru_type pru_type, >>>> bool enable); >>>> +int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux); >>>> +int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux); >>>> >>>> #else >>>> >>>> @@ -152,6 +172,16 @@ static inline int pruss_cfg_xfr_enable(struct pruss *pruss, >>>> return ERR_PTR(-EOPNOTSUPP); >>>> } >>>> >>>> +static inline int pruss_cfg_get_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 *mux) >>>> +{ >>>> + return ERR_PTR(-EOPNOTSUPP); >>>> +} >>>> + >>>> +static inline int pruss_cfg_set_gpmux(struct pruss *pruss, enum pruss_pru_id pru_id, u8 mux) >>>> +{ >>>> + return ERR_PTR(-EOPNOTSUPP); >>>> +} >>>> + >>>> #endif /* CONFIG_TI_PRUSS */ >>>> >>>> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >>>> -- >>>> 2.25.1 >>>> >> >> -- >> Thanks and Regards, >> Danish.
Hi All, The Programmable Real-Time Unit and Industrial Communication Subsystem (PRU-ICSS or simply PRUSS) on various TI SoCs consists of dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs) for program execution. There are 3 foundation components for TI PRUSS subsystem: the PRUSS platform driver, the PRUSS INTC driver and the PRUSS remoteproc driver. All of them have already been merged and can be found under: 1) drivers/soc/ti/pruss.c Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml 2) drivers/irqchip/irq-pruss-intc.c Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml 3) drivers/remoteproc/pru_rproc.c Documentation/devicetree/bindings/remoteproc/ti,pru-consumer.yaml The programmable nature of the PRUs provide flexibility to implement custom peripheral interfaces, fast real-time responses, or specialized data handling. Example of a PRU consumer drivers will be: - Software UART over PRUSS - PRU-ICSS Ethernet EMAC In order to make usage of common PRU resources and allow the consumer drivers to configure the PRU hardware for specific usage the PRU API is introduced. This is the v5 of the old patch series[1]. This doesn't have any functional changes, the old series has been rebased on linux-next. Changes from v4 [1] to v5: *) Addressed Roger's comment to change function argument in API pruss_cfg_xfr_enable(). Instead of asking user to calcualte mask, now user will just provide the pru_type and mask will be calcualted inside the API. *) Moved enum pru_type from pru_rproc.c to include/linux/remoteproc/pruss.h in patch 4 / 5. *) Moved enum pruss_gpi_mode from patch 3/5 to patch 4/5 to introduce this enum in same patch as the API using it. *) Moved enum pruss_gp_mux_sel from patch 3/5 to patch 5/5 to introduce this enum in same patch as the API using it. *) Created new headefile drivers/soc/ti/pruss.h, private to PRUSS as asked by Roger. Moved all private definitions and pruss_cfg_read () / update () APIs to this newly added headerfile. *) Renamed include/linux/pruss_driver.h to include/linux/pruss_internal.h as suggested by Andrew and Roger. Changes from v3 [2] to v4: *) Added my SoB tags in all patches as earlier SoB tags were missing in few patches. *) Added Roger's RB tags in 3 patches. *) Addressed Roger's comment in patch 4/5 of this series. Added check for invalid GPI mode in pruss_cfg_gpimode() API. *) Removed patch [5] from this series as that patch is no longer required. *) Made pruss_cfg_read() and pruss_cfg_update() APIs internal to pruss.c by removing EXPORT_SYMBOL_GPL and making them static. Now these APIs are internal to pruss.c and PRUSS CFG space is not exposed. *) Moved APIs pruss_cfg_gpimode(), pruss_cfg_miirt_enable(), pruss_cfg_xfr_enable(), pruss_cfg_get_gpmux(), pruss_cfg_set_gpmux() to pruss.c file as they are using APIs pruss_cfg_read / update. Defined these APIs in pruss.h file as other drivers use these APIs to perform respective operations. Changes from v2 to v3: *) No functional changes, the old series has been rebased on linux-next (tag: next-20230306). This series depends on another series which is already merged in the remoteproc tree [3] and is part of v6.3-rc1. This series and the remoteproc series form the PRUSS consumer API which can be used by consumer drivers to utilize the PRUs. One example of the consumer driver is the PRU-ICSSG ethernet driver [4],which depends on this series and the remoteproc series [3]. [1] https://lore.kernel.org/all/20230313111127.1229187-1-danishanwar@ti.com/ [2] https://lore.kernel.org/all/20230306110934.2736465-1-danishanwar@ti.com/ [3] https://lore.kernel.org/all/20230106121046.886863-1-danishanwar@ti.com/#t [4] https://lore.kernel.org/all/20230210114957.2667963-1-danishanwar@ti.com/ [5] https://lore.kernel.org/all/20230306110934.2736465-6-danishanwar@ti.com/ Thanks and Regards, Md Danish Anwar Andrew F. Davis (1): soc: ti: pruss: Add pruss_{request,release}_mem_region() API Suman Anna (2): soc: ti: pruss: Add pruss_cfg_read()/update() API soc: ti: pruss: Add helper functions to set GPI mode, MII_RT_event and XFR Tero Kristo (2): soc: ti: pruss: Add pruss_get()/put() API soc: ti: pruss: Add helper functions to get/set PRUSS_CFG_GPMUX drivers/remoteproc/pru_rproc.c | 17 +- drivers/soc/ti/pruss.c | 256 +++++++++++++++++- drivers/soc/ti/pruss.h | 112 ++++++++ .../{pruss_driver.h => pruss_internal.h} | 34 +-- include/linux/remoteproc/pruss.h | 139 ++++++++++ 5 files changed, 516 insertions(+), 42 deletions(-) create mode 100644 drivers/soc/ti/pruss.h rename include/linux/{pruss_driver.h => pruss_internal.h} (58%)