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

[Peter Crosthwaite]

>
>
> 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).

Does it work, can you add exynos support for SDHCI using this version?
Do you want to for the sake of immediate exynos sdhci support?

 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);

Ack, will fix v2

> 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?

I can look into cleaning up the undefined behaviours, but for the
implementation i have (as in the real hardware) bad reads and writes
perform no action. This is why I just have a warning message rather
than a hw error.

> 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.

Yeh these behaviours are outside the implemented subset.

> 8) Why "sysbus_sdhci" instead of simple "sdhci"?

The original implementation was a dual PCI as well as sysbus. If
vincent or anyone else wants to put the PCI qdev back in they can.
the sysbus prefix is thinking forward for when there needs to be a
distinction between pci_sdhci and sysbus_sdhci.

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