2018-07-07 02:52:12

by Jun Gao

[permalink] [raw]
Subject: [PATCH 0/3] Register i2c adapter driver earlier and use DMA safe buffers

This patch series based on v4.18-rc1, include i2c adapter driver register time
modification, DMA safe buffer free function and DMA safe buffers used for i2c
transactions.

Jun Gao (3):
i2c: mediatek: Register i2c adapter driver earlier
i2c: Add helper to ease DMA handling
i2c: mediatek: Use DMA safe buffers for i2c transactions

drivers/i2c/busses/i2c-mt65xx.c | 74 ++++++++++++++++++++++++++++++++++++-----
drivers/i2c/i2c-core-base.c | 14 ++++++++
include/linux/i2c.h | 1 +
3 files changed, 81 insertions(+), 8 deletions(-)

--
1.8.1.1



2018-07-07 02:51:05

by Jun Gao

[permalink] [raw]
Subject: [PATCH 2/3] i2c: Add helper to ease DMA handling

From: Jun Gao <[email protected]>

This function is needed by i2c_get_dma_safe_msg_buf() potentially.
It is used to free DMA safe buffer when DMA operation fails.

Signed-off-by: Jun Gao <[email protected]>
---
drivers/i2c/i2c-core-base.c | 14 ++++++++++++++
include/linux/i2c.h | 1 +
2 files changed, 15 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 31d16ad..2b518ea 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2288,6 +2288,20 @@ void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
}
EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);

+/**
+ * i2c_free_dma_safe_msg_buf - free DMA safe buffer
+ * @msg: the message related to DMA safe buffer
+ * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
+ */
+void i2c_free_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
+{
+ if (!buf || buf == msg->buf)
+ return;
+
+ kfree(buf);
+}
+EXPORT_SYMBOL_GPL(i2c_free_dma_safe_msg_buf);
+
MODULE_AUTHOR("Simon G. Vogl <[email protected]>");
MODULE_DESCRIPTION("I2C-Bus main module");
MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 254cd34..6d62f93 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -860,6 +860,7 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)

u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
+void i2c_free_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);

