[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM objec
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object |
Date: |
Tue, 31 Jul 2012 11:45:47 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Igor Mitsyanko <address@hidden> writes:
> A straightforward conversion of SD card implementation to a proper QEMU
> object.
> Wrapper functions were introduced for SDClass methods in order to avoid SD
> card
> users modification. Because of this, name change for several functions in
> hw/sd.c
> was required.
>
> Signed-off-by: Igor Mitsyanko <address@hidden>
> ---
> hw/sd.c | 56 ++++++++++++++++++++++++++++++++++++++--------------
> hw/sd.h | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 100 insertions(+), 23 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index f8ab045..fe2c138 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -75,6 +75,8 @@ enum {
> };
>
> struct SDState {
> + Object parent_obj;
> +
> uint32_t mode;
> int32_t state;
> uint32_t ocr;
> @@ -489,11 +491,8 @@ static const VMStateDescription sd_vmstate = {
> whether card should be in SSI or MMC/SD mode. It is also up to the
> board to ensure that ssi transfers only occur when the chip select
> is asserted. */
> -SDState *sd_init(BlockDriverState *bs, bool is_spi)
> +static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
> {
> - SDState *sd;
> -
> - sd = (SDState *) g_malloc0(sizeof(SDState));
> sd->buf = qemu_blockalign(bs, 512);
> sd->spi = is_spi;
> sd->enable = true;
> @@ -503,10 +502,9 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
> bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
> }
> vmstate_register(NULL, -1, &sd_vmstate, sd);
> - return sd;
> }
>
> -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
> +static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert)
> {
> sd->readonly_cb = readonly;
> sd->inserted_cb = insert;
> @@ -1334,7 +1332,7 @@ static int cmd_valid_while_locked(SDState *sd,
> SDRequest *req)
> return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7;
> }
>
> -int sd_do_command(SDState *sd, SDRequest *req,
> +static int sd_send_command(SDState *sd, SDRequest *req,
> uint8_t *response) {
> int last_state;
> sd_rsp_type_t rtype;
> @@ -1502,7 +1500,7 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
> uint32_t len)
> #define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
> #define APP_WRITE_BLOCK(a, len)
>
> -void sd_write_data(SDState *sd, uint8_t value)
> +static void sd_write_card_data(SDState *sd, uint8_t value)
> {
> int i;
>
> @@ -1510,7 +1508,7 @@ void sd_write_data(SDState *sd, uint8_t value)
> return;
>
> if (sd->state != sd_receivingdata_state) {
> - fprintf(stderr, "sd_write_data: not in Receiving-Data state\n");
> + fprintf(stderr, "sd_write_card_data: not in Receiving-Data state\n");
Outside this patch's scope, but here goes anyway: what kind of condition
is reported here? Programming error that should never happen? Guest
doing something weird?
Same for all the other fprintf(stderr, ...) in this file.
> return;
> }
>
> @@ -1621,12 +1619,12 @@ void sd_write_data(SDState *sd, uint8_t value)
> break;
>
> default:
> - fprintf(stderr, "sd_write_data: unknown command\n");
> + fprintf(stderr, "sd_write_card_data: unknown command\n");
> break;
> }
> }
>
> -uint8_t sd_read_data(SDState *sd)
> +static uint8_t sd_read_card_data(SDState *sd)
> {
> /* TODO: Append CRCs */
> uint8_t ret;
> @@ -1636,7 +1634,7 @@ uint8_t sd_read_data(SDState *sd)
> return 0x00;
>
> if (sd->state != sd_sendingdata_state) {
> - fprintf(stderr, "sd_read_data: not in Sending-Data state\n");
> + fprintf(stderr, "sd_read_card_data: not in Sending-Data state\n");
> return 0x00;
> }
>
> @@ -1738,19 +1736,47 @@ uint8_t sd_read_data(SDState *sd)
> break;
>
> default:
> - fprintf(stderr, "sd_read_data: unknown command\n");
> + fprintf(stderr, "sd_read_card_data: unknown command\n");
> return 0x00;
> }
>
> return ret;
> }
>
> -bool sd_data_ready(SDState *sd)
> +static bool sd_is_data_ready(SDState *sd)
> {
> return sd->state == sd_sendingdata_state;
> }
>
> -void sd_enable(SDState *sd, bool enable)
> +static void sd_enable_card(SDState *sd, bool enable)
> {
> sd->enable = enable;
> }
> +
> +static void sd_class_init(ObjectClass *klass, void *data)
> +{
> + SDClass *k = SD_CLASS(klass);
> +
> + k->init = sd_init_card;
> + k->set_cb = sd_set_callbacks;
> + k->do_command = sd_send_command;
> + k->data_ready = sd_is_data_ready;
> + k->read_data = sd_read_card_data;
> + k->write_data = sd_write_card_data;
> + k->enable = sd_enable_card;
> +}
> +
> +static const TypeInfo sd_type_info = {
> + .name = TYPE_SD_CARD,
> + .parent = TYPE_OBJECT,
Possibly ignorant question: why TYPE_OBJECT, not TYPE_DEVICE?
> + .instance_size = sizeof(SDState),
> + .class_init = sd_class_init,
> + .class_size = sizeof(SDClass)
> +};
> +
> +static void sd_register_types(void)
> +{
> + type_register_static(&sd_type_info);
> +}
> +
> +type_init(sd_register_types)
> diff --git a/hw/sd.h b/hw/sd.h
> index 4eb9679..f84e863 100644
> --- a/hw/sd.h
> +++ b/hw/sd.h
> @@ -29,6 +29,9 @@
> #ifndef __hw_sd_h
> #define __hw_sd_h 1
>
> +#include "qemu-common.h"
> +#include "qemu/object.h"
> +
> #define OUT_OF_RANGE (1 << 31)
> #define ADDRESS_ERROR (1 << 30)
> #define BLOCK_LEN_ERROR (1 << 29)
> @@ -67,13 +70,61 @@ typedef struct {
>
> typedef struct SDState SDState;
>
> -SDState *sd_init(BlockDriverState *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);
> -void sd_enable(SDState *sd, bool enable);
> +typedef struct SDClass {
> + ObjectClass parent_class;
> +
> + void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi);
> + int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
> + void (*write_data)(SDState *sd, uint8_t value);
> + uint8_t (*read_data)(SDState *sd);
> + void (*set_cb)(SDState *sd, qemu_irq readonly, qemu_irq insert);
> + bool (*data_ready)(SDState *sd);
> + void (*enable)(SDState *sd, bool enable);
> +} SDClass;
> +
> +#define TYPE_SD_CARD "sd-card"
> +#define SD_CARD(obj) \
> + OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
> +#define SD_CLASS(klass) \
> + OBJECT_CLASS_CHECK(SDClass, (klass), TYPE_SD_CARD)
> +#define SD_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(SDClass, (obj), TYPE_SD_CARD)
> +
> +static inline SDState *sd_init(BlockDriverState *bs, bool is_spi)
> +{
> + SDState *sd = SD_CARD(object_new(TYPE_SD_CARD));
> + SD_GET_CLASS(sd)->init(sd, bs, is_spi);
> + return sd;
Shouldn't bs and spi be properties? Oh, that's coming in PATCH
10-11/12.
> +}
> +
> +static inline int sd_do_command(SDState *sd, SDRequest *req, uint8_t
> *response)
> +{
> + return SD_GET_CLASS(sd)->do_command(sd, req, response);
> +}
> +
> +static inline uint8_t sd_read_data(SDState *sd)
> +{
> + return SD_GET_CLASS(sd)->read_data(sd);
> +}
> +
> +static inline void sd_write_data(SDState *sd, uint8_t value)
> +{
> + SD_GET_CLASS(sd)->write_data(sd, value);
> +}
> +
> +static inline bool sd_data_ready(SDState *sd)
> +{
> + return SD_GET_CLASS(sd)->data_ready(sd);
> +}
> +
> +static inline void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
> +{
> + SD_GET_CLASS(sd)->set_cb(sd, readonly, insert);
> +}
> +
> +static inline void sd_enable(SDState *sd, bool enable)
> +{
> + SD_GET_CLASS(sd)->enable(sd, enable);
> +}
>
> #endif /* __hw_sd_h */
- Re: [Qemu-devel] [PATCH V4 02/12] hw/sd.c: make sd_wp_addr() accept 64 bit address argument, (continued)
- [Qemu-devel] [PATCH V4 03/12] hw/sd.c: introduce wrapper for conversion address to wp group, Igor Mitsyanko, 2012/07/27
- [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase, Igor Mitsyanko, 2012/07/27
- [Qemu-devel] [PATCH V4 05/12] hw/sd.c: convert binary variables to bool, Igor Mitsyanko, 2012/07/27
- [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Igor Mitsyanko, 2012/07/27
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Peter Maydell, 2012/07/31
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Igor Mitsyanko, 2012/07/31
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Markus Armbruster, 2012/07/31
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Peter Maydell, 2012/07/31
- Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object, Igor Mitsyanko, 2012/07/31
[Qemu-devel] [PATCH V4 06/12] hw/sd.c: make sd_dataready() return bool, Igor Mitsyanko, 2012/07/27
[Qemu-devel] [PATCH V4 11/12] SD card: introduce "spi" property for SD card objects, Igor Mitsyanko, 2012/07/27