| Message ID | 1359823557-5748-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On 2 February 2013 16:45, Andreas Färber <afaerber@suse.de> wrote: > From: Andreas Färber <andreas.faerber@web.de> > > The libqos driver for omap_i2c currently does not work on Big Endian. > Introduce helpers for reading from and writing to 16-bit armel registers. > > This fixes tmp105-test failures on ppc. > > Signed-off-by: Andreas Färber <andreas.faerber@web.de> > --- > tests/libi2c-omap.c | 51 ++++++++++++++++++++++++++++++++------------------- > 1 Datei geändert, 32 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-) > > diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c > index 9be57e9..7d50ef2 100644 > --- a/tests/libi2c-omap.c > +++ b/tests/libi2c-omap.c > @@ -12,6 +12,7 @@ > #include <string.h> > > #include "qemu/osdep.h" > +#include "qemu/bswap.h" > #include "libqtest.h" > > enum OMAPI2CRegisters { > @@ -48,12 +49,24 @@ typedef struct OMAPI2C { > } OMAPI2C; > > > +static inline void omap_i2c_read16(uint64_t addr, uint16_t *data) > +{ > + memread(addr, data, 2); > + *data = le16_to_cpu(*data); > +} > + > +static inline void omap_i2c_write16(uint64_t addr, uint16_t data) > +{ > + data = cpu_to_le16(data); > + memwrite(addr, &data, 2); > +} There's nothing special about the OMAP i2c device that I know of: shouldn't the test code just be using a generic "write 16 bit value to memory with appropriate endianness for target CPU" function ? -- PMM
Am 02.02.2013 17:49, schrieb Peter Maydell: > On 2 February 2013 16:45, Andreas Färber <afaerber@suse.de> wrote: >> From: Andreas Färber <andreas.faerber@web.de> >> >> The libqos driver for omap_i2c currently does not work on Big Endian. >> Introduce helpers for reading from and writing to 16-bit armel registers. >> >> This fixes tmp105-test failures on ppc. >> >> Signed-off-by: Andreas Färber <andreas.faerber@web.de> >> --- >> tests/libi2c-omap.c | 51 ++++++++++++++++++++++++++++++++------------------- >> 1 Datei geändert, 32 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-) >> >> diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c >> index 9be57e9..7d50ef2 100644 >> --- a/tests/libi2c-omap.c >> +++ b/tests/libi2c-omap.c >> @@ -12,6 +12,7 @@ >> #include <string.h> >> >> #include "qemu/osdep.h" >> +#include "qemu/bswap.h" >> #include "libqtest.h" >> >> enum OMAPI2CRegisters { >> @@ -48,12 +49,24 @@ typedef struct OMAPI2C { >> } OMAPI2C; >> >> >> +static inline void omap_i2c_read16(uint64_t addr, uint16_t *data) >> +{ >> + memread(addr, data, 2); >> + *data = le16_to_cpu(*data); >> +} >> + >> +static inline void omap_i2c_write16(uint64_t addr, uint16_t data) >> +{ >> + data = cpu_to_le16(data); >> + memwrite(addr, &data, 2); >> +} > > There's nothing special about the OMAP i2c device that I know of: > shouldn't the test code just be using a generic "write 16 bit value > to memory with appropriate endianness for target CPU" function ? I asked about where to address this issue [1], no concrete answers, and this is the only solution I came up with. libqtest.h has no generic endian-aware memread functions unlike Alex, you or me expected. It reads a sequence of bytes from guest memory and transmits them one-by-one over the text-based qtest protocol. Since they are raw bytes I need to convert them here. tmp105-test itself was transmitting byte arrays, so it did not need changes. Looking into where that annoying "mipid" debug output is coming from is a bit further down on my to-do list for 1.4. I tested this patch to resolve the test breakage on ppc and not to introduce a regression for x86_64. I don't want arm disabled like sparc. Any qtest API remodelling would be 1.5 material now, I guess, and could be hidden inside my helper functions. Andreas [1] http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg05081.html
On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: > Am 02.02.2013 17:49, schrieb Peter Maydell: >> There's nothing special about the OMAP i2c device that I know of: >> shouldn't the test code just be using a generic "write 16 bit value >> to memory with appropriate endianness for target CPU" function ? > > I asked about where to address this issue [1], no concrete answers, and > this is the only solution I came up with. > > libqtest.h has no generic endian-aware memread functions unlike Alex, > you or me expected. It reads a sequence of bytes from guest memory and > transmits them one-by-one over the text-based qtest protocol. OK, so this is just busted for accessing devices. The protocol has to have some way of letting you do a 32 bit / 16 bit / 8 bit access (and maybe 64 bit as well while we're here). memread and memwrite are OK for RAM accesses [ie anything you'd be happy to have cached or buffered in a real system] but for memory mapped registers we need to have an equivalent of inb/inw/inl/outb/outw/outl that guarantee to do exactly one access of exactly the required width. (If you want to do a workaround for 1.4 to get the test suite running again I don't object; this is just a discussion about what the right long term fix should be.) -- PMM
Am 02.02.2013 18:44, schrieb Peter Maydell: > On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: >> Am 02.02.2013 17:49, schrieb Peter Maydell: >>> There's nothing special about the OMAP i2c device that I know of: >>> shouldn't the test code just be using a generic "write 16 bit value >>> to memory with appropriate endianness for target CPU" function ? >> >> I asked about where to address this issue [1], no concrete answers, and >> this is the only solution I came up with. >> >> libqtest.h has no generic endian-aware memread functions unlike Alex, >> you or me expected. It reads a sequence of bytes from guest memory and >> transmits them one-by-one over the text-based qtest protocol. > > OK, so this is just busted for accessing devices. The protocol > has to have some way of letting you do a 32 bit / 16 bit / 8 bit > access (and maybe 64 bit as well while we're here). memread > and memwrite are OK for RAM accesses [ie anything you'd be > happy to have cached or buffered in a real system] but for > memory mapped registers we need to have an equivalent of > inb/inw/inl/outb/outw/outl that guarantee to do exactly one > access of exactly the required width. > > (If you want to do a workaround for 1.4 to get the test suite > running again I don't object; this is just a discussion about > what the right long term fix should be.) OK. As discussed with Alex in IRC, I think the issue is that libqtest does not know about which endianness the guest uses, so the only thing we can do on the library side is generalize my helpers (e.g., memread_le16u). Anything else would need to be done in qtest.c, i.e. adding new commands to the protocol (e.g., memread16) and implementing them on both sides. m48t59-test, as only other memread() user, gets around this due to doing byte access only. Andreas
On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: >> Am 02.02.2013 17:49, schrieb Peter Maydell: >>> There's nothing special about the OMAP i2c device that I know of: >>> shouldn't the test code just be using a generic "write 16 bit value >>> to memory with appropriate endianness for target CPU" function ? >> >> I asked about where to address this issue [1], no concrete answers, and >> this is the only solution I came up with. >> >> libqtest.h has no generic endian-aware memread functions unlike Alex, >> you or me expected. It reads a sequence of bytes from guest memory and >> transmits them one-by-one over the text-based qtest protocol. > > OK, so this is just busted for accessing devices. The protocol > has to have some way of letting you do a 32 bit / 16 bit / 8 bit > access (and maybe 64 bit as well while we're here). memread > and memwrite are OK for RAM accesses [ie anything you'd be > happy to have cached or buffered in a real system] but for > memory mapped registers we need to have an equivalent of > inb/inw/inl/outb/outw/outl that guarantee to do exactly one > access of exactly the required width. I was also just making a patch (but not so nice as Andreas'). My analysis was that qtest.c and libqtest.c just pass the result of cpu_physical_memory_rw() as is, with no endian conversion. So the result needs to be converted to host CPU order just like Andreas did. I think the protocol is OK, my initial reaction was to put byte swapping there but that's not right. > > (If you want to do a workaround for 1.4 to get the test suite > running again I don't object; this is just a discussion about > what the right long term fix should be.) > > -- PMM
On 2 February 2013 18:26, Blue Swirl <blauwirbel@gmail.com> wrote: > On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: >>> Am 02.02.2013 17:49, schrieb Peter Maydell: >>> libqtest.h has no generic endian-aware memread functions unlike Alex, >>> you or me expected. It reads a sequence of bytes from guest memory and >>> transmits them one-by-one over the text-based qtest protocol. >> >> OK, so this is just busted for accessing devices. The protocol >> has to have some way of letting you do a 32 bit / 16 bit / 8 bit >> access (and maybe 64 bit as well while we're here). memread >> and memwrite are OK for RAM accesses [ie anything you'd be >> happy to have cached or buffered in a real system] but for >> memory mapped registers we need to have an equivalent of >> inb/inw/inl/outb/outw/outl that guarantee to do exactly one >> access of exactly the required width. > > I was also just making a patch (but not so nice as Andreas'). My > analysis was that qtest.c and libqtest.c just pass the result of > cpu_physical_memory_rw() as is, with no endian conversion. So the > result needs to be converted to host CPU order just like Andreas did. > I think the protocol is OK, my initial reaction was to put byte > swapping there but that's not right. No, I think the protocol is broken, even if you ignore the question of endianness. Consider that a device's MemoryRegion can have different behaviour depending on whether you access it as a byte, halfword or word size. cpu_physical_memory_rw() won't let you make a word size access to an unaligned address. -- PMM
On Sat, Feb 2, 2013 at 8:18 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 February 2013 18:26, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: >>>> Am 02.02.2013 17:49, schrieb Peter Maydell: >>>> libqtest.h has no generic endian-aware memread functions unlike Alex, >>>> you or me expected. It reads a sequence of bytes from guest memory and >>>> transmits them one-by-one over the text-based qtest protocol. >>> >>> OK, so this is just busted for accessing devices. The protocol >>> has to have some way of letting you do a 32 bit / 16 bit / 8 bit >>> access (and maybe 64 bit as well while we're here). memread >>> and memwrite are OK for RAM accesses [ie anything you'd be >>> happy to have cached or buffered in a real system] but for >>> memory mapped registers we need to have an equivalent of >>> inb/inw/inl/outb/outw/outl that guarantee to do exactly one >>> access of exactly the required width. >> >> I was also just making a patch (but not so nice as Andreas'). My >> analysis was that qtest.c and libqtest.c just pass the result of >> cpu_physical_memory_rw() as is, with no endian conversion. So the >> result needs to be converted to host CPU order just like Andreas did. >> I think the protocol is OK, my initial reaction was to put byte >> swapping there but that's not right. > > No, I think the protocol is broken, even if you ignore the > question of endianness. Consider that a device's MemoryRegion > can have different behaviour depending on whether you access it > as a byte, halfword or word size. cpu_physical_memory_rw() won't > let you make a word size access to an unaligned address. But the width is transmitted too, so any size accesses can be performed. > > -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 2 February 2013 18:26, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote: >>>> Am 02.02.2013 17:49, schrieb Peter Maydell: >>>> libqtest.h has no generic endian-aware memread functions unlike Alex, >>>> you or me expected. It reads a sequence of bytes from guest memory and >>>> transmits them one-by-one over the text-based qtest protocol. >>> >>> OK, so this is just busted for accessing devices. The protocol >>> has to have some way of letting you do a 32 bit / 16 bit / 8 bit >>> access (and maybe 64 bit as well while we're here). memread >>> and memwrite are OK for RAM accesses [ie anything you'd be >>> happy to have cached or buffered in a real system] but for >>> memory mapped registers we need to have an equivalent of >>> inb/inw/inl/outb/outw/outl that guarantee to do exactly one >>> access of exactly the required width. >> >> I was also just making a patch (but not so nice as Andreas'). My >> analysis was that qtest.c and libqtest.c just pass the result of >> cpu_physical_memory_rw() as is, with no endian conversion. So the >> result needs to be converted to host CPU order just like Andreas did. >> I think the protocol is OK, my initial reaction was to put byte >> swapping there but that's not right. > > No, I think the protocol is broken, even if you ignore the > question of endianness. Consider that a device's MemoryRegion > can have different behaviour depending on whether you access it > as a byte, halfword or word size. cpu_physical_memory_rw() won't > let you make a word size access to an unaligned address. memread/memwrite is for bulk data transfer. It's a byte API. It would be a good idea to add word-sized I/O requests to the protocol. Regards, Anthony Liguori > > -- PMM
Andreas Färber <afaerber@suse.de> writes: > From: Andreas Färber <andreas.faerber@web.de> > > The libqos driver for omap_i2c currently does not work on Big Endian. > Introduce helpers for reading from and writing to 16-bit armel registers. > > This fixes tmp105-test failures on ppc. > > Signed-off-by: Andreas Färber <andreas.faerber@web.de> > --- > tests/libi2c-omap.c | 51 ++++++++++++++++++++++++++++++++------------------- > 1 Datei geändert, 32 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-) > > diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c > index 9be57e9..7d50ef2 100644 > --- a/tests/libi2c-omap.c > +++ b/tests/libi2c-omap.c > @@ -12,6 +12,7 @@ > #include <string.h> > > #include "qemu/osdep.h" > +#include "qemu/bswap.h" > #include "libqtest.h" > > enum OMAPI2CRegisters { > @@ -48,12 +49,24 @@ typedef struct OMAPI2C { > } OMAPI2C; > > > +static inline void omap_i2c_read16(uint64_t addr, uint16_t *data) > +{ > + memread(addr, data, 2); > + *data = le16_to_cpu(*data); > +} > + > +static inline void omap_i2c_write16(uint64_t addr, uint16_t data) > +{ > + data = cpu_to_le16(data); > + memwrite(addr, &data, 2); > +} > + > static void omap_i2c_set_slave_addr(OMAPI2C *s, uint8_t addr) > { > uint16_t data = addr; > > - memwrite(s->addr + OMAP_I2C_SA, &data, 2); > - memread(s->addr + OMAP_I2C_SA, &data, 2); > + omap_i2c_write16(s->addr + OMAP_I2C_SA, data); > + omap_i2c_read16(s->addr + OMAP_I2C_SA, &data); > g_assert_cmphex(data, ==, addr); > } > > @@ -66,22 +79,22 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr, > omap_i2c_set_slave_addr(s, addr); > > data = len; > - memwrite(s->addr + OMAP_I2C_CNT, &data, 2); > + omap_i2c_write16(s->addr + OMAP_I2C_CNT, data); > > data = OMAP_I2C_CON_I2C_EN | > OMAP_I2C_CON_TRX | > OMAP_I2C_CON_MST | > OMAP_I2C_CON_STT | > OMAP_I2C_CON_STP; > - memwrite(s->addr + OMAP_I2C_CON, &data, 2); > - memread(s->addr + OMAP_I2C_CON, &data, 2); > + omap_i2c_write16(s->addr + OMAP_I2C_CON, data); > + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); > g_assert((data & OMAP_I2C_CON_STP) != 0); > > - memread(s->addr + OMAP_I2C_STAT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); > g_assert((data & OMAP_I2C_STAT_NACK) == 0); > > while (len > 1) { > - memread(s->addr + OMAP_I2C_STAT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); > g_assert((data & OMAP_I2C_STAT_XRDY) != 0); > > memwrite(s->addr + OMAP_I2C_DATA, buf, 2); > @@ -89,13 +102,13 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr, > len -= 2; > } > if (len == 1) { > - memread(s->addr + OMAP_I2C_STAT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); > g_assert((data & OMAP_I2C_STAT_XRDY) != 0); > > memwrite(s->addr + OMAP_I2C_DATA, buf, 1); > } > > - memread(s->addr + OMAP_I2C_CON, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); > g_assert((data & OMAP_I2C_CON_STP) == 0); > } > > @@ -108,32 +121,32 @@ static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr, > omap_i2c_set_slave_addr(s, addr); > > data = len; > - memwrite(s->addr + OMAP_I2C_CNT, &data, 2); > + omap_i2c_write16(s->addr + OMAP_I2C_CNT, data); > > data = OMAP_I2C_CON_I2C_EN | > OMAP_I2C_CON_MST | > OMAP_I2C_CON_STT | > OMAP_I2C_CON_STP; > - memwrite(s->addr + OMAP_I2C_CON, &data, 2); > - memread(s->addr + OMAP_I2C_CON, &data, 2); > + omap_i2c_write16(s->addr + OMAP_I2C_CON, data); > + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); > g_assert((data & OMAP_I2C_CON_STP) == 0); > > - memread(s->addr + OMAP_I2C_STAT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); > g_assert((data & OMAP_I2C_STAT_NACK) == 0); > > - memread(s->addr + OMAP_I2C_CNT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_CNT, &data); > g_assert_cmpuint(data, ==, len); > > while (len > 0) { > - memread(s->addr + OMAP_I2C_STAT, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); > g_assert((data & OMAP_I2C_STAT_RRDY) != 0); > g_assert((data & OMAP_I2C_STAT_ROVR) == 0); > > memread(s->addr + OMAP_I2C_DATA, &data, 2); > > - memread(s->addr + OMAP_I2C_STAT, &stat, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &stat); > if (unlikely(len == 1)) { > - *buf = data & 0xf; > + *buf = le16_to_cpu(data) & 0xf; I don't really get this part. You're effectively unswapping the bytes, right? Regards, Anthony Liguori > buf++; > len--; > } else { > @@ -143,7 +156,7 @@ static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr, > } > } > > - memread(s->addr + OMAP_I2C_CON, &data, 2); > + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); > g_assert((data & OMAP_I2C_CON_STP) == 0); > } > > @@ -159,7 +172,7 @@ I2CAdapter *omap_i2c_create(uint64_t addr) > i2c->recv = omap_i2c_recv; > > /* verify the mmio address by looking for a known signature */ > - memread(addr + OMAP_I2C_REV, &data, 2); > + omap_i2c_read16(addr + OMAP_I2C_REV, &data); > g_assert_cmphex(data, ==, 0x34); > > return i2c; > -- > 1.7.10.4
Am 05.02.2013 17:58, schrieb Anthony Liguori: > Andreas Färber <afaerber@suse.de> writes: > >> memread(s->addr + OMAP_I2C_DATA, &data, 2); >> >> - memread(s->addr + OMAP_I2C_STAT, &stat, 2); >> + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &stat); >> if (unlikely(len == 1)) { >> - *buf = data & 0xf; >> + *buf = le16_to_cpu(data) & 0xf; > > I don't really get this part. You're effectively unswapping the bytes, > right? No. I am reading into "data" unswapped above. This line swaps "data" when only half of the register is used for the 0xF mask, which assumes host endianness - I can re-test tonight to verify whether it breaks without. Probably I should insert a white line to make the "data" vs. "stat" distinction more clear. I was using a broken STAT register bit before, if you check the libi2c-omap patch history, where we couldn't agree on how to fix properly without spec. Thus I changed the condition to not rely on stat; reading the register seemed necessary though. Alternatively the I2C device has a Big Endian bit that we could enable - but I would rather not have the tests run different device code paths based on host. Regards, Andreas
Applied. Thanks. Regards, Anthony Liguori
diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c index 9be57e9..7d50ef2 100644 --- a/tests/libi2c-omap.c +++ b/tests/libi2c-omap.c @@ -12,6 +12,7 @@ #include <string.h> #include "qemu/osdep.h" +#include "qemu/bswap.h" #include "libqtest.h" enum OMAPI2CRegisters { @@ -48,12 +49,24 @@ typedef struct OMAPI2C { } OMAPI2C; +static inline void omap_i2c_read16(uint64_t addr, uint16_t *data) +{ + memread(addr, data, 2); + *data = le16_to_cpu(*data); +} + +static inline void omap_i2c_write16(uint64_t addr, uint16_t data) +{ + data = cpu_to_le16(data); + memwrite(addr, &data, 2); +} + static void omap_i2c_set_slave_addr(OMAPI2C *s, uint8_t addr) { uint16_t data = addr; - memwrite(s->addr + OMAP_I2C_SA, &data, 2); - memread(s->addr + OMAP_I2C_SA, &data, 2); + omap_i2c_write16(s->addr + OMAP_I2C_SA, data); + omap_i2c_read16(s->addr + OMAP_I2C_SA, &data); g_assert_cmphex(data, ==, addr); } @@ -66,22 +79,22 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr, omap_i2c_set_slave_addr(s, addr); data = len; - memwrite(s->addr + OMAP_I2C_CNT, &data, 2); + omap_i2c_write16(s->addr + OMAP_I2C_CNT, data); data = OMAP_I2C_CON_I2C_EN | OMAP_I2C_CON_TRX | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT | OMAP_I2C_CON_STP; - memwrite(s->addr + OMAP_I2C_CON, &data, 2); - memread(s->addr + OMAP_I2C_CON, &data, 2); + omap_i2c_write16(s->addr + OMAP_I2C_CON, data); + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); g_assert((data & OMAP_I2C_CON_STP) != 0); - memread(s->addr + OMAP_I2C_STAT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); g_assert((data & OMAP_I2C_STAT_NACK) == 0); while (len > 1) { - memread(s->addr + OMAP_I2C_STAT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); g_assert((data & OMAP_I2C_STAT_XRDY) != 0); memwrite(s->addr + OMAP_I2C_DATA, buf, 2); @@ -89,13 +102,13 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr, len -= 2; } if (len == 1) { - memread(s->addr + OMAP_I2C_STAT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); g_assert((data & OMAP_I2C_STAT_XRDY) != 0); memwrite(s->addr + OMAP_I2C_DATA, buf, 1); } - memread(s->addr + OMAP_I2C_CON, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); g_assert((data & OMAP_I2C_CON_STP) == 0); } @@ -108,32 +121,32 @@ static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr, omap_i2c_set_slave_addr(s, addr); data = len; - memwrite(s->addr + OMAP_I2C_CNT, &data, 2); + omap_i2c_write16(s->addr + OMAP_I2C_CNT, data); data = OMAP_I2C_CON_I2C_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT | OMAP_I2C_CON_STP; - memwrite(s->addr + OMAP_I2C_CON, &data, 2); - memread(s->addr + OMAP_I2C_CON, &data, 2); + omap_i2c_write16(s->addr + OMAP_I2C_CON, data); + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); g_assert((data & OMAP_I2C_CON_STP) == 0); - memread(s->addr + OMAP_I2C_STAT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); g_assert((data & OMAP_I2C_STAT_NACK) == 0); - memread(s->addr + OMAP_I2C_CNT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_CNT, &data); g_assert_cmpuint(data, ==, len); while (len > 0) { - memread(s->addr + OMAP_I2C_STAT, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &data); g_assert((data & OMAP_I2C_STAT_RRDY) != 0); g_assert((data & OMAP_I2C_STAT_ROVR) == 0); memread(s->addr + OMAP_I2C_DATA, &data, 2); - memread(s->addr + OMAP_I2C_STAT, &stat, 2); + omap_i2c_read16(s->addr + OMAP_I2C_STAT, &stat); if (unlikely(len == 1)) { - *buf = data & 0xf; + *buf = le16_to_cpu(data) & 0xf; buf++; len--; } else { @@ -143,7 +156,7 @@ static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr, } } - memread(s->addr + OMAP_I2C_CON, &data, 2); + omap_i2c_read16(s->addr + OMAP_I2C_CON, &data); g_assert((data & OMAP_I2C_CON_STP) == 0); } @@ -159,7 +172,7 @@ I2CAdapter *omap_i2c_create(uint64_t addr) i2c->recv = omap_i2c_recv; /* verify the mmio address by looking for a known signature */ - memread(addr + OMAP_I2C_REV, &data, 2); + omap_i2c_read16(addr + OMAP_I2C_REV, &data); g_assert_cmphex(data, ==, 0x34); return i2c;