Re: [PATCH v2] i2c: add support for microchip fpga i2c controllers

From: Hillf Danton <hdanton@sina.com>
To: Conor Dooley <conor.dooley@microchip.com>
Cc: wsa@kernel.org, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	daire.mcnamara@microchip.com
Subject: Re: [PATCH v2] i2c: add support for microchip fpga i2c controllers
Date: Wed,  4 May 2022 17:16:22 +0800	[thread overview]
Message-ID: <20220504091622.5024-1-hdanton@sina.com> (raw)
In-Reply-To: <20220321125855.3227057-1-conor.dooley@microchip.com>

On Mon, 21 Mar 2022 12:58:56 +0000 Conor Dooley wrote:
> +
> +static irqreturn_t mchp_corei2c_handle_isr(struct mchp_corei2c_dev *idev)
> +{
> +	u32 status = idev->isr_status;
> +	u8 ctrl;
> +
> +	if (!idev->buf)
> +		return IRQ_NONE;
> +
> +	switch (status) {
> +	case STATUS_M_START_SENT:
> +	case STATUS_M_REPEATED_START_SENT:
> +		ctrl = readb(idev->base + CORE_I2C_CTRL);
> +		ctrl &= ~CTRL_STA;
> +		writeb(idev->addr, idev->base + CORE_I2C_DATA);
> +		writeb(ctrl, idev->base + CORE_I2C_CTRL);
> +		if (idev->msg_len <= 0)
> +			goto finished;
> +		break;
> +	case STATUS_M_ARB_LOST:
> +		idev->msg_err = -EAGAIN;
> +		goto finished;
> +	case STATUS_M_SLAW_ACK:
> +	case STATUS_M_TX_DATA_ACK:
> +		if (idev->msg_len > 0)
> +			mchp_corei2c_fill_tx(idev);
> +		else
> +			goto last_byte;
> +		break;
> +	case STATUS_M_TX_DATA_NACK:
> +	case STATUS_M_SLAR_NACK:
> +	case STATUS_M_SLAW_NACK:
> +		idev->msg_err = -ENXIO;
> +		goto last_byte;
> +	case STATUS_M_SLAR_ACK:
> +		ctrl = readb(idev->base + CORE_I2C_CTRL);
> +		if (idev->msg_len == 1u) {
> +			ctrl &= ~CTRL_AA;
> +			writeb(ctrl, idev->base + CORE_I2C_CTRL);
> +		} else {
> +			ctrl |= CTRL_AA;
> +			writeb(ctrl, idev->base + CORE_I2C_CTRL);
> +		}
> +		if (idev->msg_len < 1u)
> +			goto last_byte;
> +		break;
> +	case STATUS_M_RX_DATA_ACKED:
> +		mchp_corei2c_empty_rx(idev);
> +		break;
> +	case STATUS_M_RX_DATA_NACKED:
> +		mchp_corei2c_empty_rx(idev);
> +		if (idev->msg_len == 0)
> +			goto last_byte;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return IRQ_HANDLED;
> +
> +last_byte:
> +	/* On the last byte to be transmitted, send STOP */
> +	mchp_corei2c_stop(idev);
> +finished:
> +	complete(&idev->msg_complete);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mchp_corei2c_isr(int irq, void *_dev)
> +{
> +	struct mchp_corei2c_dev *idev = _dev;
> +	irqreturn_t ret = IRQ_NONE;
> +	u8 ctrl;
> +
> +	ctrl = readb(idev->base + CORE_I2C_CTRL);
> +	if (ctrl & CTRL_SI) {
> +		idev->isr_status = readb(idev->base + CORE_I2C_STATUS);
> +		ret = mchp_corei2c_handle_isr(idev);
> +	}
> +
> +	/* Clear the si flag */
> +	ctrl = readb(idev->base + CORE_I2C_CTRL);
> +	ctrl &= ~CTRL_SI;
> +	writeb(ctrl, idev->base + CORE_I2C_CTRL);
> +
> +	return ret;
> +}
> +
> +static int mchp_corei2c_xfer_msg(struct mchp_corei2c_dev *idev,
> +				 struct i2c_msg *msg)
> +{
> +	u8 ctrl;
> +	unsigned long time_left;
> +
> +	if (msg->len == 0)
> +		return -EINVAL;
> +
> +	idev->addr = i2c_8bit_addr_from_msg(msg);
> +	idev->msg_len = msg->len;
> +	idev->buf = msg->buf;
> +	idev->msg_err = 0;
> +	idev->msg_read = (msg->flags & I2C_M_RD);
> +
> +	reinit_completion(&idev->msg_complete);

Would you specify why you need reinit completion? Is it likely to race
with the complete() above in mchp_corei2c_handle_isr()?

Hillf
> +
> +	mchp_corei2c_core_enable(idev);
> +
> +	ctrl = readb(idev->base + CORE_I2C_CTRL);
> +	ctrl |= CTRL_STA;
> +	writeb(ctrl, idev->base + CORE_I2C_CTRL);
> +
> +	time_left = wait_for_completion_timeout(&idev->msg_complete,
> +						MICROCHIP_I2C_TIMEOUT);
> +	if (!time_left)
> +		return -ETIMEDOUT;
> +
> +	return idev->msg_err;
> +}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv