On Thu, 9 Jul 2020, Miquel Raynal wrote:
> Add support for Silvaco I3C dual-role IP. The master role is supported
> in SDR mode only. I2C transfers have not been tested but are shared
> because they are so close to the I3C transfers in terms of registers
> configuration.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
I'm worried by constructs like these:
> +static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master,
> + struct i3c_dev_desc *dev)
> +{
> + struct svc_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> + struct i3c_ibi_slot *slot;
> + unsigned int count;
> + u32 mdatactrl;
> + u8 *buf;
> + int ret;
> + u32 reg;
> +
> + spin_lock(&master->ibi.lock);
> +
> + slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
> + if (!slot) {
> + ret = -ENOSPC;
> + goto unlock;
> + }
> +
> + slot->len = 0;
> + buf = slot->data;
> +
> + ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + reg & SVC_I3C_MINT_RXPEND, 0, 1000);
Here you're in atomic context due to the lock, and
readl_poll_timeout(() is built using usleep() which may ... sleep.
Also, is it actually possible for execution to reach this point if
SVC_I3C_MINT_RXPEND is not set?
The rest looks reasonable to me.
Nicolas
