From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDFpe-00030z-Qi for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:09:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDFpb-000739-JN for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:09:42 -0500 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:54631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDFpb-000732-8v for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:09:39 -0500 Received: by mail-wg0-f51.google.com with SMTP id l18so10806643wgh.10 for ; Mon, 19 Jan 2015 09:09:38 -0800 (PST) Sender: Paolo Bonzini Message-ID: <54BD3A4E.3030102@redhat.com> Date: Mon, 19 Jan 2015 18:09:34 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1421120079-987-1-git-send-email-jsnow@redhat.com> <1421120079-987-12-git-send-email-jsnow@redhat.com> In-Reply-To: <1421120079-987-12-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 11/14] libqos/ahci: Functional register helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, marc.mari.barcelo@gmail.com, armbru@redhat.com, stefanha@redhat.com, mreitz@redhat.com On 13/01/2015 04:34, John Snow wrote: > Introduce a set of "static inline" register helpers that are intended to > replace the current set of macros with more functional versions that are > better suited to inclusion in libqos than porcelain macros. > > As a stopgap measure before eliminating the porcelain macros, define them > to use the new functions defined in the ahci.h header. > > Signed-off-by: John Snow > --- > tests/ahci-test.c | 25 ++++++++++----------- > tests/libqos/ahci.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+), 14 deletions(-) > > diff --git a/tests/ahci-test.c b/tests/ahci-test.c > index b84f942..ef751a4 100644 > --- a/tests/ahci-test.c > +++ b/tests/ahci-test.c > @@ -47,22 +47,19 @@ static char tmp_path[] = "/tmp/qtest.XXXXXX"; > static bool ahci_pedantic; > > /*** IO macros for the AHCI memory registers. ***/ > -#define AHCI_READ(OFST) qpci_io_readl(ahci->dev, ahci->hba_base + (OFST)) > -#define AHCI_WRITE(OFST, VAL) qpci_io_writel(ahci->dev, \ > - ahci->hba_base + (OFST), (VAL)) > -#define AHCI_RREG(regno) AHCI_READ(4 * (regno)) > -#define AHCI_WREG(regno, val) AHCI_WRITE(4 * (regno), (val)) > -#define AHCI_SET(regno, mask) AHCI_WREG((regno), AHCI_RREG(regno) | (mask)) > -#define AHCI_CLR(regno, mask) AHCI_WREG((regno), AHCI_RREG(regno) & ~(mask)) > +#define AHCI_READ(OFST) ahci_mread(ahci, (OFST)) > +#define AHCI_WRITE(OFST, VAL) ahci_mwrite(ahci, (OFST), (VAL)) > +#define AHCI_RREG(regno) ahci_rreg(ahci, (regno)) > +#define AHCI_WREG(regno, val) ahci_wreg(ahci, (regno), (val)) > +#define AHCI_SET(regno, mask) ahci_set(ahci, (regno), (mask)) > +#define AHCI_CLR(regno, mask) ahci_clr(ahci, (regno), (mask)) > > /*** IO macros for port-specific offsets inside of AHCI memory. ***/ > -#define PX_OFST(port, regno) (HBA_PORT_NUM_REG * (port) + AHCI_PORTS + (regno)) > -#define PX_RREG(port, regno) AHCI_RREG(PX_OFST((port), (regno))) > -#define PX_WREG(port, regno, val) AHCI_WREG(PX_OFST((port), (regno)), (val)) > -#define PX_SET(port, reg, mask) PX_WREG((port), (reg), \ > - PX_RREG((port), (reg)) | (mask)); > -#define PX_CLR(port, reg, mask) PX_WREG((port), (reg), \ > - PX_RREG((port), (reg)) & ~(mask)); > +#define PX_OFST(port, regno) ahci_px_ofst((port), (regno)) > +#define PX_RREG(port, regno) ahci_px_rreg(ahci, (port), (regno)) > +#define PX_WREG(port, regno, val) ahci_px_wreg(ahci, (port), (regno), (val)) > +#define PX_SET(port, reg, mask) ahci_px_set(ahci, (port), (reg), (mask)) > +#define PX_CLR(port, reg, mask) ahci_px_clr(ahci, (port), (reg), (mask)) > > /*** Function Declarations ***/ > static QPCIDevice *get_ahci_device(uint32_t *fingerprint); > diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h > index 8e92385..56b1dfc 100644 > --- a/tests/libqos/ahci.h > +++ b/tests/libqos/ahci.h > @@ -354,4 +354,67 @@ typedef struct PRD { > /* For calculating how big the PRD table needs to be: */ > #define CMD_TBL_SIZ(n) ((0x80 + ((n) * sizeof(PRD)) + 0x7F) & ~0x7F) > > +/* Helpers for reading/writing AHCI HBA register values */ > + > +static inline uint32_t ahci_mread(AHCIQState *ahci, size_t offset) > +{ > + return qpci_io_readl(ahci->dev, ahci->hba_base + offset); > +} > + > +static inline void ahci_mwrite(AHCIQState *ahci, size_t offset, uint32_t value) > +{ > + qpci_io_writel(ahci->dev, ahci->hba_base + offset, value); > +} > + > +static inline uint32_t ahci_rreg(AHCIQState *ahci, uint32_t reg_num) > +{ > + return ahci_mread(ahci, 4 * reg_num); > +} > + > +static inline void ahci_wreg(AHCIQState *ahci, uint32_t reg_num, uint32_t value) > +{ > + ahci_mwrite(ahci, 4 * reg_num, value); > +} > + > +static inline void ahci_set(AHCIQState *ahci, uint32_t reg_num, uint32_t mask) > +{ > + ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) | mask); > +} > + > +static inline void ahci_clr(AHCIQState *ahci, uint32_t reg_num, uint32_t mask) > +{ > + ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) & ~mask); > +} > + > +static inline size_t ahci_px_offset(uint8_t port, uint32_t reg_num) > +{ > + return AHCI_PORTS + (HBA_PORT_NUM_REG * port) + reg_num; > +} __attribute__((__pure__)) This attribute applies to ahci_px_rreg, which I think is not what you want. The compiler should figure out the "purity" anyway for a static inline function, so my advice is to just drop it. Paolo > +static inline uint32_t ahci_px_rreg(AHCIQState *ahci, uint8_t port, > + uint32_t reg_num) > +{ > + return ahci_rreg(ahci, ahci_px_offset(port, reg_num)); > +} > + > +static inline void ahci_px_wreg(AHCIQState *ahci, uint8_t port, > + uint32_t reg_num, uint32_t value) > +{ > + ahci_wreg(ahci, ahci_px_offset(port, reg_num), value); > +} > + > +static inline void ahci_px_set(AHCIQState *ahci, uint8_t port, > + uint32_t reg_num, uint32_t mask) > +{ > + ahci_px_wreg(ahci, port, reg_num, > + ahci_px_rreg(ahci, port, reg_num) | mask); > +} > + > +static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port, > + uint32_t reg_num, uint32_t mask) > +{ > + ahci_px_wreg(ahci, port, reg_num, > + ahci_px_rreg(ahci, port, reg_num) & ~mask); > +} > + > #endif >