From: Romain Naour <romain.naour@smile.fr> To: linux-kernel@vger.kernel.org, Md Danish Anwar <danishanwar@ti.com> Cc: bjorn.andersson@linaro.org, mathieu.poirier@linaro.org, krzysztof.kozlowski+dt@linaro.org, linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org, nm@ti.com, ssantosh@kernel.org, s-anna@ti.com, linux-arm-kernel@lists.infradead.org, rogerq@kernel.org, grygorii.strashko@ti.com, vigneshr@ti.com, kishon@ti.com Subject: Re: [PATCH v2 6/6] soc: ti: pruss: Add helper functions to get/set PRUSS_CFG_GPMUX Date: Mon, 12 Sep 2022 16:20:17 +0200 [thread overview] Message-ID: <94b57cbc-b865-e0b4-0d52-3da72f2dd026@smile.fr> (raw) In-Reply-To: <20220418123004.9332-7-p-mohan@ti.com> +Danish Hi Danish, (Removed Puranjay (as he is no longer with TI) and adding Danish.) Le 18/04/2022 à 14:30, Puranjay Mohan a écrit : > 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. Actually I'm curious about how the GP MUX mode are supposed to work in some cases. The register mapping in the AM57xx TRM seems confusing. See the "PRU-ICSS I/O Interface" part about the "PRU-ICSS Internal Wrapper Multiplexing" [1]. The commit "ARM: dts: am57xx-idk: Add prueth on ICSS" [2] (only in the TI kernel tree) adds pruss1 and pruss2 for the am571x-idk board. But this commit doesn't really explain the ti,pruss-gp-mux-sel setting from pruss1_eth and pruss2_eth: /* Dual mac ethernet application node on icss1 */ pruss1_eth { status = "okay"; compatible = "ti,am57-prueth"; ti,pruss-gp-mux-sel = <0>, /* GP, default */ <4>; /* MII2, needed for PRUSS1_MII1 */ } &pruss2_eth { ti,pruss-gp-mux-sel = <4>, /* MII2, needed for PRUSS1_MII0 */ <4>; /* MII2, needed for PRUSS1_MII1 */ }; At the first look, the two comments in pruss2_eth node about PRUSS1_MIIx seems dubious. Indeed, it would means that the PRUSS2 setting (ti,pruss-gp-mux-sel) is required to makes PRUSS1 work. In my use case, only the pruss1 is expected to be used with the prueth driver. Actually, the prueth on PRUSS1 partially works with only pruss1_eth's gp-mux initialized: pruss1_eth { status = "okay"; compatible = "ti,am57-prueth"; ti,pruss-gp-mux-sel = <0>, /* GP, default */ <4>; /* MII2, needed for PRUSS1_MII1 */ } pruss2_eth { status = "disabled"; } (Tests done with the ti-linux-kernel 5.10.y) On wireshark I noticed ethernet frames (ping) sent from the board but the reply from the remote PC is never received on the board. It really seems we need pruss2_eth's gp-mux initialized. The problem here is that I don't want to enable PRUSS2 just to configure pruss2_eth's gp-mux for the sake of pruss1. I had to write manually (using devmem2) the "good" value (0x10002003) in PRUSS2_CFG0 and PRUSS2_CFG1 to configure entirely the PRUSS1_MII1. I'm not sure how the driver should handle this register mapping properly. [1] https://www.ti.com/lit/ds/symlink/am5749.pdf [2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=linux-5.10.y&id=2a3b089f5697fe2f9a9875b2fba1bef88d196a53 Best regards, Romain > > 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> > --- > include/linux/pruss_driver.h | 44 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/include/linux/pruss_driver.h b/include/linux/pruss_driver.h > index e2d5477225c6..3312281ef4c1 100644 > --- a/include/linux/pruss_driver.h > +++ b/include/linux/pruss_driver.h > @@ -35,4 +35,48 @@ struct pruss { > struct clk *iep_clk_mux; > }; > > +/** > + * 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 > + */ > +static inline 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); > + return ret; > +} > + > +/** > + * 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 > + */ > +static inline 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); > +} > + > #endif /* _PRUSS_DRIVER_H_ */
WARNING: multiple messages have this Message-ID (diff)
From: Romain Naour <romain.naour@smile.fr> To: linux-kernel@vger.kernel.org, Md Danish Anwar <danishanwar@ti.com> Cc: bjorn.andersson@linaro.org, mathieu.poirier@linaro.org, krzysztof.kozlowski+dt@linaro.org, linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org, nm@ti.com, ssantosh@kernel.org, s-anna@ti.com, linux-arm-kernel@lists.infradead.org, rogerq@kernel.org, grygorii.strashko@ti.com, vigneshr@ti.com, kishon@ti.com Subject: Re: [PATCH v2 6/6] soc: ti: pruss: Add helper functions to get/set PRUSS_CFG_GPMUX Date: Mon, 12 Sep 2022 16:20:17 +0200 [thread overview] Message-ID: <94b57cbc-b865-e0b4-0d52-3da72f2dd026@smile.fr> (raw) In-Reply-To: <20220418123004.9332-7-p-mohan@ti.com> +Danish Hi Danish, (Removed Puranjay (as he is no longer with TI) and adding Danish.) Le 18/04/2022 à 14:30, Puranjay Mohan a écrit : > 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. Actually I'm curious about how the GP MUX mode are supposed to work in some cases. The register mapping in the AM57xx TRM seems confusing. See the "PRU-ICSS I/O Interface" part about the "PRU-ICSS Internal Wrapper Multiplexing" [1]. The commit "ARM: dts: am57xx-idk: Add prueth on ICSS" [2] (only in the TI kernel tree) adds pruss1 and pruss2 for the am571x-idk board. But this commit doesn't really explain the ti,pruss-gp-mux-sel setting from pruss1_eth and pruss2_eth: /* Dual mac ethernet application node on icss1 */ pruss1_eth { status = "okay"; compatible = "ti,am57-prueth"; ti,pruss-gp-mux-sel = <0>, /* GP, default */ <4>; /* MII2, needed for PRUSS1_MII1 */ } &pruss2_eth { ti,pruss-gp-mux-sel = <4>, /* MII2, needed for PRUSS1_MII0 */ <4>; /* MII2, needed for PRUSS1_MII1 */ }; At the first look, the two comments in pruss2_eth node about PRUSS1_MIIx seems dubious. Indeed, it would means that the PRUSS2 setting (ti,pruss-gp-mux-sel) is required to makes PRUSS1 work. In my use case, only the pruss1 is expected to be used with the prueth driver. Actually, the prueth on PRUSS1 partially works with only pruss1_eth's gp-mux initialized: pruss1_eth { status = "okay"; compatible = "ti,am57-prueth"; ti,pruss-gp-mux-sel = <0>, /* GP, default */ <4>; /* MII2, needed for PRUSS1_MII1 */ } pruss2_eth { status = "disabled"; } (Tests done with the ti-linux-kernel 5.10.y) On wireshark I noticed ethernet frames (ping) sent from the board but the reply from the remote PC is never received on the board. It really seems we need pruss2_eth's gp-mux initialized. The problem here is that I don't want to enable PRUSS2 just to configure pruss2_eth's gp-mux for the sake of pruss1. I had to write manually (using devmem2) the "good" value (0x10002003) in PRUSS2_CFG0 and PRUSS2_CFG1 to configure entirely the PRUSS1_MII1. I'm not sure how the driver should handle this register mapping properly. [1] https://www.ti.com/lit/ds/symlink/am5749.pdf [2] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=linux-5.10.y&id=2a3b089f5697fe2f9a9875b2fba1bef88d196a53 Best regards, Romain > > 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> > --- > include/linux/pruss_driver.h | 44 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/include/linux/pruss_driver.h b/include/linux/pruss_driver.h > index e2d5477225c6..3312281ef4c1 100644 > --- a/include/linux/pruss_driver.h > +++ b/include/linux/pruss_driver.h > @@ -35,4 +35,48 @@ struct pruss { > struct clk *iep_clk_mux; > }; > > +/** > + * 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 > + */ > +static inline 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); > + return ret; > +} > + > +/** > + * 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 > + */ > +static inline 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); > +} > + > #endif /* _PRUSS_DRIVER_H_ */ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-09-12 14:20 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-18 12:29 [PATCH v2 0/6] Introduce PRU platform consumer API Puranjay Mohan 2022-04-18 12:29 ` Puranjay Mohan 2022-04-18 12:29 ` [PATCH v2 1/6] soc: ti: pruss: Add pruss_get()/put() API Puranjay Mohan 2022-04-18 12:29 ` Puranjay Mohan 2022-04-18 12:30 ` [PATCH v2 2/6] soc: ti: pruss: Add pruss_{request, release}_mem_region() API Puranjay Mohan 2022-04-18 12:30 ` [PATCH v2 2/6] soc: ti: pruss: Add pruss_{request,release}_mem_region() API Puranjay Mohan 2022-04-18 12:30 ` [PATCH v2 3/6] soc: ti: pruss: Add pruss_cfg_read()/update() API Puranjay Mohan 2022-04-18 12:30 ` Puranjay Mohan 2022-04-18 12:30 ` [PATCH v2 4/6] soc: ti: pruss: Add helper functions to set GPI mode, MII_RT_event and XFR Puranjay Mohan 2022-04-18 12:30 ` Puranjay Mohan 2022-04-18 12:30 ` [PATCH v2 5/6] soc: ti: pruss: Add helper function to enable OCP master ports Puranjay Mohan 2022-04-18 12:30 ` Puranjay Mohan 2022-04-21 6:51 ` Tony Lindgren 2022-04-21 6:51 ` Tony Lindgren 2022-04-18 12:30 ` [PATCH v2 6/6] soc: ti: pruss: Add helper functions to get/set PRUSS_CFG_GPMUX Puranjay Mohan 2022-04-18 12:30 ` Puranjay Mohan 2022-09-12 14:20 ` Romain Naour [this message] 2022-09-12 14:20 ` Romain Naour 2022-09-14 13:15 ` Roger Quadros 2022-09-14 13:15 ` Roger Quadros 2022-09-15 8:27 ` Romain Naour 2022-09-15 8:27 ` Romain Naour
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=94b57cbc-b865-e0b4-0d52-3da72f2dd026@smile.fr \ --to=romain.naour@smile.fr \ --cc=bjorn.andersson@linaro.org \ --cc=danishanwar@ti.com \ --cc=devicetree@vger.kernel.org \ --cc=grygorii.strashko@ti.com \ --cc=kishon@ti.com \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-remoteproc@vger.kernel.org \ --cc=mathieu.poirier@linaro.org \ --cc=nm@ti.com \ --cc=rogerq@kernel.org \ --cc=s-anna@ti.com \ --cc=ssantosh@kernel.org \ --cc=vigneshr@ti.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.