| Message ID | 20240805144754.2384663-1-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: fec: Remove duplicated code | expand |
On Mon, Aug 05, 2024 at 04:47:55PM +0200, Csókás, Bence wrote: > `fec_ptp_pps_perout()` reimplements logic already > in `fec_ptp_read()`. Replace with function call. > > Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> > --- > drivers/net/ethernet/freescale/fec_ptp.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c > index 0cc96dac8b53..19e8fcfab3bc 100644 > --- a/drivers/net/ethernet/freescale/fec_ptp.c > +++ b/drivers/net/ethernet/freescale/fec_ptp.c > @@ -211,13 +211,7 @@ static int fec_ptp_pps_perout(struct fec_enet_private *fep) > timecounter_read(&fep->tc); > > /* Get the current ptp hardware time counter */ > - temp_val = readl(fep->hwp + FEC_ATIME_CTRL); > - temp_val |= FEC_T_CTRL_CAPTURE; > - writel(temp_val, fep->hwp + FEC_ATIME_CTRL); > - if (fep->quirks & FEC_QUIRK_BUG_CAPTURE) > - udelay(1); > - > - ptp_hc = readl(fep->hwp + FEC_ATIME); > + ptp_hc = fep->cc.read(&fep->cc); why not call fec_ptp_read() directly? Frank > > /* Convert the ptp local counter to 1588 timestamp */ > curr_time = timecounter_cyc2time(&fep->tc, ptp_hc); > -- > 2.34.1 > >
On 8/5/24 17:31, Frank Li wrote: > On Mon, Aug 05, 2024 at 04:47:55PM +0200, Csókás, Bence wrote: >> - ptp_hc = readl(fep->hwp + FEC_ATIME); >> + ptp_hc = fep->cc.read(&fep->cc); > > why not call fec_ptp_read() directly? > > Frank It is defined later in the file. Using the struct cyclecounter was the solution for this we settled on in 61d5e2a251fb ("fec: Fix timer capture timing in `fec_ptp_enable_pps()`") as well. Bence
On Mon, Aug 05, 2024 at 05:36:49PM +0200, Csókás Bence wrote: > > > On 8/5/24 17:31, Frank Li wrote: > > On Mon, Aug 05, 2024 at 04:47:55PM +0200, Csókás, Bence wrote: > > > - ptp_hc = readl(fep->hwp + FEC_ATIME); > > > + ptp_hc = fep->cc.read(&fep->cc); > > > > why not call fec_ptp_read() directly? > > > > Frank > > It is defined later in the file. Using the struct cyclecounter was the > solution for this we settled on in 61d5e2a251fb ("fec: Fix timer capture > timing in `fec_ptp_enable_pps()`") as well. Actually I prefer move fec_ptp_read() ahead. It is not big deal. Direct call just little bit better because reduce a indirect call. Generally ->cc.read() supposed to be called only timercount.c otherwice, it should provide an API call in timercount.h. Sorry, I missed review 61d5e2a251fb. Frank Li > > Bence >
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 0cc96dac8b53..19e8fcfab3bc 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -211,13 +211,7 @@ static int fec_ptp_pps_perout(struct fec_enet_private *fep) timecounter_read(&fep->tc); /* Get the current ptp hardware time counter */ - temp_val = readl(fep->hwp + FEC_ATIME_CTRL); - temp_val |= FEC_T_CTRL_CAPTURE; - writel(temp_val, fep->hwp + FEC_ATIME_CTRL); - if (fep->quirks & FEC_QUIRK_BUG_CAPTURE) - udelay(1); - - ptp_hc = readl(fep->hwp + FEC_ATIME); + ptp_hc = fep->cc.read(&fep->cc); /* Convert the ptp local counter to 1588 timestamp */ curr_time = timecounter_cyc2time(&fep->tc, ptp_hc);
`fec_ptp_pps_perout()` reimplements logic already in `fec_ptp_read()`. Replace with function call. Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> --- drivers/net/ethernet/freescale/fec_ptp.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)