Change in coreboot[master]: WP: soc/mediatek/mt8192: remove dma reset after i2c transfer
Jg Daolongzhu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WP: soc/mediatek/mt8192: remove dma reset after i2c transfer ......................................................................
WP: soc/mediatek/mt8192: remove dma reset after i2c transfer
remove dma reset after per i2c transfer
TEST=Boots correctly on MT8192P1
Signed-off-by: jg_daolongzhu jg_daolongzhu@mediatek.corp-partner.google.com Change-Id: Ie5ba7f5514ce415422e586cdfc411382c8b4a539 --- M src/soc/mediatek/common/i2c.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/47392/1
diff --git a/src/soc/mediatek/common/i2c.c b/src/soc/mediatek/common/i2c.c old mode 100644 new mode 100755 index 931e36d..ab3551d --- a/src/soc/mediatek/common/i2c.c +++ b/src/soc/mediatek/common/i2c.c @@ -215,7 +215,7 @@ /* reset the i2c controller for next i2c transfer. */ write32(®s->softreset, 0x1);
- i2c_dma_reset(dma_regs); + //i2c_dma_reset(dma_regs);
return ret_code; }
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47392
to look at the new patch set (#2).
Change subject: WP: soc/mediatek/mt8192: remove dma reset after i2c transfer ......................................................................
WP: soc/mediatek/mt8192: remove dma reset after i2c transfer
remove dma reset after per i2c transfer
TEST=Boots correctly on MT8192P1
Signed-off-by: jg_daolongzhu jg_daolongzhu@mediatek.corp-partner.google.com Change-Id: Ie5ba7f5514ce415422e586cdfc411382c8b4a539 --- M src/soc/mediatek/common/i2c.c M src/soc/mediatek/common/include/soc/i2c_common.h 2 files changed, 38 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/47392/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47392
to look at the new patch set (#3).
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
WIP: soc/mediatek/mt8192: modify i2c and dma reset flow
modify i2c and dma reset flow
TEST=Boots correctly on MT8192P1
Signed-off-by: jg_daolongzhu jg_daolongzhu@mediatek.corp-partner.google.com Change-Id: Ie5ba7f5514ce415422e586cdfc411382c8b4a539 --- M src/soc/mediatek/common/i2c.c M src/soc/mediatek/common/include/soc/i2c_common.h 2 files changed, 38 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/47392/3
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Patch Set 3:
(11 comments)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... File src/soc/mediatek/common/i2c.c:
PS3: Please remove execute permission.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 13: inline I think we don't necessarily need this to be inline.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 15: struct const struct
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 16: struct const struct
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 17: Check bus < ARRAY_SIZE(mtk_i2c_bus_controller)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 26: ( No need for parentheses.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 27: ( No need for parentheses.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 35: udelay(50); Why do we need 50us delay here, but only 10us for I2C_APDMA_ASYNC?
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/inc... File src/soc/mediatek/common/include/soc/i2c_common.h:
PS3: Please remove execute permission.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/inc... PS3, Line 67: r R
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/inc... PS3, Line 74: i2c Take the chance to fix this to "I2C"?
yongqiang niu has uploaded a new patch set (#4) to the change originally created by Jg Daolongzhu. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
WIP: soc/mediatek/mt8192: modify i2c and dma reset flow
modify i2c and dma reset flow
TEST=Boots correctly on MT8192P1
Signed-off-by: jg_daolongzhu jg_daolongzhu@mediatek.corp-partner.google.com Change-Id: Ie5ba7f5514ce415422e586cdfc411382c8b4a539 --- M src/soc/mediatek/common/i2c.c M src/soc/mediatek/common/include/soc/i2c_common.h 2 files changed, 38 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/47392/4
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... File src/soc/mediatek/common/i2c.c:
PS3:
Please remove execute permission.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/inc... File src/soc/mediatek/common/include/soc/i2c_common.h:
PS3:
Please remove execute permission.
Done
wang qii has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Patch Set 6:
(7 comments)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... File src/soc/mediatek/common/i2c.c:
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 13: inline
I think we don't necessarily need this to be inline.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 15: struct
const struct
In order to maintain consistency with other references, const is not used in particular, and it may be possible to add another patch for uniform modification.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 16: struct
const struct
In order to maintain consistency with other references, const is not used in particular, and it may be possible to add another patch for uniform modification.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 17:
Check bus < ARRAY_SIZE(mtk_i2c_bus_controller)
mtk_i2c_bus_init has used it.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 26: (
No need for parentheses.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 27: (
No need for parentheses.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 35: udelay(50);
Why do we need 50us delay here, but only 10us for I2C_APDMA_ASYNC?
That’s only required by the HW designers
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Patch Set 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... File src/soc/mediatek/common/i2c.c:
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 13: inline
Done
Not done.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 15: struct
In order to maintain consistency with other references, const is not used in particular, and it may […]
Ack
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 16: struct
In order to maintain consistency with other references, const is not used in particular, and it may […]
Ack
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 17:
mtk_i2c_bus_init has used it.
What do you mean?
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 35: udelay(50);
That’s only required by the HW designers
Ack
Jg Daolongzhu has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Abandoned
-
Jg Daolongzhu (Code Review) -
wang qii (Code Review)
-
yongqiang niu (Code Review)
-
Yu-Ping Wu (Code Review)