From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDGG2-0006Wx-MQ for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:37:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDGFy-0008Dw-LP for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:36:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47001) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDGFy-0008Dr-Dg for qemu-devel@nongnu.org; Mon, 19 Jan 2015 12:36:54 -0500 Message-ID: <54BD40B2.2010003@redhat.com> Date: Mon, 19 Jan 2015 12:36:50 -0500 From: John Snow MIME-Version: 1.0 References: <1421120079-987-1-git-send-email-jsnow@redhat.com> <1421120079-987-12-git-send-email-jsnow@redhat.com> <54BD3A4E.3030102@redhat.com> In-Reply-To: <54BD3A4E.3030102@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed 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: Paolo Bonzini , qemu-devel@nongnu.org Cc: kwolf@redhat.com, marc.mari.barcelo@gmail.com, armbru@redhat.com, stefanha@redhat.com, mreitz@redhat.com On 01/19/2015 12:09 PM, Paolo Bonzini wrote: > > > 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 > No, whoops. Was thinking of the struct syntax. I am surprised it doesn't complain. I'll just remove it. >> +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 >> >