Message ID | 1382175183-27090-1-git-send-email-plagnioj@jcrosoft.com |
---|---|
State | New |
Headers | show |
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 <plagnioj@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 >
On 25 October 2013 12:04, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@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
On 18:44 Fri 25 Oct , Peter Maydell wrote: > On 25 October 2013 12:04, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@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.
On 28 October 2013 13:24, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@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
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);
as it's depend on current direction Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> --- hw/sd/pl181.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)