On Mon, Oct 29, 2018 at 1:04 AM Cédric Le Goater <c...@kaod.org> wrote:
>
> Hello Joe,
>
> On 10/22/18 9:55 PM, Joe Hershberger wrote:
> > Hi Cedric,
> >
> > On Tue, Oct 16, 2018 at 4:32 AM Cédric Le Goater <c...@kaod.org> wrote:
> >>
> >> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >> Reviewed-by: Joel Stanley <j...@jms.id.au>
> >> ---
> >>
> >> Changes since v3 :
> >>
> >> - introduced a ftgmac100_wait_for_txdone() function similar to the
> >> wait_for_bit_*() macros.
> >>
> >> drivers/net/ftgmac100.c | 44 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 44 insertions(+)
> >>
> >> diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
> >> index bf8600814690..9adfe109ebc2 100644
> >> --- a/drivers/net/ftgmac100.c
> >> +++ b/drivers/net/ftgmac100.c
> >> @@ -14,6 +14,7 @@
> >> #include <dm.h>
> >> #include <miiphy.h>
> >> #include <net.h>
> >> +#include <wait_bit.h>
> >> #include <linux/io.h>
> >> #include <linux/iopoll.h>
> >>
> >> @@ -28,6 +29,9 @@
> >> /* PKTBUFSTX/PKTBUFSRX must both be power of 2 */
> >> #define PKTBUFSTX 4 /* must be power of 2 */
> >>
> >> +/* Timeout for transmit */
> >> +#define FTGMAC100_TX_TIMEOUT_MS 1000
> >> +
> >> /* Timeout for a mdio read/write operation */
> >> #define FTGMAC100_MDIO_TIMEOUT_USEC 10000
> >>
> >> @@ -401,6 +405,41 @@ static int ftgmac100_recv(struct udevice *dev, int
> >> flags, uchar **packetp)
> >> return rxlen;
> >> }
> >>
> >> +/*
> >> + * The wait_for_bit_*() macros require a register value. This define a
> >> + * similar routine which loops on the in-memory transmit descriptor to
> >> + * wait for the MAC to clear the DMA_OWN bit.
> >> + */
> >> +static int ftgmac100_wait_for_txdone(struct ftgmac100_txdes *txdes,
> >> + const unsigned int timeout_ms,
> >> + const bool breakable)
> >> +{
> >
> > I was hoping to see something like this instead:
> >
> > static u32 ftgmac100_read_txdesc(void *desc)
> > {
> > struct ftgmac100_txdes *txdes = desc;
> > ulong des_start = (ulong)txdes;
> > ulong des_end = des_start + roundup(sizeof(*txdes),
> > ARCH_DMA_MINALIGN);
> >
> > invalidate_dcache_range(des_start, des_end);
> >
> > return txdes->txdes0;
> > }
> >
> > BUILD_WAIT_FOR_BIT(ftgmac100_txdone, u32, ftgmac100_read_txdesc)
> >
> > [ ... ]
> >
> > ftgmac100_send( ... )
> > {
> > [ ... ]
> >
> > rc = wait_for_bit_ftgmac100_txdone(curr_des,
> > FTGMAC100_TXDES0_TXDMA_OWN, false, FTGMAC100_TX_TIMEOUT_MS, true);
> > if (rc)
> > return rc;
> >
> > [ ... ]
> > }
>
>
> Yes, this is much better. A few other drivers could make use of a similar
> macro.
I agree... spread the word to people with hardware who can make the
change and test. :)
> Sending a v5 with the proposed change.
Looks great, thanks!
>
> Thanks,
>
> C.
>
>
> >> + ulong des_start = (ulong)txdes;
> >> + ulong des_end = des_start + roundup(sizeof(*txdes),
> >> ARCH_DMA_MINALIGN);
> >> + ulong start = get_timer(0);
> >> +
> >> + while (1) {
> >> + invalidate_dcache_range(des_start, des_end);
> >> +
> >> + if (!(txdes->txdes0 & FTGMAC100_TXDES0_TXDMA_OWN))
> >> + return 0;
> >> +
> >> + if (get_timer(start) > timeout_ms)
> >> + break;
> >> +
> >> + if (breakable && ctrlc()) {
> >> + puts("Abort\n");
> >> + return -EINTR;
> >> + }
> >> +
> >> + udelay(1);
> >> + WATCHDOG_RESET();
> >> + }
> >> +
> >> + dev_err(dev, "transmit timeout\n");
> >> + return -ETIMEDOUT;
> >> +}
> >> +
> >> /*
> >> * Send a data block via Ethernet
> >> */
> >> @@ -414,6 +453,7 @@ static int ftgmac100_send(struct udevice *dev, void
> >> *packet, int length)
> >> roundup(sizeof(*curr_des), ARCH_DMA_MINALIGN);
> >> ulong data_start;
> >> ulong data_end;
> >> + int rc;
> >>
> >> invalidate_dcache_range(des_start, des_end);
> >>
> >> @@ -446,6 +486,10 @@ static int ftgmac100_send(struct udevice *dev, void
> >> *packet, int length)
> >> /* Start transmit */
> >> writel(1, &ftgmac100->txpd);
> >>
> >> + rc = ftgmac100_wait_for_txdone(curr_des, FTGMAC100_TX_TIMEOUT_MS,
> >> true);
> >> + if (rc)
> >> + return rc;
> >> +
> >> debug("%s(): packet sent\n", __func__);
> >>
> >> /* Move to next descriptor */
> >> --
> >> 2.17.2
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot@lists.denx.de
> >> https://lists.denx.de/listinfo/u-boot
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
