[PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[Boris Lysov]

Example uses values for MT6589 SoC, but MT6577 was specified in "compatible" property.

Signed-off-by: Boris Lysov <arzamas-16@mail.ee>
---
 Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
index 7f0194fdd0cc..acf3d4d28b98 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
@@ -36,7 +36,7 @@ Optional properties:
 Example:
 
 	i2c0: i2c@1100d000 {
-			compatible = "mediatek,mt6577-i2c";
+			compatible = "mediatek,mt6589-i2c";
 			reg = <0x1100d000 0x70>,
 			      <0x11000300 0x80>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_LOW>;
-- 
2.28.0

Re: [PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[Qii Wang]

On Fri, 2020-09-04 at 22:33 +0300, Boris Lysov wrote:
> Example uses values for MT6589 SoC, but MT6577 was specified in "compatible" property.
> 

Why do you think the example is MT6589 SoC, not MT6577?

> Signed-off-by: Boris Lysov <arzamas-16@mail.ee>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
> index 7f0194fdd0cc..acf3d4d28b98 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
> @@ -36,7 +36,7 @@ Optional properties:
>  Example:
>  
>  	i2c0: i2c@1100d000 {
> -			compatible = "mediatek,mt6577-i2c";
> +			compatible = "mediatek,mt6589-i2c";
>  			reg = <0x1100d000 0x70>,
>  			      <0x11000300 0x80>;
>  			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_LOW>;

Re: [PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[Boris Lysov]

On Mon, 7 Sep 2020 09:28:07 +0800
Qii Wang <qii.wang@mediatek.com> wrote:

> On Fri, 2020-09-04 at 22:33 +0300, Boris Lysov wrote:
> > Example uses values for MT6589 SoC, but MT6577 was specified in "compatible" property.
> >   
> 
> Why do you think the example is MT6589 SoC, not MT6577?
> 

The best way to explain why it's for MT6589 instead of MT6577 is to provide
an example ;) I will refer to various downstream Linux kernel sources, I hope
sharing GitHub links is appropriate.


This is the kernel source code of Lenovo P780 (MT6589)
https://github.com/andreya108/bindu-kernel-mediatek

mediatek/platform/mt6589/kernel/core/include/mach/mt_reg_base.h , line 115:
> #define I2C0_BASE                  0xF100D000
This address is virtual, and it translates into physical address 0x1100D000
0x1100D000 equals to the value in example

mediatek/platform/mt6589/kernel/core/mt_devs.c , line 846:
> .end    = IO_VIRT_TO_PHYS(I2C0_BASE) + 0x70,
0x70 shows length of memory region, which also equals to the value in example

mediatek/platform/mt6589/kernel/drivers/i2c/i2c.c , line 1140:
> i2c->pdmabase = AP_DMA_BASE + 0x300 + (0x80*(i2c->id));
For id=0, physical pdmabase is 0x11000300 which also matches the address in example

mediatek/platform/mt6589/kernel/core/include/mach/mt_irq.h , line 29:
> #define MT_I2C0_IRQ_ID                      (GIC_PRIVATE_SIGNALS + 44)
The IRQ ID (44) equals to one specified in dt-binding example:
> interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_LOW>;

These values are same for other MT6589 devices, here are few other repositories
containing same lines of code as above (line numbers might be off for a few lines,
but the values I am referring to are same):
Micromax A116 (MT6589) - https://github.com/neomanu/NeoKernel-MT6589-A116
Acer V370 (MT6589) - https://github.com/Shr3ps/android_kernel_acer_V370_MT6589
bq Aquaris 5.7 (MT6589) - https://github.com/luckasfb/aquaris-5.7


Now lets take a look at MT6577 devices. This is the kernel source code of ZTE v970:
https://github.com/dragonpt/Kernel_3.4.67_KK_ZTE_v970

mediatek/platform/mt6589/kernel/core/include/mach/mt_reg_base.h , line 68:
> #define I2C0_BASE 0xF1012000
This address is virtual, and it translates into physical address 0xC1012000
0xC1012000 does not equal to 0x1100d000 listed in example!

mediatek/platform/mt6577/kernel/drivers/i2c/i2c.c
No mentions of pdmabase. There are no DMA addresses in this file!

mediatek/platform/mt6577/kernel/core/include/mach/mt_irq.h , line 70:
> #define MT_I2C0_IRQ_ID                  (GIC_PRIVATE_SIGNALS + 49)
The IRQ ID (49) does not match the ID (44) specified in example!

Other MT6577 kernels with same values:
Acer V360 (MT6577) - https://github.com/aquila-dev/mt6577_kernel3.4
Acer C10 (MT6577) - https://github.com/Dr-Shadow/android_kernel_acer_c10
Wiko Cink Slim (MT6577) - https://github.com/theboleslaw/kernel_wiko_s8073


As you can see, current dt-binding example represents MT6589 SoC values, not MT6577.
I have sent additional email about i2c-mt65xx and MT6577 compatibility, where
I voiced my concerns about I2C DMA on that SoC. I hope you can look into it.

Re: [PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[Qii Wang]

On Mon, 2020-09-07 at 18:08 +0300, Boris Lysov wrote:
> On Mon, 7 Sep 2020 09:28:07 +0800
> Qii Wang <qii.wang@mediatek.com> wrote:
> 
> > On Fri, 2020-09-04 at 22:33 +0300, Boris Lysov wrote:
> > > Example uses values for MT6589 SoC, but MT6577 was specified in "compatible" property.
> > >   
> > 
> > Why do you think the example is MT6589 SoC, not MT6577?
> > 
> 
> The best way to explain why it's for MT6589 instead of MT6577 is to provide
> an example ;) I will refer to various downstream Linux kernel sources, I hope
> sharing GitHub links is appropriate.
> 
> 
> This is the kernel source code of Lenovo P780 (MT6589)
> https://github.com/andreya108/bindu-kernel-mediatek
> 
> mediatek/platform/mt6589/kernel/core/include/mach/mt_reg_base.h , line 115:
> > #define I2C0_BASE                  0xF100D000
> This address is virtual, and it translates into physical address 0x1100D000
> 0x1100D000 equals to the value in example
> 
> mediatek/platform/mt6589/kernel/core/mt_devs.c , line 846:
> > .end    = IO_VIRT_TO_PHYS(I2C0_BASE) + 0x70,
> 0x70 shows length of memory region, which also equals to the value in example
> 
> mediatek/platform/mt6589/kernel/drivers/i2c/i2c.c , line 1140:
> > i2c->pdmabase = AP_DMA_BASE + 0x300 + (0x80*(i2c->id));
> For id=0, physical pdmabase is 0x11000300 which also matches the address in example
> 
> mediatek/platform/mt6589/kernel/core/include/mach/mt_irq.h , line 29:
> > #define MT_I2C0_IRQ_ID                      (GIC_PRIVATE_SIGNALS + 44)
> The IRQ ID (44) equals to one specified in dt-binding example:
> > interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_LOW>;
> 
> These values are same for other MT6589 devices, here are few other repositories
> containing same lines of code as above (line numbers might be off for a few lines,
> but the values I am referring to are same):
> Micromax A116 (MT6589) - https://github.com/neomanu/NeoKernel-MT6589-A116
> Acer V370 (MT6589) - https://github.com/Shr3ps/android_kernel_acer_V370_MT6589
> bq Aquaris 5.7 (MT6589) - https://github.com/luckasfb/aquaris-5.7
> 
> 
> Now lets take a look at MT6577 devices. This is the kernel source code of ZTE v970:
> https://github.com/dragonpt/Kernel_3.4.67_KK_ZTE_v970
> 
> mediatek/platform/mt6589/kernel/core/include/mach/mt_reg_base.h , line 68:
> > #define I2C0_BASE 0xF1012000
> This address is virtual, and it translates into physical address 0xC1012000
> 0xC1012000 does not equal to 0x1100d000 listed in example!
> 
> mediatek/platform/mt6577/kernel/drivers/i2c/i2c.c
> No mentions of pdmabase. There are no DMA addresses in this file!
> 
> mediatek/platform/mt6577/kernel/core/include/mach/mt_irq.h , line 70:
> > #define MT_I2C0_IRQ_ID                  (GIC_PRIVATE_SIGNALS + 49)
> The IRQ ID (49) does not match the ID (44) specified in example!
> 
> Other MT6577 kernels with same values:
> Acer V360 (MT6577) - https://github.com/aquila-dev/mt6577_kernel3.4
> Acer C10 (MT6577) - https://github.com/Dr-Shadow/android_kernel_acer_c10
> Wiko Cink Slim (MT6577) - https://github.com/theboleslaw/kernel_wiko_s8073
> 
> 
> As you can see, current dt-binding example represents MT6589 SoC values, not MT6577.
> I have sent additional email about i2c-mt65xx and MT6577 compatibility, where
> I voiced my concerns about I2C DMA on that SoC. I hope you can look into it.

None of the examples you cited are the upstream code of our official
release, and the name of customer's SOC cannot be accurately evaluated. 

Re: [PATCH] dt-bindings: i2c: i2c-mt65xx: Update binding example

[Boris Lysov]

On Mon, 14 Sep 2020 10:09:46 +0800
Qii Wang <qii.wang@mediatek.com> wrote:
> None of the examples you cited are the upstream code of our official
> release

Where can I access the official upstream release for both MT6577 and MT6589? Me and other developers would certainly like to research it.

> , and the name of customer's SOC cannot be accurately evaluated. 

I'm sorry, but I don't understand what do you mean by "customer's SOC". I own a device with MT6577, and in all source code bundles I had listed in the previous message the SoCs are either MT6577 or MT6589.

As of now, the code in the example of i2c-mt65xx driver documentation is declared compatible with MT6577, but it clearly does *not* work on actual MT6577 SoC.