Re: [Qemu-devel] [PATCH v1 1/2] SDHCI: inital version

[Igor Mitsyanko]

On 04/02/2012 03:05 PM, Peter Crosthwaite wrote:
> On Mon, Apr 2, 2012 at 6:38 PM, Igor Mitsyanko<address@hidden>  wrote:
> > On 04/02/2012 12:00 PM, Andreas Färber wrote:
> > > Am 02.04.2012 09:20, schrieb Peter Maydell:
> > > > On 2 April 2012 07:24, Peter A. G. Crosthwaite
> > > > <address@hidden>    wrote:
> > > > > device more for standard SD host controller interface (SDHCI).
> > > > >
> > > > > Signed-off-by: Peter A. G. Crosthwaite<address@hidden>
> > > >
> > > >
> > > > So how does this compare with Vincent Palatin's version?
> > > > (http://patchwork.ozlabs.org/patch/106767/) I'm guessing from
> > > > the copyright string that it's a modified version -- it would
> > > > be nice to say what the differences are.
> > >
> > >
> > > ...and what the differences to Samsung's version are.
> > >
> > > We should probably also cc Kevin on the topic.
> > >
> > > Andreas
> >
> >
> > It looks like this sdhc implements version 1 of standard SDHC specification,
> > while ours implements second version.
>
> The implementation is not a fully 100% complete SDHCI (indeed very few
> QEMU models of anything are 100% true to the hardware),   Id have to
> have a good look through the spec to give you what v2 features are and
> arent implemented, but this is supposed to have some level of v2
> support.
>
>   Second version should be backwards
> > compatible with first,
>
> Yes, I had to add somebackward compatibility to Vincents model around
> the ADMA stuff.
>
> I didn't want to submit it yet to see if vmstate
> > issue will be resolved or not. And also the whole QEMU SD emulation scheme
> > needs some refactoring which I've been busy with for a while now. The
> > biggest issues are that currently we cannot hot-insert SD card into virtual
> > board interface and we do not take any advantage of QOM (which is hard until
> > Paolo's and Andrea's patches land in master).
> > The one advantage of our SDHCI implementation is that it provides interface
> > to implement SoC-specific successors of standart SDHCI. I'll send these
> > patches today (even though I wanted to do a lot more refactoring with them),
> > Peter, could you please test them to find out if you can use them instead of
> > your SDHCI implementation?
>
> Yes i can but will they these patches be immediately mergable?
>
> My primary motivation for pushing this is to get my platform upstream
> and maintained. To that end, if major construction effort is required
> to get your SDHCI up to acceptable (which you have indicated in your
> comments) then can we push this anyway, and then when your refactoring
> happens, your model can replace this model.

Yes, I've been trying to get my sdhc accepted since last year :) I tried 
to comply with specification entirely, your implementation is obviously 
much smaller but enough for use with Linux driver (i've tested it with 
exynos board emulation). I have some comments though:

1) It doesn't compile because adma_stride could be used uninitialized in 
sdhci_dma_transfer() (QEMU is compiled with -Werror);
2) You shouldn't hw_error() on bad reads/writes.
3) Why do you register memory region with 0x200 size and then print 
error messages on read and writes in a range 0x100-0x200?
4) Controller transfers data in blocks of blksize (512 usually) bytes, 
but you transfer data immediately on writes to registers, you probably 
should use a buffer for this.
5) You assume that BUFFER_DATAPORT register is always accessed with 4 
byte system bus transfers, but it's not true in general.
6) SDMA transfer must continue when you write to the upper byte of 
SDMA_SYSAD register if it was stoped at page boundary.
7) Maybe it's not important, but you have no support of 64-bit target 
emulation.
8) Why "sysbus_sdhci" instead of simple "sdhci"?



--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden