Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-28 Thread Jean-Christophe PLAGNIOL-VILLARD 
On 18:44 Fri 25 Oct , Peter Maydell wrote:
 On 25 October 2013 12:04, Jean-Christophe PLAGNIOL-VILLARD
 plagn...@jcrosoft.com wrote:
  On 11:33 Sat 19 Oct , Jean-Christophe PLAGNIOL-VILLARD wrote:
  as it's depend on current direction
 
  ony change to get that applied?
 
  Barebox relay on it so it can work on both qemu and real hw
 
 I can't see anything obvious in the PL181 data sheet that
 says this register should change behaviour like this based
 on the direction of transfer, so I'm afraid I can't accept
 this patch without a much more detailed analysis of why
 it is correct. (Just as a for-starters, how does this change
 relate to the comment immediately above that mentions vagueness
 in the documentation and claims we don't need to emulate things
 to an exact level of detail? Is this change supposed to fix
 that? Does the comment need to change? Which bit of the
 PL181 documentation describes the behaviour the patch is
 affecting? etc)

it's

the register is supposed to report the status of the fifo

and in qemu if always report the datacnt which is the number of byte to
write and in case of reading it's always 0 so bootloader that use polling will
never be get any data.

And yes the readl hw does report the fifo lenght in case of read and write
not always 0 when reading.

Best Regards,
J.

Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-28 Thread Peter Maydell 
On 28 October 2013 13:24, Jean-Christophe PLAGNIOL-VILLARD
plagn...@jcrosoft.com wrote:
 the register is supposed to report the status of the fifo

 and in qemu if always report the datacnt which is the number of byte to
 write and in case of reading it's always 0 so bootloader that use polling will
 never be get any data.

No, datacnt in our implementation is the number of
bytes to transfer: we use it to count number of bytes
to read for read transfers (see pl181_fifo_run() which
loops based on it both for pushing data into the
fifo and for popping data out of it.) This is also the
value we report as the hardware datacnt register, which
according to the spec is used for both read and write
transfers.

-- PMM

Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-25 Thread Jean-Christophe PLAGNIOL-VILLARD 
On 11:33 Sat 19 Oct , Jean-Christophe PLAGNIOL-VILLARD wrote:
 as it's depend on current direction

ony change to get that applied?

Barebox relay on it so it can work on both qemu and real hw

Best Regards,
J.
 
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
 ---
  hw/sd/pl181.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
 index 03875bf..91adbbd 100644
 --- a/hw/sd/pl181.c
 +++ b/hw/sd/pl181.c
 @@ -344,7 +344,11 @@ static uint64_t pl181_read(void *opaque, hwaddr offset,
 data engine.  DataCnt is decremented after each byte is
 transferred between the serial engine and the card.
 We don't emulate this level of detail, so both can be the same.  
 */
 -tmp = (s-datacnt + 3)  2;
 + if (s-datactrl  PL181_DATA_DIRECTION)
 + tmp = s-fifo_len;
 + else
 + tmp = s-datacnt;
 +tmp = (tmp + 3)  2;
  if (s-linux_hack) {
  s-linux_hack = 0;
  pl181_fifo_run(s);
 -- 
 1.8.4.rc3

Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-25 Thread Peter Maydell 
On 25 October 2013 12:04, Jean-Christophe PLAGNIOL-VILLARD
plagn...@jcrosoft.com wrote:
 On 11:33 Sat 19 Oct , Jean-Christophe PLAGNIOL-VILLARD wrote:
 as it's depend on current direction

 ony change to get that applied?

 Barebox relay on it so it can work on both qemu and real hw

I can't see anything obvious in the PL181 data sheet that
says this register should change behaviour like this based
on the direction of transfer, so I'm afraid I can't accept
this patch without a much more detailed analysis of why
it is correct. (Just as a for-starters, how does this change
relate to the comment immediately above that mentions vagueness
in the documentation and claims we don't need to emulate things
to an exact level of detail? Is this change supposed to fix
that? Does the comment need to change? Which bit of the
PL181 documentation describes the behaviour the patch is
affecting? etc)

I'd also appreciate it if you could read
http://wiki.qemu.org/Contribute/SubmitAPatch
In particular, your patch has some obvious coding
style errors.

thanks
-- PMM

[Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support

2013-10-19 Thread Jean-Christophe PLAGNIOL-VILLARD 
as it's depend on current direction

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
---
 hw/sd/pl181.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 03875bf..91adbbd 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -344,7 +344,11 @@ static uint64_t pl181_read(void *opaque, hwaddr offset,
data engine.  DataCnt is decremented after each byte is
transferred between the serial engine and the card.
We don't emulate this level of detail, so both can be the same.  */
-tmp = (s-datacnt + 3)  2;
+   if (s-datactrl  PL181_DATA_DIRECTION)
+   tmp = s-fifo_len;
+   else
+   tmp = s-datacnt;
+tmp = (tmp + 3)  2;
 if (s-linux_hack) {
 s-linux_hack = 0;
 pl181_fifo_run(s);
-- 
1.8.4.rc3