Hi Sean,
[...]
> @@ -905,38 +906,43 @@ static void stm32f7_i2c_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
> cr2 |= STM32F7_I2C_CR2_NBYTES(f7_msg->count);
> }
>
> - /* Enable NACK, STOP, error and transfer complete interrupts */
> - cr1 |= STM32F7_I2C_CR1_ERRIE | STM32F7_I2C_CR1_TCIE |
> - STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE;
> -
> - /* Clear DMA req and TX/RX interrupt */
> - cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE |
> - STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN);
> -
> - /* Configure DMA or enable RX/TX interrupt */
> - i2c_dev->use_dma = false;
> - if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
> - ret = stm32_i2c_prep_dma_xfer(i2c_dev->dev, i2c_dev->dma,
> - msg->flags & I2C_M_RD,
> - f7_msg->count, f7_msg->buf,
> - stm32f7_i2c_dma_callback,
> - i2c_dev);
> - if (!ret)
> - i2c_dev->use_dma = true;
> - else
> - dev_warn(i2c_dev->dev, "can't use DMA\n");
> - }
> + if (!i2c_dev->atomic) {
> + /* Enable NACK, STOP, error and transfer complete interrupts */
> + cr1 |= STM32F7_I2C_CR1_ERRIE | STM32F7_I2C_CR1_TCIE |
> + STM32F7_I2C_CR1_STOPIE | STM32F7_I2C_CR1_NACKIE;
> +
> + /* Clear DMA req and TX/RX interrupt */
> + cr1 &= ~(STM32F7_I2C_CR1_RXIE | STM32F7_I2C_CR1_TXIE |
> + STM32F7_I2C_CR1_RXDMAEN | STM32F7_I2C_CR1_TXDMAEN);
> +
> + /* Configure DMA or enable RX/TX interrupt */
> + i2c_dev->use_dma = false;
> + if (i2c_dev->dma && f7_msg->count >= STM32F7_I2C_DMA_LEN_MIN) {
> + ret = stm32_i2c_prep_dma_xfer(i2c_dev->dev, i2c_dev->dma,
> + msg->flags & I2C_M_RD,
> + f7_msg->count, f7_msg->buf,
> + stm32f7_i2c_dma_callback,
> + i2c_dev);
> + if (!ret)
> + i2c_dev->use_dma = true;
> + else
> + dev_warn(i2c_dev->dev, "can't use DMA\n");
> + }
>
> - if (!i2c_dev->use_dma) {
> - if (msg->flags & I2C_M_RD)
> - cr1 |= STM32F7_I2C_CR1_RXIE;
> - else
> - cr1 |= STM32F7_I2C_CR1_TXIE;
> + if (!i2c_dev->use_dma) {
> + if (msg->flags & I2C_M_RD)
> + cr1 |= STM32F7_I2C_CR1_RXIE;
> + else
> + cr1 |= STM32F7_I2C_CR1_TXIE;
> + } else {
> + if (msg->flags & I2C_M_RD)
> + cr1 |= STM32F7_I2C_CR1_RXDMAEN;
> + else
> + cr1 |= STM32F7_I2C_CR1_TXDMAEN;
> + }
> } else {
> - if (msg->flags & I2C_M_RD)
> - cr1 |= STM32F7_I2C_CR1_RXDMAEN;
> - else
> - cr1 |= STM32F7_I2C_CR1_TXDMAEN;
> + /* Disable all interrupts */
> + cr1 &= ~STM32F7_I2C_ALL_IRQ_MASK;
if you do
if (i2c_dev->atomic) {
/* Disable all interrupts */
cr1 &= ~STM32F7_I2C_ALL_IRQ_MASK;
return;
}
you save all the above from a leveel of indentation.
> }
>
> /* Configure Start/Repeated Start */
> @@ -1670,7 +1676,22 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
> +static int stm32f7_i2c_wait_polling(struct stm32f7_i2c_dev *i2c_dev)
> +{
> + ktime_t timeout = ktime_add_ms(ktime_get(), i2c_dev->adap.timeout);
> +
> + while (ktime_compare(ktime_get(), timeout) < 0) {
> + udelay(5);
> + stm32f7_i2c_isr_event(0, i2c_dev);
> +
> + if (try_wait_for_completion(&i2c_dev->complete))
> + return 1;
I'm wondering if it makes really sense to have a complete() and
wait_for_completion() scheme here.
> + }
> +
> + return 0;
> +}
> +
> +static int stm32f7_i2c_xfer_core(struct i2c_adapter *i2c_adap,
> struct i2c_msg msgs[], int num)
> {
> struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> @@ -1694,8 +1715,13 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
>
> stm32f7_i2c_xfer_msg(i2c_dev, msgs);
>
> - time_left = wait_for_completion_timeout(&i2c_dev->complete,
> - i2c_dev->adap.timeout);
> + if (!i2c_dev->atomic) {
> + time_left = wait_for_completion_timeout(&i2c_dev->complete,
> + i2c_dev->adap.timeout);
> + } else {
> + time_left = stm32f7_i2c_wait_polling(i2c_dev);
> + }
please, drop the brackets here... and time_left here serves only
not to get the -ETIMEDOUT... looks a bit ugly to me, but can't
think of a better way.
> +
> ret = f7_msg->result;
> if (ret) {
> if (i2c_dev->use_dma)
> @@ -1727,6 +1753,24 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
> return (ret < 0) ? ret : num;
> }
>
> +static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
> + struct i2c_msg msgs[], int num)
> +{
> + struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +
> + i2c_dev->atomic = 0;
false
> + return stm32f7_i2c_xfer_core(i2c_adap, msgs, num);
> +}
> +
> +static int stm32f7_i2c_xfer_atomic(struct i2c_adapter *i2c_adap,
> + struct i2c_msg msgs[], int num)
> +{
> + struct stm32f7_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +
> + i2c_dev->atomic = 1;
true
Andi
> + return stm32f7_i2c_xfer_core(i2c_adap, msgs, num);
> +}
> +
