From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: "Ninad Palsule" <ninad@linux.ibm.com>,
"Cédric Le Goater" <clg@kaod.org>,
qemu-devel@nongnu.org, peter.maydell@linaro.org, joel@jms.id.au,
pbonzini@redhat.com, marcandre.lureau@redhat.com,
berrange@redhat.com, thuth@redhat.com, philmd@linaro.org,
lvivier@redhat.com
Cc: qemu-arm@nongnu.org, Andrew Jeffery <andrew@aj.id.au>
Subject: Re: [PATCH v6 06/10] hw/fsi: Aspeed APB2OPB interface
Date: Fri, 27 Oct 2023 15:55:47 +1030 [thread overview]
Message-ID: <6976919f53a8643d23903d05d3642083da5f9421.camel@codeconstruct.com.au> (raw)
In-Reply-To: <8269cf50-05d1-4f57-8fc8-0a074721a0df@linux.ibm.com>
On Thu, 2023-10-26 at 10:27 -0500, Ninad Palsule wrote:
> Hello Cedric,
>
>
> On 10/24/23 10:21, Cédric Le Goater wrote:
> > On 10/24/23 17:00, Ninad Palsule wrote:
> > > Hello Cedric,
> > >
> > > On 10/24/23 02:46, Cédric Le Goater wrote:
> > > > and the fsi_opb_* routines are useless to me.
> > > We are trying to keep the separation between OPB implementation and
> > > interface hence we have all those fsi_opb_*. I feel that we should
> > > keep as it is so that future extensions will be easier. Please let me
> > > know.
> >
> > Well, I can't really tell because I don't know enough about FSI :/
> >
> > The models look fragile and I have spent already a lot of time trying
> > to untangle what they are trying to do. Please ask your teammates or
> > let's see in the next QEMU cycle.
>
> I have decided to go with the approach you suggested and it looks much
> better. Fixed it.
I intended to reply to this before Ninad sent out v7, but life
intervened.
If we can't justify it with the code we have now I think it's right to
pull it out. Add the code to support the things we're trying to do when
we need to do them. As long as we don't do anything that precludes us
from adding that code later (and I can't really imagine how we'd corner
ourselves like that).
We should bear in mind I wrote the initial models several years ago in
the space of about a week while I was trying to learn FSI (and more
deeply about the QEMU bus and address space modelling). I think I was
doing that to unblock some CI due to the introduction of the kernel
driver for the Aspeed FSI hardware. The models were pretty rough -
prior to all this review the code reflected my hazy understanding of
the problems. I didn't get time to remove the complexities introduced
by my misunderstandings, and now it's been so long that I'm not much
help with fixing them.
Andrew
next prev parent reply other threads:[~2023-10-27 5:27 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-21 21:17 [PATCH v6 00/10] Introduce model for IBM's FSI Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 01/10] hw/fsi: Introduce IBM's Local bus Ninad Palsule
2023-10-23 14:58 ` Philippe Mathieu-Daudé
2023-10-23 17:10 ` Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 02/10] hw/fsi: Introduce IBM's scratchpad Ninad Palsule
2023-10-23 15:00 ` Philippe Mathieu-Daudé
2023-10-23 17:08 ` Ninad Palsule
2023-10-24 7:08 ` Philippe Mathieu-Daudé
2023-10-26 15:24 ` Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 03/10] hw/fsi: Introduce IBM's cfam,fsi-slave Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 04/10] hw/fsi: Introduce IBM's FSI Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 05/10] hw/fsi: IBM's On-chip Peripheral Bus Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 06/10] hw/fsi: Aspeed APB2OPB interface Ninad Palsule
2023-10-24 7:46 ` Cédric Le Goater
2023-10-24 15:00 ` Ninad Palsule
2023-10-24 15:21 ` Cédric Le Goater
2023-10-24 18:42 ` Ninad Palsule
2023-10-26 15:27 ` Ninad Palsule
2023-10-27 5:25 ` Andrew Jeffery [this message]
2023-10-21 21:17 ` [PATCH v6 07/10] hw/arm: Hook up FSI module in AST2600 Ninad Palsule
2023-10-23 15:03 ` Philippe Mathieu-Daudé
2023-10-21 21:17 ` [PATCH v6 08/10] hw/fsi: Added qtest Ninad Palsule
2023-10-23 6:51 ` Thomas Huth
2023-10-23 15:25 ` Ninad Palsule
2023-10-24 7:34 ` Cédric Le Goater
2023-10-26 15:30 ` Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 09/10] hw/fsi: Added FSI documentation Ninad Palsule
2023-10-24 7:37 ` Cédric Le Goater
2023-10-26 15:32 ` Ninad Palsule
2023-10-21 21:17 ` [PATCH v6 10/10] hw/fsi: Update MAINTAINER list Ninad Palsule
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=6976919f53a8643d23903d05d3642083da5f9421.camel@codeconstruct.com.au \
--to=andrew@codeconstruct.com.au \
--cc=andrew@aj.id.au \
--cc=berrange@redhat.com \
--cc=clg@kaod.org \
--cc=joel@jms.id.au \
--cc=lvivier@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=ninad@linux.ibm.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=thuth@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).