Re: [RFC 0/1] i2c/aspeed: Add slave device handling in new register mode

From: "Cédric Le Goater" <clg@kaod.org>
To: Peter Delevoryas <pdel@fb.com>
Cc: <qemu-arm@nongnu.org>, <qemu-devel@nongnu.org>, <zhdaniel@fb.com>,
	<troy_lee@aspeedtech.com>, <jamin_lin@aspeedtech.com>,
	<steven_lee@aspeedtech.com>, <k.jensen@samsung.com>,
	Joe Komlodi <komlodi@google.com>, Joel Stanley <joel@jms.id.au>,
	Andrew Jeffery <andrew@aj.id.au>
Subject: Re: [RFC 0/1] i2c/aspeed: Add slave device handling in new register mode
Date: Wed, 1 Jun 2022 09:10:15 +0200	[thread overview]
Message-ID: <983fc0e3-6166-74d4-1164-161c277e4a96@kaod.org> (raw)
In-Reply-To: <20220525205024.1158075-1-pdel@fb.com>

[ Adding Joe ]

On 5/25/22 22:50, Peter Delevoryas wrote:
> The AST2600/AST1030 new register mode patches[1] and the I2C slave device
> patches[2] will be really useful, but we still need DMA slave device
> handling in the new register mode too for the use-cases I'm thinking of
> (OpenBIC Zephyr kernel using Aspeed SDK drivers[3]).
> 
> My test images are on Github[4]. They can be used with the ast1030-evb, or
> the oby35-cl and oby35-bb machines in the fb qemu branch[5].
> 
> I'm submitting this as an RFC cause I just want to see how other people
> expect these changes to be made based on the previously submitted "new
> register mode" and "old register mode slave device" patches.

Currently, my preferred approach would be to start with Joe's patchset
because the registerfields conversion is a huge effort and it's adding
new mode support which should cover the needs for the AST1030 SoC [1].

Troy, could you please confirm this is OK with you ? I have pushed
the patches on :

   https://github.com/legoater/qemu/commits/aspeed-7.1

Then, adding slave support for old [2] and new mode (this patch)
shouldn't be too much of a problem since they are small.

we lack a test case for this controller and writing a I2C Aspeed bus
driver for qtest is not an easy task.

It might be easier to start an ast2600-evb machine with a lightweight
userspace (buildroot, I can host that somewhere on GH) and run some I2C
get/set/detect commands from a python/expect framework, like avocado.
I2C devices can be added on the command line for the purpose.

Thanks,

C.

> Thanks,
> Peter
> 
> [1] https://patchwork.kernel.org/project/qemu-devel/list/?series=626028&archive=both
> [2] https://patchwork.kernel.org/project/qemu-devel/list/?series=627914&archive=both
> [3] https://github.com/AspeedTech-BMC/zephyr/blob/db3dbcc9c52e67a47180890ac938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L1362-L1368
> [4] https://github.com/peterdelevoryas/OpenBIC/releases/tag/oby35-cl-2022.13.01
> [5] https://github.com/facebook/openbmc-qemu
> 
> Peter Delevoryas (1):
>    i2c/aspeed: Add slave device handling in new register mode
> 
>   hw/i2c/aspeed_i2c.c         | 118 ++++++++++++++++++++++++++++++++++--
>   include/hw/i2c/aspeed_i2c.h |  14 +++--
>   2 files changed, 124 insertions(+), 8 deletions(-)
> 