Re: [PATCH v2 03/11] hw/misc: add a toy i2c echo device

[Thomas Huth <thuth@redhat.com>]

On 01/03/2023 17.56, Cédric Le Goater wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Add an example I2C device to demonstrate how a slave may master the bus
> and send data asynchronously to another slave.
> 
> The device will echo whatever it is sent to the device identified by the
> first byte received.
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> [ clg: integrated fixes :
>    https://lore.kernel.org/qemu-devel/Y3yMKAhOkYGtnkOp@cormorant.local/ ]
> Message-Id: <20220601210831.67259-7-its@irrelevant.dk>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/misc/i2c-echo.c  | 156 ++++++++++++++++++++++++++++++++++++++++++++
>   hw/misc/meson.build |   2 +
>   2 files changed, 158 insertions(+)
>   create mode 100644 hw/misc/i2c-echo.c
> 
> diff --git a/hw/misc/i2c-echo.c b/hw/misc/i2c-echo.c
> new file mode 100644
> index 0000000000..5705ab5d73
> --- /dev/null
> +++ b/hw/misc/i2c-echo.c
> @@ -0,0 +1,156 @@
> +#include "qemu/osdep.h"
> +#include "qemu/timer.h"
> +#include "qemu/main-loop.h"
> +#include "block/aio.h"
> +#include "hw/i2c/i2c.h"

  Hi Klaus,

I've got two questions with regards to this new devices:

1) The file lacks a license statement (and a short comment at the beginning 
what it is all about). Could you maybe provide a follow up patch with a 
proper header comment that includes a license and a short description about 
the device?

2) Why is it in hw/misc/ and not in hw/i2c/ ?

I think we should also have a proper Kconfig switch for this device, so we 
can disable it with --without-default-devices, what do you think?

  Thomas