On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > Now only SD 'producers' are able to use the "sd-internal.h" API, > while SD 'consumers' are restricted to the "hw/sd/sd.h" 'public' API. > > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
I don't see what this gets us. Why bother moving this code into an internal header? Alistair > --- > hw/sd/sd-internal.h | 119 > ++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/sd/sd.h | 95 +++--------------------------------- > hw/sd/core.c | 3 +- > hw/sd/milkymist-memcard.c | 2 +- > hw/sd/omap_mmc.c | 1 + > hw/sd/pl181.c | 2 +- > hw/sd/pxa2xx_mmci.c | 1 + > hw/sd/sd.c | 6 +-- > hw/sd/sdhci.c | 2 +- > hw/sd/ssi-sd.c | 2 +- > 10 files changed, 135 insertions(+), 98 deletions(-) > create mode 100644 hw/sd/sd-internal.h > > diff --git a/hw/sd/sd-internal.h b/hw/sd/sd-internal.h > new file mode 100644 > index 0000000000..afd5dbf194 > --- /dev/null > +++ b/hw/sd/sd-internal.h > @@ -0,0 +1,119 @@ > +/* > + * SD Memory Card emulation. > + * > + * Copyright (c) 2006 Andrzej Zaborowski <bal...@zabor.org> > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the > + * distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, > + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, > + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR > + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY > + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > +#ifndef SD_INTERNAL_H > +#define SD_INTERNAL_H > + > +#include "hw/qdev.h" > +#include "sysemu/block-backend.h" > + > +#define OUT_OF_RANGE (1 << 31) > +#define ADDRESS_ERROR (1 << 30) > +#define BLOCK_LEN_ERROR (1 << 29) > +#define ERASE_SEQ_ERROR (1 << 28) > +#define ERASE_PARAM (1 << 27) > +#define WP_VIOLATION (1 << 26) > +#define CARD_IS_LOCKED (1 << 25) > +#define LOCK_UNLOCK_FAILED (1 << 24) > +#define COM_CRC_ERROR (1 << 23) > +#define ILLEGAL_COMMAND (1 << 22) > +#define CARD_ECC_FAILED (1 << 21) > +#define CC_ERROR (1 << 20) > +#define SD_ERROR (1 << 19) > +#define CID_CSD_OVERWRITE (1 << 16) > +#define WP_ERASE_SKIP (1 << 15) > +#define CARD_ECC_DISABLED (1 << 14) > +#define ERASE_RESET (1 << 13) > +#define CURRENT_STATE (7 << 9) > +#define READY_FOR_DATA (1 << 8) > +#define APP_CMD (1 << 5) > +#define AKE_SEQ_ERROR (1 << 3) > + > +#define OCR_CCS_BITN 30 > + > +typedef enum { > + sd_none = -1, > + sd_bc = 0, /* broadcast -- no response */ > + sd_bcr, /* broadcast with response */ > + sd_ac, /* addressed -- no data transfer */ > + sd_adtc, /* addressed with data transfer */ > +} sd_cmd_type_t; > + > +typedef struct SDState SDState; > + > +#define SD_CARD_CLASS(klass) \ > + OBJECT_CLASS_CHECK(SDCardClass, (klass), TYPE_SD_CARD) > +#define SD_CARD_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(SDCardClass, (obj), TYPE_SD_CARD) > + > +typedef struct { > + /*< private >*/ > + DeviceClass parent_class; > + /*< public >*/ > + > + int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response); > + void (*write_data)(SDState *sd, uint8_t value); > + uint8_t (*read_data)(SDState *sd); > + bool (*data_ready)(SDState *sd); > + void (*enable)(SDState *sd, bool enable); > + bool (*get_inserted)(SDState *sd); > + bool (*get_readonly)(SDState *sd); > +} SDCardClass; > + > +#define SD_BUS_CLASS(klass) OBJECT_CLASS_CHECK(SDBusClass, (klass), > TYPE_SD_BUS) > +#define SD_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(SDBusClass, (obj), > TYPE_SD_BUS) > + > +typedef struct { > + /*< private >*/ > + BusClass parent_class; > + /*< public >*/ > + > + /* These methods are called by the SD device to notify the controller > + * when the card insertion or readonly status changes > + */ > + void (*set_inserted)(DeviceState *dev, bool inserted); > + void (*set_readonly)(DeviceState *dev, bool readonly); > +} SDBusClass; > + > +/* Legacy functions to be used only by non-qdevified callers */ > +SDState *sd_init(BlockBackend *bs, bool is_spi); > +int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response); > +void sd_write_data(SDState *sd, uint8_t value); > +uint8_t sd_read_data(SDState *sd); > +void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); > +bool sd_data_ready(SDState *sd); > +/* sd_enable should not be used -- it is only used on the nseries boards, > + * where it is part of a broken implementation of the MMC card slot switch > + * (there should be two card slots which are multiplexed to a single MMC > + * controller, but instead we model it with one card and controller and > + * disable the card when the second slot is selected, so it looks like the > + * second slot is always empty). > + */ > +void sd_enable(SDState *sd, bool enable); > + > +#endif > diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h > index 96caefe373..f6994e61f2 100644 > --- a/include/hw/sd/sd.h > +++ b/include/hw/sd/sd.h > @@ -32,108 +32,25 @@ > > #include "hw/qdev.h" > > -#define OUT_OF_RANGE (1 << 31) > -#define ADDRESS_ERROR (1 << 30) > -#define BLOCK_LEN_ERROR (1 << 29) > -#define ERASE_SEQ_ERROR (1 << 28) > -#define ERASE_PARAM (1 << 27) > -#define WP_VIOLATION (1 << 26) > -#define CARD_IS_LOCKED (1 << 25) > -#define LOCK_UNLOCK_FAILED (1 << 24) > -#define COM_CRC_ERROR (1 << 23) > -#define ILLEGAL_COMMAND (1 << 22) > -#define CARD_ECC_FAILED (1 << 21) > -#define CC_ERROR (1 << 20) > -#define SD_ERROR (1 << 19) > -#define CID_CSD_OVERWRITE (1 << 16) > -#define WP_ERASE_SKIP (1 << 15) > -#define CARD_ECC_DISABLED (1 << 14) > -#define ERASE_RESET (1 << 13) > -#define CURRENT_STATE (7 << 9) > -#define READY_FOR_DATA (1 << 8) > -#define APP_CMD (1 << 5) > -#define AKE_SEQ_ERROR (1 << 3) > -#define OCR_CCS_BITN 30 > - > -typedef enum { > - sd_none = -1, > - sd_bc = 0, /* broadcast -- no response */ > - sd_bcr, /* broadcast with response */ > - sd_ac, /* addressed -- no data transfer */ > - sd_adtc, /* addressed with data transfer */ > -} sd_cmd_type_t; > - > typedef struct { > - uint8_t cmd; > - uint32_t arg; > - uint8_t crc; > -} SDRequest; > + uint8_t cmd; /* 6 bits */ > + uint32_t arg; /* 32 bits */ > + uint8_t crc; /* 7 bits */ > +} SDRequest; /* total: 48 bits shifted */ > > -typedef struct SDState SDState; > typedef struct SDBus SDBus; > > #define TYPE_SD_CARD "sd-card" > #define SD_CARD(obj) OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD) > -#define SD_CARD_CLASS(klass) \ > - OBJECT_CLASS_CHECK(SDCardClass, (klass), TYPE_SD_CARD) > -#define SD_CARD_GET_CLASS(obj) \ > - OBJECT_GET_CLASS(SDCardClass, (obj), TYPE_SD_CARD) > - > -typedef struct { > - /*< private >*/ > - DeviceClass parent_class; > - /*< public >*/ > - > - int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response); > - void (*write_data)(SDState *sd, uint8_t value); > - uint8_t (*read_data)(SDState *sd); > - bool (*data_ready)(SDState *sd); > - void (*enable)(SDState *sd, bool enable); > - bool (*get_inserted)(SDState *sd); > - bool (*get_readonly)(SDState *sd); > -} SDCardClass; > > #define TYPE_SD_BUS "sd-bus" > #define SD_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SD_BUS) > -#define SD_BUS_CLASS(klass) OBJECT_CLASS_CHECK(SDBusClass, (klass), > TYPE_SD_BUS) > -#define SD_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(SDBusClass, (obj), > TYPE_SD_BUS) > > struct SDBus { > BusState qbus; > }; > > -typedef struct { > - /*< private >*/ > - BusClass parent_class; > - /*< public >*/ > - > - /* These methods are called by the SD device to notify the controller > - * when the card insertion or readonly status changes > - */ > - void (*set_inserted)(DeviceState *dev, bool inserted); > - void (*set_readonly)(DeviceState *dev, bool readonly); > -} SDBusClass; > - > -/* Legacy functions to be used only by non-qdevified callers */ > -SDState *sd_init(BlockBackend *bs, bool is_spi); > -int sd_do_command(SDState *sd, SDRequest *req, > - uint8_t *response); > -void sd_write_data(SDState *sd, uint8_t value); > -uint8_t sd_read_data(SDState *sd); > -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); > -bool sd_data_ready(SDState *sd); > -/* sd_enable should not be used -- it is only used on the nseries boards, > - * where it is part of a broken implementation of the MMC card slot switch > - * (there should be two card slots which are multiplexed to a single MMC > - * controller, but instead we model it with one card and controller and > - * disable the card when the second slot is selected, so it looks like the > - * second slot is always empty). > - */ > -void sd_enable(SDState *sd, bool enable); > - > -/* Functions to be used by qdevified callers (working via > - * an SDBus rather than directly with SDState) > - */ > +/* Functions to be used by qdevified callers */ > int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response); > void sdbus_write_data(SDBus *sd, uint8_t value); > uint8_t sdbus_read_data(SDBus *sd); > @@ -154,6 +71,6 @@ void sdbus_reparent_card(SDBus *from, SDBus *to); > > /* Functions to be used by SD devices to report back to qdevified > controllers */ > void sdbus_set_inserted(SDBus *sd, bool inserted); > -void sdbus_set_readonly(SDBus *sd, bool inserted); > +void sdbus_set_readonly(SDBus *sd, bool readonly); > > #endif /* HW_SD_H */ > diff --git a/hw/sd/core.c b/hw/sd/core.c > index 295dc44ab7..bd9350d21c 100644 > --- a/hw/sd/core.c > +++ b/hw/sd/core.c > @@ -20,9 +20,8 @@ > */ > > #include "qemu/osdep.h" > -#include "hw/qdev-core.h" > -#include "sysemu/block-backend.h" > #include "hw/sd/sd.h" > +#include "sd-internal.h" > > static SDState *get_card(SDBus *sdbus) > { > diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c > index 4008c81002..8c9bb27229 100644 > --- a/hw/sd/milkymist-memcard.c > +++ b/hw/sd/milkymist-memcard.c > @@ -27,9 +27,9 @@ > #include "sysemu/sysemu.h" > #include "trace.h" > #include "qemu/error-report.h" > -#include "sysemu/block-backend.h" > #include "sysemu/blockdev.h" > #include "hw/sd/sd.h" > +#include "sd-internal.h" > > enum { > ENABLE_CMD_TX = (1<<0), > diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c > index e934cd3656..f7e42b3525 100644 > --- a/hw/sd/omap_mmc.c > +++ b/hw/sd/omap_mmc.c > @@ -20,6 +20,7 @@ > #include "hw/hw.h" > #include "hw/arm/omap.h" > #include "hw/sd/sd.h" > +#include "sd-internal.h" > > struct omap_mmc_s { > qemu_irq irq; > diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c > index 55c8098ecd..d568b12818 100644 > --- a/hw/sd/pl181.c > +++ b/hw/sd/pl181.c > @@ -8,10 +8,10 @@ > */ > > #include "qemu/osdep.h" > -#include "sysemu/block-backend.h" > #include "sysemu/blockdev.h" > #include "hw/sysbus.h" > #include "hw/sd/sd.h" > +#include "sd-internal.h" > #include "qemu/log.h" > #include "qapi/error.h" > > diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c > index 3deccf02c9..f34951e1d1 100644 > --- a/hw/sd/pxa2xx_mmci.c > +++ b/hw/sd/pxa2xx_mmci.c > @@ -16,6 +16,7 @@ > #include "hw/sysbus.h" > #include "hw/arm/pxa.h" > #include "hw/sd/sd.h" > +#include "sd-internal.h" > #include "hw/qdev.h" > #include "hw/qdev-properties.h" > #include "qemu/error-report.h" > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 35347a5bbc..9b7dee2ec4 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -32,7 +32,6 @@ > #include "qemu/osdep.h" > #include "hw/qdev.h" > #include "hw/hw.h" > -#include "sysemu/block-backend.h" > #include "hw/sd/sd.h" > #include "qapi/error.h" > #include "qemu/bitmap.h" > @@ -40,6 +39,7 @@ > #include "qemu/error-report.h" > #include "qemu/timer.h" > #include "qemu/log.h" > +#include "sd-internal.h" > > //#define DEBUG_SD 1 > > @@ -1466,8 +1466,8 @@ static int cmd_valid_while_locked(SDState *sd, > SDRequest *req) > || sd_cmd_class[req->cmd & 0x3F] == 7; > } > > -int sd_do_command(SDState *sd, SDRequest *req, > - uint8_t *response) { > +int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response) > +{ > int last_state; > sd_rsp_type_t rtype; > int rsplen; > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index b064a087c9..e7cd3258a3 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -24,12 +24,12 @@ > > #include "qemu/osdep.h" > #include "hw/hw.h" > -#include "sysemu/block-backend.h" > #include "sysemu/blockdev.h" > #include "sysemu/dma.h" > #include "qemu/timer.h" > #include "qemu/bitops.h" > #include "sdhci-internal.h" > +#include "sd-internal.h" > #include "qemu/log.h" > > /* host controller debug messages */ > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c > index 24001dc3e6..bd0b593b6e 100644 > --- a/hw/sd/ssi-sd.c > +++ b/hw/sd/ssi-sd.c > @@ -11,11 +11,11 @@ > */ > > #include "qemu/osdep.h" > -#include "sysemu/block-backend.h" > #include "sysemu/blockdev.h" > #include "hw/ssi/ssi.h" > #include "hw/sd/sd.h" > #include "qapi/error.h" > +#include "sd-internal.h" > > //#define DEBUG_SSI_SD 1 > > -- > 2.15.1 > >