[V4,11/12] SD card: introduce "spi" property for SD card objects

And drop passing is_spi argument to SDCardClass::init function.
"spi" property could be set only while SD card object is not
attached to any BlockDriverState.
It defaults to "false".

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
Cc: Paul Brook <paul@codesourcery.com>
---
 hw/sd.c |   33 +++++++++++++++++++++++++++++++--
 hw/sd.h |    8 ++++++--
 2 files changed, 37 insertions(+), 4 deletions(-)

Igor Mitsyanko <i.mitsyanko@samsung.com> writes:

> And drop passing is_spi argument to SDCardClass::init function.
> "spi" property could be set only while SD card object is not
> attached to any BlockDriverState.
> It defaults to "false".
>
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
> Cc: Paul Brook <paul@codesourcery.com>
> ---
>  hw/sd.c |   33 +++++++++++++++++++++++++++++++--
>  hw/sd.h |    8 ++++++--
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index fe2c138..611e1f3 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -491,10 +491,9 @@ 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.  */
> -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
> +static void sd_init_card(SDState *sd, BlockDriverState *bs)
>  {
>      sd->buf = qemu_blockalign(bs, 512);
> -    sd->spi = is_spi;
>      sd->enable = true;
>      sd_reset(sd, bs);
>      if (sd->bdrv) {
> @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void *data)
>      k->enable = sd_enable_card;
>  }
>  
> +static void sd_is_spi(Object *obj, Visitor *v, void *opaque,
> +                         const char *name, Error **errp)
> +{
> +    SDState *sd = SD_CARD(obj);
> +
> +    visit_type_bool(v, &sd->spi, name, errp);
> +}
> +
> +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
> +                         const char *name, Error **errp)
> +{
> +    SDState *sd = SD_CARD(obj);
> +
> +    if (sd->bdrv) {
> +        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
> +    } else {
> +        visit_type_bool(v, &sd->spi, name, errp);
> +    }
> +}
> +
> +static void sd_initfn(Object *obj)
> +{
> +    SDState *sd = SD_CARD(obj);
> +
> +    sd->spi = false;
> +    object_property_add(obj, "spi", "boolean", sd_is_spi, sd_set_spimode,
> +            NULL, NULL, NULL);
> +}
> +
>  static const TypeInfo sd_type_info = {
>      .name = TYPE_SD_CARD,
>      .parent = TYPE_OBJECT,
>      .instance_size = sizeof(SDState),
> +    .instance_init = sd_initfn,
>      .class_init = sd_class_init,
>      .class_size = sizeof(SDClass)
>  };

I suspect this would be much simpler the declarative way qdevs normally
use.  For an example, check out scsi_hd_properties[] and its use in
hw/scsi-disk.c.

> diff --git a/hw/sd.h b/hw/sd.h
> index f84e863..c78eaa1 100644
> --- a/hw/sd.h
> +++ b/hw/sd.h
> @@ -31,6 +31,7 @@
>  
>  #include "qemu-common.h"
>  #include "qemu/object.h"
> +#include "qapi/qapi-visit-core.h"
>  
>  #define OUT_OF_RANGE		(1 << 31)
>  #define ADDRESS_ERROR		(1 << 30)
> @@ -73,7 +74,7 @@ typedef struct SDState SDState;
>  typedef struct SDClass {
>      ObjectClass parent_class;
>  
> -    void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi);
> +    void (*init)(SDState *sd, BlockDriverState *bdrv);
>      int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
>      void (*write_data)(SDState *sd, uint8_t value);
>      uint8_t (*read_data)(SDState *sd);
> @@ -93,7 +94,10 @@ typedef struct SDClass {
>  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);
> +    Error *errp = NULL;
> +    object_property_set_bool(OBJECT(sd), is_spi, "spi", &errp);
> +    assert_no_error(errp);
> +    SD_GET_CLASS(sd)->init(sd, bs);
>      return sd;
>  }

Am 31.07.2012 11:54, schrieb Markus Armbruster:
> Igor Mitsyanko <i.mitsyanko@samsung.com> writes:
> 
>> And drop passing is_spi argument to SDCardClass::init function.
>> "spi" property could be set only while SD card object is not
>> attached to any BlockDriverState.
>> It defaults to "false".
>>
>> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
>> Cc: Paul Brook <paul@codesourcery.com>
>> ---
>>  hw/sd.c |   33 +++++++++++++++++++++++++++++++--
>>  hw/sd.h |    8 ++++++--
>>  2 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/sd.c b/hw/sd.c
>> index fe2c138..611e1f3 100644
>> --- a/hw/sd.c
>> +++ b/hw/sd.c
>> @@ -491,10 +491,9 @@ 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.  */
>> -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
>> +static void sd_init_card(SDState *sd, BlockDriverState *bs)
>>  {
>>      sd->buf = qemu_blockalign(bs, 512);
>> -    sd->spi = is_spi;
>>      sd->enable = true;
>>      sd_reset(sd, bs);
>>      if (sd->bdrv) {
>> @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void *data)
>>      k->enable = sd_enable_card;
>>  }
>>  
>> +static void sd_is_spi(Object *obj, Visitor *v, void *opaque,
>> +                         const char *name, Error **errp)
>> +{
>> +    SDState *sd = SD_CARD(obj);
>> +
>> +    visit_type_bool(v, &sd->spi, name, errp);
>> +}
>> +
>> +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
>> +                         const char *name, Error **errp)
>> +{
>> +    SDState *sd = SD_CARD(obj);
>> +
>> +    if (sd->bdrv) {
>> +        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
>> +    } else {
>> +        visit_type_bool(v, &sd->spi, name, errp);
>> +    }
>> +}
>> +
>> +static void sd_initfn(Object *obj)
>> +{
>> +    SDState *sd = SD_CARD(obj);
>> +
>> +    sd->spi = false;
>> +    object_property_add(obj, "spi", "boolean", sd_is_spi, sd_set_spimode,
>> +            NULL, NULL, NULL);
>> +}
>> +
>>  static const TypeInfo sd_type_info = {
>>      .name = TYPE_SD_CARD,
>>      .parent = TYPE_OBJECT,
>>      .instance_size = sizeof(SDState),
>> +    .instance_init = sd_initfn,
>>      .class_init = sd_class_init,
>>      .class_size = sizeof(SDClass)
>>  };
> 
> I suspect this would be much simpler the declarative way qdevs normally
> use.  For an example, check out scsi_hd_properties[] and its use in
> hw/scsi-disk.c.
[snip]

For static properties bool support was missing some time ago...

Andreas

Il 31/07/2012 14:19, Andreas Färber ha scritto:
>>> >> +    sd->spi = false;
>>> >> +    object_property_add(obj, "spi", "boolean", sd_is_spi, sd_set_spimode,
>>> >> +            NULL, NULL, NULL);
>>> >> +}
>>> >> +
>>> >>  static const TypeInfo sd_type_info = {
>>> >>      .name = TYPE_SD_CARD,
>>> >>      .parent = TYPE_OBJECT,
>>> >>      .instance_size = sizeof(SDState),
>>> >> +    .instance_init = sd_initfn,
>>> >>      .class_init = sd_class_init,
>>> >>      .class_size = sizeof(SDClass)
>>> >>  };
>> > 
>> > I suspect this would be much simpler the declarative way qdevs normally
>> > use.  For an example, check out scsi_hd_properties[] and its use in
>> > hw/scsi-disk.c.
> [snip]
> 
> For static properties bool support was missing some time ago...

There are bitfields, which are really the same thing except they expect
an u32 field.

Paolo