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

[Klaus Jensen]

On Aug 22 19:20, Thomas Huth wrote:
> 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
> 
> 

Hi Thomas,

1) My apologies, I'll add the proper GPL license comment.

2) It's an example of using the asynchronous i2c send API. It's not a
"real" device, its more like the edu-device. A proper note in the file
should make this clear.

I'll send a patch to fix this up!


Thanks,
Klaus

signature.asc
Description: PGP signature