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

Peter Delevoryas posted 1 patch 1 year, 11 months ago
Failed in applying to current master (apply log)
hw/i2c/aspeed_i2c.c         | 118 ++++++++++++++++++++++++++++++++++--
include/hw/i2c/aspeed_i2c.h |  14 +++--
2 files changed, 124 insertions(+), 8 deletions(-)
[RFC 0/1] i2c/aspeed: Add slave device handling in new register mode
Posted by Peter Delevoryas 1 year, 11 months ago
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.

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(-)

-- 
2.30.2
Re: [RFC 0/1] i2c/aspeed: Add slave device handling in new register mode
Posted by Cédric Le Goater 1 year, 11 months ago
[ 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(-)
>
RE: [RFC 0/1] i2c/aspeed: Add slave device handling in new register mode
Posted by Troy Lee 1 year, 11 months ago
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(-)
> >