Questions about the sh_eth driver and hardware

Questions about the sh_eth driver and hardware

[Ben Hutchings]

I found several more bugs in the sh_eth driver, but for some of them I'm
really unsure what to do.  I only have a manual for the R8A7790 (R-Car
H2) and I don't know how all the other supported chips will behave.
Maybe you can answer some of these questions.

1. When freeing packet buffers, we currently try to stop the DMA engines
by clearing EDRRR and EDTRR but we *don't* wait after that.  This seems
unsafe because in general register writes are not serialised with DMA.

The R8A7790 (R-Car H2) manual specifically says that the R bit of EDTRR
(aka CXR2) cannot be cleared by writing to it.  I think that we could
stop TX DMA by clearing the active flags of all the descriptors and then
polling the R bit until it clears.  What do you think?

As for RX DMA, I think we should wait some time after clearing the R bit
that we can be sure is long enough to transfer one packet.  Do you know
how long that could be?

2. In case of a Receive Descriptor Empty error (RDE), we currently read
the RDFAR register to find the next descriptor the DMA engine will use.
But this register is not documented for the R8A7790 and the driver does
not define an offset for it on R-Car chips.  The manual doesn't say how
to set the address of the next descriptor to use.  Maybe we should
assume that R-Car chips will never skip descriptors after RDE?

Ben.

RE: Questions about the sh_eth driver and hardware

[yoshihiro shimoda]

Hi Ben,

This is Shimoda of Renesas. I'm a software engineer.
I asked hardware team about your questions. So, I wrote some comments below.

> 1. When freeing packet buffers, we currently try to stop the DMA engines
> by clearing EDRRR and EDTRR but we *don't* wait after that.  This seems
> unsafe because in general register writes are not serialised with DMA.
> 
> The R8A7790 (R-Car H2) manual specifically says that the R bit of EDTRR
> (aka CXR2) cannot be cleared by writing to it.  I think that we could
> stop TX DMA by clearing the active flags of all the descriptors and then
> polling the R bit until it clears.  What do you think?
> 
> As for RX DMA, I think we should wait some time after clearing the R bit
> that we can be sure is long enough to transfer one packet.  Do you know
> how long that could be?

They said that a software should wait for "set amount of time" or RX interruption.
So, I think that your patch (commit id = 740c7f31c in net-next.git) is enough.

> 2. In case of a Receive Descriptor Empty error (RDE), we currently read
> the RDFAR register to find the next descriptor the DMA engine will use.
> But this register is not documented for the R8A7790 and the driver does
> not define an offset for it on R-Car chips.  The manual doesn't say how
> to set the address of the next descriptor to use.  Maybe we should
> assume that R-Car chips will never skip descriptors after RDE?

They said that software should find the next descriptor without RDFAR register somehow
after RDE. However, I checked this driver handling on the R-Car M2, and the following
code didn't run actually. So, we should assume the R-Car chips will never skip descriptors
after RDE.

< sh_eth.c: sh_eth_rx() >
	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
		/* fix the values for the next receiving if RDE is set */
		if (intr_status & EESR_RDE) {
/*vvvvvvv The code below didn't run on R-Car Gen2 vvvvvvv*/
			u32 count = (sh_eth_read(ndev, RDFAR) -
				     sh_eth_read(ndev, RDLAR)) >> 4;

			mdp->cur_rx = count;
			mdp->dirty_rx = count;
/*^^^^^^^ The code above didn't run on R-Car Gen2 ^^^^^^^*/
		}
		sh_eth_write(ndev, EDRRR_R, EDRRR);
	}

< Test method >
 Host PC $ sudo ping -f -l 128 <the board's ip address>

Best regards,
Yoshihiro Shimoda

Re: Questions about the sh_eth driver and hardware

[Ben Hutchings]

On Thu, 2015-02-19 at 04:41 +0000, yoshihiro shimoda wrote:
> Hi Ben,
> 
> This is Shimoda of Renesas. I'm a software engineer.
> I asked hardware team about your questions.

Thank you.

[...]
> > 2. In case of a Receive Descriptor Empty error (RDE), we currently read
> > the RDFAR register to find the next descriptor the DMA engine will use.
> > But this register is not documented for the R8A7790 and the driver does
> > not define an offset for it on R-Car chips.  The manual doesn't say how
> > to set the address of the next descriptor to use.  Maybe we should
> > assume that R-Car chips will never skip descriptors after RDE?
> 
> They said that software should find the next descriptor without RDFAR register somehow
> after RDE. However, I checked this driver handling on the R-Car M2, and the following
> code didn't run actually.

Is that because the hardware doesn't actually set RDE?  Otherwise, I
don't see what would prevent this code from running.

> So, we should assume the R-Car chips will never skip descriptors
> after RDE.

OK.

> < sh_eth.c: sh_eth_rx() >
> 	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
> 		/* fix the values for the next receiving if RDE is set */
> 		if (intr_status & EESR_RDE) {
> /*vvvvvvv The code below didn't run on R-Car Gen2 vvvvvvv*/
> 			u32 count = (sh_eth_read(ndev, RDFAR) -
> 				     sh_eth_read(ndev, RDLAR)) >> 4;
> 
> 			mdp->cur_rx = count;
> 			mdp->dirty_rx = count;
> /*^^^^^^^ The code above didn't run on R-Car Gen2 ^^^^^^^*/
> 		}
> 		sh_eth_write(ndev, EDRRR_R, EDRRR);
> 	}
> 
> < Test method >
>  Host PC $ sudo ping -f -l 128 <the board's ip address>

I've tried things like that, but the CPU cores seem to be fast enough to
handle a few gigabits per second.  I think we would need to slow down
the CPUs or inject memory allocation failures to provoke an RDE.

Ben.