All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mitsyanko <i.mitsyanko@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	michael@walle.cc, quintela@redhat.com, qemu-devel@nongnu.org,
	kyungmin.park@samsung.com, paul@codesourcery.com,
	afaerber@suse.de
Subject: Re: [Qemu-devel] [PATCH 6/6] hw/sd.c: convert to QOM object
Date: Mon, 02 Apr 2012 23:56:56 +0300	[thread overview]
Message-ID: <4F7A1298.70507@samsung.com> (raw)
In-Reply-To: <CAFEAcA_V1HfQOyh+3dvEEXKzE0+k5USNm5w8u7zY+A1kYxtZig@mail.gmail.com>

On 02.04.2012 7:48 PM, Peter Maydell wrote:
> On 2 April 2012 15:28, Igor Mitsyanko<i.mitsyanko@samsung.com>  wrote:
>> +    s->card = SD_CARD(object_new(TYPE_SD_CARD));
>>      dinfo = drive_get_next(IF_SD);
>> -    s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
>> +    SD_GET_CLASS(s->card)->init(s->card, dinfo ? dinfo->bdrv : NULL, false);
>
> Ideally rather than having a class specific init function we
> should make the 'is_spi' and 'bdrv' be object properties, and
> then you do something like (syntax probably wrong but):
>    s->card = SD_CARD(object_new(TYPE_SD_CARD));
>    if (dinfo) {
>       object_property_set_bdrv(s->card, dinfo->bdrv, "bdrv", errp);
>    }
>    realize(s->card);
>
> (where the default for bdrv is NULL and the default for is_spi is false
> so we don't need to set that).
>
> This needs realize support for QOM objects, though.
>
> -- PMM
>
>

I absolutely agree, that's how I wanted to do this in my working tree at 
first, but then several issues arose:
1) Pointer properties are obviously interfere with QOM principles, BDRV 
should be a link instead;
2) At first I made SD card child of SD host controller, but it most 
certainly wrong, it should be a link. But where should it be placed, 
under /block-dev/* maybe?
3) There is no point in creating SD card object until user specified an 
image.
4) Not every host controller on specific board is actually active, and 
nothing good will happen if you will try to hot-insert SD card in 
controller which was not activated by kernel. Having a child<SD> 
property in every controller instance is wrong.

What I want to do now is introduce CardSlot object and probably also 
HostControllerClass. Boards will instantiate as much CardSlots as it 
actually has and connect them with SDHC with links. SDCard object will 
be dynamically created and destroyed by setting CardSlot::inserted and 
CardSlot::image properties, SDcards will be childs of CardSlots.
But I havn't figured out how to avoid additional overhead in such a scheme.

  parent reply	other threads:[~2012-04-02 19:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-02 14:28 [Qemu-devel] [PATCH 0/6] SD save/load support and SD qomification Igor Mitsyanko
2012-04-02 14:28 ` [Qemu-devel] [PATCH 1/6] hw/sd.c: convert wp_groups in SDState to bitfield Igor Mitsyanko
2012-04-02 14:42   ` Peter Maydell
2012-04-02 20:17     ` Igor Mitsyanko
2012-04-02 14:28 ` [Qemu-devel] [PATCH 2/6] hw/sd.c: convert binary variables to bool Igor Mitsyanko
2012-04-02 16:32   ` Peter Maydell
2012-04-02 14:28 ` [Qemu-devel] [PATCH 3/6] hw/sd.c: make sd_dataready() return bool Igor Mitsyanko
2012-04-02 16:33   ` Peter Maydell
2012-04-02 14:28 ` [Qemu-devel] [PATCH 4/6] hw/sd.c: make sd_wp_addr() " Igor Mitsyanko
2012-04-02 14:28 ` [Qemu-devel] [PATCH 5/6] hw/sd.c: add SD card save/load support Igor Mitsyanko
2012-04-02 16:55   ` Peter Maydell
2012-04-02 14:28 ` [Qemu-devel] [PATCH 6/6] hw/sd.c: convert to QOM object Igor Mitsyanko
2012-04-02 16:48   ` Peter Maydell
2012-04-02 19:48     ` Paolo Bonzini
2012-04-02 20:56     ` Igor Mitsyanko [this message]
2012-04-02 20:11       ` Peter Maydell
2012-04-02 21:02       ` Paolo Bonzini
2012-04-03  8:35         ` Igor Mitsyanko
2012-04-03  9:50           ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F7A1298.70507@samsung.com \
    --to=i.mitsyanko@gmail.com \
    --cc=afaerber@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=i.mitsyanko@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=michael@walle.cc \
    --cc=paul@codesourcery.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.