int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
/**
--
1.8.1.1


2018-07-07 02:51:12

by Jun Gao

[permalink] [raw]
Subject: [PATCH 1/3] i2c: mediatek: Register i2c adapter driver earlier

From: Jun Gao <[email protected]>

As i2c adapter, i2c slave devices will depend on it. In order not to
block the initializations of i2c slave devices, register i2c adapter
driver at appropriate time.

Signed-off-by: Jun Gao <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 1e57f58..806e8b90 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -888,7 +888,17 @@ static int mtk_i2c_resume(struct device *dev)
},
};

-module_platform_driver(mtk_i2c_driver);
+static int __init mtk_i2c_adap_init(void)
+{
+ return platform_driver_register(&mtk_i2c_driver);
+}
+subsys_initcall(mtk_i2c_adap_init);
+
+static void __exit mtk_i2c_adap_exit(void)
+{
+ platform_driver_unregister(&mtk_i2c_driver);
+}
+module_exit(mtk_i2c_adap_exit);

MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("MediaTek I2C Bus Driver");
--
1.8.1.1


2018-07-07 02:51:31

by Jun Gao

[permalink] [raw]
Subject: [PATCH 3/3] i2c: mediatek: Use DMA safe buffers for i2c transactions

From: Jun Gao <[email protected]>

DMA mode will always be used in i2c transactions, try to allocate
a DMA safe buffer if the buf of struct i2c_msg used is not DMA safe.

Signed-off-by: Jun Gao <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 62 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 806e8b90..dd014ee 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -441,6 +441,8 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
u16 control_reg;
u16 restart_flag = 0;
u32 reg_4g_mode;
+ u8 *dma_rd_buf;
+ u8 *dma_wr_buf;
dma_addr_t rpaddr = 0;
dma_addr_t wpaddr = 0;
int ret;
@@ -500,10 +502,18 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
if (i2c->op == I2C_MASTER_RD) {
writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
writel(I2C_DMA_CON_RX, i2c->pdmabase + OFFSET_CON);
- rpaddr = dma_map_single(i2c->dev, msgs->buf,
+
+ dma_rd_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
+ if (!dma_rd_buf)
+ return -ENOMEM;
+
+ rpaddr = dma_map_single(i2c->dev, dma_rd_buf,
msgs->len, DMA_FROM_DEVICE);
- if (dma_mapping_error(i2c->dev, rpaddr))
+ if (dma_mapping_error(i2c->dev, rpaddr)) {
+ i2c_free_dma_safe_msg_buf(msgs, dma_rd_buf);
+
return -ENOMEM;
+ }

if (i2c->dev_comp->support_33bits) {
reg_4g_mode = mtk_i2c_set_4g_mode(rpaddr);
@@ -515,10 +525,18 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
} else if (i2c->op == I2C_MASTER_WR) {
writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
- wpaddr = dma_map_single(i2c->dev, msgs->buf,
+
+ dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
+ if (!dma_wr_buf)
+ return -ENOMEM;
+
+ wpaddr = dma_map_single(i2c->dev, dma_wr_buf,
msgs->len, DMA_TO_DEVICE);
- if (dma_mapping_error(i2c->dev, wpaddr))
+ if (dma_mapping_error(i2c->dev, wpaddr)) {
+ i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
+
return -ENOMEM;
+ }

if (i2c->dev_comp->support_33bits) {
reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
@@ -530,16 +548,39 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
} else {
writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_INT_FLAG);
writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_CON);
- wpaddr = dma_map_single(i2c->dev, msgs->buf,
+
+ dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
+ if (!dma_wr_buf)
+ return -ENOMEM;
+
+ wpaddr = dma_map_single(i2c->dev, dma_wr_buf,
msgs->len, DMA_TO_DEVICE);
- if (dma_mapping_error(i2c->dev, wpaddr))
+ if (dma_mapping_error(i2c->dev, wpaddr)) {
+ i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
+
return -ENOMEM;
- rpaddr = dma_map_single(i2c->dev, (msgs + 1)->buf,
+ }
+
+ dma_rd_buf = i2c_get_dma_safe_msg_buf((msgs + 1), 0);
+ if (!dma_rd_buf) {
+ dma_unmap_single(i2c->dev, wpaddr,
+ msgs->len, DMA_TO_DEVICE);
+
+ i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
+
+ return -ENOMEM;
+ }
+
+ rpaddr = dma_map_single(i2c->dev, dma_rd_buf,
(msgs + 1)->len,
DMA_FROM_DEVICE);
if (dma_mapping_error(i2c->dev, rpaddr)) {
dma_unmap_single(i2c->dev, wpaddr,
msgs->len, DMA_TO_DEVICE);
+
+ i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
+ i2c_free_dma_safe_msg_buf((msgs + 1), dma_rd_buf);
+
return -ENOMEM;
}

@@ -578,14 +619,21 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
if (i2c->op == I2C_MASTER_WR) {
dma_unmap_single(i2c->dev, wpaddr,
msgs->len, DMA_TO_DEVICE);
+
+ i2c_release_dma_safe_msg_buf(msgs, dma_wr_buf);
} else if (i2c->op == I2C_MASTER_RD) {
dma_unmap_single(i2c->dev, rpaddr,
msgs->len, DMA_FROM_DEVICE);
+
+ i2c_release_dma_safe_msg_buf(msgs, dma_rd_buf);
} else {
dma_unmap_single(i2c->dev, wpaddr, msgs->len,
DMA_TO_DEVICE);
dma_unmap_single(i2c->dev, rpaddr, (msgs + 1)->len,
DMA_FROM_DEVICE);
+
+ i2c_release_dma_safe_msg_buf(msgs, dma_wr_buf);
+ i2c_release_dma_safe_msg_buf((msgs + 1), dma_rd_buf);
}

if (ret == 0) {
--
1.8.1.1


2018-07-07 07:10:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 3/3] i2c: mediatek: Use DMA safe buffers for i2c transactions

Hi Jun,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.18-rc3 next-20180706]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Jun-Gao/Register-i2c-adapter-driver-earlier-and-use-DMA-safe-buffers/20180707-105514
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

drivers/i2c//busses/i2c-mt65xx.c: In function 'mtk_i2c_do_transfer':
>> drivers/i2c//busses/i2c-mt65xx.c:635:3: warning: 'dma_wr_buf' may be used uninitialized in this function [-Wmaybe-uninitialized]
i2c_release_dma_safe_msg_buf(msgs, dma_wr_buf);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c//busses/i2c-mt65xx.c:636:3: warning: 'dma_rd_buf' may be used uninitialized in this function [-Wmaybe-uninitialized]
i2c_release_dma_safe_msg_buf((msgs + 1), dma_rd_buf);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/dma_wr_buf +635 drivers/i2c//busses/i2c-mt65xx.c

435
436 static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
437 int num, int left_num)
438 {
439 u16 addr_reg;
440 u16 start_reg;
441 u16 control_reg;
442 u16 restart_flag = 0;
443 u32 reg_4g_mode;
444 u8 *dma_rd_buf;
445 u8 *dma_wr_buf;
446 dma_addr_t rpaddr = 0;
447 dma_addr_t wpaddr = 0;
448 int ret;
449
450 i2c->irq_stat = 0;
451
452 if (i2c->auto_restart)
453 restart_flag = I2C_RS_TRANSFER;
454
455 reinit_completion(&i2c->msg_complete);
456
457 control_reg = readw(i2c->base + OFFSET_CONTROL) &
458 ~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS);
459 if ((i2c->speed_hz > 400000) || (left_num >= 1))
460 control_reg |= I2C_CONTROL_RS;
461
462 if (i2c->op == I2C_MASTER_WRRD)
463 control_reg |= I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS;
464
465 writew(control_reg, i2c->base + OFFSET_CONTROL);
466
467 /* set start condition */
468 if (i2c->speed_hz <= 100000)
469 writew(I2C_ST_START_CON, i2c->base + OFFSET_EXT_CONF);
470 else
471 writew(I2C_FS_START_CON, i2c->base + OFFSET_EXT_CONF);
472
473 addr_reg = i2c_8bit_addr_from_msg(msgs);
474 writew(addr_reg, i2c->base + OFFSET_SLAVE_ADDR);
475
476 /* Clear interrupt status */
477 writew(restart_flag | I2C_HS_NACKERR | I2C_ACKERR |
478 I2C_TRANSAC_COMP, i2c->base + OFFSET_INTR_STAT);
479 writew(I2C_FIFO_ADDR_CLR, i2c->base + OFFSET_FIFO_ADDR_CLR);
480
481 /* Enable interrupt */
482 writew(restart_flag | I2C_HS_NACKERR | I2C_ACKERR |
483 I2C_TRANSAC_COMP, i2c->base + OFFSET_INTR_MASK);
484
485 /* Set transfer and transaction len */
486 if (i2c->op == I2C_MASTER_WRRD) {
487 if (i2c->dev_comp->aux_len_reg) {
488 writew(msgs->len, i2c->base + OFFSET_TRANSFER_LEN);
489 writew((msgs + 1)->len, i2c->base +
490 OFFSET_TRANSFER_LEN_AUX);
491 } else {
492 writew(msgs->len | ((msgs + 1)->len) << 8,
493 i2c->base + OFFSET_TRANSFER_LEN);
494 }
495 writew(I2C_WRRD_TRANAC_VALUE, i2c->base + OFFSET_TRANSAC_LEN);
496 } else {
497 writew(msgs->len, i2c->base + OFFSET_TRANSFER_LEN);
498 writew(num, i2c->base + OFFSET_TRANSAC_LEN);
499 }
500
501 /* Prepare buffer data to start transfer */
502 if (i2c->op == I2C_MASTER_RD) {
503 writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
504 writel(I2C_DMA_CON_RX, i2c->pdmabase + OFFSET_CON);
505
506 dma_rd_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
507 if (!dma_rd_buf)
508 return -ENOMEM;
509
510 rpaddr = dma_map_single(i2c->dev, dma_rd_buf,
511 msgs->len, DMA_FROM_DEVICE);
512 if (dma_mapping_error(i2c->dev, rpaddr)) {
513 i2c_free_dma_safe_msg_buf(msgs, dma_rd_buf);
514
515 return -ENOMEM;
516 }
517
518 if (i2c->dev_comp->support_33bits) {
519 reg_4g_mode = mtk_i2c_set_4g_mode(rpaddr);
520 writel(reg_4g_mode, i2c->pdmabase + OFFSET_RX_4G_MODE);
521 }
522
523 writel((u32)rpaddr, i2c->pdmabase + OFFSET_RX_MEM_ADDR);
524 writel(msgs->len, i2c->pdmabase + OFFSET_RX_LEN);
525 } else if (i2c->op == I2C_MASTER_WR) {
526 writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
527 writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
528
529 dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
530 if (!dma_wr_buf)
531 return -ENOMEM;
532
533 wpaddr = dma_map_single(i2c->dev, dma_wr_buf,
534 msgs->len, DMA_TO_DEVICE);
535 if (dma_mapping_error(i2c->dev, wpaddr)) {
536 i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
537
538 return -ENOMEM;
539 }
540
541 if (i2c->dev_comp->support_33bits) {
542 reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
543 writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
544 }
545
546 writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
547 writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
548 } else {
549 writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_INT_FLAG);
550 writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_CON);
551
552 dma_wr_buf = i2c_get_dma_safe_msg_buf(msgs, 0);
553 if (!dma_wr_buf)
554 return -ENOMEM;
555
556 wpaddr = dma_map_single(i2c->dev, dma_wr_buf,
557 msgs->len, DMA_TO_DEVICE);
558 if (dma_mapping_error(i2c->dev, wpaddr)) {
559 i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
560
561 return -ENOMEM;
562 }
563
564 dma_rd_buf = i2c_get_dma_safe_msg_buf((msgs + 1), 0);
565 if (!dma_rd_buf) {
566 dma_unmap_single(i2c->dev, wpaddr,
567 msgs->len, DMA_TO_DEVICE);
568
569 i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
570
571 return -ENOMEM;
572 }
573
574 rpaddr = dma_map_single(i2c->dev, dma_rd_buf,
575 (msgs + 1)->len,
576 DMA_FROM_DEVICE);
577 if (dma_mapping_error(i2c->dev, rpaddr)) {
578 dma_unmap_single(i2c->dev, wpaddr,
579 msgs->len, DMA_TO_DEVICE);
580
581 i2c_free_dma_safe_msg_buf(msgs, dma_wr_buf);
582 i2c_free_dma_safe_msg_buf((msgs + 1), dma_rd_buf);
583
584 return -ENOMEM;
585 }
586
587 if (i2c->dev_comp->support_33bits) {
588 reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
589 writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
590
591 reg_4g_mode = mtk_i2c_set_4g_mode(rpaddr);
592 writel(reg_4g_mode, i2c->pdmabase + OFFSET_RX_4G_MODE);
593 }
594
595 writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
596 writel((u32)rpaddr, i2c->pdmabase + OFFSET_RX_MEM_ADDR);
597 writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
598 writel((msgs + 1)->len, i2c->pdmabase + OFFSET_RX_LEN);
599 }
600
601 writel(I2C_DMA_START_EN, i2c->pdmabase + OFFSET_EN);
602
603 if (!i2c->auto_restart) {
604 start_reg = I2C_TRANSAC_START;
605 } else {
606 start_reg = I2C_TRANSAC_START | I2C_RS_MUL_TRIG;
607 if (left_num >= 1)
608 start_reg |= I2C_RS_MUL_CNFG;
609 }
610 writew(start_reg, i2c->base + OFFSET_START);
611
612 ret = wait_for_completion_timeout(&i2c->msg_complete,
613 i2c->adap.timeout);
614
615 /* Clear interrupt mask */
616 writew(~(restart_flag | I2C_HS_NACKERR | I2C_ACKERR |
617 I2C_TRANSAC_COMP), i2c->base + OFFSET_INTR_MASK);
618
619 if (i2c->op == I2C_MASTER_WR) {
620 dma_unmap_single(i2c->dev, wpaddr,
621 msgs->len, DMA_TO_DEVICE);
622
623 i2c_release_dma_safe_msg_buf(msgs, dma_wr_buf);
624 } else if (i2c->op == I2C_MASTER_RD) {
625 dma_unmap_single(i2c->dev, rpaddr,
626 msgs->len, DMA_FROM_DEVICE);
627
628 i2c_release_dma_safe_msg_buf(msgs, dma_rd_buf);
629 } else {
630 dma_unmap_single(i2c->dev, wpaddr, msgs->len,
631 DMA_TO_DEVICE);
632 dma_unmap_single(i2c->dev, rpaddr, (msgs + 1)->len,
633 DMA_FROM_DEVICE);
634
> 635 i2c_release_dma_safe_msg_buf(msgs, dma_wr_buf);
> 636 i2c_release_dma_safe_msg_buf((msgs + 1), dma_rd_buf);
637 }
638
639 if (ret == 0) {
640 dev_dbg(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr);
641 mtk_i2c_init_hw(i2c);
642 return -ETIMEDOUT;
643 }
644
645 completion_done(&i2c->msg_complete);
646
647 if (i2c->irq_stat & (I2C_HS_NACKERR | I2C_ACKERR)) {
648 dev_dbg(i2c->dev, "addr: %x, transfer ACK error\n", msgs->addr);
649 mtk_i2c_init_hw(i2c);
650 return -ENXIO;
651 }
652
653 return 0;
654 }
655

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (9.58 kB)
.config.gz (52.76 kB)
Download all attachments