On 1/2/24 17:24, Peter Maydell wrote:
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  hw/arm/stellaris.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)


@@ -658,8 +661,6 @@ static void stellaris_i2c_init(Object *obj)
      memory_region_init_io(&s->iomem, obj, &stellaris_i2c_ops, s,
                            "i2c", 0x1000);
      sysbus_init_mmio(sbd, &s->iomem);
-    /* ??? For now we only implement the master interface.  */

I'm not 100%, but I think this comment is a general one,
not reset specific, so it should stay in the init function.

-    stellaris_i2c_reset(s);
  }

I think that the i2c_end_transfer() should be in the
"enter" phase, and the clearing of the state fields in
"hold", and then the stellaris_i2c_update() call in "exit".

Indeed.

Though usually we don't bother to do an update in reset
if it's just "device has reset and now its outbound IRQ
line is not set", so we could alternatively just delete that.

I'll let that as an possible cleanup on top.

Thanks for the review,

Phil.

Reply via email to