Re: [PATCH net v3 1/2] net: ti: icssg-prueth: Fix firmware load sequence.

[Diogo Ivo]

Hi all,

On 12/9/24 10:34 AM, Meghana Malladi wrote:
> On 05/12/24 18:38, Roger Quadros wrote:
> > Hi,
> >
> > On 05/12/2024 10:28, Meghana Malladi wrote:
> > > From: MD Danish Anwar <danishanwar@xxxxxx>
> > >
> > > 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 resective 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.
> > >
> > > As there are interdependencies between SLICE0 and SLICE1 firmwares,
> > > fix this by running both PRU0 and PRU1 firmwares as long as at least 1
> > > ICSSG interface is up. Add new flag in prueth struct to check if all
> > > firmwares are running.
> > >
> > > Use emacs_initialized as reference count to load the firmwares for the
> > > first and last interface up/down. Moving init_emac_mode and 
> > > fw_offload_mode
> > > API outside of icssg_config to icssg_common_start API as they need
> > > to be called only once per firmware boot.
> > >
> > > Fixes: c1e0230eeaab ("net: ti: icss-iep: Add IEP driver")
> > > Signed-off-by: MD Danish Anwar <danishanwar@xxxxxx>
> > > Signed-off-by: Meghana Malladi <m-malladi@xxxxxx>
> > > ---
> > >
> > > diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/ 
> > > net/ethernet/ti/icssg/icssg_prueth.h
> > > index f5c1d473e9f9..b30f2e9a73d8 100644
> > > --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> > > +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> > > @@ -257,6 +257,7 @@ struct icssg_firmwares {
> > >    * @is_switchmode_supported: indicates platform support for switch 
> > > mode
> > >    * @switch_id: ID for mapping switch ports to bridge
> > >    * @default_vlan: Default VLAN for host
> > > + * @prus_running: flag to indicate if all pru cores are running
> > >    */
> > >   struct prueth {
> > >       struct device *dev;
> > > @@ -298,6 +299,7 @@ struct prueth {
> > >       int default_vlan;
> > >       /** @vtbl_lock: Lock for vtbl in shared memory */
> > >       spinlock_t vtbl_lock;
> > > +    bool prus_running;
> >
> > I think you don't need fw_running flag anymore. Could you please 
> > remove it
> > from struct prueth_emac?
>
> This flag is still being used by SR1, for which this patch doesn't 
> apply. So I prefer not touching this flag for the sake of SR1.

Currently for SR1.0 this flag is set but not used anywhere in the code, 
so it can be removed.

Best regards,
Diogo