| Message ID | 20220525205024.1158075-1-pdel@fb.com |
|---|---|
| Headers | show |
| Series | i2c/aspeed: Add slave device handling in new register mode | expand |
[ 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(-) >
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(-) > >