RE: [PATCH v3 2/3] mmc: renesas_sdhi_internal_dmac: Add R7S9210 support

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

 



Hi Wolfram,

On Friday, October 12, 2018, Wolfram Sang wrote:
> > +/* RZ/A2 does not have the ADRR_MODE bit */
> > +#define SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED 2
> 
> First, there is a typo: s/ADRR/ADDR/g

Thanks!

Even after you pointed that out...I had to stare real hard to see it. I 
guess the brain corrects what you see.


> Also, I think it would make the code much more comprehensible if this
> macro was named SDHI_INTERNAL_DMAC_ADDR_MODE_BROKEN. Or maybe
> SDHI_INTERNAL_DMAC_FIXED_ADDR_MODE_BROKEN. Currently, on first read I
> thought this mode was fixed on this SoC and broken on all the others and
> was confused.

To be honest, I was not able to fully understand the issue in detail. I was getting information through someone that was not in the design team. So to be fair to the chip design guys, I want to avoid the word "broken".
They took the bit out of the hardware manual, and since the HW was designed to work either way (and the default register setting is for fixed), I consider it an 'unsupported feature'.

So, how about a compromise of:

#define SDHI_INTERNAL_DMAC_ADDR_MODE_FIXED_ONLY 2


>From your other email:
> > As you can imagine, it does have this bit. And it worked fine from me.
> > But the chip guys said they found something not right with it, so they
> > removed it from the v1.0 Hardware Manual.
> 
> Do you happen to know if this applies for Gen3 SDHI as well?

I don't think this applies to Gen3. This is just a RZ/A2 thing.

With that said, there are some Gen3 HW bugs that are in RZ/A2 HW, like the QSPI read bug. Since the RZ/A2 has the exact same HW as Gen3, it has the same HW bug. Now that one is a "bug", and I don't mind calling that one broken.


Chris




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux