Re: [PATCH v3 1/3] hw/i2c: Implement Broadcom Serial Controller (BSC)

[Philippe Mathieu-Daudé]

On 20/2/24 14:41, Rayhan Faizel wrote:
> A few deficiencies in the current device model need to be noted.
>
> 1. FIFOs are not used. All sends and receives are done directly.
> 2. Repeated starts are not emulated. Repeated starts can be triggered in real
> hardware by sending a new read transfer request in the window time between
> transfer active set of write transfer request and done bit set of the same.
>
> Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
> ---
>   docs/system/arm/raspi.rst    |   1 +
>   hw/i2c/Kconfig               |   4 +
>   hw/i2c/bcm2835_i2c.c         | 278 +++++++++++++++++++++++++++++++++++
>   hw/i2c/meson.build           |   1 +
>   include/hw/i2c/bcm2835_i2c.h |  80 ++++++++++
>   5 files changed, 364 insertions(+)
>   create mode 100644 hw/i2c/bcm2835_i2c.c
>   create mode 100644 include/hw/i2c/bcm2835_i2c.h


> +static const MemoryRegionOps bcm2835_i2c_ops = {
> +    .read = bcm2835_i2c_read,
> +    .write = bcm2835_i2c_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

Watch out, your implementation is 32-bit, so this misses:

      .impl = {
          .min_access_size = 4,
          .max_access_size = 4,
      },

> +};


> diff --git a/include/hw/i2c/bcm2835_i2c.h b/include/hw/i2c/bcm2835_i2c.h
> new file mode 100644
> index 0000000000..0a56df4720
> --- /dev/null
> +++ b/include/hw/i2c/bcm2835_i2c.h


> +#define BCM2835_I2C_C       0x0                   /* Control */
> +#define BCM2835_I2C_S       0x4                   /* Status */
> +#define BCM2835_I2C_DLEN    0x8                   /* Data Length */
> +#define BCM2835_I2C_A       0xc                   /* Slave Address */
> +#define BCM2835_I2C_FIFO    0x10                  /* FIFO */
> +#define BCM2835_I2C_DIV     0x14                  /* Clock Divider */
> +#define BCM2835_I2C_DEL     0x18                  /* Data Delay */
> +#define BCM2835_I2C_CLKT    0x20                  /* Clock Stretch Timeout */