Hello Romain,
On 12/09/2022 17:20, Romain Naour wrote:
> +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@xxxxxx>
>>
>> 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.
Yes, if I remember right this is only applicable to AM571 Soc which had
an overloaded pinmuxing design and it resulted in this kind of weird constraint.
i.e. gp-mux-sel of PRUSS2 needs to be set to get PRUSS1 MII to 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
cheers,
-roger
>
>>
>> Co-developed-by: Suman Anna <s-anna@xxxxxx>
>> Signed-off-by: Suman Anna <s-anna@xxxxxx>
>> Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
>> Co-developed-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@xxxxxxxxxx>
>> Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@xxxxxxxxxx>
>> Signed-off-by: Puranjay Mohan <p-mohan@xxxxxx>
>> ---
>> 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_ */
>
