* [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends @ 2018-12-14 15:23 Ahmad Fatoum 2018-12-14 15:23 ` [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error Ahmad Fatoum 2018-12-17 9:48 ` [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Sascha Hauer 0 siblings, 2 replies; 5+ messages in thread From: Ahmad Fatoum @ 2018-12-14 15:23 UTC (permalink / raw) To: barebox Linux has them in include/linux/time64.h and they are useful for making (especially microsecond) use readable such as in read*_poll_timeout. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- include/linux/time.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/linux/time.h b/include/linux/time.h index 3a1bb50020fd..7903139a65b2 100644 --- a/include/linux/time.h +++ b/include/linux/time.h @@ -3,7 +3,13 @@ #include <linux/types.h> +#define MSEC_PER_SEC 1000L +#define USEC_PER_MSEC 1000L +#define NSEC_PER_USEC 1000L +#define NSEC_PER_MSEC 1000000L +#define USEC_PER_SEC 1000000L #define NSEC_PER_SEC 1000000000L +#define FSEC_PER_SEC 1000000000000000LL struct timespec { time_t tv_sec; /* seconds */ -- 2.19.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error 2018-12-14 15:23 [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Ahmad Fatoum @ 2018-12-14 15:23 ` Ahmad Fatoum 2018-12-14 15:29 ` Ahmad Fatoum 2018-12-17 9:48 ` [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Sascha Hauer 1 sibling, 1 reply; 5+ messages in thread From: Ahmad Fatoum @ 2018-12-14 15:23 UTC (permalink / raw) To: barebox read*_poll_timeout's final timeout parameter is in microseconds, but the supplied arguments in fec_imx.c were in nanoseconds, which might lead to barebox getting seemingly stuck in fec_halt (loops for a thousand seconds instead of one). I've tested this still works on an i.MX6D by copying a file over TFTP and verifying the hash is correct. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- drivers/net/fec_imx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c index d304b94c567b..642517de6e98 100644 --- a/drivers/net/fec_imx.c +++ b/drivers/net/fec_imx.c @@ -61,7 +61,7 @@ static int fec_miibus_read(struct mii_bus *bus, int phyAddr, int regAddr) * wait for the related interrupt */ if (readl_poll_timeout(fec->regs + FEC_IEVENT, reg, - reg & FEC_IEVENT_MII, MSECOND)) { + reg & FEC_IEVENT_MII, USEC_PER_MSEC)) { dev_err(&fec->edev.dev, "Read MDIO failed...\n"); return -1; } @@ -98,7 +98,7 @@ static int fec_miibus_write(struct mii_bus *bus, int phyAddr, * wait for the MII interrupt */ if (readl_poll_timeout(fec->regs + FEC_IEVENT, reg, - reg & FEC_IEVENT_MII, MSECOND)) { + reg & FEC_IEVENT_MII, USEC_PER_MSEC)) { dev_err(&fec->edev.dev, "Write MDIO failed...\n"); return -1; } @@ -411,7 +411,7 @@ static void fec_halt(struct eth_device *dev) /* wait for graceful stop to register */ if (readl_poll_timeout(fec->regs + FEC_IEVENT, reg, - reg & FEC_IEVENT_GRA, SECOND)) + reg & FEC_IEVENT_GRA, USEC_PER_SEC)) dev_err(&dev->dev, "graceful stop timeout\n"); /* Disable SmartDMA tasks */ @@ -485,7 +485,7 @@ static int fec_send(struct eth_device *dev, void *eth_data, int data_length) fec_tx_task_enable(fec); if (readw_poll_timeout(&fec->tbd_base[fec->tbd_index].status, - status, !(status & FEC_TBD_READY), SECOND)) + status, !(status & FEC_TBD_READY), USEC_PER_SEC)) dev_err(&dev->dev, "transmission timeout\n"); dma_unmap_single(fec->dev, dma, data_length, DMA_TO_DEVICE); @@ -806,7 +806,7 @@ static int fec_probe(struct device_d *dev) /* Reset chip. */ writel(FEC_ECNTRL_RESET, fec->regs + FEC_ECNTRL); ret = readl_poll_timeout(fec->regs + FEC_ECNTRL, reg, - !(reg & FEC_ECNTRL_RESET), SECOND); + !(reg & FEC_ECNTRL_RESET), USEC_PER_SEC); if (ret) goto free_gpio; -- 2.19.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error 2018-12-14 15:23 ` [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error Ahmad Fatoum @ 2018-12-14 15:29 ` Ahmad Fatoum 2018-12-17 9:50 ` Sascha Hauer 0 siblings, 1 reply; 5+ messages in thread From: Ahmad Fatoum @ 2018-12-14 15:29 UTC (permalink / raw) To: barebox Hello, I didn't change read*_poll_timeout to take nanoseconds because it is copied over from Linux. On 14/12/18 16:23, Ahmad Fatoum wrote: > read*_poll_timeout's final timeout parameter is in microseconds, > but the supplied arguments in fec_imx.c were in nanoseconds, > which might lead to barebox getting seemingly stuck in fec_halt > (loops for a thousand seconds instead of one). So, how about dropping the (U|M|)SECOND defines in include/clock.h and replace them across the tree by the corresponding NSEC_PER_(U|M|)SEC? I find them more readable and they would avoid such mistakes in future. Cheers Ahmad -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error 2018-12-14 15:29 ` Ahmad Fatoum @ 2018-12-17 9:50 ` Sascha Hauer 0 siblings, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2018-12-17 9:50 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox On Fri, Dec 14, 2018 at 04:29:02PM +0100, Ahmad Fatoum wrote: > Hello, > > I didn't change read*_poll_timeout to take nanoseconds because it is copied > over from Linux. > > On 14/12/18 16:23, Ahmad Fatoum wrote: > > read*_poll_timeout's final timeout parameter is in microseconds, > > but the supplied arguments in fec_imx.c were in nanoseconds, > > which might lead to barebox getting seemingly stuck in fec_halt > > (loops for a thousand seconds instead of one). > > So, how about dropping the (U|M|)SECOND defines in include/clock.h and replace > them across the tree by the corresponding NSEC_PER_(U|M|)SEC? > > I find them more readable and they would avoid such mistakes in future. You are probably right that these are more readable, but still you have to know that is_timeout() takes nanoseconds and readl_poll_timeout takes microseconds. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends 2018-12-14 15:23 [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Ahmad Fatoum 2018-12-14 15:23 ` [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error Ahmad Fatoum @ 2018-12-17 9:48 ` Sascha Hauer 1 sibling, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2018-12-17 9:48 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox On Fri, Dec 14, 2018 at 04:23:05PM +0100, Ahmad Fatoum wrote: > Linux has them in include/linux/time64.h and they are useful for > making (especially microsecond) use readable such as in read*_poll_timeout. > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > include/linux/time.h | 6 ++++++ > 1 file changed, 6 insertions(+) Applied, thanks Sascha > > diff --git a/include/linux/time.h b/include/linux/time.h > index 3a1bb50020fd..7903139a65b2 100644 > --- a/include/linux/time.h > +++ b/include/linux/time.h > @@ -3,7 +3,13 @@ > > #include <linux/types.h> > > +#define MSEC_PER_SEC 1000L > +#define USEC_PER_MSEC 1000L > +#define NSEC_PER_USEC 1000L > +#define NSEC_PER_MSEC 1000000L > +#define USEC_PER_SEC 1000000L > #define NSEC_PER_SEC 1000000000L > +#define FSEC_PER_SEC 1000000000000000LL > > struct timespec { > time_t tv_sec; /* seconds */ > -- > 2.19.1 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-17 9:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-14 15:23 [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Ahmad Fatoum 2018-12-14 15:23 ` [PATCH 2/2] net: fec_imx: fix timeout off by *1000 error Ahmad Fatoum 2018-12-14 15:29 ` Ahmad Fatoum 2018-12-17 9:50 ` Sascha Hauer 2018-12-17 9:48 ` [PATCH 1/2] include/linux/time.h: define USEC_PER_SEC and friends Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox