Xilinx QEMU implements a TCA6416 device model which may be useful for the
broader QEMU community, so upstream it. In the Xilinx fork, the device model
gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
"hw/*/cadence_*" maintainers.
The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
modifications:
* Use OBJECT_DECLARE_SIMPLE_TYPE()
* Port from DPRINTF() to trace events
* Follow QEMU conventions for naming of state struct
* Rename type to not contain a ','
* Use DEFINE_TYPES() macro
* Implement under hw/gpio since device is i2c client and gpio provider
* Have dedicated Kconfig switch
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
--
I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
willing to maintain this device model?
---
MAINTAINERS | 1 +
hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
hw/gpio/Kconfig | 5 ++
hw/gpio/meson.build | 1 +
hw/gpio/trace-events | 4 ++
5 files changed, 133 insertions(+)
create mode 100644 hw/gpio/tca6416.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7531d65429..0cac9c90bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1030,6 +1030,7 @@ S: Maintained
F: hw/*/xilinx_*
F: hw/*/cadence_*
F: hw/misc/zynq_slcr.c
+F: hw/gpio/tca6416.c
F: hw/adc/zynq-xadc.c
F: include/hw/misc/zynq_slcr.h
F: include/hw/adc/zynq-xadc.h
diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
new file mode 100644
index 0000000000..81ed7a654d
--- /dev/null
+++ b/hw/gpio/tca6416.c
@@ -0,0 +1,122 @@
+/*
+ * QEMU model of TCA6416 16-Bit I/O Expander
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ *
+ * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "hw/i2c/i2c.h"
+#include "trace.h"
+
+#define TYPE_TCA6416 "tca6416"
+OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
+
+#define IN_PORT0 0
+#define IN_PORT1 1
+#define OUT_PORT0 2
+#define OUT_PORT1 3
+#define POL_INV0 4
+#define POL_INV1 5
+#define CONF_PORT0 6
+#define CONF_PORT1 7
+#define RMAX (CONF_PORT1 + 1)
+
+enum tca6416_events {
+ ADDR_DONE,
+ ADDRESSING,
+};
+
+struct Tca6416State {
+ I2CSlave i2c;
+
+ uint8_t addr;
+ uint8_t state;
+ uint8_t regs[RMAX];
+};
+
+static uint8_t tca6416_read(I2CSlave *i2c)
+{
+ Tca6416State *s = TCA6416(i2c);
+ uint8_t ret;
+
+ ret = s->regs[s->addr];
+ trace_tca6416_read(ret);
+ return ret;
+}
+
+static int tca6416_write(I2CSlave *i2c, uint8_t data)
+{
+ Tca6416State *s = TCA6416(i2c);
+
+ trace_tca6416_write(data);
+ if (s->state == ADDRESSING) {
+ s->addr = data;
+ } else {
+ s->regs[s->addr] = data;
+ }
+
+ return 0;
+}
+
+static void tca6416_realize(DeviceState *dev, Error **errp)
+{
+ Tca6416State *s = TCA6416(dev);
+
+ s->regs[CONF_PORT0] = 0xFF;
+ s->regs[CONF_PORT1] = 0xFF;
+}
+
+static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
+{
+ Tca6416State *s = TCA6416(i2c);
+
+ switch (event) {
+ case I2C_START_SEND:
+ s->state = ADDRESSING;
+ break;
+ default:
+ s->state = ADDR_DONE;
+ };
+ return 0;
+}
+
+static void tca6416_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+
+ dc->realize = tca6416_realize;
+ k->recv = tca6416_read;
+ k->send = tca6416_write;
+ k->event = tca6416_event;
+}
+
+static const TypeInfo tca6416_types[] = {
+ {
+ .name = TYPE_TCA6416,
+ .parent = TYPE_I2C_SLAVE,
+ .class_init = tca6416_class_init,
+ .instance_size = sizeof(Tca6416State),
+ }
+};
+
+DEFINE_TYPES(tca6416_types)
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index c423e10f59..a240cf2de2 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -20,5 +20,10 @@ config PCF8574
bool
depends on I2C
+config TCA6416
+ bool
+ depends on I2C
+ default y if I2C_DEVICES
+
config ZAURUS_SCOOP
bool
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 74840619c0..b3ff7c7460 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
+system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index cea896b28f..6724f2efb8 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
+
+# tca6416.c
+tca6416_write(uint8_t value) "0x%02x"
+tca6416_read(uint8_t value) "0x%02x"
--
2.48.1Cc'ing AMD folks
Hi Bernhard,
TL;DR; can't you use the PCF8574 which is a more complete model of I/O
expander? (See hw/gpio/pcf8574.c)
On 20/1/25 21:37, Bernhard Beschow wrote:
> Xilinx QEMU implements a TCA6416 device model which may be useful for the
> broader QEMU community, so upstream it. In the Xilinx fork, the device model
> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
> "hw/*/cadence_*" maintainers.
>
> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
> modifications:
> * Use OBJECT_DECLARE_SIMPLE_TYPE()
> * Port from DPRINTF() to trace events
> * Follow QEMU conventions for naming of state struct
> * Rename type to not contain a ','
> * Use DEFINE_TYPES() macro
> * Implement under hw/gpio since device is i2c client and gpio provider
> * Have dedicated Kconfig switch
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
> --
> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
> willing to maintain this device model?
> ---
> MAINTAINERS | 1 +
> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
> hw/gpio/Kconfig | 5 ++
> hw/gpio/meson.build | 1 +
> hw/gpio/trace-events | 4 ++
> 5 files changed, 133 insertions(+)
> create mode 100644 hw/gpio/tca6416.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7531d65429..0cac9c90bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1030,6 +1030,7 @@ S: Maintained
> F: hw/*/xilinx_*
> F: hw/*/cadence_*
> F: hw/misc/zynq_slcr.c
> +F: hw/gpio/tca6416.c
> F: hw/adc/zynq-xadc.c
> F: include/hw/misc/zynq_slcr.h
> F: include/hw/adc/zynq-xadc.h
> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
> new file mode 100644
> index 0000000000..81ed7a654d
> --- /dev/null
> +++ b/hw/gpio/tca6416.c
> @@ -0,0 +1,122 @@
> +/*
> + * QEMU model of TCA6416 16-Bit I/O Expander
Please add datasheet reference.
> + *
> + * Copyright (c) 2018 Xilinx Inc.
> + *
> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
Sai/Edgar/Francisco, should we update to AMD here? Remove or update the
email address? Also, can you Ack the replacement of this license by a
SPDX tag?
> + */
> +#include "qemu/osdep.h"
> +#include "hw/i2c/i2c.h"
> +#include "trace.h"
> +
> +#define TYPE_TCA6416 "tca6416"
> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
> +
> +#define IN_PORT0 0
> +#define IN_PORT1 1
> +#define OUT_PORT0 2
> +#define OUT_PORT1 3
> +#define POL_INV0 4
> +#define POL_INV1 5
> +#define CONF_PORT0 6
> +#define CONF_PORT1 7
enum up to here?
> +#define RMAX (CONF_PORT1 + 1)
> +
> +enum tca6416_events {
> + ADDR_DONE,
> + ADDRESSING,
> +};
> +
> +struct Tca6416State {
> + I2CSlave i2c;
> +
> + uint8_t addr;
> + uint8_t state;
> + uint8_t regs[RMAX];
> +};
> +
> +static uint8_t tca6416_read(I2CSlave *i2c)
> +{
> + Tca6416State *s = TCA6416(i2c);
> + uint8_t ret;
> +
> + ret = s->regs[s->addr];
> + trace_tca6416_read(ret);
> + return ret;
> +}
> +
> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + trace_tca6416_write(data);
> + if (s->state == ADDRESSING) {
> + s->addr = data;
I suppose HW masks here.
s->addr = data & 0xf;
> + } else {
> + s->regs[s->addr] = data;
(otherwise this could overflow).
So this device isn't doing anything actually (I'd have
expected 1 IRQ and 16 GPIO lines). What is the point,
at least in this state?
> + }
> +
> + return 0;
> +}
> +
> +static void tca6416_realize(DeviceState *dev, Error **errp)
> +{
> + Tca6416State *s = TCA6416(dev);
> +
Missing:
s->regs[OUT_PORT0] = 0xff;
s->regs[OUT_PORT1] = 0xff;
s->regs[POL_INV0] = 0x00;
s->regs[POL_INV1] = 0x00;
> + s->regs[CONF_PORT0] = 0xFF;
> + s->regs[CONF_PORT1] = 0xFF;
Style is 0xff.
That said, this is not a REALIZE but RESET handler.
> +}
> +
> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + switch (event) {
> + case I2C_START_SEND:
> + s->state = ADDRESSING;
> + break;
> + default:
> + s->state = ADDR_DONE;
> + };
> + return 0;
> +}
> +
> +static void tca6416_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> + dc->realize = tca6416_realize;
(tca6416_realize is a reset handler).
Please complete:
dc->desc = "TCA6416 I/O Expander";
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
No migration state?
> + k->recv = tca6416_read;
> + k->send = tca6416_write;
> + k->event = tca6416_event;
> +}> +
> +static const TypeInfo tca6416_types[] = {
> + {
> + .name = TYPE_TCA6416,
> + .parent = TYPE_I2C_SLAVE,
> + .class_init = tca6416_class_init,
> + .instance_size = sizeof(Tca6416State),
> + }
> +};
> +
> +DEFINE_TYPES(tca6416_types)
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index c423e10f59..a240cf2de2 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -20,5 +20,10 @@ config PCF8574
> bool
> depends on I2C
>
> +config TCA6416
> + bool
> + depends on I2C
> + default y if I2C_DEVICES
> +
> config ZAURUS_SCOOP
> bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 74840619c0..b3ff7c7460 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
> index cea896b28f..6724f2efb8 100644
> --- a/hw/gpio/trace-events
> +++ b/hw/gpio/trace-events
> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
> +
> +# tca6416.c
> +tca6416_write(uint8_t value) "0x%02x"
"wr 0x%02x"
> +tca6416_read(uint8_t value) "0x%02x"
"rd ..."
Regards,
Phil.
Am 30. Januar 2025 23:05:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Cc'ing AMD folks
>
>Hi Bernhard,
>
>TL;DR; can't you use the PCF8574 which is a more complete model of I/O
>expander? (See hw/gpio/pcf8574.c)
If it is software-compatible then I could use it. I'm modeling a real board whose device tree I'd like to use unchanged, so I don't have much choice. The only reason I need it is because its absence clogs the i2c bus.
Best regards,
Bernhard
>
>On 20/1/25 21:37, Bernhard Beschow wrote:
>> Xilinx QEMU implements a TCA6416 device model which may be useful for the
>> broader QEMU community, so upstream it. In the Xilinx fork, the device model
>> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
>> "hw/*/cadence_*" maintainers.
>>
>> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
>> modifications:
>> * Use OBJECT_DECLARE_SIMPLE_TYPE()
>> * Port from DPRINTF() to trace events
>> * Follow QEMU conventions for naming of state struct
>> * Rename type to not contain a ','
>> * Use DEFINE_TYPES() macro
>> * Implement under hw/gpio since device is i2c client and gpio provider
>> * Have dedicated Kconfig switch
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>
>> --
>> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
>> willing to maintain this device model?
>> ---
>> MAINTAINERS | 1 +
>> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>> hw/gpio/Kconfig | 5 ++
>> hw/gpio/meson.build | 1 +
>> hw/gpio/trace-events | 4 ++
>> 5 files changed, 133 insertions(+)
>> create mode 100644 hw/gpio/tca6416.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7531d65429..0cac9c90bc 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1030,6 +1030,7 @@ S: Maintained
>> F: hw/*/xilinx_*
>> F: hw/*/cadence_*
>> F: hw/misc/zynq_slcr.c
>> +F: hw/gpio/tca6416.c
>> F: hw/adc/zynq-xadc.c
>> F: include/hw/misc/zynq_slcr.h
>> F: include/hw/adc/zynq-xadc.h
>> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
>> new file mode 100644
>> index 0000000000..81ed7a654d
>> --- /dev/null
>> +++ b/hw/gpio/tca6416.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * QEMU model of TCA6416 16-Bit I/O Expander
>
>Please add datasheet reference.
>
>> + *
>> + * Copyright (c) 2018 Xilinx Inc.
>> + *
>> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>
>Sai/Edgar/Francisco, should we update to AMD here? Remove or update the
>email address? Also, can you Ack the replacement of this license by a
>SPDX tag?
>
>> + */
>> +#include "qemu/osdep.h"
>> +#include "hw/i2c/i2c.h"
>> +#include "trace.h"
>> +
>> +#define TYPE_TCA6416 "tca6416"
>> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
>> +
>> +#define IN_PORT0 0
>> +#define IN_PORT1 1
>> +#define OUT_PORT0 2
>> +#define OUT_PORT1 3
>> +#define POL_INV0 4
>> +#define POL_INV1 5
>> +#define CONF_PORT0 6
>> +#define CONF_PORT1 7
>
>enum up to here?
>
>> +#define RMAX (CONF_PORT1 + 1)
>> +
>> +enum tca6416_events {
>> + ADDR_DONE,
>> + ADDRESSING,
>> +};
>> +
>> +struct Tca6416State {
>> + I2CSlave i2c;
>> +
>> + uint8_t addr;
>> + uint8_t state;
>> + uint8_t regs[RMAX];
>> +};
>> +
>> +static uint8_t tca6416_read(I2CSlave *i2c)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> + uint8_t ret;
>> +
>> + ret = s->regs[s->addr];
>> + trace_tca6416_read(ret);
>> + return ret;
>> +}
>> +
>> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> +
>> + trace_tca6416_write(data);
>> + if (s->state == ADDRESSING) {
>> + s->addr = data;
>
>I suppose HW masks here.
>
> s->addr = data & 0xf;
>
>> + } else {
>> + s->regs[s->addr] = data;
>
>(otherwise this could overflow).
>
>So this device isn't doing anything actually (I'd have
>expected 1 IRQ and 16 GPIO lines). What is the point,
>at least in this state?
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tca6416_realize(DeviceState *dev, Error **errp)
>> +{
>> + Tca6416State *s = TCA6416(dev);
>> +
>
>Missing:
>
> s->regs[OUT_PORT0] = 0xff;
> s->regs[OUT_PORT1] = 0xff;
> s->regs[POL_INV0] = 0x00;
> s->regs[POL_INV1] = 0x00;
>
>> + s->regs[CONF_PORT0] = 0xFF;
>> + s->regs[CONF_PORT1] = 0xFF;
>
>Style is 0xff.
>
>That said, this is not a REALIZE but RESET handler.
>
>> +}
>> +
>> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> +
>> + switch (event) {
>> + case I2C_START_SEND:
>> + s->state = ADDRESSING;
>> + break;
>> + default:
>> + s->state = ADDR_DONE;
>> + };
>> + return 0;
>> +}
>> +
>> +static void tca6416_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>> +
>> + dc->realize = tca6416_realize;
>
>(tca6416_realize is a reset handler).
>
>Please complete:
>
> dc->desc = "TCA6416 I/O Expander";
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>
>No migration state?
>
>> + k->recv = tca6416_read;
>> + k->send = tca6416_write;
>> + k->event = tca6416_event;
>> +}> +
>> +static const TypeInfo tca6416_types[] = {
>> + {
>> + .name = TYPE_TCA6416,
>> + .parent = TYPE_I2C_SLAVE,
>> + .class_init = tca6416_class_init,
>> + .instance_size = sizeof(Tca6416State),
>> + }
>> +};
>> +
>> +DEFINE_TYPES(tca6416_types)
>> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
>> index c423e10f59..a240cf2de2 100644
>> --- a/hw/gpio/Kconfig
>> +++ b/hw/gpio/Kconfig
>> @@ -20,5 +20,10 @@ config PCF8574
>> bool
>> depends on I2C
>> +config TCA6416
>> + bool
>> + depends on I2C
>> + default y if I2C_DEVICES
>> +
>> config ZAURUS_SCOOP
>> bool
>> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
>> index 74840619c0..b3ff7c7460 100644
>> --- a/hw/gpio/meson.build
>> +++ b/hw/gpio/meson.build
>> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
>> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
>> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
>> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>> index cea896b28f..6724f2efb8 100644
>> --- a/hw/gpio/trace-events
>> +++ b/hw/gpio/trace-events
>> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
>> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
>> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
>> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
>> +
>> +# tca6416.c
>> +tca6416_write(uint8_t value) "0x%02x"
>
>"wr 0x%02x"
>
>> +tca6416_read(uint8_t value) "0x%02x"
>
>"rd ..."
>
>Regards,
>
>Phil.
On 1/2/25 16:28, Bernhard Beschow wrote:
>
>
> Am 30. Januar 2025 23:05:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Cc'ing AMD folks
>>
>> Hi Bernhard,
>>
>> TL;DR; can't you use the PCF8574 which is a more complete model of I/O
>> expander? (See hw/gpio/pcf8574.c)
>
> If it is software-compatible then I could use it. I'm modeling a real board whose device tree I'd like to use unchanged, so I don't have much choice. The only reason I need it is because its absence clogs the i2c bus.
No clue about compatibility. If you unfortunately need to add it,
then please address my comments in the next version.
>
> Best regards,
> Bernhard
>
>>
>> On 20/1/25 21:37, Bernhard Beschow wrote:
>>> Xilinx QEMU implements a TCA6416 device model which may be useful for the
>>> broader QEMU community, so upstream it. In the Xilinx fork, the device model
>>> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
>>> "hw/*/cadence_*" maintainers.
>>>
>>> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
>>> modifications:
>>> * Use OBJECT_DECLARE_SIMPLE_TYPE()
>>> * Port from DPRINTF() to trace events
>>> * Follow QEMU conventions for naming of state struct
>>> * Rename type to not contain a ','
>>> * Use DEFINE_TYPES() macro
>>> * Implement under hw/gpio since device is i2c client and gpio provider
>>> * Have dedicated Kconfig switch
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>
>>> --
>>> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
>>> willing to maintain this device model?
>>> ---
>>> MAINTAINERS | 1 +
>>> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>>> hw/gpio/Kconfig | 5 ++
>>> hw/gpio/meson.build | 1 +
>>> hw/gpio/trace-events | 4 ++
>>> 5 files changed, 133 insertions(+)
>>> create mode 100644 hw/gpio/tca6416.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 7531d65429..0cac9c90bc 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1030,6 +1030,7 @@ S: Maintained
>>> F: hw/*/xilinx_*
>>> F: hw/*/cadence_*
>>> F: hw/misc/zynq_slcr.c
>>> +F: hw/gpio/tca6416.c
>>> F: hw/adc/zynq-xadc.c
>>> F: include/hw/misc/zynq_slcr.h
>>> F: include/hw/adc/zynq-xadc.h
>>> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
>>> new file mode 100644
>>> index 0000000000..81ed7a654d
>>> --- /dev/null
>>> +++ b/hw/gpio/tca6416.c
>>> @@ -0,0 +1,122 @@
>>> +/*
>>> + * QEMU model of TCA6416 16-Bit I/O Expander
>>
>> Please add datasheet reference.
>>
>>> + *
>>> + * Copyright (c) 2018 Xilinx Inc.
>>> + *
>>> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>>> + * of this software and associated documentation files (the "Software"), to deal
>>> + * in the Software without restriction, including without limitation the rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be included in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>>> + * THE SOFTWARE.
>>
>> Sai/Edgar/Francisco, should we update to AMD here? Remove or update the
>> email address? Also, can you Ack the replacement of this license by a
>> SPDX tag?
>>
>>> + */
>>> +#include "qemu/osdep.h"
>>> +#include "hw/i2c/i2c.h"
>>> +#include "trace.h"
>>> +
>>> +#define TYPE_TCA6416 "tca6416"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
>>> +
>>> +#define IN_PORT0 0
>>> +#define IN_PORT1 1
>>> +#define OUT_PORT0 2
>>> +#define OUT_PORT1 3
>>> +#define POL_INV0 4
>>> +#define POL_INV1 5
>>> +#define CONF_PORT0 6
>>> +#define CONF_PORT1 7
>>
>> enum up to here?
>>
>>> +#define RMAX (CONF_PORT1 + 1)
>>> +
>>> +enum tca6416_events {
>>> + ADDR_DONE,
>>> + ADDRESSING,
>>> +};
>>> +
>>> +struct Tca6416State {
>>> + I2CSlave i2c;
>>> +
>>> + uint8_t addr;
>>> + uint8_t state;
>>> + uint8_t regs[RMAX];
>>> +};
>>> +
>>> +static uint8_t tca6416_read(I2CSlave *i2c)
>>> +{
>>> + Tca6416State *s = TCA6416(i2c);
>>> + uint8_t ret;
>>> +
>>> + ret = s->regs[s->addr];
>>> + trace_tca6416_read(ret);
>>> + return ret;
>>> +}
>>> +
>>> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
>>> +{
>>> + Tca6416State *s = TCA6416(i2c);
>>> +
>>> + trace_tca6416_write(data);
>>> + if (s->state == ADDRESSING) {
>>> + s->addr = data;
>>
>> I suppose HW masks here.
>>
>> s->addr = data & 0xf;
>>
>>> + } else {
>>> + s->regs[s->addr] = data;
>>
>> (otherwise this could overflow).
>>
>> So this device isn't doing anything actually (I'd have
>> expected 1 IRQ and 16 GPIO lines). What is the point,
>> at least in this state?
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void tca6416_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + Tca6416State *s = TCA6416(dev);
>>> +
>>
>> Missing:
>>
>> s->regs[OUT_PORT0] = 0xff;
>> s->regs[OUT_PORT1] = 0xff;
>> s->regs[POL_INV0] = 0x00;
>> s->regs[POL_INV1] = 0x00;
>>
>>> + s->regs[CONF_PORT0] = 0xFF;
>>> + s->regs[CONF_PORT1] = 0xFF;
>>
>> Style is 0xff.
>>
>> That said, this is not a REALIZE but RESET handler.
>>
>>> +}
>>> +
>>> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
>>> +{
>>> + Tca6416State *s = TCA6416(i2c);
>>> +
>>> + switch (event) {
>>> + case I2C_START_SEND:
>>> + s->state = ADDRESSING;
>>> + break;
>>> + default:
>>> + s->state = ADDR_DONE;
>>> + };
>>> + return 0;
>>> +}
>>> +
>>> +static void tca6416_class_init(ObjectClass *klass, void *data)
>>> +{
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>>> +
>>> + dc->realize = tca6416_realize;
>>
>> (tca6416_realize is a reset handler).
>>
>> Please complete:
>>
>> dc->desc = "TCA6416 I/O Expander";
>> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>
>> No migration state?
>>
>>> + k->recv = tca6416_read;
>>> + k->send = tca6416_write;
>>> + k->event = tca6416_event;
>>> +}> +
>>> +static const TypeInfo tca6416_types[] = {
>>> + {
>>> + .name = TYPE_TCA6416,
>>> + .parent = TYPE_I2C_SLAVE,
>>> + .class_init = tca6416_class_init,
>>> + .instance_size = sizeof(Tca6416State),
>>> + }
>>> +};
>>> +
>>> +DEFINE_TYPES(tca6416_types)
>>> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
>>> index c423e10f59..a240cf2de2 100644
>>> --- a/hw/gpio/Kconfig
>>> +++ b/hw/gpio/Kconfig
>>> @@ -20,5 +20,10 @@ config PCF8574
>>> bool
>>> depends on I2C
>>> +config TCA6416
>>> + bool
>>> + depends on I2C
>>> + default y if I2C_DEVICES
>>> +
>>> config ZAURUS_SCOOP
>>> bool
>>> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
>>> index 74840619c0..b3ff7c7460 100644
>>> --- a/hw/gpio/meson.build
>>> +++ b/hw/gpio/meson.build
>>> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
>>> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>>> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
>>> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
>>> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
>>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>>> index cea896b28f..6724f2efb8 100644
>>> --- a/hw/gpio/trace-events
>>> +++ b/hw/gpio/trace-events
>>> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
>>> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
>>> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
>>> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
>>> +
>>> +# tca6416.c
>>> +tca6416_write(uint8_t value) "0x%02x"
>>
>> "wr 0x%02x"
>>
>>> +tca6416_read(uint8_t value) "0x%02x"
>>
>> "rd ..."
>>
>> Regards,
>>
>> Phil.
Am 2. Februar 2025 17:09:06 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 1/2/25 16:28, Bernhard Beschow wrote:
>>
>>
>> Am 30. Januar 2025 23:05:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> Cc'ing AMD folks
>>>
>>> Hi Bernhard,
>>>
>>> TL;DR; can't you use the PCF8574 which is a more complete model of I/O
>>> expander? (See hw/gpio/pcf8574.c)
>>
>> If it is software-compatible then I could use it. I'm modeling a real board whose device tree I'd like to use unchanged, so I don't have much choice. The only reason I need it is because its absence clogs the i2c bus.
>
>No clue about compatibility. If you unfortunately need to add it,
>then please address my comments in the next version.
Sure, I'll link your and Zoltan's comments as todos in the next version.
Best regards,
Bernhard
>
>>
>> Best regards,
>> Bernhard
>>
>>>
>>> On 20/1/25 21:37, Bernhard Beschow wrote:
>>>> Xilinx QEMU implements a TCA6416 device model which may be useful for the
>>>> broader QEMU community, so upstream it. In the Xilinx fork, the device model
>>>> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
>>>> "hw/*/cadence_*" maintainers.
>>>>
>>>> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
>>>> modifications:
>>>> * Use OBJECT_DECLARE_SIMPLE_TYPE()
>>>> * Port from DPRINTF() to trace events
>>>> * Follow QEMU conventions for naming of state struct
>>>> * Rename type to not contain a ','
>>>> * Use DEFINE_TYPES() macro
>>>> * Implement under hw/gpio since device is i2c client and gpio provider
>>>> * Have dedicated Kconfig switch
>>>>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>
>>>> --
>>>> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
>>>> willing to maintain this device model?
>>>> ---
>>>> MAINTAINERS | 1 +
>>>> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>>>> hw/gpio/Kconfig | 5 ++
>>>> hw/gpio/meson.build | 1 +
>>>> hw/gpio/trace-events | 4 ++
>>>> 5 files changed, 133 insertions(+)
>>>> create mode 100644 hw/gpio/tca6416.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 7531d65429..0cac9c90bc 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -1030,6 +1030,7 @@ S: Maintained
>>>> F: hw/*/xilinx_*
>>>> F: hw/*/cadence_*
>>>> F: hw/misc/zynq_slcr.c
>>>> +F: hw/gpio/tca6416.c
>>>> F: hw/adc/zynq-xadc.c
>>>> F: include/hw/misc/zynq_slcr.h
>>>> F: include/hw/adc/zynq-xadc.h
>>>> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
>>>> new file mode 100644
>>>> index 0000000000..81ed7a654d
>>>> --- /dev/null
>>>> +++ b/hw/gpio/tca6416.c
>>>> @@ -0,0 +1,122 @@
>>>> +/*
>>>> + * QEMU model of TCA6416 16-Bit I/O Expander
>>>
>>> Please add datasheet reference.
>>>
>>>> + *
>>>> + * Copyright (c) 2018 Xilinx Inc.
>>>> + *
>>>> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> + *
>>>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>>>> + * of this software and associated documentation files (the "Software"), to deal
>>>> + * in the Software without restriction, including without limitation the rights
>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>>>> + * copies of the Software, and to permit persons to whom the Software is
>>>> + * furnished to do so, subject to the following conditions:
>>>> + *
>>>> + * The above copyright notice and this permission notice shall be included in
>>>> + * all copies or substantial portions of the Software.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>>>> + * THE SOFTWARE.
>>>
>>> Sai/Edgar/Francisco, should we update to AMD here? Remove or update the
>>> email address? Also, can you Ack the replacement of this license by a
>>> SPDX tag?
>>>
>>>> + */
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/i2c/i2c.h"
>>>> +#include "trace.h"
>>>> +
>>>> +#define TYPE_TCA6416 "tca6416"
>>>> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
>>>> +
>>>> +#define IN_PORT0 0
>>>> +#define IN_PORT1 1
>>>> +#define OUT_PORT0 2
>>>> +#define OUT_PORT1 3
>>>> +#define POL_INV0 4
>>>> +#define POL_INV1 5
>>>> +#define CONF_PORT0 6
>>>> +#define CONF_PORT1 7
>>>
>>> enum up to here?
>>>
>>>> +#define RMAX (CONF_PORT1 + 1)
>>>> +
>>>> +enum tca6416_events {
>>>> + ADDR_DONE,
>>>> + ADDRESSING,
>>>> +};
>>>> +
>>>> +struct Tca6416State {
>>>> + I2CSlave i2c;
>>>> +
>>>> + uint8_t addr;
>>>> + uint8_t state;
>>>> + uint8_t regs[RMAX];
>>>> +};
>>>> +
>>>> +static uint8_t tca6416_read(I2CSlave *i2c)
>>>> +{
>>>> + Tca6416State *s = TCA6416(i2c);
>>>> + uint8_t ret;
>>>> +
>>>> + ret = s->regs[s->addr];
>>>> + trace_tca6416_read(ret);
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
>>>> +{
>>>> + Tca6416State *s = TCA6416(i2c);
>>>> +
>>>> + trace_tca6416_write(data);
>>>> + if (s->state == ADDRESSING) {
>>>> + s->addr = data;
>>>
>>> I suppose HW masks here.
>>>
>>> s->addr = data & 0xf;
>>>
>>>> + } else {
>>>> + s->regs[s->addr] = data;
>>>
>>> (otherwise this could overflow).
>>>
>>> So this device isn't doing anything actually (I'd have
>>> expected 1 IRQ and 16 GPIO lines). What is the point,
>>> at least in this state?
>>>
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void tca6416_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> + Tca6416State *s = TCA6416(dev);
>>>> +
>>>
>>> Missing:
>>>
>>> s->regs[OUT_PORT0] = 0xff;
>>> s->regs[OUT_PORT1] = 0xff;
>>> s->regs[POL_INV0] = 0x00;
>>> s->regs[POL_INV1] = 0x00;
>>>
>>>> + s->regs[CONF_PORT0] = 0xFF;
>>>> + s->regs[CONF_PORT1] = 0xFF;
>>>
>>> Style is 0xff.
>>>
>>> That said, this is not a REALIZE but RESET handler.
>>>
>>>> +}
>>>> +
>>>> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
>>>> +{
>>>> + Tca6416State *s = TCA6416(i2c);
>>>> +
>>>> + switch (event) {
>>>> + case I2C_START_SEND:
>>>> + s->state = ADDRESSING;
>>>> + break;
>>>> + default:
>>>> + s->state = ADDR_DONE;
>>>> + };
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void tca6416_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>>>> +
>>>> + dc->realize = tca6416_realize;
>>>
>>> (tca6416_realize is a reset handler).
>>>
>>> Please complete:
>>>
>>> dc->desc = "TCA6416 I/O Expander";
>>> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>>
>>> No migration state?
>>>
>>>> + k->recv = tca6416_read;
>>>> + k->send = tca6416_write;
>>>> + k->event = tca6416_event;
>>>> +}> +
>>>> +static const TypeInfo tca6416_types[] = {
>>>> + {
>>>> + .name = TYPE_TCA6416,
>>>> + .parent = TYPE_I2C_SLAVE,
>>>> + .class_init = tca6416_class_init,
>>>> + .instance_size = sizeof(Tca6416State),
>>>> + }
>>>> +};
>>>> +
>>>> +DEFINE_TYPES(tca6416_types)
>>>> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
>>>> index c423e10f59..a240cf2de2 100644
>>>> --- a/hw/gpio/Kconfig
>>>> +++ b/hw/gpio/Kconfig
>>>> @@ -20,5 +20,10 @@ config PCF8574
>>>> bool
>>>> depends on I2C
>>>> +config TCA6416
>>>> + bool
>>>> + depends on I2C
>>>> + default y if I2C_DEVICES
>>>> +
>>>> config ZAURUS_SCOOP
>>>> bool
>>>> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
>>>> index 74840619c0..b3ff7c7460 100644
>>>> --- a/hw/gpio/meson.build
>>>> +++ b/hw/gpio/meson.build
>>>> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
>>>> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>>>> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
>>>> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
>>>> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
>>>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>>>> index cea896b28f..6724f2efb8 100644
>>>> --- a/hw/gpio/trace-events
>>>> +++ b/hw/gpio/trace-events
>>>> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
>>>> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
>>>> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
>>>> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
>>>> +
>>>> +# tca6416.c
>>>> +tca6416_write(uint8_t value) "0x%02x"
>>>
>>> "wr 0x%02x"
>>>
>>>> +tca6416_read(uint8_t value) "0x%02x"
>>>
>>> "rd ..."
>>>
>>> Regards,
>>>
>>> Phil.
>
At Sun, 02/02/2025 at 18:09 +0100, Philippe Mathieu-Daudé writes: > No clue about compatibility. If you unfortunately need to add it, > then please address my comments in the next version. TCA6416 is _way_ more complex device than PCF8574. Basically PCF8574 is shift register directly connected to IO lines, while TCA6416 is more like fully-fledged GPIO controller with output direction, drive strength, interrupt mask configuration etc etc. In Linux kernel these devices are handled by family-compatible driver drivers/gpio/gpio-pca953x.c Closest things by implementation in QEMU source tree are hw/gpio/pca9552.c and hw/gpio/pca9554.c However they are NOT register-compatible with pca953x. I suppose, best decision would be new driver for TCA6416 which eventually should support whole pca953x family of I2C GPIO expanders. Best regards, Dmitrii
Am 3. Februar 2025 05:42:55 UTC schrieb Dmitriy Sharikhin <d.sharikhin@yadro.com>: >At Sun, 02/02/2025 at 18:09 +0100, Philippe Mathieu-Daudé writes: >> No clue about compatibility. If you unfortunately need to add it, >> then please address my comments in the next version. >TCA6416 is _way_ more complex device than PCF8574. Basically PCF8574 is >shift register directly connected to IO lines, while TCA6416 is more like >fully-fledged GPIO controller with output direction, drive strength, interrupt >mask configuration etc etc. > >In Linux kernel these devices are handled by family-compatible driver > drivers/gpio/gpio-pca953x.c >Closest things by implementation in QEMU source tree are > hw/gpio/pca9552.c and > hw/gpio/pca9554.c >However they are NOT register-compatible with pca953x. I suppose, best >decision would be new driver for TCA6416 which eventually should support whole >pca953x family of I2C GPIO expanders. Just for the record: There has been an attempt to implement it: <https://lore.kernel.org/qemu-devel/20241107195453.2684138-3-titusr@google.com/> > >Best regards, >Dmitrii
Am 3. Februar 2025 05:42:55 UTC schrieb Dmitriy Sharikhin <d.sharikhin@yadro.com>: >At Sun, 02/02/2025 at 18:09 +0100, Philippe Mathieu-Daudé writes: >> No clue about compatibility. If you unfortunately need to add it, >> then please address my comments in the next version. >TCA6416 is _way_ more complex device than PCF8574. Basically PCF8574 is >shift register directly connected to IO lines, while TCA6416 is more like >fully-fledged GPIO controller with output direction, drive strength, interrupt >mask configuration etc etc. > >In Linux kernel these devices are handled by family-compatible driver > drivers/gpio/gpio-pca953x.c >Closest things by implementation in QEMU source tree are > hw/gpio/pca9552.c and > hw/gpio/pca9554.c >However they are NOT register-compatible with pca953x. Thanks Dimitrii for the valuable hint! The pca9552 model works fine for my purposes -- which is just to avoid clogging the i2c bus. >I suppose, best >decision would be new driver for TCA6416 which eventually should support whole >pca953x family of I2C GPIO expanders. I'll drop this patch then. Best regards, Bernhard > >Best regards, >Dmitrii
I'm not sure about ownership here, but from an I2C point of view this
all looks ok.
Reviewed-by: Corey Minyard <cminyard@mvista.com>
On Mon, Jan 20, 2025 at 12:43 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Xilinx QEMU implements a TCA6416 device model which may be useful for the
> broader QEMU community, so upstream it. In the Xilinx fork, the device model
> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
> "hw/*/cadence_*" maintainers.
>
> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
> modifications:
> * Use OBJECT_DECLARE_SIMPLE_TYPE()
> * Port from DPRINTF() to trace events
> * Follow QEMU conventions for naming of state struct
> * Rename type to not contain a ','
> * Use DEFINE_TYPES() macro
> * Implement under hw/gpio since device is i2c client and gpio provider
> * Have dedicated Kconfig switch
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
> --
> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
> willing to maintain this device model?
> ---
> MAINTAINERS | 1 +
> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
> hw/gpio/Kconfig | 5 ++
> hw/gpio/meson.build | 1 +
> hw/gpio/trace-events | 4 ++
> 5 files changed, 133 insertions(+)
> create mode 100644 hw/gpio/tca6416.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7531d65429..0cac9c90bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1030,6 +1030,7 @@ S: Maintained
> F: hw/*/xilinx_*
> F: hw/*/cadence_*
> F: hw/misc/zynq_slcr.c
> +F: hw/gpio/tca6416.c
> F: hw/adc/zynq-xadc.c
> F: include/hw/misc/zynq_slcr.h
> F: include/hw/adc/zynq-xadc.h
> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
> new file mode 100644
> index 0000000000..81ed7a654d
> --- /dev/null
> +++ b/hw/gpio/tca6416.c
> @@ -0,0 +1,122 @@
> +/*
> + * QEMU model of TCA6416 16-Bit I/O Expander
> + *
> + * Copyright (c) 2018 Xilinx Inc.
> + *
> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "hw/i2c/i2c.h"
> +#include "trace.h"
> +
> +#define TYPE_TCA6416 "tca6416"
> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
> +
> +#define IN_PORT0 0
> +#define IN_PORT1 1
> +#define OUT_PORT0 2
> +#define OUT_PORT1 3
> +#define POL_INV0 4
> +#define POL_INV1 5
> +#define CONF_PORT0 6
> +#define CONF_PORT1 7
> +#define RMAX (CONF_PORT1 + 1)
> +
> +enum tca6416_events {
> + ADDR_DONE,
> + ADDRESSING,
> +};
> +
> +struct Tca6416State {
> + I2CSlave i2c;
> +
> + uint8_t addr;
> + uint8_t state;
> + uint8_t regs[RMAX];
> +};
> +
> +static uint8_t tca6416_read(I2CSlave *i2c)
> +{
> + Tca6416State *s = TCA6416(i2c);
> + uint8_t ret;
> +
> + ret = s->regs[s->addr];
> + trace_tca6416_read(ret);
> + return ret;
> +}
> +
> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + trace_tca6416_write(data);
> + if (s->state == ADDRESSING) {
> + s->addr = data;
> + } else {
> + s->regs[s->addr] = data;
> + }
> +
> + return 0;
> +}
> +
> +static void tca6416_realize(DeviceState *dev, Error **errp)
> +{
> + Tca6416State *s = TCA6416(dev);
> +
> + s->regs[CONF_PORT0] = 0xFF;
> + s->regs[CONF_PORT1] = 0xFF;
> +}
> +
> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + switch (event) {
> + case I2C_START_SEND:
> + s->state = ADDRESSING;
> + break;
> + default:
> + s->state = ADDR_DONE;
> + };
> + return 0;
> +}
> +
> +static void tca6416_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> + dc->realize = tca6416_realize;
> + k->recv = tca6416_read;
> + k->send = tca6416_write;
> + k->event = tca6416_event;
> +}
> +
> +static const TypeInfo tca6416_types[] = {
> + {
> + .name = TYPE_TCA6416,
> + .parent = TYPE_I2C_SLAVE,
> + .class_init = tca6416_class_init,
> + .instance_size = sizeof(Tca6416State),
> + }
> +};
> +
> +DEFINE_TYPES(tca6416_types)
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index c423e10f59..a240cf2de2 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -20,5 +20,10 @@ config PCF8574
> bool
> depends on I2C
>
> +config TCA6416
> + bool
> + depends on I2C
> + default y if I2C_DEVICES
> +
> config ZAURUS_SCOOP
> bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 74840619c0..b3ff7c7460 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
> index cea896b28f..6724f2efb8 100644
> --- a/hw/gpio/trace-events
> +++ b/hw/gpio/trace-events
> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
> +
> +# tca6416.c
> +tca6416_write(uint8_t value) "0x%02x"
> +tca6416_read(uint8_t value) "0x%02x"
> --
> 2.48.1
>
>
On Mon, 20 Jan 2025, Bernhard Beschow wrote:
> Xilinx QEMU implements a TCA6416 device model which may be useful for the
> broader QEMU community, so upstream it. In the Xilinx fork, the device model
> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
> "hw/*/cadence_*" maintainers.
>
> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
> modifications:
> * Use OBJECT_DECLARE_SIMPLE_TYPE()
> * Port from DPRINTF() to trace events
> * Follow QEMU conventions for naming of state struct
> * Rename type to not contain a ','
> * Use DEFINE_TYPES() macro
> * Implement under hw/gpio since device is i2c client and gpio provider
> * Have dedicated Kconfig switch
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
> --
> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
> willing to maintain this device model?
> ---
> MAINTAINERS | 1 +
> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
> hw/gpio/Kconfig | 5 ++
> hw/gpio/meson.build | 1 +
> hw/gpio/trace-events | 4 ++
> 5 files changed, 133 insertions(+)
> create mode 100644 hw/gpio/tca6416.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7531d65429..0cac9c90bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1030,6 +1030,7 @@ S: Maintained
> F: hw/*/xilinx_*
> F: hw/*/cadence_*
> F: hw/misc/zynq_slcr.c
> +F: hw/gpio/tca6416.c
> F: hw/adc/zynq-xadc.c
> F: include/hw/misc/zynq_slcr.h
> F: include/hw/adc/zynq-xadc.h
> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
> new file mode 100644
> index 0000000000..81ed7a654d
> --- /dev/null
> +++ b/hw/gpio/tca6416.c
> @@ -0,0 +1,122 @@
> +/*
> + * QEMU model of TCA6416 16-Bit I/O Expander
> + *
> + * Copyright (c) 2018 Xilinx Inc.
> + *
> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "hw/i2c/i2c.h"
> +#include "trace.h"
> +
> +#define TYPE_TCA6416 "tca6416"
> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
> +
> +#define IN_PORT0 0
> +#define IN_PORT1 1
> +#define OUT_PORT0 2
> +#define OUT_PORT1 3
> +#define POL_INV0 4
> +#define POL_INV1 5
> +#define CONF_PORT0 6
> +#define CONF_PORT1 7
> +#define RMAX (CONF_PORT1 + 1)
Maybe make this list an enum? (But also fine as it is.)
Regards,
BALATON Zoltan
> +
> +enum tca6416_events {
> + ADDR_DONE,
> + ADDRESSING,
> +};
> +
> +struct Tca6416State {
> + I2CSlave i2c;
> +
> + uint8_t addr;
> + uint8_t state;
> + uint8_t regs[RMAX];
> +};
> +
> +static uint8_t tca6416_read(I2CSlave *i2c)
> +{
> + Tca6416State *s = TCA6416(i2c);
> + uint8_t ret;
> +
> + ret = s->regs[s->addr];
> + trace_tca6416_read(ret);
> + return ret;
> +}
> +
> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + trace_tca6416_write(data);
> + if (s->state == ADDRESSING) {
> + s->addr = data;
> + } else {
> + s->regs[s->addr] = data;
> + }
> +
> + return 0;
> +}
> +
> +static void tca6416_realize(DeviceState *dev, Error **errp)
> +{
> + Tca6416State *s = TCA6416(dev);
> +
> + s->regs[CONF_PORT0] = 0xFF;
> + s->regs[CONF_PORT1] = 0xFF;
> +}
> +
> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + Tca6416State *s = TCA6416(i2c);
> +
> + switch (event) {
> + case I2C_START_SEND:
> + s->state = ADDRESSING;
> + break;
> + default:
> + s->state = ADDR_DONE;
> + };
> + return 0;
> +}
> +
> +static void tca6416_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> + dc->realize = tca6416_realize;
> + k->recv = tca6416_read;
> + k->send = tca6416_write;
> + k->event = tca6416_event;
> +}
> +
> +static const TypeInfo tca6416_types[] = {
> + {
> + .name = TYPE_TCA6416,
> + .parent = TYPE_I2C_SLAVE,
> + .class_init = tca6416_class_init,
> + .instance_size = sizeof(Tca6416State),
> + }
> +};
> +
> +DEFINE_TYPES(tca6416_types)
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index c423e10f59..a240cf2de2 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -20,5 +20,10 @@ config PCF8574
> bool
> depends on I2C
>
> +config TCA6416
> + bool
> + depends on I2C
> + default y if I2C_DEVICES
> +
> config ZAURUS_SCOOP
> bool
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 74840619c0..b3ff7c7460 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
> index cea896b28f..6724f2efb8 100644
> --- a/hw/gpio/trace-events
> +++ b/hw/gpio/trace-events
> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
> +
> +# tca6416.c
> +tca6416_write(uint8_t value) "0x%02x"
> +tca6416_read(uint8_t value) "0x%02x"
>
Am 21. Januar 2025 03:07:39 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Mon, 20 Jan 2025, Bernhard Beschow wrote:
>> Xilinx QEMU implements a TCA6416 device model which may be useful for the
>> broader QEMU community, so upstream it. In the Xilinx fork, the device model
>> gets compiled whenever CONFIG_CADENCE is true, so have it maintained by the
>> "hw/*/cadence_*" maintainers.
>>
>> The code is based on Xilinx QEMU version xilinx_v2024.2 plus the following
>> modifications:
>> * Use OBJECT_DECLARE_SIMPLE_TYPE()
>> * Port from DPRINTF() to trace events
>> * Follow QEMU conventions for naming of state struct
>> * Rename type to not contain a ','
>> * Use DEFINE_TYPES() macro
>> * Implement under hw/gpio since device is i2c client and gpio provider
>> * Have dedicated Kconfig switch
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>
>> --
>> I have a use for TCA6416 emulation. Question: Are Xilinx Zynq maintainers
>> willing to maintain this device model?
>> ---
>> MAINTAINERS | 1 +
>> hw/gpio/tca6416.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>> hw/gpio/Kconfig | 5 ++
>> hw/gpio/meson.build | 1 +
>> hw/gpio/trace-events | 4 ++
>> 5 files changed, 133 insertions(+)
>> create mode 100644 hw/gpio/tca6416.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7531d65429..0cac9c90bc 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1030,6 +1030,7 @@ S: Maintained
>> F: hw/*/xilinx_*
>> F: hw/*/cadence_*
>> F: hw/misc/zynq_slcr.c
>> +F: hw/gpio/tca6416.c
>> F: hw/adc/zynq-xadc.c
>> F: include/hw/misc/zynq_slcr.h
>> F: include/hw/adc/zynq-xadc.h
>> diff --git a/hw/gpio/tca6416.c b/hw/gpio/tca6416.c
>> new file mode 100644
>> index 0000000000..81ed7a654d
>> --- /dev/null
>> +++ b/hw/gpio/tca6416.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * QEMU model of TCA6416 16-Bit I/O Expander
>> + *
>> + * Copyright (c) 2018 Xilinx Inc.
>> + *
>> + * Written by Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to deal
>> + * in the Software without restriction, including without limitation the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include "qemu/osdep.h"
>> +#include "hw/i2c/i2c.h"
>> +#include "trace.h"
>> +
>> +#define TYPE_TCA6416 "tca6416"
>> +OBJECT_DECLARE_SIMPLE_TYPE(Tca6416State, TCA6416)
>> +
>> +#define IN_PORT0 0
>> +#define IN_PORT1 1
>> +#define OUT_PORT0 2
>> +#define OUT_PORT1 3
>> +#define POL_INV0 4
>> +#define POL_INV1 5
>> +#define CONF_PORT0 6
>> +#define CONF_PORT1 7
>> +#define RMAX (CONF_PORT1 + 1)
>
>Maybe make this list an enum? (But also fine as it is.)
Good idea indeed. I'd address this once maintainership is clarified.
Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> +
>> +enum tca6416_events {
>> + ADDR_DONE,
>> + ADDRESSING,
>> +};
>> +
>> +struct Tca6416State {
>> + I2CSlave i2c;
>> +
>> + uint8_t addr;
>> + uint8_t state;
>> + uint8_t regs[RMAX];
>> +};
>> +
>> +static uint8_t tca6416_read(I2CSlave *i2c)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> + uint8_t ret;
>> +
>> + ret = s->regs[s->addr];
>> + trace_tca6416_read(ret);
>> + return ret;
>> +}
>> +
>> +static int tca6416_write(I2CSlave *i2c, uint8_t data)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> +
>> + trace_tca6416_write(data);
>> + if (s->state == ADDRESSING) {
>> + s->addr = data;
>> + } else {
>> + s->regs[s->addr] = data;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tca6416_realize(DeviceState *dev, Error **errp)
>> +{
>> + Tca6416State *s = TCA6416(dev);
>> +
>> + s->regs[CONF_PORT0] = 0xFF;
>> + s->regs[CONF_PORT1] = 0xFF;
>> +}
>> +
>> +static int tca6416_event(I2CSlave *i2c, enum i2c_event event)
>> +{
>> + Tca6416State *s = TCA6416(i2c);
>> +
>> + switch (event) {
>> + case I2C_START_SEND:
>> + s->state = ADDRESSING;
>> + break;
>> + default:
>> + s->state = ADDR_DONE;
>> + };
>> + return 0;
>> +}
>> +
>> +static void tca6416_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>> +
>> + dc->realize = tca6416_realize;
>> + k->recv = tca6416_read;
>> + k->send = tca6416_write;
>> + k->event = tca6416_event;
>> +}
>> +
>> +static const TypeInfo tca6416_types[] = {
>> + {
>> + .name = TYPE_TCA6416,
>> + .parent = TYPE_I2C_SLAVE,
>> + .class_init = tca6416_class_init,
>> + .instance_size = sizeof(Tca6416State),
>> + }
>> +};
>> +
>> +DEFINE_TYPES(tca6416_types)
>> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
>> index c423e10f59..a240cf2de2 100644
>> --- a/hw/gpio/Kconfig
>> +++ b/hw/gpio/Kconfig
>> @@ -20,5 +20,10 @@ config PCF8574
>> bool
>> depends on I2C
>>
>> +config TCA6416
>> + bool
>> + depends on I2C
>> + default y if I2C_DEVICES
>> +
>> config ZAURUS_SCOOP
>> bool
>> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
>> index 74840619c0..b3ff7c7460 100644
>> --- a/hw/gpio/meson.build
>> +++ b/hw/gpio/meson.build
>> @@ -18,3 +18,4 @@ system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
>> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
>> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
>> +system_ss.add(when: 'CONFIG_TCA6416', if_true: files('tca6416.c'))
>> diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
>> index cea896b28f..6724f2efb8 100644
>> --- a/hw/gpio/trace-events
>> +++ b/hw/gpio/trace-events
>> @@ -46,3 +46,7 @@ stm32l4x5_gpio_read(char *gpio, uint64_t addr) "GPIO%s addr: 0x%" PRIx64 " "
>> stm32l4x5_gpio_write(char *gpio, uint64_t addr, uint64_t data) "GPIO%s addr: 0x%" PRIx64 " val: 0x%" PRIx64 ""
>> stm32l4x5_gpio_update_idr(char *gpio, uint32_t old_idr, uint32_t new_idr) "GPIO%s from: 0x%x to: 0x%x"
>> stm32l4x5_gpio_pins(char *gpio, uint16_t disconnected, uint16_t high) "GPIO%s disconnected pins: 0x%x levels: 0x%x"
>> +
>> +# tca6416.c
>> +tca6416_write(uint8_t value) "0x%02x"
>> +tca6416_read(uint8_t value) "0x%02x"
>>
© 2016 - 2025 Red Hat, Inc.