lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <YvKZNcVfYdLw7bkm@lunn.ch> Date: Tue, 9 Aug 2022 19:28:21 +0200 From: Andrew Lunn <andrew@...n.ch> To: Csókás Bence <csokas.bence@...lan.hu> Cc: netdev@...r.kernel.org, Richard Cochran <richardcochran@...il.com>, Fugang Duan <fugang.duan@....com> Subject: Re: [PATCH] fec: Restart PPS after link state change On Tue, Aug 09, 2022 at 02:41:19PM +0200, Csókás Bence wrote: > On link state change, the controller gets reset, > causing PPS to drop out. So we restart it if needed. > > Signed-off-by: Csókás Bence <csokas.bence@...lan.hu> > --- > drivers/net/ethernet/freescale/fec_main.c | 27 ++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index ca5d49361fdf..c264b1dd5286 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -954,6 +954,7 @@ fec_restart(struct net_device *ndev) > u32 temp_mac[2]; > u32 rcntl = OPT_FRAME_SIZE | 0x04; > u32 ecntl = 0x2; /* ETHEREN */ > + struct ptp_clock_request ptp_rq = { .type = PTP_CLK_REQ_PPS }; Is it safe to hard code this? What if the user configured PTP_CLK_REQ_EXTTS or PTP_CLK_REQ_PEROUT? > /* Whack a reset. We should wait for this. > * For i.MX6SX SOC, enet use AXI bus, we use disable MAC > @@ -1119,6 +1120,13 @@ fec_restart(struct net_device *ndev) > if (fep->bufdesc_ex) > fec_ptp_start_cyclecounter(ndev); > > + /* Restart PPS if needed */ > + if (fep->pps_enable) { > + /* Clear flag so fec_ptp_enable_pps() doesn't return immediately */ > + fep->pps_enable = 0; If reset causes PPS to stop, maybe it would be better to do this unconditionally? fep->pps_enable = 0; fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1); > + if (fep->bufdesc_ex) > + ecntl |= (1 << 4); Please replace (1 << 4) with a #define to make it clear what this is doing. > + > /* We have to keep ENET enabled to have MII interrupt stay working */ > if (fep->quirks & FEC_QUIRK_ENET_MAC && > !(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) { > - writel(2, fep->hwp + FEC_ECNTRL); > + ecntl |= 0x2; > writel(rmii_mode, fep->hwp + FEC_R_CNTRL); > } > + > + writel(ecntl, fep->hwp + FEC_ECNTRL); > + > + if (fep->bufdesc_ex) > + fec_ptp_start_cyclecounter(ndev); > + > + /* Restart PPS if needed */ > + if (fep->pps_enable) { > + /* Clear flag so fec_ptp_enable_pps() doesn't return immediately */ > + fep->pps_enable = 0; > + fep->ptp_caps.enable(&fep->ptp_caps, &ptp_rq, 1); > + } So you re-start PPS in stop()? Should it keep outputting when the interface is down? Also, if it is always outputting, don't you need to stop it in fec_drv_remove(). You probably don't want to still going after the driver is unloaded. Andrew
Powered by blists - more mailing lists