Re: [PATCH v4 07/10] i3c: master: Add driver for Cadence IP
From: Boris Brezillon
Date: Mon Jun 04 2018 - 07:26:36 EST
Hi Przemek,
On Mon, 4 Jun 2018 09:24:51 +0000
Przemyslaw Gaj <pgaj@xxxxxxxxxxx> wrote:
> Hi Boris,
>
> Few things regarding Cadence IP driver:
>
> ïOn 6/4/18, 9:31 AM, "Boris Brezillon" <boris.brezillon@xxxxxxxxxxx> wrote:
>
> +static void cdns_i3c_master_handle_ibi(struct cdns_i3c_master *master,
> + u32 ibir)
> +{
> + struct cdns_i3c_i2c_dev_data *data;
> + bool data_consumed = false;
> + struct i3c_ibi_slot *slot;
> + u32 id = IBIR_SLVID(ibir);
> + struct i3c_device *dev;
> + int len, i, j;
> + u8 *buf;
> +
> + /*
> + * FIXME: maybe we should report the FIFO OVF errors to the upper
> + * layer.
> + */
> + if (id >= master->ibi.num_slots || (ibir & IBIR_ERROR))
> + goto out;
> +
> + dev = master->ibi.slots[id];
> + spin_lock(&master->ibi.lock);
> +
> + data = i3c_device_get_master_data(dev);
> + slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
> + if (!slot)
> + goto out_unlock;
> +
> + buf = slot->data;
> +
> + len = IBIR_XFER_BYTES(ibir);
> + for (i = 0; i < IBIR_XFER_BYTES(ibir); i += 4) {
> + u32 tmp = readl(master->regs + IBI_DATA_FIFO);
> +
> + for (j = 0; j < 4 && i + j < dev->ibi->max_payload_len; j++)
> + buf[i + j] = tmp >> (j * 8);
> +
> + }
> + slot->len = min_t(unsigned int, IBIR_XFER_BYTES(ibir),
> + dev->ibi->max_payload_len);
> + i3c_master_queue_ibi(dev, slot);
> + data_consumed = true;
> +
> +out_unlock:
> + spin_unlock(&master->ibi.lock);
> +
> +out:
> + /* Consume data from the FIFO if it's not been done already. */
> + if (!data_consumed) {
> + for (i = 0; i < IBIR_XFER_BYTES(ibir); i += 4)
> + readl(master->regs + IBI_DATA_FIFO);
> + }
> +}
>
> len variable is unneeded.
Will get rid of len.
> +
> + /* Device ID0 is reserved to describe this master. */
> + master->maxdevs = CONF_STATUS0_DEVS_NUM(val);
> + master->free_rr_slots = GENMASK(master->maxdevs, 1);
> +
> + val = readl(master->regs + CONF_STATUS1);
> + master->caps.cmdfifodepth = CONF_STATUS1_CMD_DEPTH(val);
> + master->caps.rxfifodepth = CONF_STATUS1_RX_DEPTH(val);
> + master->caps.txfifodepth = CONF_STATUS1_TX_DEPTH(val);
> + master->caps.ibirfifodepth = 16;
>
> IBI fifo depth is hardcoded. You can read this value from CONF_STATUS0 register.
>
> + master->caps.cmdrfifodepth = 16;
>
> CMDR fifo depth is hardcoded. You can read this value from CONF_STATUS0 register also.
Sure, I'll use the FIFO depth exposed in CONF_STATUS0.
Thanks,
Boris