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

[Troy Lee]

Hi Cedric,

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, June 1, 2022 3:10 PM
> To: Peter Delevoryas <pdel@fb.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org; zhdaniel@fb.com; Troy
> Lee <troy_lee@aspeedtech.com>; Jamin Lin <jamin_lin@aspeedtech.com>;
> Steven Lee <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
> 
> [ 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
> 

Yes, I'm ok with this. We have tested Joe's patch set as well.

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

I'm looking forward to have slave device support, with that we could have more 
firmware test case in QEMU.

Thanks,
Troy Lee

> 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&ar
> > chive=both [2]
> > https://patchwork.kernel.org/project/qemu-devel/list/?series=627914&ar
> > chive=both [3]
> > https://github.com/AspeedTech-
> BMC/zephyr/blob/db3dbcc9c52e67a47180890a
> > c938ed380b33f91c/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(-)
> >