| Message ID | 1548057574-8061-6-git-send-email-qii.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add i2c support for mt7629 and mt8183 | expand |
> + - mediatek,share-i3c: i3c controller can share i2c function.
I am not happy with this binding. There must be a better way of using
the I3C controller in I2C mode. I think it would be easier to tell if we
had an I3C driver to see how it implements I2C fallback there. Is the
I3C driver on the way?
On Tue, 2019-02-05 at 14:16 +0100, Wolfram Sang wrote: > > + - mediatek,share-i3c: i3c controller can share i2c function. > > I am not happy with this binding. There must be a better way of using > the I3C controller in I2C mode. I think it would be easier to tell if we > had an I3C driver to see how it implements I2C fallback there. Is the > I3C driver on the way? > I am very sorry for the late reply due to the Chinese New Year holiday. After confirmation, for I2C mode in I3C controller, we will push an I3C driver to implement I2C fallback there later, and we will remove "mediatek,share-i3c" for i2c controller driver in the next patch.
On Thu, Feb 14, 2019 at 09:54:28AM +0800, Qii Wang wrote: > On Tue, 2019-02-05 at 14:16 +0100, Wolfram Sang wrote: > > > + - mediatek,share-i3c: i3c controller can share i2c function. > > > > I am not happy with this binding. There must be a better way of using > > the I3C controller in I2C mode. I think it would be easier to tell if we > > had an I3C driver to see how it implements I2C fallback there. Is the > > I3C driver on the way? > > > > I am very sorry for the late reply due to the Chinese New Year holiday. No worries. I hope you enjoyed the holidays! > After confirmation, for I2C mode in I3C controller, we will push an I3C > driver to implement I2C fallback there later, and we will remove > "mediatek,share-i3c" for i2c controller driver in the next patch. The problem with DT bindings is that you need to support them forever once you introduced them. Because there might be old device trees on boards out there which need the property. So, I don't like the idea of having a temporary binding which is meant to be removed anyway. My take is that you should just send out the I3C driver which will handle the I2C support then. Adding Rob to CC in case he has a different opinion.
On Fri, 2019-02-15 at 10:02 +0100, Wolfram Sang wrote: > On Thu, Feb 14, 2019 at 09:54:28AM +0800, Qii Wang wrote: > > On Tue, 2019-02-05 at 14:16 +0100, Wolfram Sang wrote: > > > > + - mediatek,share-i3c: i3c controller can share i2c function. > > > > > > I am not happy with this binding. There must be a better way of using > > > the I3C controller in I2C mode. I think it would be easier to tell if we > > > had an I3C driver to see how it implements I2C fallback there. Is the > > > I3C driver on the way? > > > > > > > I am very sorry for the late reply due to the Chinese New Year holiday. > > No worries. I hope you enjoyed the holidays! > > > After confirmation, for I2C mode in I3C controller, we will push an I3C > > driver to implement I2C fallback there later, and we will remove > > "mediatek,share-i3c" for i2c controller driver in the next patch. > > The problem with DT bindings is that you need to support them forever > once you introduced them. Because there might be old device trees on > boards out there which need the property. So, I don't like the idea of > having a temporary binding which is meant to be removed anyway. My take > is that you should just send out the I3C driver which will handle the > I2C support then. > > Adding Rob to CC in case he has a different opinion. > Maybe you think that MT8183 SoC only has the I3C controllers. Actually, there are I2C and I3C controllers on MT8183 SoC. We will remove "mediatek,share-i3c" for I2C controller driver, so these patches are just to support the I2C controllers on MT8183 SoC. For the I3C controller we will send some new patches which are based on i3c framework directories(kernel/drivers/i3c), not based on i2c-mt65xx.c.
> Maybe you think that MT8183 SoC only has the I3C controllers. Actually, > there are I2C and I3C controllers on MT8183 SoC. We will remove > "mediatek,share-i3c" for I2C controller driver, so these patches are > just to support the I2C controllers on MT8183 SoC. For the I3C > controller we will send some new patches which are based on i3c > framework directories(kernel/drivers/i3c), not based on > i2c-mt65xx.c. Sounds perfect to me. If it turns out that the I3C driver can share things with the I2C driver, we can discuss it then. But unless we have an I3C driver and know for sure, the above strategy seems good to me.
On Sat, 2019-02-16 at 13:29 +0100, Wolfram Sang wrote: > > Maybe you think that MT8183 SoC only has the I3C controllers. Actually, > > there are I2C and I3C controllers on MT8183 SoC. We will remove > > "mediatek,share-i3c" for I2C controller driver, so these patches are > > just to support the I2C controllers on MT8183 SoC. For the I3C > > controller we will send some new patches which are based on i3c > > framework directories(kernel/drivers/i3c), not based on > > i2c-mt65xx.c. > > Sounds perfect to me. If it turns out that the I3C driver can share > things with the I2C driver, we can discuss it then. But unless we have > an I3C driver and know for sure, the above strategy seems good to me. > Can I push the V4 patches of I2C controller driver now? or you just want an I3C driver that is compatible with the I2C controller? According to our plan, I2C controller will use i2c-mt65xx.c(kernel/drivers/i2c/busses/)and I3C controller will use i3c-xxx.c(kernel/drivers/i3c/master/)
> > > Maybe you think that MT8183 SoC only has the I3C controllers. Actually, > > > there are I2C and I3C controllers on MT8183 SoC. We will remove > > > "mediatek,share-i3c" for I2C controller driver, so these patches are > > > just to support the I2C controllers on MT8183 SoC. For the I3C > > > controller we will send some new patches which are based on i3c > > > framework directories(kernel/drivers/i3c), not based on > > > i2c-mt65xx.c. > > > > Sounds perfect to me. If it turns out that the I3C driver can share > > things with the I2C driver, we can discuss it then. But unless we have > > an I3C driver and know for sure, the above strategy seems good to me. > > > > Can I push the V4 patches of I2C controller driver now? or you just want > an I3C driver that is compatible with the I2C controller? If the V4 series does not use the "mediatek,share-i3c" binding anymore, please resend it. From what I recall, it should be OK then. > According to our plan, I2C controller will use > i2c-mt65xx.c(kernel/drivers/i2c/busses/)and I3C controller will use > i3c-xxx.c(kernel/drivers/i3c/master/) Sounds OK from what I understood. The I2C fallback from I3C will be interesting.
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mtk.txt b/Documentation/devicetree/bindings/i2c/i2c-mtk.txt index ee4c324..0d29a5b 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mtk.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-mtk.txt @@ -12,14 +12,15 @@ Required properties: "mediatek,mt7623-i2c", "mediatek,mt6577-i2c": for MediaTek MT7623 "mediatek,mt7629-i2c", "mediatek,mt2712-i2c": for MediaTek MT7629 "mediatek,mt8173-i2c": for MediaTek MT8173 + "mediatek,mt8183-i2c": for MediaTek MT8183 - reg: physical base address of the controller and dma base, length of memory mapped region. - interrupts: interrupt number to the cpu. - clock-div: the fixed value for frequency divider of clock source in i2c module. Each IC may be different. - clocks: clock name from clock manager - - clock-names: Must include "main" and "dma", if enable have-pmic need include - "pmic" extra. + - clock-names: Must include "main" and "dma", "arb" is optional, if enable + have-pmic need include "pmic" extra. Optional properties: - clock-frequency: Frequency in Hz of the bus when transfer, the default value @@ -27,6 +28,7 @@ Optional properties: - mediatek,have-pmic: platform can control i2c form special pmic side. Only mt6589 and mt8135 support this feature. - mediatek,use-push-pull: IO config use push-pull mode. + - mediatek,share-i3c: i3c controller can share i2c function. Example: