[v1] hw/i2c: Add designware i2c controller

[PATCH 0/4] hw/i2c: Add designware i2c controller

Posted by Nicholas Piggin 3 weeks, 2 days ago

Hi,

This series contains the DW I2C model written by Chris Rauer and
updated for the Tenstorrent Atlantis machine recently. There was
some more review comment on that submission and so we decided to
take the I2C device out of that series and work on it separately,
see here:

https://lore.kernel.org/qemu-devel/20260425131721.932250-1-joel@jms.id.au/T/#mb1ef2824c2f1f37bf4574dc1ef0fb95566c3a2f2

The big thing suggested was to move to the QEMU register API. That
is a big change and difficult to review, so I have split that and
a some smaller changes out into their own patches. I don't expect
detailed reviews on the register API patch -- it's quite mechanical
and I did attempt to verify it by diff'ing register traces. But it
would be good to make sure maintainers are happy to go that way.

Unfortunately the patch 1 was quite well reviewed and tested so
incremental changes would be preferable, but it is painful to maintain
migration compatibility across these changes.

Thanks,
Nick

Chris Rauer (1):
  hw/i2c: Add designware i2c controller

Nicholas Piggin (3):
  [RFC] hw/i2c/designware_i2c: Switch to Fifo8
  [RFC] hw/i2c/designware_i2c: Switch to QEMU register API
  [RFC] hw/i2c/designware_i2c: add SMBUS_INTR_MASK

 MAINTAINERS                     |   8 +
 hw/i2c/Kconfig                  |   5 +
 hw/i2c/designware_i2c.c         | 742 ++++++++++++++++++++++++++++++++
 hw/i2c/meson.build              |   1 +
 hw/i2c/trace-events             |   4 +
 include/hw/i2c/designware_i2c.h |  56 +++
 roms/seabios-hppa               |   2 +-
 7 files changed, 817 insertions(+), 1 deletion(-)
 create mode 100644 hw/i2c/designware_i2c.c
 create mode 100644 include/hw/i2c/designware_i2c.h

-- 
2.53.0

Re: [PATCH 0/4] hw/i2c: Add designware i2c controller

Posted by Corey Minyard 2 weeks, 6 days ago

On Thu, May 07, 2026 at 10:05:18PM +1000, Nicholas Piggin wrote:
> Hi,
> 
> This series contains the DW I2C model written by Chris Rauer and
> updated for the Tenstorrent Atlantis machine recently. There was
> some more review comment on that submission and so we decided to
> take the I2C device out of that series and work on it separately,
> see here:
> 
> https://lore.kernel.org/qemu-devel/20260425131721.932250-1-joel@jms.id.au/T/#mb1ef2824c2f1f37bf4574dc1ef0fb95566c3a2f2
> 
> The big thing suggested was to move to the QEMU register API. That
> is a big change and difficult to review, so I have split that and
> a some smaller changes out into their own patches. I don't expect
> detailed reviews on the register API patch -- it's quite mechanical
> and I did attempt to verify it by diff'ing register traces. But it
> would be good to make sure maintainers are happy to go that way.
> 
> Unfortunately the patch 1 was quite well reviewed and tested so
> incremental changes would be preferable, but it is painful to maintain
> migration compatibility across these changes.

I had a few comments on the first patch, but they were all fixed in
later patches.  From my review this all looks good.

Yes, please squash these as you suggested in the second patch.

Acked-by: Corey Minyard <cminyard@mvista.com>

-corey

> 
> Thanks,
> Nick
> 
> Chris Rauer (1):
>   hw/i2c: Add designware i2c controller
> 
> Nicholas Piggin (3):
>   [RFC] hw/i2c/designware_i2c: Switch to Fifo8
>   [RFC] hw/i2c/designware_i2c: Switch to QEMU register API
>   [RFC] hw/i2c/designware_i2c: add SMBUS_INTR_MASK
> 
>  MAINTAINERS                     |   8 +
>  hw/i2c/Kconfig                  |   5 +
>  hw/i2c/designware_i2c.c         | 742 ++++++++++++++++++++++++++++++++
>  hw/i2c/meson.build              |   1 +
>  hw/i2c/trace-events             |   4 +
>  include/hw/i2c/designware_i2c.h |  56 +++
>  roms/seabios-hppa               |   2 +-
>  7 files changed, 817 insertions(+), 1 deletion(-)
>  create mode 100644 hw/i2c/designware_i2c.c
>  create mode 100644 include/hw/i2c/designware_i2c.h
> 
> -- 
> 2.53.0
>

Re: [PATCH 0/4] hw/i2c: Add designware i2c controller

Posted by Nicholas Piggin 2 weeks ago

On Sun, May 10, 2026 at 08:03:53AM -0500, Corey Minyard wrote:
> On Thu, May 07, 2026 at 10:05:18PM +1000, Nicholas Piggin wrote:
> > Hi,
> > 
> > This series contains the DW I2C model written by Chris Rauer and
> > updated for the Tenstorrent Atlantis machine recently. There was
> > some more review comment on that submission and so we decided to
> > take the I2C device out of that series and work on it separately,
> > see here:
> > 
> > https://lore.kernel.org/qemu-devel/20260425131721.932250-1-joel@jms.id.au/T/#mb1ef2824c2f1f37bf4574dc1ef0fb95566c3a2f2
> > 
> > The big thing suggested was to move to the QEMU register API. That
> > is a big change and difficult to review, so I have split that and
> > a some smaller changes out into their own patches. I don't expect
> > detailed reviews on the register API patch -- it's quite mechanical
> > and I did attempt to verify it by diff'ing register traces. But it
> > would be good to make sure maintainers are happy to go that way.
> > 
> > Unfortunately the patch 1 was quite well reviewed and tested so
> > incremental changes would be preferable, but it is painful to maintain
> > migration compatibility across these changes.
> 
> I had a few comments on the first patch, but they were all fixed in
> later patches.  From my review this all looks good.
> 
> Yes, please squash these as you suggested in the second patch.
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>

Thank you for the Ack, Corey. Since everybody is happier with it now
I will squash and submit it with the next revision of the tt-atlantis
series.

Thanks,
Nick