| Message ID | 20230306110934.2736465-4-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 |
Hi, On 06/03/2023 13:09, 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 allow other drivers to read and program > respectively a register within the PRUSS CFG sub-module represented > by a syscon driver. This interface provides a simple way for client Do you really need these 2 functions to be public? I see that later patches (4-6) add APIs for doing specific things and that should be sufficient than exposing entire CFG space via pruss_cfg_read/update(). > drivers without having them to include and parse the CFG syscon node > within their respective device nodes. Various useful registers and > macros for certain register bit-fields and their values have also > been added. > > It is the responsibility of the client drivers to reconfigure or > reset a particular register upon any failures. > > 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 | 41 +++++++++++++ > include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ > 2 files changed, 143 insertions(+) > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > index c8053c0d735f..537a3910ffd8 100644 > --- a/drivers/soc/ti/pruss.c > +++ b/drivers/soc/ti/pruss.c > @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, > } > EXPORT_SYMBOL_GPL(pruss_release_mem_region); cheers, -roger
Hi Roger, On 08/03/23 13:57, Roger Quadros wrote: > Hi, > > On 06/03/2023 13:09, 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 allow other drivers to read and program >> respectively a register within the PRUSS CFG sub-module represented >> by a syscon driver. This interface provides a simple way for client > > Do you really need these 2 functions to be public? > I see that later patches (4-6) add APIs for doing specific things > and that should be sufficient than exposing entire CFG space via > pruss_cfg_read/update(). > > I think the intention here is to keep this APIs pruss_cfg_read() and pruss_cfg_update() public so that other drivers can read / modify PRUSS config when needed. The later patches (4-6) add APIs to do specific thing, but those APIs also eventually call pruss_cfg_read/update(). >> drivers without having them to include and parse the CFG syscon node >> within their respective device nodes. Various useful registers and >> macros for certain register bit-fields and their values have also >> been added. >> >> It is the responsibility of the client drivers to reconfigure or >> reset a particular register upon any failures. >> >> 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 | 41 +++++++++++++ >> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >> 2 files changed, 143 insertions(+) >> >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >> index c8053c0d735f..537a3910ffd8 100644 >> --- a/drivers/soc/ti/pruss.c >> +++ b/drivers/soc/ti/pruss.c >> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >> } >> EXPORT_SYMBOL_GPL(pruss_release_mem_region); > > cheers, > -roger
On 08/03/2023 13:36, Md Danish Anwar wrote: > Hi Roger, > > On 08/03/23 13:57, Roger Quadros wrote: >> Hi, >> >> On 06/03/2023 13:09, 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 allow other drivers to read and program >>> respectively a register within the PRUSS CFG sub-module represented >>> by a syscon driver. This interface provides a simple way for client >> >> Do you really need these 2 functions to be public? >> I see that later patches (4-6) add APIs for doing specific things >> and that should be sufficient than exposing entire CFG space via >> pruss_cfg_read/update(). >> >> > > I think the intention here is to keep this APIs pruss_cfg_read() and > pruss_cfg_update() public so that other drivers can read / modify PRUSS config > when needed. Where are these other drivers? If they don't exist then let's not make provision for it now. We can provide necessary API helpers when needed instead of letting client drivers do what they want as they can be misused and hard to debug. > > The later patches (4-6) add APIs to do specific thing, but those APIs also > eventually call pruss_cfg_read/update(). They can still call them but they need to be private to pruss.c > >>> drivers without having them to include and parse the CFG syscon node >>> within their respective device nodes. Various useful registers and >>> macros for certain register bit-fields and their values have also >>> been added. >>> >>> It is the responsibility of the client drivers to reconfigure or >>> reset a particular register upon any failures. >>> >>> 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 | 41 +++++++++++++ >>> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >>> 2 files changed, 143 insertions(+) >>> >>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>> index c8053c0d735f..537a3910ffd8 100644 >>> --- a/drivers/soc/ti/pruss.c >>> +++ b/drivers/soc/ti/pruss.c >>> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >>> } >>> EXPORT_SYMBOL_GPL(pruss_release_mem_region); >> >> cheers, >> -roger >
Hi Roger, On 08/03/23 17:12, Roger Quadros wrote: > > > On 08/03/2023 13:36, Md Danish Anwar wrote: >> Hi Roger, >> >> On 08/03/23 13:57, Roger Quadros wrote: >>> Hi, >>> >>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>> respectively a register within the PRUSS CFG sub-module represented >>>> by a syscon driver. This interface provides a simple way for client >>> >>> Do you really need these 2 functions to be public? >>> I see that later patches (4-6) add APIs for doing specific things >>> and that should be sufficient than exposing entire CFG space via >>> pruss_cfg_read/update(). >>> >>> >> >> I think the intention here is to keep this APIs pruss_cfg_read() and >> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >> when needed. > > Where are these other drivers? If they don't exist then let's not make provision > for it now. > We can provide necessary API helpers when needed instead of letting client drivers > do what they want as they can be misused and hard to debug. > The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in the series [1]. The ethernet driver series is dependent on this series. In series [1] we are using pruss_cfg_update() in icssg_config.c file, icssg_config() API. So for this, the API pruss_cfg_update() needs to be public. [1] https://lore.kernel.org/all/20230210114957.2667963-3-danishanwar@ti.com/ >> >> The later patches (4-6) add APIs to do specific thing, but those APIs also >> eventually call pruss_cfg_read/update(). > > They can still call them but they need to be private to pruss.c > >> >>>> drivers without having them to include and parse the CFG syscon node >>>> within their respective device nodes. Various useful registers and >>>> macros for certain register bit-fields and their values have also >>>> been added. >>>> >>>> It is the responsibility of the client drivers to reconfigure or >>>> reset a particular register upon any failures. >>>> >>>> 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 | 41 +++++++++++++ >>>> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >>>> 2 files changed, 143 insertions(+) >>>> >>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>> index c8053c0d735f..537a3910ffd8 100644 >>>> --- a/drivers/soc/ti/pruss.c >>>> +++ b/drivers/soc/ti/pruss.c >>>> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >>>> } >>>> EXPORT_SYMBOL_GPL(pruss_release_mem_region); >>> >>> cheers, >>> -roger >>
Hi Roger, On 09/03/23 17:00, Md Danish Anwar wrote: > Hi Roger, > > On 08/03/23 17:12, Roger Quadros wrote: >> >> >> On 08/03/2023 13:36, Md Danish Anwar wrote: >>> Hi Roger, >>> >>> On 08/03/23 13:57, Roger Quadros wrote: >>>> Hi, >>>> >>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>> respectively a register within the PRUSS CFG sub-module represented >>>>> by a syscon driver. This interface provides a simple way for client >>>> >>>> Do you really need these 2 functions to be public? >>>> I see that later patches (4-6) add APIs for doing specific things >>>> and that should be sufficient than exposing entire CFG space via >>>> pruss_cfg_read/update(). >>>> >>>> >>> >>> I think the intention here is to keep this APIs pruss_cfg_read() and >>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>> when needed. >> >> Where are these other drivers? If they don't exist then let's not make provision >> for it now. >> We can provide necessary API helpers when needed instead of letting client drivers >> do what they want as they can be misused and hard to debug. >> > > The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in > the series [1]. The ethernet driver series is dependent on this series. In > series [1] we are using pruss_cfg_update() in icssg_config.c file, > icssg_config() API. > > So for this, the API pruss_cfg_update() needs to be public. > > [1] https://lore.kernel.org/all/20230210114957.2667963-3-danishanwar@ti.com/ > I will keep this patch as it is as pruss_cfg_update() needs to be public for ICSSG Ethernet driver and pruss_cfg_read() is kind of a complementary function to update. I will do required changes in other patches and send next revision if that's OK with you. Please let me know. >>> >>> The later patches (4-6) add APIs to do specific thing, but those APIs also >>> eventually call pruss_cfg_read/update(). >> >> They can still call them but they need to be private to pruss.c >> >>> >>>>> drivers without having them to include and parse the CFG syscon node >>>>> within their respective device nodes. Various useful registers and >>>>> macros for certain register bit-fields and their values have also >>>>> been added. >>>>> >>>>> It is the responsibility of the client drivers to reconfigure or >>>>> reset a particular register upon any failures. >>>>> >>>>> 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 | 41 +++++++++++++ >>>>> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >>>>> 2 files changed, 143 insertions(+) >>>>> >>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>>> index c8053c0d735f..537a3910ffd8 100644 >>>>> --- a/drivers/soc/ti/pruss.c >>>>> +++ b/drivers/soc/ti/pruss.c >>>>> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >>>>> } >>>>> EXPORT_SYMBOL_GPL(pruss_release_mem_region); >>>> >>>> cheers, >>>> -roger >>> >
Hi Danish, On 10/03/2023 13:53, Md Danish Anwar wrote: > Hi Roger, > > On 09/03/23 17:00, Md Danish Anwar wrote: >> Hi Roger, >> >> On 08/03/23 17:12, Roger Quadros wrote: >>> >>> >>> On 08/03/2023 13:36, Md Danish Anwar wrote: >>>> Hi Roger, >>>> >>>> On 08/03/23 13:57, Roger Quadros wrote: >>>>> Hi, >>>>> >>>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>>> respectively a register within the PRUSS CFG sub-module represented >>>>>> by a syscon driver. This interface provides a simple way for client >>>>> >>>>> Do you really need these 2 functions to be public? >>>>> I see that later patches (4-6) add APIs for doing specific things >>>>> and that should be sufficient than exposing entire CFG space via >>>>> pruss_cfg_read/update(). >>>>> >>>>> >>>> >>>> I think the intention here is to keep this APIs pruss_cfg_read() and >>>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>>> when needed. >>> >>> Where are these other drivers? If they don't exist then let's not make provision >>> for it now. >>> We can provide necessary API helpers when needed instead of letting client drivers >>> do what they want as they can be misused and hard to debug. >>> >> >> The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in >> the series [1]. The ethernet driver series is dependent on this series. In >> series [1] we are using pruss_cfg_update() in icssg_config.c file, >> icssg_config() API. You can instead add a new API on what exactly you want it to do rather than exposing entire CFG space. >> >> So for this, the API pruss_cfg_update() needs to be public. >> >> [1] https://lore.kernel.org/all/20230210114957.2667963-3-danishanwar@ti.com/ >> > > I will keep this patch as it is as pruss_cfg_update() needs to be public for > ICSSG Ethernet driver and pruss_cfg_read() is kind of a complementary function > to update. I will do required changes in other patches and send next revision > if that's OK with you. Please let me know. > >>>> >>>> The later patches (4-6) add APIs to do specific thing, but those APIs also >>>> eventually call pruss_cfg_read/update(). >>> >>> They can still call them but they need to be private to pruss.c >>> >>>> >>>>>> drivers without having them to include and parse the CFG syscon node >>>>>> within their respective device nodes. Various useful registers and >>>>>> macros for certain register bit-fields and their values have also >>>>>> been added. >>>>>> >>>>>> It is the responsibility of the client drivers to reconfigure or >>>>>> reset a particular register upon any failures. >>>>>> >>>>>> 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 | 41 +++++++++++++ >>>>>> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 143 insertions(+) >>>>>> >>>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>>>> index c8053c0d735f..537a3910ffd8 100644 >>>>>> --- a/drivers/soc/ti/pruss.c >>>>>> +++ b/drivers/soc/ti/pruss.c >>>>>> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(pruss_release_mem_region); >>>>> cheers, -roger
Hi Roger, On 10/03/23 18:53, Roger Quadros wrote: > Hi Danish, > > On 10/03/2023 13:53, Md Danish Anwar wrote: >> Hi Roger, >> >> On 09/03/23 17:00, Md Danish Anwar wrote: >>> Hi Roger, >>> >>> On 08/03/23 17:12, Roger Quadros wrote: >>>> >>>> >>>> On 08/03/2023 13:36, Md Danish Anwar wrote: >>>>> Hi Roger, >>>>> >>>>> On 08/03/23 13:57, Roger Quadros wrote: >>>>>> Hi, >>>>>> >>>>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>>>> respectively a register within the PRUSS CFG sub-module represented >>>>>>> by a syscon driver. This interface provides a simple way for client >>>>>> >>>>>> Do you really need these 2 functions to be public? >>>>>> I see that later patches (4-6) add APIs for doing specific things >>>>>> and that should be sufficient than exposing entire CFG space via >>>>>> pruss_cfg_read/update(). >>>>>> >>>>>> >>>>> >>>>> I think the intention here is to keep this APIs pruss_cfg_read() and >>>>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>>>> when needed. >>>> >>>> Where are these other drivers? If they don't exist then let's not make provision >>>> for it now. >>>> We can provide necessary API helpers when needed instead of letting client drivers >>>> do what they want as they can be misused and hard to debug. >>>> >>> >>> The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in >>> the series [1]. The ethernet driver series is dependent on this series. In >>> series [1] we are using pruss_cfg_update() in icssg_config.c file, >>> icssg_config() API. > > You can instead add a new API on what exactly you want it to do rather than exposing > entire CFG space. > Sure. In icssg_config.c, a call to pruss_cfg_update() is made to enable XFR shift for PRU and RTU, /* enable XFR shift for PRU and RTU */ mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; pruss_cfg_update(prueth->pruss, PRUSS_CFG_SPP, mask, mask); I will add the below API as part of Patch 4 of the series. We'll call this API and entire CFG space will not be exposed. /** * pruss_cfg_xfr_pru_rtu_enable() - Enable/disable XFR shift for PRU and RTU * @pruss: the pruss instance * @enable: enable/disable * * Return: 0 on success, or an error code otherwise */ static inline int pruss_cfg_xfr_pru_rtu_enable(struct pruss *pruss, bool enable) { u32 mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; u32 set = enable ? mask : 0; return pruss_cfg_update(pruss, PRUSS_CFG_SPP, mask, set); } To make pruss_cfg_update() and pruss_cfg_read() API internal to pruss.c, I will add the below change to pruss.h file and pruss.c file. Let me know if this change looks okay to you. diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c index 537a3910ffd8..9f01c8809deb 100644 --- a/drivers/soc/ti/pruss.c +++ b/drivers/soc/ti/pruss.c @@ -182,7 +182,6 @@ int pruss_cfg_read(struct pruss *pruss, unsigned int reg, unsigned int *val) return regmap_read(pruss->cfg_regmap, reg, val); } -EXPORT_SYMBOL_GPL(pruss_cfg_read); /** * pruss_cfg_update() - configure a PRUSS CFG sub-module register @@ -203,7 +202,6 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, return regmap_update_bits(pruss->cfg_regmap, reg, mask, val); } -EXPORT_SYMBOL_GPL(pruss_cfg_update); static void pruss_of_free_clk_provider(void *data) { diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h index d41bec448f06..12ef10b9fe9a 100644 --- a/include/linux/remoteproc/pruss.h +++ b/include/linux/remoteproc/pruss.h @@ -165,9 +165,6 @@ 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); -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); #else @@ -191,18 +188,6 @@ static inline int pruss_release_mem_region(struct pruss *pruss, return -EOPNOTSUPP; } -static inline int pruss_cfg_read(struct pruss *pruss, unsigned int reg, - unsigned int *val) -{ - return -EOPNOTSUPP; -} - -static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, - unsigned int mask, unsigned int val) -{ - return -EOPNOTSUPP; -} - #endif /* CONFIG_TI_PRUSS */ #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) Please have a look and let me know if above API and code changes looks OK to you. >>> >>> So for this, the API pruss_cfg_update() needs to be public. >>> >>> [1] https://lore.kernel.org/all/20230210114957.2667963-3-danishanwar@ti.com/ >>> >> >> I will keep this patch as it is as pruss_cfg_update() needs to be public for >> ICSSG Ethernet driver and pruss_cfg_read() is kind of a complementary function >> to update. I will do required changes in other patches and send next revision >> if that's OK with you. Please let me know. >> >>>>> >>>>> The later patches (4-6) add APIs to do specific thing, but those APIs also >>>>> eventually call pruss_cfg_read/update(). >>>> >>>> They can still call them but they need to be private to pruss.c >>>> >>>>> >>>>>>> drivers without having them to include and parse the CFG syscon node >>>>>>> within their respective device nodes. Various useful registers and >>>>>>> macros for certain register bit-fields and their values have also >>>>>>> been added. >>>>>>> >>>>>>> It is the responsibility of the client drivers to reconfigure or >>>>>>> reset a particular register upon any failures. >>>>>>> >>>>>>> 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 | 41 +++++++++++++ >>>>>>> include/linux/remoteproc/pruss.h | 102 +++++++++++++++++++++++++++++++ >>>>>>> 2 files changed, 143 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>>>>> index c8053c0d735f..537a3910ffd8 100644 >>>>>>> --- a/drivers/soc/ti/pruss.c >>>>>>> +++ b/drivers/soc/ti/pruss.c >>>>>>> @@ -164,6 +164,47 @@ int pruss_release_mem_region(struct pruss *pruss, >>>>>>> } >>>>>>> EXPORT_SYMBOL_GPL(pruss_release_mem_region); >>>>>> > > cheers, > -roger
Hi Danish, On 10/03/2023 17:36, Md Danish Anwar wrote: > Hi Roger, > > On 10/03/23 18:53, Roger Quadros wrote: >> Hi Danish, >> >> On 10/03/2023 13:53, Md Danish Anwar wrote: >>> Hi Roger, >>> >>> On 09/03/23 17:00, Md Danish Anwar wrote: >>>> Hi Roger, >>>> >>>> On 08/03/23 17:12, Roger Quadros wrote: >>>>> >>>>> >>>>> On 08/03/2023 13:36, Md Danish Anwar wrote: >>>>>> Hi Roger, >>>>>> >>>>>> On 08/03/23 13:57, Roger Quadros wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>>>>> respectively a register within the PRUSS CFG sub-module represented >>>>>>>> by a syscon driver. This interface provides a simple way for client >>>>>>> >>>>>>> Do you really need these 2 functions to be public? >>>>>>> I see that later patches (4-6) add APIs for doing specific things >>>>>>> and that should be sufficient than exposing entire CFG space via >>>>>>> pruss_cfg_read/update(). >>>>>>> >>>>>>> >>>>>> >>>>>> I think the intention here is to keep this APIs pruss_cfg_read() and >>>>>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>>>>> when needed. >>>>> >>>>> Where are these other drivers? If they don't exist then let's not make provision >>>>> for it now. >>>>> We can provide necessary API helpers when needed instead of letting client drivers >>>>> do what they want as they can be misused and hard to debug. >>>>> >>>> >>>> The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in >>>> the series [1]. The ethernet driver series is dependent on this series. In >>>> series [1] we are using pruss_cfg_update() in icssg_config.c file, >>>> icssg_config() API. >> >> You can instead add a new API on what exactly you want it to do rather than exposing >> entire CFG space. >> > > Sure. > > In icssg_config.c, a call to pruss_cfg_update() is made to enable XFR shift for > PRU and RTU, > > /* enable XFR shift for PRU and RTU */ > mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; > pruss_cfg_update(prueth->pruss, PRUSS_CFG_SPP, mask, mask); > > I will add the below API as part of Patch 4 of the series. We'll call this API > and entire CFG space will not be exposed. > > /** > * pruss_cfg_xfr_pru_rtu_enable() - Enable/disable XFR shift for PRU and RTU > * @pruss: the pruss instance > * @enable: enable/disable > * > * Return: 0 on success, or an error code otherwise > */ > static inline int pruss_cfg_xfr_pru_rtu_enable(struct pruss *pruss, bool enable) > { > u32 mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; > u32 set = enable ? mask : 0; > > return pruss_cfg_update(pruss, PRUSS_CFG_SPP, mask, set); > } I would suggest to make separate APIs for PRU XFR vs RTU XFR. > > To make pruss_cfg_update() and pruss_cfg_read() API internal to pruss.c, I will > add the below change to pruss.h file and pruss.c file. Let me know if this > change looks okay to you. > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > > index 537a3910ffd8..9f01c8809deb 100644 > > --- a/drivers/soc/ti/pruss.c > > +++ b/drivers/soc/ti/pruss.c > > @@ -182,7 +182,6 @@ int pruss_cfg_read(struct pruss *pruss, unsigned int reg, > unsigned int *val) Need to declare this as 'static'. > > > > return regmap_read(pruss->cfg_regmap, reg, val); > > } > > -EXPORT_SYMBOL_GPL(pruss_cfg_read); > > > > /** > > * pruss_cfg_update() - configure a PRUSS CFG sub-module register > > @@ -203,7 +202,6 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, this as well. > > > > return regmap_update_bits(pruss->cfg_regmap, reg, mask, val); > > } > > -EXPORT_SYMBOL_GPL(pruss_cfg_update); > > > > static void pruss_of_free_clk_provider(void *data) > > { > > diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h > > index d41bec448f06..12ef10b9fe9a 100644 > > --- a/include/linux/remoteproc/pruss.h > > +++ b/include/linux/remoteproc/pruss.h > > @@ -165,9 +165,6 @@ 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); > > -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); > > > > #else > > > > @@ -191,18 +188,6 @@ static inline int pruss_release_mem_region(struct pruss > *pruss, > > return -EOPNOTSUPP; > > } > > > > -static inline int pruss_cfg_read(struct pruss *pruss, unsigned int reg, > > - unsigned int *val) > > -{ > > - return -EOPNOTSUPP; > > -} > > - > > -static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > > - unsigned int mask, unsigned int val) > > -{ > > - return -EOPNOTSUPP; > > -} > > - > > #endif /* CONFIG_TI_PRUSS */ > > > > #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) > > > Please have a look and let me know if above API and code changes looks OK to you. > Rest looks OK. cheers, -roger
Hi Roger On 11/03/23 17:36, Roger Quadros wrote: > Hi Danish, > > On 10/03/2023 17:36, Md Danish Anwar wrote: >> Hi Roger, >> >> On 10/03/23 18:53, Roger Quadros wrote: >>> Hi Danish, >>> >>> On 10/03/2023 13:53, Md Danish Anwar wrote: >>>> Hi Roger, >>>> >>>> On 09/03/23 17:00, Md Danish Anwar wrote: >>>>> Hi Roger, >>>>> >>>>> On 08/03/23 17:12, Roger Quadros wrote: >>>>>> >>>>>> >>>>>> On 08/03/2023 13:36, Md Danish Anwar wrote: >>>>>>> Hi Roger, >>>>>>> >>>>>>> On 08/03/23 13:57, Roger Quadros wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>>>>>> respectively a register within the PRUSS CFG sub-module represented >>>>>>>>> by a syscon driver. This interface provides a simple way for client >>>>>>>> >>>>>>>> Do you really need these 2 functions to be public? >>>>>>>> I see that later patches (4-6) add APIs for doing specific things >>>>>>>> and that should be sufficient than exposing entire CFG space via >>>>>>>> pruss_cfg_read/update(). >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> I think the intention here is to keep this APIs pruss_cfg_read() and >>>>>>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>>>>>> when needed. >>>>>> >>>>>> Where are these other drivers? If they don't exist then let's not make provision >>>>>> for it now. >>>>>> We can provide necessary API helpers when needed instead of letting client drivers >>>>>> do what they want as they can be misused and hard to debug. >>>>>> >>>>> >>>>> The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in >>>>> the series [1]. The ethernet driver series is dependent on this series. In >>>>> series [1] we are using pruss_cfg_update() in icssg_config.c file, >>>>> icssg_config() API. >>> >>> You can instead add a new API on what exactly you want it to do rather than exposing >>> entire CFG space. >>> >> >> Sure. >> >> In icssg_config.c, a call to pruss_cfg_update() is made to enable XFR shift for >> PRU and RTU, >> >> /* enable XFR shift for PRU and RTU */ >> mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; >> pruss_cfg_update(prueth->pruss, PRUSS_CFG_SPP, mask, mask); >> >> I will add the below API as part of Patch 4 of the series. We'll call this API >> and entire CFG space will not be exposed. >> >> /** >> * pruss_cfg_xfr_pru_rtu_enable() - Enable/disable XFR shift for PRU and RTU >> * @pruss: the pruss instance >> * @enable: enable/disable >> * >> * Return: 0 on success, or an error code otherwise >> */ >> static inline int pruss_cfg_xfr_pru_rtu_enable(struct pruss *pruss, bool enable) >> { >> u32 mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; >> u32 set = enable ? mask : 0; >> >> return pruss_cfg_update(pruss, PRUSS_CFG_SPP, mask, set); >> } > > I would suggest to make separate APIs for PRU XFR vs RTU XFR. > How about making only one API for XFR shift and passing PRU or RTU as argument to the API. The API along with struct pruss and bool enable will take another argument u32 mask. mask = PRUSS_SPP_XFER_SHIFT_EN for PRU mask = PRUSS_SPP_RTU_XFR_SHIFT_EN for RTU mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN for PRU and RTU So one API will be able to do all three jobs. How does this seem? >> >> To make pruss_cfg_update() and pruss_cfg_read() API internal to pruss.c, I will >> add the below change to pruss.h file and pruss.c file. Let me know if this >> change looks okay to you. >> >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >> >> index 537a3910ffd8..9f01c8809deb 100644 >> >> --- a/drivers/soc/ti/pruss.c >> >> +++ b/drivers/soc/ti/pruss.c >> >> @@ -182,7 +182,6 @@ int pruss_cfg_read(struct pruss *pruss, unsigned int reg, >> unsigned int *val) > > Need to declare this as 'static'. > >> >> >> >> return regmap_read(pruss->cfg_regmap, reg, val); >> >> } >> >> -EXPORT_SYMBOL_GPL(pruss_cfg_read); >> >> >> >> /** >> >> * pruss_cfg_update() - configure a PRUSS CFG sub-module register >> >> @@ -203,7 +202,6 @@ int pruss_cfg_update(struct pruss *pruss, unsigned int reg, > > this as well. > >> >> >> >> return regmap_update_bits(pruss->cfg_regmap, reg, mask, val); >> >> } >> >> -EXPORT_SYMBOL_GPL(pruss_cfg_update); >> >> >> >> static void pruss_of_free_clk_provider(void *data) >> >> { >> >> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h >> >> index d41bec448f06..12ef10b9fe9a 100644 >> >> --- a/include/linux/remoteproc/pruss.h >> >> +++ b/include/linux/remoteproc/pruss.h >> >> @@ -165,9 +165,6 @@ 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); >> >> -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); >> >> >> >> #else >> >> >> >> @@ -191,18 +188,6 @@ static inline int pruss_release_mem_region(struct pruss >> *pruss, >> >> return -EOPNOTSUPP; >> >> } >> >> >> >> -static inline int pruss_cfg_read(struct pruss *pruss, unsigned int reg, >> >> - unsigned int *val) >> >> -{ >> >> - return -EOPNOTSUPP; >> >> -} >> >> - >> >> -static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >> >> - unsigned int mask, unsigned int val) >> >> -{ >> >> - return -EOPNOTSUPP; >> >> -} >> >> - >> >> #endif /* CONFIG_TI_PRUSS */ >> >> >> >> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >> >> >> Please have a look and let me know if above API and code changes looks OK to you. >> > > Rest looks OK. > > cheers, > -roger
On 13/03/2023 07:01, Md Danish Anwar wrote: > Hi Roger > > On 11/03/23 17:36, Roger Quadros wrote: >> Hi Danish, >> >> On 10/03/2023 17:36, Md Danish Anwar wrote: >>> Hi Roger, >>> >>> On 10/03/23 18:53, Roger Quadros wrote: >>>> Hi Danish, >>>> >>>> On 10/03/2023 13:53, Md Danish Anwar wrote: >>>>> Hi Roger, >>>>> >>>>> On 09/03/23 17:00, Md Danish Anwar wrote: >>>>>> Hi Roger, >>>>>> >>>>>> On 08/03/23 17:12, Roger Quadros wrote: >>>>>>> >>>>>>> >>>>>>> On 08/03/2023 13:36, Md Danish Anwar wrote: >>>>>>>> Hi Roger, >>>>>>>> >>>>>>>> On 08/03/23 13:57, Roger Quadros wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 06/03/2023 13:09, 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 allow other drivers to read and program >>>>>>>>>> respectively a register within the PRUSS CFG sub-module represented >>>>>>>>>> by a syscon driver. This interface provides a simple way for client >>>>>>>>> >>>>>>>>> Do you really need these 2 functions to be public? >>>>>>>>> I see that later patches (4-6) add APIs for doing specific things >>>>>>>>> and that should be sufficient than exposing entire CFG space via >>>>>>>>> pruss_cfg_read/update(). >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> I think the intention here is to keep this APIs pruss_cfg_read() and >>>>>>>> pruss_cfg_update() public so that other drivers can read / modify PRUSS config >>>>>>>> when needed. >>>>>>> >>>>>>> Where are these other drivers? If they don't exist then let's not make provision >>>>>>> for it now. >>>>>>> We can provide necessary API helpers when needed instead of letting client drivers >>>>>>> do what they want as they can be misused and hard to debug. >>>>>>> >>>>>> >>>>>> The ICSSG Ethernet driver uses pruss_cfg_update() API. It is posted upstream in >>>>>> the series [1]. The ethernet driver series is dependent on this series. In >>>>>> series [1] we are using pruss_cfg_update() in icssg_config.c file, >>>>>> icssg_config() API. >>>> >>>> You can instead add a new API on what exactly you want it to do rather than exposing >>>> entire CFG space. >>>> >>> >>> Sure. >>> >>> In icssg_config.c, a call to pruss_cfg_update() is made to enable XFR shift for >>> PRU and RTU, >>> >>> /* enable XFR shift for PRU and RTU */ >>> mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; >>> pruss_cfg_update(prueth->pruss, PRUSS_CFG_SPP, mask, mask); >>> >>> I will add the below API as part of Patch 4 of the series. We'll call this API >>> and entire CFG space will not be exposed. >>> >>> /** >>> * pruss_cfg_xfr_pru_rtu_enable() - Enable/disable XFR shift for PRU and RTU >>> * @pruss: the pruss instance >>> * @enable: enable/disable >>> * >>> * Return: 0 on success, or an error code otherwise >>> */ >>> static inline int pruss_cfg_xfr_pru_rtu_enable(struct pruss *pruss, bool enable) >>> { >>> u32 mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; >>> u32 set = enable ? mask : 0; >>> >>> return pruss_cfg_update(pruss, PRUSS_CFG_SPP, mask, set); >>> } >> >> I would suggest to make separate APIs for PRU XFR vs RTU XFR. >> > > How about making only one API for XFR shift and passing PRU or RTU as argument > to the API. The API along with struct pruss and bool enable will take another > argument u32 mask. > > mask = PRUSS_SPP_XFER_SHIFT_EN for PRU > mask = PRUSS_SPP_RTU_XFR_SHIFT_EN for RTU > mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN for PRU and RTU > > So one API will be able to do all three jobs. > > How does this seem? Yes, that is also fine. cheers, -roger
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c index c8053c0d735f..537a3910ffd8 100644 --- a/drivers/soc/ti/pruss.c +++ b/drivers/soc/ti/pruss.c @@ -164,6 +164,47 @@ 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 + */ +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); +} +EXPORT_SYMBOL_GPL(pruss_cfg_read); + +/** + * 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 + */ +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); +} +EXPORT_SYMBOL_GPL(pruss_cfg_update); + 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..d41bec448f06 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. @@ -78,6 +165,9 @@ 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); +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); #else @@ -101,6 +191,18 @@ static inline int pruss_release_mem_region(struct pruss *pruss, return -EOPNOTSUPP; } +static inline int pruss_cfg_read(struct pruss *pruss, unsigned int reg, + unsigned int *val) +{ + return -EOPNOTSUPP; +} + +static inline int pruss_cfg_update(struct pruss *pruss, unsigned int reg, + unsigned int mask, unsigned int val) +{ + return -EOPNOTSUPP; +} + #endif /* CONFIG_TI_PRUSS */ #if IS_ENABLED(CONFIG_PRU_REMOTEPROC)