Re: [PATCH v4 07/10] i3c: master: Add driver for Cadence IP

[Przemyslaw Gaj <pgaj@cadence.com>]

Hi Boris,

Few things regarding Cadence IP driver:

On 6/4/18, 9:31 AM, "Boris Brezillon" <boris.brezillon@bootlin.com> 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.

    +static int cdns_i3c_master_probe(struct platform_device *pdev)
    +{
    +	struct cdns_i3c_master *master;
    +	struct resource *res;
    +	int ret, irq;
    +	u32 val;
    +
    +	master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
    +	if (!master)
    +		return -ENOMEM;
    +
    +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
    +	master->regs = devm_ioremap_resource(&pdev->dev, res);
    +	if (IS_ERR(master->regs))
    +		return PTR_ERR(master->regs);
    +
    +	master->pclk = devm_clk_get(&pdev->dev, "pclk");
    +	if (IS_ERR(master->pclk))
    +		return PTR_ERR(master->pclk);
    +
    +	master->sysclk = devm_clk_get(&pdev->dev, "sysclk");
    +	if (IS_ERR(master->pclk))
    +		return PTR_ERR(master->pclk);
    +
    +	irq = platform_get_irq(pdev, 0);
    +	if (irq < 0)
    +		return irq;
    +
    +	ret = clk_prepare_enable(master->pclk);
    +	if (ret)
    +		return ret;
    +
    +	ret = clk_prepare_enable(master->sysclk);
    +	if (ret)
    +		goto err_disable_pclk;
    +
    +	if (readl(master->regs + DEV_ID) != DEV_ID_I3C_MASTER) {
    +		ret = -EINVAL;
    +		goto err_disable_sysclk;
    +	}
    +
    +	spin_lock_init(&master->xferqueue.lock);
    +	INIT_LIST_HEAD(&master->xferqueue.list);
    +
    +	INIT_WORK(&master->hj_work, cdns_i3c_master_hj);
    +	writel(0xffffffff, master->regs + MST_IDR);
    +	writel(0xffffffff, master->regs + SLV_IDR);
    +	ret = devm_request_irq(&pdev->dev, irq, cdns_i3c_master_interrupt, 0,
    +			       dev_name(&pdev->dev), master);
    +	if (ret)
    +		goto err_disable_sysclk;
    +
    +	platform_set_drvdata(pdev, master);
    +
    +	val = readl(master->regs + CONF_STATUS0);
    +
    +	/* 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.

    +
    +	spin_lock_init(&master->ibi.lock);
    +	master->ibi.num_slots = CONF_STATUS1_IBI_HW_RES(val);
    +	master->ibi.slots = devm_kzalloc(&pdev->dev,
    +					 sizeof(*master->ibi.slots) *
    +					 master->ibi.num_slots,
    +					 GFP_KERNEL);
    +	if (!master->ibi.slots)
    +		goto err_disable_sysclk;
    +
    +	writel(IBIR_THR(1), master->regs + CMD_IBI_THR_CTRL);
    +	writel(MST_INT_IBIR_THR, master->regs + MST_IER);
    +	writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
    +
    +	ret = i3c_master_register(&master->base, &pdev->dev,
    +				  &cdns_i3c_master_ops, false);
    +	if (ret)
    +		goto err_disable_sysclk;
    +
    +	return 0;
    +
    +err_disable_sysclk:
    +	clk_disable_unprepare(master->sysclk);
    +
    +err_disable_pclk:
    +	clk_disable_unprepare(master->pclk);
    +
    +	return ret;
    +}
    
Regards,
Przemyslaw Gaj