RE: [PATCH 00/10] usb: Add host and device support for RZ/A2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chrisさん

> From: Chris Brandt, Sent: Thursday, May 9, 2019 3:08 AM
> 
> Hi Shimodaさん
> 
> > From: Yoshihiro Shimoda
> > Sent: Tuesday, May 07, 2019 5:17 AM
> > > For the most part, the RZ/A2 has the same USB 2.0 host and device
> > > HW as the R-Car Gen3, so we can reuse a lot of the code.
> > >
> > > However, there are a couple extra register bits, and the CFIFO
> > > register 8-bit access works a little different (weird, no idea why).
> >
> > This is just my gut feeling, but if we set the BIGEND bit in the CFIFOSEL
> > of RZ/A2M (R-Car Gen3 doesn't have such a bit though), could the original
> > code work correctly?
> 
> I just tried to set CFIFOSEL.BIGEND = 1

Thank you for trying it!

>  * Set CFIFOSEL.BIGEND = 1
>  * Write 8-bit values to CFIFO (same method as R-Car)
>  * Set CFIFOSEL.BIGEND = 0
> 
> The result is bad.

I got it...

> But, then I tried this:
>  * Set CFIFOSEL.MBW = 0   (CFIFO port access = 8-bit)
>  * Write 8-bit values to CFIFO
>  * Set CFIFOSEL.MBW = 2   (CFIFO port access = 32-bit)
> 
> Code:
> u16 cfifosel = usbhs_read(priv, fifo->sel);
> 
> usbhs_write(priv, fifo->sel, cfifosel & 0xF3FF); // MBW = 8-bit
> 
> 		for (i = 0; i < len; i++)
> 			iowrite8(buf[i], addr); //same address each time
> 
> usbhs_write(priv, fifo->sel, cfifosel);	// MBW = 32-bit
> 
> 
> This method works good.

I got it.

>   (I assume this method would work with R-Car also)

Unfortunately, R-Car cannot work with this method...
But, "iowrite8(buf[i], addr + 3);" or "iowrite32(buf[i], addr);" works on the R-Car.
And then, I realized that R-Car CFIFO register allows 32-bit access only...
# So, I'm asking HW guy whether the 8-bit access can be allowed or not now...

> But...then we have extra register reads and writes.
> Register accesses are slower, so performance is lower.
> 
> So, I prefer my original method:
> 	if (usbhsc_flags_has(priv, USBHSF_CFIFO_BYTE_ADDR))
> 		for (i = 0; i < len; i++)
> 			iowrite8(buf[i], addr + (i & 0x03));
> 	else
> 		for (i = 0; i < len; i++)
> 			iowrite8(buf[i], addr + (0x03 - (i & 0x03)));
> 
> 
> Do you agree?

I agree.
However, I have some comments about the patch. So, I'll reply on the patch later.

Best regards,
Yoshihiro Shimoda

> Chris





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux