All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: <pdel@fb.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	andrew@aj.id.au, qemu-devel@nongnu.org, f4bug@amsat.org,
	patrick@stwcx.xyz, qemu-arm@nongnu.org,
	Joel Stanley <joel@jms.id.au>
Subject: Re: [PATCH 0/1] hw/arm/aspeed: Allow machine to set serial_hd(0)
Date: Wed, 1 Sep 2021 08:44:41 +0200	[thread overview]
Message-ID: <af023c9a-8ec5-27ec-9055-089969c88d1a@kaod.org> (raw)
In-Reply-To: <20210831233140.2659116-1-pdel@fb.com>

On 9/1/21 1:31 AM, pdel@fb.com wrote:
> From: Peter Delevoryas <pdel@fb.com>
> 
> This is a follow-up to a discussion in a previous series I sent:
> 
> https://lore.kernel.org/qemu-devel/20210827210417.4022054-1-pdel@fb.com/
> 
> I tried to add a new machine type called Fuji that required the ability
> to specify the UART connected to the first serial device on the command
> line (serial_hd(0)).
> 
> After some discussion, we concluded that we could add a serial_dev
> option to the machine class and the SoC to support this:
> 
> https://lore.kernel.org/qemu-devel/a802ecb1-aa49-fd4c-5bd2-2bb19af56ac9@kaod.org/
> 
> I didn't follow Cedric's advice _exactly_, so let me know if you have
> suggestions. I used "uint32_t serial_hd0", because I think it more
> clearly indicates that this is the device to connect to the first serial
> device, serial_hd(0).

I don't have a strong opinion on the name but it is part of the user
API and can be set from the command line  : 

  -global driver=ast2600-a3,property=serial-hd0,value=5

I would prefer something like "uart-default" which makes more sense.

> Also, I didn't know how to transfer data from the machine class to the
> device state, so I just added the attribute to the machine class and
> used 'qdev_get_machine' to within aspeed_soc_realize() based on some
> code I found in hw/ppc/spapr_cpu_core.c. 

This is called from the reset execution path which is a bit special.
The use of qdev_get_machine() should be avoided when possible. 

> I expect that I'm missing
> something, I've just been having some trouble figuring out the QEMU
> object model.

Look at the "dram" property of the SoC.

Thanks,

C. 

> If this patch is accepted, I can follow-up with another patch adding the
> fuji machine type with "serial_hd0 = ASPEED_DEV_UART1".
> 
> Thanks,
> Peter
> 
> Peter Delevoryas (1):
>   hw/arm/aspeed: Allow machine to set serial_hd(0)
> 
>  hw/arm/aspeed.c         |  1 +
>  hw/arm/aspeed_ast2600.c | 11 +++++++----
>  hw/arm/aspeed_soc.c     |  9 ++++++---
>  include/hw/arm/aspeed.h |  1 +
>  4 files changed, 15 insertions(+), 7 deletions(-)
> 



      parent reply	other threads:[~2021-09-01  6:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31 23:31 [PATCH 0/1] hw/arm/aspeed: Allow machine to set serial_hd(0) pdel
2021-08-31 23:31 ` [PATCH 1/1] " pdel
2021-09-01  2:30   ` Patrick Williams
2021-09-01  6:34   ` Cédric Le Goater
2021-09-01 14:05     ` Peter Delevoryas
2021-09-01  6:44 ` Cédric Le Goater [this message]

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=af023c9a-8ec5-27ec-9055-089969c88d1a@kaod.org \
    --to=clg@kaod.org \
    --cc=andrew@aj.id.au \
    --cc=f4bug@amsat.org \
    --cc=joel@jms.id.au \
    --cc=patrick@stwcx.xyz \
    --cc=pdel@fb.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.