Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-11 Thread Bernhard Beschow



Am 7. März 2025 19:18:34 UTC schrieb Bernhard Beschow :
>
>
>Am 4. März 2025 18:53:10 UTC schrieb Bernhard Beschow :
>>
>>
>>Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow :
>>>The implementation just allows Linux to determine date and time.
>>>
>>>Signed-off-by: Bernhard Beschow 
>>>---
>>> MAINTAINERS|   2 +
>>> hw/rtc/rs5c372.c   | 236 +
>>> tests/qtest/rs5c372-test.c |  43 +++
>>> hw/rtc/Kconfig |   5 +
>>> hw/rtc/meson.build |   1 +
>>> hw/rtc/trace-events|   4 +
>>> tests/qtest/meson.build|   1 +
>>> 7 files changed, 292 insertions(+)
>>> create mode 100644 hw/rtc/rs5c372.c
>>> create mode 100644 tests/qtest/rs5c372-test.c
>>
>>Ping for just this patch. I'd like to have it merged for 10.0.
>
>Ping^2 -- just few days left before soft freeze.

Last ping before the freeze

It would really be nice to have this device model in 10.0 since this would 
allow me to use upstream QEMU.

Thanks,
Bernhard

>
>AFAICS no open issues and I'd really like to have this RTC merged for 10.0. 
>What is holding it back?
>
>Best regards,
>Bernhard
>
>>
>>Thanks,
>>Bernhard
>>
>>>
>>>diff --git a/MAINTAINERS b/MAINTAINERS
>>>index 489e426d85..2552cfd65c 100644
>>>--- a/MAINTAINERS
>>>+++ b/MAINTAINERS
>>>@@ -828,10 +828,12 @@ F: hw/arm/imx8mp-evk.c
>>> F: hw/arm/fsl-imx8mp.c
>>> F: hw/misc/imx8mp_*.c
>>> F: hw/pci-host/fsl_imx8m_phy.c
>>>+F: hw/rtc/rs5c372.c
>>> F: include/hw/arm/fsl-imx8mp.h
>>> F: include/hw/misc/imx8mp_*.h
>>> F: include/hw/pci-host/fsl_imx8m_phy.h
>>> F: pc-bios/imx8mp*
>>>+F: tests/qtest/rs5c372-test.c
>>> F: docs/system/arm/imx8mp-evk.rst
>>> 
>>> MPS2 / MPS3
>>>diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
>>>new file mode 100644
>>>index 00..5542f74085
>>>--- /dev/null
>>>+++ b/hw/rtc/rs5c372.c
>>>@@ -0,0 +1,236 @@
>>>+/*
>>>+ * Ricoh RS5C372, R222x I2C RTC
>>>+ *
>>>+ * Copyright (c) 2025 Bernhard Beschow 
>>>+ *
>>>+ * Based on hw/rtc/ds1338.c
>>>+ *
>>>+ * SPDX-License-Identifier: GPL-2.0-or-later
>>>+ */
>>>+
>>>+#include "qemu/osdep.h"
>>>+#include "hw/i2c/i2c.h"
>>>+#include "hw/qdev-properties.h"
>>>+#include "hw/resettable.h"
>>>+#include "migration/vmstate.h"
>>>+#include "qemu/bcd.h"
>>>+#include "qom/object.h"
>>>+#include "system/rtc.h"
>>>+#include "trace.h"
>>>+
>>>+#define NVRAM_SIZE 0x10
>>>+
>>>+/* Flags definitions */
>>>+#define SECONDS_CH 0x80
>>>+#define HOURS_PM   0x20
>>>+#define CTRL2_24   0x20
>>>+
>>>+#define TYPE_RS5C372 "rs5c372"
>>>+OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
>>>+
>>>+struct RS5C372State {
>>>+I2CSlave parent_obj;
>>>+
>>>+int64_t offset;
>>>+uint8_t wday_offset;
>>>+uint8_t nvram[NVRAM_SIZE];
>>>+uint8_t ptr;
>>>+uint8_t tx_format;
>>>+bool addr_byte;
>>>+};
>>>+
>>>+static void capture_current_time(RS5C372State *s)
>>>+{
>>>+/*
>>>+ * Capture the current time into the secondary registers which will be
>>>+ * actually read by the data transfer operation.
>>>+ */
>>>+struct tm now;
>>>+qemu_get_timedate(&now, s->offset);
>>>+s->nvram[0] = to_bcd(now.tm_sec);
>>>+s->nvram[1] = to_bcd(now.tm_min);
>>>+if (s->nvram[0xf] & CTRL2_24) {
>>>+s->nvram[2] = to_bcd(now.tm_hour);
>>>+} else {
>>>+int tmp = now.tm_hour;
>>>+if (tmp % 12 == 0) {
>>>+tmp += 12;
>>>+}
>>>+if (tmp <= 12) {
>>>+s->nvram[2] = to_bcd(tmp);
>>>+} else {
>>>+s->nvram[2] = HOURS_PM | to_bcd(tmp - 12);
>>>+}
>>>+}
>>>+s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
>>>+s->nvram[4] = to_bcd(now.tm_mday);
>>>+s->nvram[5] = to_bcd(now.tm_mon + 1);
>>>+s->nvram[6] = to_bcd(now.tm_year - 100);
>>>+}
>>>+
>>>+static void inc_regptr(RS5C372State *s)
>>>+{
>>>+s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
>>>+}
>>>+
>>>+static int rs5c372_event(I2CSlave *i2c, enum i2c_event event)
>>>+{
>>>+RS5C372State *s = RS5C372(i2c);
>>>+
>>>+switch (event) {
>>>+case I2C_START_RECV:
>>>+/*
>>>+ * In h/w, capture happens on any START condition, not just a
>>>+ * START_RECV, but there is no need to actually capture on
>>>+ * START_SEND, because the guest can't get at that data
>>>+ * without going through a START_RECV which would overwrite it.
>>>+ */
>>>+capture_current_time(s);
>>>+s->ptr = 0xf;
>>>+break;
>>>+case I2C_START_SEND:
>>>+s->addr_byte = true;
>>>+break;
>>>+default:
>>>+break;
>>>+}
>>>+
>>>+return 0;
>>>+}
>>>+
>>>+static uint8_t rs5c372_recv(I2CSlave *i2c)
>>>+{
>>>+RS5C372State *s = RS5C372(i2c);
>>>+uint8_t res;
>>>+
>>>+res  = s->nvram[s->ptr];
>>>+
>>>+trace_rs5c372_recv(s->ptr, res);
>>>+
>>>+inc_regptr(s);
>>>+return res;
>>>+}
>>>+
>>>+static int rs5c372_send(I2CSlave *i2c, uint8_t data)
>>>+{
>>>+RS5C372State *s = RS5C372(i2c);
>>>+
>

Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-11 Thread Philippe Mathieu-Daudé

On 11/3/25 16:49, Corey Minyard wrote:

On Tue, Mar 11, 2025 at 10:20:03AM +0100, Philippe Mathieu-Daudé wrote:

On 11/3/25 08:34, Bernhard Beschow wrote:



Am 7. März 2025 19:18:34 UTC schrieb Bernhard Beschow :



Am 4. März 2025 18:53:10 UTC schrieb Bernhard Beschow :



Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow :

The implementation just allows Linux to determine date and time.

Signed-off-by: Bernhard Beschow 
---
MAINTAINERS|   2 +
hw/rtc/rs5c372.c   | 236 +
tests/qtest/rs5c372-test.c |  43 +++
hw/rtc/Kconfig |   5 +
hw/rtc/meson.build |   1 +
hw/rtc/trace-events|   4 +
tests/qtest/meson.build|   1 +
7 files changed, 292 insertions(+)
create mode 100644 hw/rtc/rs5c372.c
create mode 100644 tests/qtest/rs5c372-test.c


Ping for just this patch. I'd like to have it merged for 10.0.


Ping^2 -- just few days left before soft freeze.


Last ping before the freeze

It would really be nice to have this device model in 10.0 since this would 
allow me to use upstream QEMU.


Apparently I2C maintainer wasn't Cc'ed (now is):

   Corey Minyard  (maintainer:I2C and SMBus)

At a glance patch LGTM, so:
Reviewed-by: Philippe Mathieu-Daudé 


Well, it's fairly hard to read this way :-).  But this looks good.  My
only comment is on:


+#define NVRAM_SIZE 0x10
+
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_PM   0x20
+#define CTRL2_24   0x20


Those are fairly generic names; when I see things like that I worry
about conflicting with other generic names that might come into an
include later.  Not a huge deal, though.

Acked-by: Corey Minyard 


Thank you! Patch queued via my hw-misc tree then.

Regards,

Phil.



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-11 Thread Corey Minyard
On Tue, Mar 11, 2025 at 10:20:03AM +0100, Philippe Mathieu-Daudé wrote:
> On 11/3/25 08:34, Bernhard Beschow wrote:
> > 
> > 
> > Am 7. März 2025 19:18:34 UTC schrieb Bernhard Beschow :
> > > 
> > > 
> > > Am 4. März 2025 18:53:10 UTC schrieb Bernhard Beschow :
> > > > 
> > > > 
> > > > Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow 
> > > > :
> > > > > The implementation just allows Linux to determine date and time.
> > > > > 
> > > > > Signed-off-by: Bernhard Beschow 
> > > > > ---
> > > > > MAINTAINERS|   2 +
> > > > > hw/rtc/rs5c372.c   | 236 +
> > > > > tests/qtest/rs5c372-test.c |  43 +++
> > > > > hw/rtc/Kconfig |   5 +
> > > > > hw/rtc/meson.build |   1 +
> > > > > hw/rtc/trace-events|   4 +
> > > > > tests/qtest/meson.build|   1 +
> > > > > 7 files changed, 292 insertions(+)
> > > > > create mode 100644 hw/rtc/rs5c372.c
> > > > > create mode 100644 tests/qtest/rs5c372-test.c
> > > > 
> > > > Ping for just this patch. I'd like to have it merged for 10.0.
> > > 
> > > Ping^2 -- just few days left before soft freeze.
> > 
> > Last ping before the freeze
> > 
> > It would really be nice to have this device model in 10.0 since this would 
> > allow me to use upstream QEMU.
> 
> Apparently I2C maintainer wasn't Cc'ed (now is):
> 
>   Corey Minyard  (maintainer:I2C and SMBus)
> 
> At a glance patch LGTM, so:
> Reviewed-by: Philippe Mathieu-Daudé 

Well, it's fairly hard to read this way :-).  But this looks good.  My
only comment is on:

 +#define NVRAM_SIZE 0x10
 +
 +/* Flags definitions */
 +#define SECONDS_CH 0x80
 +#define HOURS_PM   0x20
 +#define CTRL2_24   0x20

Those are fairly generic names; when I see things like that I worry
about conflicting with other generic names that might come into an
include later.  Not a huge deal, though.

Acked-by: Corey Minyard 

> 
> > 
> > Thanks,
> > Bernhard
> > 
> > > 
> > > AFAICS no open issues and I'd really like to have this RTC merged for 
> > > 10.0. What is holding it back?
> > > 
> > > Best regards,
> > > Bernhard
> > > 
> > > > 
> > > > Thanks,
> > > > Bernhard
> > > > 
> > > > > 
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 489e426d85..2552cfd65c 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -828,10 +828,12 @@ F: hw/arm/imx8mp-evk.c
> > > > > F: hw/arm/fsl-imx8mp.c
> > > > > F: hw/misc/imx8mp_*.c
> > > > > F: hw/pci-host/fsl_imx8m_phy.c
> > > > > +F: hw/rtc/rs5c372.c
> > > > > F: include/hw/arm/fsl-imx8mp.h
> > > > > F: include/hw/misc/imx8mp_*.h
> > > > > F: include/hw/pci-host/fsl_imx8m_phy.h
> > > > > F: pc-bios/imx8mp*
> > > > > +F: tests/qtest/rs5c372-test.c
> > > > > F: docs/system/arm/imx8mp-evk.rst
> > > > > 
> > > > > MPS2 / MPS3
> > > > > diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
> > > > > new file mode 100644
> > > > > index 00..5542f74085
> > > > > --- /dev/null
> > > > > +++ b/hw/rtc/rs5c372.c
> > > > > @@ -0,0 +1,236 @@
> > > > > +/*
> > > > > + * Ricoh RS5C372, R222x I2C RTC
> > > > > + *
> > > > > + * Copyright (c) 2025 Bernhard Beschow 
> > > > > + *
> > > > > + * Based on hw/rtc/ds1338.c
> > > > > + *
> > > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > > + */
> > > > > +
> > > > > +#include "qemu/osdep.h"
> > > > > +#include "hw/i2c/i2c.h"
> > > > > +#include "hw/qdev-properties.h"
> > > > > +#include "hw/resettable.h"
> > > > > +#include "migration/vmstate.h"
> > > > > +#include "qemu/bcd.h"
> > > > > +#include "qom/object.h"
> > > > > +#include "system/rtc.h"
> > > > > +#include "trace.h"
> > > > > +
> > > > > +#define NVRAM_SIZE 0x10
> > > > > +
> > > > > +/* Flags definitions */
> > > > > +#define SECONDS_CH 0x80
> > > > > +#define HOURS_PM   0x20
> > > > > +#define CTRL2_24   0x20
> > > > > +
> > > > > +#define TYPE_RS5C372 "rs5c372"
> > > > > +OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
> > > > > +
> > > > > +struct RS5C372State {
> > > > > +I2CSlave parent_obj;
> > > > > +
> > > > > +int64_t offset;
> > > > > +uint8_t wday_offset;
> > > > > +uint8_t nvram[NVRAM_SIZE];
> > > > > +uint8_t ptr;
> > > > > +uint8_t tx_format;
> > > > > +bool addr_byte;
> > > > > +};
> > > > > +
> > > > > +static void capture_current_time(RS5C372State *s)
> > > > > +{
> > > > > +/*
> > > > > + * Capture the current time into the secondary registers which 
> > > > > will be
> > > > > + * actually read by the data transfer operation.
> > > > > + */
> > > > > +struct tm now;
> > > > > +qemu_get_timedate(&now, s->offset);
> > > > > +s->nvram[0] = to_bcd(now.tm_sec);
> > > > > +s->nvram[1] = to_bcd(now.tm_min);
> > > > > +if (s->nvram[0xf] & CTRL2_24) {
> > > > > +s->nvram[2] = to_bcd(now.tm_hour);
> > > > > +} else {
> > > > > +int tmp = now.tm_hour;
> > > > > +if (tmp % 12 == 0) {
> > > > > +tmp += 12;
> > > > > +}
> > > > > +  

Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-11 Thread Fabiano Rosas
Bernhard Beschow  writes:

> The implementation just allows Linux to determine date and time.
>
> Signed-off-by: Bernhard Beschow 

For qtest:

Acked-by: Fabiano Rosas 



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-11 Thread Philippe Mathieu-Daudé

On 11/3/25 08:34, Bernhard Beschow wrote:



Am 7. März 2025 19:18:34 UTC schrieb Bernhard Beschow :



Am 4. März 2025 18:53:10 UTC schrieb Bernhard Beschow :



Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow :

The implementation just allows Linux to determine date and time.

Signed-off-by: Bernhard Beschow 
---
MAINTAINERS|   2 +
hw/rtc/rs5c372.c   | 236 +
tests/qtest/rs5c372-test.c |  43 +++
hw/rtc/Kconfig |   5 +
hw/rtc/meson.build |   1 +
hw/rtc/trace-events|   4 +
tests/qtest/meson.build|   1 +
7 files changed, 292 insertions(+)
create mode 100644 hw/rtc/rs5c372.c
create mode 100644 tests/qtest/rs5c372-test.c


Ping for just this patch. I'd like to have it merged for 10.0.


Ping^2 -- just few days left before soft freeze.


Last ping before the freeze

It would really be nice to have this device model in 10.0 since this would 
allow me to use upstream QEMU.


Apparently I2C maintainer wasn't Cc'ed (now is):

  Corey Minyard  (maintainer:I2C and SMBus)

At a glance patch LGTM, so:
Reviewed-by: Philippe Mathieu-Daudé 



Thanks,
Bernhard



AFAICS no open issues and I'd really like to have this RTC merged for 10.0. 
What is holding it back?

Best regards,
Bernhard



Thanks,
Bernhard



diff --git a/MAINTAINERS b/MAINTAINERS
index 489e426d85..2552cfd65c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -828,10 +828,12 @@ F: hw/arm/imx8mp-evk.c
F: hw/arm/fsl-imx8mp.c
F: hw/misc/imx8mp_*.c
F: hw/pci-host/fsl_imx8m_phy.c
+F: hw/rtc/rs5c372.c
F: include/hw/arm/fsl-imx8mp.h
F: include/hw/misc/imx8mp_*.h
F: include/hw/pci-host/fsl_imx8m_phy.h
F: pc-bios/imx8mp*
+F: tests/qtest/rs5c372-test.c
F: docs/system/arm/imx8mp-evk.rst

MPS2 / MPS3
diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
new file mode 100644
index 00..5542f74085
--- /dev/null
+++ b/hw/rtc/rs5c372.c
@@ -0,0 +1,236 @@
+/*
+ * Ricoh RS5C372, R222x I2C RTC
+ *
+ * Copyright (c) 2025 Bernhard Beschow 
+ *
+ * Based on hw/rtc/ds1338.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/i2c.h"
+#include "hw/qdev-properties.h"
+#include "hw/resettable.h"
+#include "migration/vmstate.h"
+#include "qemu/bcd.h"
+#include "qom/object.h"
+#include "system/rtc.h"
+#include "trace.h"
+
+#define NVRAM_SIZE 0x10
+
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_PM   0x20
+#define CTRL2_24   0x20
+
+#define TYPE_RS5C372 "rs5c372"
+OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
+
+struct RS5C372State {
+I2CSlave parent_obj;
+
+int64_t offset;
+uint8_t wday_offset;
+uint8_t nvram[NVRAM_SIZE];
+uint8_t ptr;
+uint8_t tx_format;
+bool addr_byte;
+};
+
+static void capture_current_time(RS5C372State *s)
+{
+/*
+ * Capture the current time into the secondary registers which will be
+ * actually read by the data transfer operation.
+ */
+struct tm now;
+qemu_get_timedate(&now, s->offset);
+s->nvram[0] = to_bcd(now.tm_sec);
+s->nvram[1] = to_bcd(now.tm_min);
+if (s->nvram[0xf] & CTRL2_24) {
+s->nvram[2] = to_bcd(now.tm_hour);
+} else {
+int tmp = now.tm_hour;
+if (tmp % 12 == 0) {
+tmp += 12;
+}
+if (tmp <= 12) {
+s->nvram[2] = to_bcd(tmp);
+} else {
+s->nvram[2] = HOURS_PM | to_bcd(tmp - 12);
+}
+}
+s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
+s->nvram[4] = to_bcd(now.tm_mday);
+s->nvram[5] = to_bcd(now.tm_mon + 1);
+s->nvram[6] = to_bcd(now.tm_year - 100);
+}
+
+static void inc_regptr(RS5C372State *s)
+{
+s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
+}
+
+static int rs5c372_event(I2CSlave *i2c, enum i2c_event event)
+{
+RS5C372State *s = RS5C372(i2c);
+
+switch (event) {
+case I2C_START_RECV:
+/*
+ * In h/w, capture happens on any START condition, not just a
+ * START_RECV, but there is no need to actually capture on
+ * START_SEND, because the guest can't get at that data
+ * without going through a START_RECV which would overwrite it.
+ */
+capture_current_time(s);
+s->ptr = 0xf;
+break;
+case I2C_START_SEND:
+s->addr_byte = true;
+break;
+default:
+break;
+}
+
+return 0;
+}
+
+static uint8_t rs5c372_recv(I2CSlave *i2c)
+{
+RS5C372State *s = RS5C372(i2c);
+uint8_t res;
+
+res  = s->nvram[s->ptr];
+
+trace_rs5c372_recv(s->ptr, res);
+
+inc_regptr(s);
+return res;
+}
+
+static int rs5c372_send(I2CSlave *i2c, uint8_t data)
+{
+RS5C372State *s = RS5C372(i2c);
+
+if (s->addr_byte) {
+s->ptr = data >> 4;
+s->tx_format = data & 0xf;
+s->addr_byte = false;
+return 0;
+}
+
+trace_rs5c372_send(s->ptr, data);
+
+if (s->ptr < 7) {
+/* Time register. */
+struct tm now;
+qemu_get_timedate(&now, 

Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-07 Thread Bernhard Beschow



Am 4. März 2025 18:53:10 UTC schrieb Bernhard Beschow :
>
>
>Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow :
>>The implementation just allows Linux to determine date and time.
>>
>>Signed-off-by: Bernhard Beschow 
>>---
>> MAINTAINERS|   2 +
>> hw/rtc/rs5c372.c   | 236 +
>> tests/qtest/rs5c372-test.c |  43 +++
>> hw/rtc/Kconfig |   5 +
>> hw/rtc/meson.build |   1 +
>> hw/rtc/trace-events|   4 +
>> tests/qtest/meson.build|   1 +
>> 7 files changed, 292 insertions(+)
>> create mode 100644 hw/rtc/rs5c372.c
>> create mode 100644 tests/qtest/rs5c372-test.c
>
>Ping for just this patch. I'd like to have it merged for 10.0.

Ping^2 -- just few days left before soft freeze.

AFAICS no open issues and I'd really like to have this RTC merged for 10.0. 
What is holding it back?

Best regards,
Bernhard

>
>Thanks,
>Bernhard
>
>>
>>diff --git a/MAINTAINERS b/MAINTAINERS
>>index 489e426d85..2552cfd65c 100644
>>--- a/MAINTAINERS
>>+++ b/MAINTAINERS
>>@@ -828,10 +828,12 @@ F: hw/arm/imx8mp-evk.c
>> F: hw/arm/fsl-imx8mp.c
>> F: hw/misc/imx8mp_*.c
>> F: hw/pci-host/fsl_imx8m_phy.c
>>+F: hw/rtc/rs5c372.c
>> F: include/hw/arm/fsl-imx8mp.h
>> F: include/hw/misc/imx8mp_*.h
>> F: include/hw/pci-host/fsl_imx8m_phy.h
>> F: pc-bios/imx8mp*
>>+F: tests/qtest/rs5c372-test.c
>> F: docs/system/arm/imx8mp-evk.rst
>> 
>> MPS2 / MPS3
>>diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
>>new file mode 100644
>>index 00..5542f74085
>>--- /dev/null
>>+++ b/hw/rtc/rs5c372.c
>>@@ -0,0 +1,236 @@
>>+/*
>>+ * Ricoh RS5C372, R222x I2C RTC
>>+ *
>>+ * Copyright (c) 2025 Bernhard Beschow 
>>+ *
>>+ * Based on hw/rtc/ds1338.c
>>+ *
>>+ * SPDX-License-Identifier: GPL-2.0-or-later
>>+ */
>>+
>>+#include "qemu/osdep.h"
>>+#include "hw/i2c/i2c.h"
>>+#include "hw/qdev-properties.h"
>>+#include "hw/resettable.h"
>>+#include "migration/vmstate.h"
>>+#include "qemu/bcd.h"
>>+#include "qom/object.h"
>>+#include "system/rtc.h"
>>+#include "trace.h"
>>+
>>+#define NVRAM_SIZE 0x10
>>+
>>+/* Flags definitions */
>>+#define SECONDS_CH 0x80
>>+#define HOURS_PM   0x20
>>+#define CTRL2_24   0x20
>>+
>>+#define TYPE_RS5C372 "rs5c372"
>>+OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
>>+
>>+struct RS5C372State {
>>+I2CSlave parent_obj;
>>+
>>+int64_t offset;
>>+uint8_t wday_offset;
>>+uint8_t nvram[NVRAM_SIZE];
>>+uint8_t ptr;
>>+uint8_t tx_format;
>>+bool addr_byte;
>>+};
>>+
>>+static void capture_current_time(RS5C372State *s)
>>+{
>>+/*
>>+ * Capture the current time into the secondary registers which will be
>>+ * actually read by the data transfer operation.
>>+ */
>>+struct tm now;
>>+qemu_get_timedate(&now, s->offset);
>>+s->nvram[0] = to_bcd(now.tm_sec);
>>+s->nvram[1] = to_bcd(now.tm_min);
>>+if (s->nvram[0xf] & CTRL2_24) {
>>+s->nvram[2] = to_bcd(now.tm_hour);
>>+} else {
>>+int tmp = now.tm_hour;
>>+if (tmp % 12 == 0) {
>>+tmp += 12;
>>+}
>>+if (tmp <= 12) {
>>+s->nvram[2] = to_bcd(tmp);
>>+} else {
>>+s->nvram[2] = HOURS_PM | to_bcd(tmp - 12);
>>+}
>>+}
>>+s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
>>+s->nvram[4] = to_bcd(now.tm_mday);
>>+s->nvram[5] = to_bcd(now.tm_mon + 1);
>>+s->nvram[6] = to_bcd(now.tm_year - 100);
>>+}
>>+
>>+static void inc_regptr(RS5C372State *s)
>>+{
>>+s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
>>+}
>>+
>>+static int rs5c372_event(I2CSlave *i2c, enum i2c_event event)
>>+{
>>+RS5C372State *s = RS5C372(i2c);
>>+
>>+switch (event) {
>>+case I2C_START_RECV:
>>+/*
>>+ * In h/w, capture happens on any START condition, not just a
>>+ * START_RECV, but there is no need to actually capture on
>>+ * START_SEND, because the guest can't get at that data
>>+ * without going through a START_RECV which would overwrite it.
>>+ */
>>+capture_current_time(s);
>>+s->ptr = 0xf;
>>+break;
>>+case I2C_START_SEND:
>>+s->addr_byte = true;
>>+break;
>>+default:
>>+break;
>>+}
>>+
>>+return 0;
>>+}
>>+
>>+static uint8_t rs5c372_recv(I2CSlave *i2c)
>>+{
>>+RS5C372State *s = RS5C372(i2c);
>>+uint8_t res;
>>+
>>+res  = s->nvram[s->ptr];
>>+
>>+trace_rs5c372_recv(s->ptr, res);
>>+
>>+inc_regptr(s);
>>+return res;
>>+}
>>+
>>+static int rs5c372_send(I2CSlave *i2c, uint8_t data)
>>+{
>>+RS5C372State *s = RS5C372(i2c);
>>+
>>+if (s->addr_byte) {
>>+s->ptr = data >> 4;
>>+s->tx_format = data & 0xf;
>>+s->addr_byte = false;
>>+return 0;
>>+}
>>+
>>+trace_rs5c372_send(s->ptr, data);
>>+
>>+if (s->ptr < 7) {
>>+/* Time register. */
>>+struct tm now;
>>+qemu_get_timedate(&now, s->offset);
>>+switch (s->ptr) {
>>+case 0:
>>+  

Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-03-04 Thread Bernhard Beschow



Am 23. Februar 2025 11:47:08 UTC schrieb Bernhard Beschow :
>The implementation just allows Linux to determine date and time.
>
>Signed-off-by: Bernhard Beschow 
>---
> MAINTAINERS|   2 +
> hw/rtc/rs5c372.c   | 236 +
> tests/qtest/rs5c372-test.c |  43 +++
> hw/rtc/Kconfig |   5 +
> hw/rtc/meson.build |   1 +
> hw/rtc/trace-events|   4 +
> tests/qtest/meson.build|   1 +
> 7 files changed, 292 insertions(+)
> create mode 100644 hw/rtc/rs5c372.c
> create mode 100644 tests/qtest/rs5c372-test.c

Ping for just this patch. I'd like to have it merged for 10.0.

Thanks,
Bernhard

>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 489e426d85..2552cfd65c 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -828,10 +828,12 @@ F: hw/arm/imx8mp-evk.c
> F: hw/arm/fsl-imx8mp.c
> F: hw/misc/imx8mp_*.c
> F: hw/pci-host/fsl_imx8m_phy.c
>+F: hw/rtc/rs5c372.c
> F: include/hw/arm/fsl-imx8mp.h
> F: include/hw/misc/imx8mp_*.h
> F: include/hw/pci-host/fsl_imx8m_phy.h
> F: pc-bios/imx8mp*
>+F: tests/qtest/rs5c372-test.c
> F: docs/system/arm/imx8mp-evk.rst
> 
> MPS2 / MPS3
>diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
>new file mode 100644
>index 00..5542f74085
>--- /dev/null
>+++ b/hw/rtc/rs5c372.c
>@@ -0,0 +1,236 @@
>+/*
>+ * Ricoh RS5C372, R222x I2C RTC
>+ *
>+ * Copyright (c) 2025 Bernhard Beschow 
>+ *
>+ * Based on hw/rtc/ds1338.c
>+ *
>+ * SPDX-License-Identifier: GPL-2.0-or-later
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "hw/i2c/i2c.h"
>+#include "hw/qdev-properties.h"
>+#include "hw/resettable.h"
>+#include "migration/vmstate.h"
>+#include "qemu/bcd.h"
>+#include "qom/object.h"
>+#include "system/rtc.h"
>+#include "trace.h"
>+
>+#define NVRAM_SIZE 0x10
>+
>+/* Flags definitions */
>+#define SECONDS_CH 0x80
>+#define HOURS_PM   0x20
>+#define CTRL2_24   0x20
>+
>+#define TYPE_RS5C372 "rs5c372"
>+OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
>+
>+struct RS5C372State {
>+I2CSlave parent_obj;
>+
>+int64_t offset;
>+uint8_t wday_offset;
>+uint8_t nvram[NVRAM_SIZE];
>+uint8_t ptr;
>+uint8_t tx_format;
>+bool addr_byte;
>+};
>+
>+static void capture_current_time(RS5C372State *s)
>+{
>+/*
>+ * Capture the current time into the secondary registers which will be
>+ * actually read by the data transfer operation.
>+ */
>+struct tm now;
>+qemu_get_timedate(&now, s->offset);
>+s->nvram[0] = to_bcd(now.tm_sec);
>+s->nvram[1] = to_bcd(now.tm_min);
>+if (s->nvram[0xf] & CTRL2_24) {
>+s->nvram[2] = to_bcd(now.tm_hour);
>+} else {
>+int tmp = now.tm_hour;
>+if (tmp % 12 == 0) {
>+tmp += 12;
>+}
>+if (tmp <= 12) {
>+s->nvram[2] = to_bcd(tmp);
>+} else {
>+s->nvram[2] = HOURS_PM | to_bcd(tmp - 12);
>+}
>+}
>+s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
>+s->nvram[4] = to_bcd(now.tm_mday);
>+s->nvram[5] = to_bcd(now.tm_mon + 1);
>+s->nvram[6] = to_bcd(now.tm_year - 100);
>+}
>+
>+static void inc_regptr(RS5C372State *s)
>+{
>+s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
>+}
>+
>+static int rs5c372_event(I2CSlave *i2c, enum i2c_event event)
>+{
>+RS5C372State *s = RS5C372(i2c);
>+
>+switch (event) {
>+case I2C_START_RECV:
>+/*
>+ * In h/w, capture happens on any START condition, not just a
>+ * START_RECV, but there is no need to actually capture on
>+ * START_SEND, because the guest can't get at that data
>+ * without going through a START_RECV which would overwrite it.
>+ */
>+capture_current_time(s);
>+s->ptr = 0xf;
>+break;
>+case I2C_START_SEND:
>+s->addr_byte = true;
>+break;
>+default:
>+break;
>+}
>+
>+return 0;
>+}
>+
>+static uint8_t rs5c372_recv(I2CSlave *i2c)
>+{
>+RS5C372State *s = RS5C372(i2c);
>+uint8_t res;
>+
>+res  = s->nvram[s->ptr];
>+
>+trace_rs5c372_recv(s->ptr, res);
>+
>+inc_regptr(s);
>+return res;
>+}
>+
>+static int rs5c372_send(I2CSlave *i2c, uint8_t data)
>+{
>+RS5C372State *s = RS5C372(i2c);
>+
>+if (s->addr_byte) {
>+s->ptr = data >> 4;
>+s->tx_format = data & 0xf;
>+s->addr_byte = false;
>+return 0;
>+}
>+
>+trace_rs5c372_send(s->ptr, data);
>+
>+if (s->ptr < 7) {
>+/* Time register. */
>+struct tm now;
>+qemu_get_timedate(&now, s->offset);
>+switch (s->ptr) {
>+case 0:
>+now.tm_sec = from_bcd(data & 0x7f);
>+break;
>+case 1:
>+now.tm_min = from_bcd(data & 0x7f);
>+break;
>+case 2:
>+if (s->nvram[0xf] & CTRL2_24) {
>+now.tm_hour = from_bcd(data & 0x3f);
>+} else {
>+int tmp = from_bcd(data & (HOURS_PM - 1));
>+if (data & HOURS_PM) {
>+  

Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-02-23 Thread Bernhard Beschow



Am 10. Februar 2025 22:48:24 UTC schrieb Bernhard Beschow :
>
>
>Am 10. Februar 2025 14:26:00 UTC schrieb "Philippe Mathieu-Daudé" 
>:
>>On 6/2/25 22:58, Bernhard Beschow wrote:
>>> 
>>> 
>>> Am 6. Februar 2025 17:32:31 UTC schrieb Peter Maydell 
>>> :
 On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
> 
> The implementation just allows Linux to determine date and time.
> 
> Signed-off-by: Bernhard Beschow 
> ---
>   MAINTAINERS |   1 +
>   hw/rtc/rs5c372.c| 227 
>   hw/rtc/Kconfig  |   5 +
>   hw/rtc/meson.build  |   1 +
>   hw/rtc/trace-events |   4 +
>   5 files changed, 238 insertions(+)
>   create mode 100644 hw/rtc/rs5c372.c
 
 Should there be a patch after this one that adds this device
 to your board ?
>>> 
>>> As per Kconfig the board selects I2C_DEVICES and this device is "default y 
>>> if I2C_DEVICES". I've deliberately not hardcoded this device to the board 
>>> to make it emulate a plain i.MX 8M Plus SoC (see also board documentation).
>>
>>Then maybe add a test to be sure it is not bitroting?
>
>Good idea. I haven't written a test in QEMU yet but I could certainly draw 
>some inspiration from ds1338-test.c. This may take an iteration longer but 
>won't be forgotten.

There will be a test similar to ds1338-test.c in v3.

>
>Best regards,
>Bernhard



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-02-10 Thread Bernhard Beschow



Am 10. Februar 2025 14:26:00 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 6/2/25 22:58, Bernhard Beschow wrote:
>> 
>> 
>> Am 6. Februar 2025 17:32:31 UTC schrieb Peter Maydell 
>> :
>>> On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
 
 The implementation just allows Linux to determine date and time.
 
 Signed-off-by: Bernhard Beschow 
 ---
   MAINTAINERS |   1 +
   hw/rtc/rs5c372.c| 227 
   hw/rtc/Kconfig  |   5 +
   hw/rtc/meson.build  |   1 +
   hw/rtc/trace-events |   4 +
   5 files changed, 238 insertions(+)
   create mode 100644 hw/rtc/rs5c372.c
>>> 
>>> Should there be a patch after this one that adds this device
>>> to your board ?
>> 
>> As per Kconfig the board selects I2C_DEVICES and this device is "default y 
>> if I2C_DEVICES". I've deliberately not hardcoded this device to the board to 
>> make it emulate a plain i.MX 8M Plus SoC (see also board documentation).
>
>Then maybe add a test to be sure it is not bitroting?

Good idea. I haven't written a test in QEMU yet but I could certainly draw some 
inspiration from ds1338-test.c. This may take an iteration longer but won't be 
forgotten.

Best regards,
Bernhard



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-02-10 Thread Philippe Mathieu-Daudé

On 6/2/25 22:58, Bernhard Beschow wrote:



Am 6. Februar 2025 17:32:31 UTC schrieb Peter Maydell 
:

On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:


The implementation just allows Linux to determine date and time.

Signed-off-by: Bernhard Beschow 
---
  MAINTAINERS |   1 +
  hw/rtc/rs5c372.c| 227 
  hw/rtc/Kconfig  |   5 +
  hw/rtc/meson.build  |   1 +
  hw/rtc/trace-events |   4 +
  5 files changed, 238 insertions(+)
  create mode 100644 hw/rtc/rs5c372.c


Should there be a patch after this one that adds this device
to your board ?


As per Kconfig the board selects I2C_DEVICES and this device is "default y if 
I2C_DEVICES". I've deliberately not hardcoded this device to the board to make it 
emulate a plain i.MX 8M Plus SoC (see also board documentation).


Then maybe add a test to be sure it is not bitroting?



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-02-06 Thread Bernhard Beschow



Am 6. Februar 2025 17:32:31 UTC schrieb Peter Maydell 
:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
>>
>> The implementation just allows Linux to determine date and time.
>>
>> Signed-off-by: Bernhard Beschow 
>> ---
>>  MAINTAINERS |   1 +
>>  hw/rtc/rs5c372.c| 227 
>>  hw/rtc/Kconfig  |   5 +
>>  hw/rtc/meson.build  |   1 +
>>  hw/rtc/trace-events |   4 +
>>  5 files changed, 238 insertions(+)
>>  create mode 100644 hw/rtc/rs5c372.c
>
>Should there be a patch after this one that adds this device
>to your board ?

As per Kconfig the board selects I2C_DEVICES and this device is "default y if 
I2C_DEVICES". I've deliberately not hardcoded this device to the board to make 
it emulate a plain i.MX 8M Plus SoC (see also board documentation).

Best regards,
Bernhard
 
>
>thanks
>-- PMM



Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation

2025-02-06 Thread Peter Maydell
On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  wrote:
>
> The implementation just allows Linux to determine date and time.
>
> Signed-off-by: Bernhard Beschow 
> ---
>  MAINTAINERS |   1 +
>  hw/rtc/rs5c372.c| 227 
>  hw/rtc/Kconfig  |   5 +
>  hw/rtc/meson.build  |   1 +
>  hw/rtc/trace-events |   4 +
>  5 files changed, 238 insertions(+)
>  create mode 100644 hw/rtc/rs5c372.c

Should there be a patch after this one that adds this device
to your board ?

thanks
-- PMM