Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks

[Leon Romanovsky <leon@kernel.org>]

On Wed, Jan 04, 2023 at 03:59:17PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Jan 04, 2023 at 03:03:14PM +0100, Lorenzo Bianconi wrote:
> > > Introduce reset and reset_complete wlan callback to schedule WLAN driver
> > > reset when ethernet/wed driver is resetting.
> > > 
> > > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > > Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  6 ++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
> > >  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
> > >  4 files changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index bafae4f0312e..2d74e26f45c9 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> > >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> > >  	}
> > >  
> > > +	rtnl_unlock();
> > > +	mtk_wed_fe_reset();
> > > +	rtnl_lock();
> > 
> > Is it safe to call rtnl_unlock(), perform some work and lock again?
> 
> Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
> scheduled until this bit is cleared

I'm more worried about opening a window for user-space access while you
are performing FW reset.

<...>

> > > diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h
> > > index db637a13888d..ddff54fc9717 100644
> > > --- a/include/linux/soc/mediatek/mtk_wed.h
> > > +++ b/include/linux/soc/mediatek/mtk_wed.h
> > > @@ -150,6 +150,8 @@ struct mtk_wed_device {
> > >  		void (*release_rx_buf)(struct mtk_wed_device *wed);
> > >  		void (*update_wo_rx_stats)(struct mtk_wed_device *wed,
> > >  					   struct mtk_wed_wo_rx_stats *stats);
> > > +		int (*reset)(struct mtk_wed_device *wed);
> > > +		int (*reset_complete)(struct mtk_wed_device *wed);
> > 
> > I don't see any driver implementation of these callbacks in this series.
> > Did I miss it?
> 
> These callbacks are implemented in the mt76 driver. I have not added these
> patches to the series since mt76 patches usually go through Felix/Kalle's
> trees (anyway I am fine to add them to the series if they can go into net-next
> directly).

Usually patches that use specific functionality are submitted together
with API changes.

> 
> Regards,
> Lorenzo
> 
> > 
> > Thanks
> > 
> > >  	} wlan;
> > >  #endif
> > >  };
> > > -- 
> > > 2.39.0
> > >