From: Simon Horman <horms@kernel•org>
To: Meghana Malladi <m-malladi@ti•com>
Cc: vigneshr@ti•com, m-karicheri2@ti•com, jan.kiszka@siemens•com,
javier.carrasco.cruz@gmail•com, jacob.e.keller@intel•com,
diogo.ivo@siemens•com, pabeni@redhat•com, kuba@kernel•org,
edumazet@google•com, davem@davemloft•net, andrew+netdev@lunn•ch,
linux-kernel@vger•kernel.org, netdev@vger•kernel.org,
linux-arm-kernel@lists•infradead.org, srk@ti•com,
Roger Quadros <rogerq@kernel•org>,
danishanwar@ti•com
Subject: Re: [PATCH net 1/2] net: ti: icssg-prueth: Fix firmware load sequence.
Date: Fri, 8 Nov 2024 13:30:44 +0000 [thread overview]
Message-ID: <20241108133044.GB4507@kernel.org> (raw)
In-Reply-To: <20241106074040.3361730-2-m-malladi@ti.com>
On Wed, Nov 06, 2024 at 01:10:39PM +0530, Meghana Malladi wrote:
> From: MD Danish Anwar <danishanwar@ti•com>
>
> Timesync related operations are ran in PRU0 cores for both ICSSG SLICE0
> and SLICE1. Currently whenever any ICSSG interface comes up we load the
> respective firmwares to PRU cores and whenever interface goes down, we
> stop the respective cores. Due to this, when SLICE0 goes down while
> SLICE1 is still active, PRU0 firmwares are unloaded and PRU0 core is
> stopped. This results in clock jump for SLICE1 interface as the timesync
> related operations are no longer running.
>
> Fix this by running both PRU0 and PRU1 firmwares as long as at least 1
> ICSSG interface is up.
>
> rx_flow_id is updated before firmware is loaded. Once firmware is loaded,
> it reads the flow_id and uses it for rx. emac_fdb_flow_id_updated() is
> used to let firmware know that the flow_id has been updated and to use the
> latest rx_flow_id.
>
> Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
> Signed-off-by: MD Danish Anwar <danishanwar@ti•com>
> Signed-off-by: Meghana Malladi <m-malladi@ti•com>
...
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
...
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 0556910938fa..9df67539285b 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -534,6 +534,7 @@ static int emac_ndo_open(struct net_device *ndev)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> int ret, i, num_data_chn = emac->tx_ch_num;
> + struct icssg_flow_cfg __iomem *flow_cfg;
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> struct device *dev = prueth->dev;
> @@ -549,8 +550,12 @@ static int emac_ndo_open(struct net_device *ndev)
> /* set h/w MAC as user might have re-configured */
> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>
> + if (!prueth->emacs_initialized) {
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE0, 0, false);
> + icssg_class_default(prueth->miig_rt, ICSS_SLICE1, 0, false);
> + }
> +
> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
> - icssg_class_default(prueth->miig_rt, slice, 0, false);
> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>
> /* Notify the stack of the actual queue counts. */
> @@ -588,10 +593,31 @@ static int emac_ndo_open(struct net_device *ndev)
> goto cleanup_napi;
> }
>
> - /* reset and start PRU firmware */
> - ret = prueth_emac_start(prueth, emac);
> - if (ret)
> - goto free_rx_irq;
> + if (!prueth->emacs_initialized) {
> + if (prueth->emac[ICSS_SLICE0]) {
> + ret = prueth_emac_start(prueth, prueth->emac[ICSS_SLICE0]);
I wonder if it is worth simplifying this by having prueth_emac_start()
check if it's 2nd parameter is NULL. Likewise for prueth_emac_stop().
> + if (ret) {
> + netdev_err(ndev, "unable to start fw for slice %d", ICSS_SLICE0);
> + goto free_rx_irq;
> + }
> + }
> + if (prueth->emac[ICSS_SLICE1]) {
> + ret = prueth_emac_start(prueth, prueth->emac[ICSS_SLICE1]);
> + if (ret) {
> + netdev_err(ndev, "unable to start fw for slice %d", ICSS_SLICE1);
> + goto halt_slice0_prus;
> + }
> + }
> + }
> +
> + flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
> + writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
> + ret = emac_fdb_flow_id_updated(emac);
> +
> + if (ret) {
> + netdev_err(ndev, "Failed to update Rx Flow ID %d", ret);
> + goto stop;
Branching to stop may result in calls to prueth_emac_stop() which does not
seem symmetrical with the condition on prueth->emacs_initialized for calls
to prueth_emac_start() above.
If so, is this intended?
> + }
>
> icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>
...
next prev parent reply other threads:[~2024-11-08 13:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-06 7:40 [PATCH net 0/2] IEP clock module bug fixes Meghana Malladi
2024-11-06 7:40 ` [PATCH net 1/2] net: ti: icssg-prueth: Fix firmware load sequence Meghana Malladi
2024-11-08 13:30 ` Simon Horman [this message]
2024-11-11 11:53 ` Meghana Malladi
2024-11-11 13:03 ` Roger Quadros
2024-11-11 13:35 ` Anwar, Md Danish
2024-11-11 14:25 ` Roger Quadros
2024-11-11 15:29 ` Anwar, Md Danish
2024-11-12 8:29 ` Roger Quadros
2024-11-06 7:40 ` [PATCH net 2/2] net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init Meghana Malladi
2024-11-11 13:53 ` Roger Quadros
2024-11-12 9:04 ` Meghana Malladi
2024-11-12 13:17 ` Roger Quadros
2024-11-12 14:36 ` Meghana Malladi
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=20241108133044.GB4507@kernel.org \
--to=horms@kernel$(echo .)org \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=danishanwar@ti$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=diogo.ivo@siemens$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=jan.kiszka@siemens$(echo .)com \
--cc=javier.carrasco.cruz@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=m-karicheri2@ti$(echo .)com \
--cc=m-malladi@ti$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=rogerq@kernel$(echo .)org \
--cc=srk@ti$(echo .)com \
--cc=vigneshr@ti$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox