Messages in this thread | | | From | Wei Fang <> | Subject | RE: [EXTERNAL] [PATCH net-next] net: fec: Convert fec driver to use lock guards | Date | Wed, 8 May 2024 02:46:31 +0000 |
| |
> -----Original Message----- > From: Suman Ghosh <sumang@marvell.com> > Sent: 2024年5月7日 19:55 > To: Wei Fang <wei.fang@nxp.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Shenwei > Wang <shenwei.wang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>; > richardcochran@gmail.com; netdev@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; imx@lists.linux.dev > Subject: RE: [EXTERNAL] [PATCH net-next] net: fec: Convert fec driver to use > lock guards > > > drivers/net/ethernet/freescale/fec_main.c | 37 ++++---- > >drivers/net/ethernet/freescale/fec_ptp.c | 104 +++++++++------------- > > 2 files changed, 58 insertions(+), 83 deletions(-) > > > >diff --git a/drivers/net/ethernet/freescale/fec_main.c > >b/drivers/net/ethernet/freescale/fec_main.c > >index 8bd213da8fb6..5f98c0615115 100644 > >--- a/drivers/net/ethernet/freescale/fec_main.c > >+++ b/drivers/net/ethernet/freescale/fec_main.c > >@@ -1397,12 +1397,11 @@ static void > > fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, > > struct skb_shared_hwtstamps *hwtstamps) { > >- unsigned long flags; > > u64 ns; > > > >- spin_lock_irqsave(&fep->tmreg_lock, flags); > >- ns = timecounter_cyc2time(&fep->tc, ts); > >- spin_unlock_irqrestore(&fep->tmreg_lock, flags); > >+ scoped_guard(spinlock_irqsave, &fep->tmreg_lock) { > >+ ns = timecounter_cyc2time(&fep->tc, ts); > >+ } > > > > memset(hwtstamps, 0, sizeof(*hwtstamps)); > > hwtstamps->hwtstamp = ns_to_ktime(ns); @@ -2313,15 +2312,13 @@ > static > >int fec_enet_clk_enable(struct net_device *ndev, bool enable) > > return ret; > > > > if (fep->clk_ptp) { > >- mutex_lock(&fep->ptp_clk_mutex); > >- ret = clk_prepare_enable(fep->clk_ptp); > >- if (ret) { > >- mutex_unlock(&fep->ptp_clk_mutex); > >- goto failed_clk_ptp; > >- } else { > >- fep->ptp_clk_on = true; > >+ scoped_guard(mutex, &fep->ptp_clk_mutex) { > >+ ret = clk_prepare_enable(fep->clk_ptp); > >+ if (ret) > >+ goto failed_clk_ptp; > >+ else > >+ fep->ptp_clk_on = true; > > } > >- mutex_unlock(&fep->ptp_clk_mutex); > > } > > > > ret = clk_prepare_enable(fep->clk_ref); @@ -2336,10 +2333,10 > @@ > >static int fec_enet_clk_enable(struct net_device *ndev, bool enable) > > } else { > > clk_disable_unprepare(fep->clk_enet_out); > > if (fep->clk_ptp) { > >- mutex_lock(&fep->ptp_clk_mutex); > >- clk_disable_unprepare(fep->clk_ptp); > >- fep->ptp_clk_on = false; > >- mutex_unlock(&fep->ptp_clk_mutex); > >+ scoped_guard(mutex, &fep->ptp_clk_mutex) { > >+ clk_disable_unprepare(fep->clk_ptp); > >+ fep->ptp_clk_on = false; > >+ } > > } > > clk_disable_unprepare(fep->clk_ref); > > clk_disable_unprepare(fep->clk_2x_txclk); > >@@ -2352,10 +2349,10 @@ static int fec_enet_clk_enable(struct > net_device > >*ndev, bool enable) > > clk_disable_unprepare(fep->clk_ref); > > failed_clk_ref: > > if (fep->clk_ptp) { > >- mutex_lock(&fep->ptp_clk_mutex); > >- clk_disable_unprepare(fep->clk_ptp); > >- fep->ptp_clk_on = false; > >- mutex_unlock(&fep->ptp_clk_mutex); > >+ scoped_guard(mutex, &fep->ptp_clk_mutex) { > [Suman] Hi Wei, > I am new to the use of scoped_guard() and have a confusion here. Above, > "goto failed_clk_ref" is getting called after scoped_guard() declaration and > you are again doing scoped_guard() inside the goto label failed_clk_ref. Why > is this double declaration needed?
The lock will be freed when it goes out of scope. And the second scope_guard() is not nested in the first scope_guard().
> >+ clk_disable_unprepare(fep->clk_ptp); > >+ fep->ptp_clk_on = false; > >+ } > > } > > failed_clk_ptp: > > clk_disable_unprepare(fep->clk_enet_out);
| |