Message ID | 1431967209-5261-3-git-send-email-eddie.huang@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hello, On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > From: Xudong Chen <xudong.chen@mediatek.com> > > The mediatek SoCs have I2C controller that handle I2C transfer. > This patch include common I2C bus driver. > This driver is compatible with I2C controller on mt65xx/mt81xx. > > Signed-off-by: Xudong Chen <xudong.chen@mediatek.com> > Signed-off-by: Liguo Zhang <liguo.zhang@mediatek.com> > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> > Acked-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-mt65xx.c | 675 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 685 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-mt65xx.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 2255af2..14c7266 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -620,6 +620,15 @@ config I2C_MPC > This driver can also be built as a module. If so, the module > will be called i2c-mpc. > > +config I2C_MT65XX > + tristate "MediaTek I2C adapter" > + depends on ARCH_MEDIATEK || COMPILE_TEST > + help > + This selects the MediaTek(R) Integrated Inter Circuit bus driver > + for MT65xx and MT81xx. > + If you want to use MediaTek(R) I2C interface, say Y or M here. > + If unsure, say N. > + > config I2C_MV64XXX > tristate "Marvell mv64xxx I2C Controller" > depends on MV64X60 || PLAT_ORION || ARCH_SUNXI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index cdf941d..abbf422 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -60,6 +60,7 @@ obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o > obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o > obj-$(CONFIG_I2C_MESON) += i2c-meson.o > obj-$(CONFIG_I2C_MPC) += i2c-mpc.o > +obj-$(CONFIG_I2C_MT65XX) += i2c-mt65xx.o > obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o > obj-$(CONFIG_I2C_MXS) += i2c-mxs.o > obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o > diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c > new file mode 100644 > index 0000000..7462f05 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-mt65xx.c > @@ -0,0 +1,675 @@ > +/* > + * Copyright (c) 2014 MediaTek Inc. > + * Author: Xudong.chen <xudong.chen@mediatek.com> s/Xudong.chen/Xudong Chen/ > +#define I2C_DRV_NAME "mt-i2c" i2c-mt65xx ? > +} > + > +/* calculate i2c port speed */ It would be nice to summarize the clock frequency settings here. Something like: /* * The input clock is divided by the value specified in the * device tree as clock-div. The actual bus speed is then * derived from this frequency by the following formula: * .... This would make it possible to verify your calculations below. > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > +{ > + unsigned int khz; > + unsigned int step_cnt; > + unsigned int sample_cnt; > + unsigned int sclk; > + unsigned int hclk; > + unsigned int max_step_cnt; > + unsigned int sample_div = MAX_SAMPLE_CNT_DIV; > + unsigned int step_div; > + unsigned int min_div; > + unsigned int best_mul; > + unsigned int cnt_mul; > + > + if (i2c->speed_hz > MAX_HS_MODE_SPEED) > + return -EINVAL; > + else if (i2c->speed_hz > MAX_FS_MODE_SPEED) > + max_step_cnt = MAX_HS_STEP_CNT_DIV; > + else > + max_step_cnt = MAX_STEP_CNT_DIV; > + > + step_div = max_step_cnt; > + /* Find the best combination */ > + khz = i2c->speed_hz / 1000; > + hclk = clk_src_in_hz / 1000; > + min_div = ((hclk >> 1) + khz - 1) / khz; This is DIV_ROUND_UP(hclk >> 1, khz). > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > + > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; > + cnt_mul = step_cnt * sample_cnt; > + if (step_cnt > max_step_cnt) > + continue; > + > + if (cnt_mul < best_mul) { > + best_mul = cnt_mul; > + sample_div = sample_cnt; > + step_div = step_cnt; > + if (best_mul == min_div) > + break; > + } > + } > + > + sample_cnt = sample_div; > + step_cnt = step_div; > + sclk = hclk / (2 * sample_cnt * step_cnt); > + if (sclk > khz) { > + dev_dbg(i2c->dev, "%s mode: unsupported speed (%ldkhz)\n", > + (i2c->speed_hz > MAX_HS_MODE_SPEED) ? "HS" : "ST/FT", > + (long int)khz); > + return -EINVAL; > + } > + > + step_cnt--; > + sample_cnt--; > + > + if (i2c->speed_hz > MAX_FS_MODE_SPEED) { > + /* Set the hign speed mode register */ > + i2c->timing_reg = I2C_FS_TIME_INIT_VALUE; > + i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE | > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 12 | > + (step_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8; > + } else { > + i2c->timing_reg = > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8 | > + (step_cnt & I2C_TIMING_STEP_DIV_MASK) << 0; > + /* Disable the high speed transaction */ > + i2c->high_speed_reg = I2C_TIME_CLR_VALUE; Can it happen that sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK != sample_cnt? If yes, what is the influence on correctness? Same question for step_cnt. Best regards Uwe
2015-05-18 20:43 GMT+02:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > Hello, > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: >> From: Xudong Chen <xudong.chen@mediatek.com> >> >> The mediatek SoCs have I2C controller that handle I2C transfer. >> This patch include common I2C bus driver. >> This driver is compatible with I2C controller on mt65xx/mt81xx. >> >> Signed-off-by: Xudong Chen <xudong.chen@mediatek.com> >> Signed-off-by: Liguo Zhang <liguo.zhang@mediatek.com> >> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> >> Acked-by: Sascha Hauer <s.hauer@pengutronix.de> >> --- >> drivers/i2c/busses/Kconfig | 9 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-mt65xx.c | 675 ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 685 insertions(+) >> create mode 100644 drivers/i2c/busses/i2c-mt65xx.c >> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> index 2255af2..14c7266 100644 >> --- a/drivers/i2c/busses/Kconfig >> +++ b/drivers/i2c/busses/Kconfig >> @@ -620,6 +620,15 @@ config I2C_MPC >> This driver can also be built as a module. If so, the module >> will be called i2c-mpc. >> >> +config I2C_MT65XX >> + tristate "MediaTek I2C adapter" >> + depends on ARCH_MEDIATEK || COMPILE_TEST >> + help >> + This selects the MediaTek(R) Integrated Inter Circuit bus driver >> + for MT65xx and MT81xx. >> + If you want to use MediaTek(R) I2C interface, say Y or M here. >> + If unsure, say N. >> + >> config I2C_MV64XXX >> tristate "Marvell mv64xxx I2C Controller" >> depends on MV64X60 || PLAT_ORION || ARCH_SUNXI >> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile >> index cdf941d..abbf422 100644 >> --- a/drivers/i2c/busses/Makefile >> +++ b/drivers/i2c/busses/Makefile >> @@ -60,6 +60,7 @@ obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o >> obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o >> obj-$(CONFIG_I2C_MESON) += i2c-meson.o >> obj-$(CONFIG_I2C_MPC) += i2c-mpc.o >> +obj-$(CONFIG_I2C_MT65XX) += i2c-mt65xx.o >> obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o >> obj-$(CONFIG_I2C_MXS) += i2c-mxs.o >> obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o >> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c >> new file mode 100644 >> index 0000000..7462f05 >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-mt65xx.c >> @@ -0,0 +1,675 @@ >> +/* >> + * Copyright (c) 2014 MediaTek Inc. >> + * Author: Xudong.chen <xudong.chen@mediatek.com> > s/Xudong.chen/Xudong Chen/ > >> +#define I2C_DRV_NAME "mt-i2c" > i2c-mt65xx ? As this works for all SoCs so far, I would propose: i2c-mtk
Hello Matthias, On Tue, May 19, 2015 at 04:48:23PM +0200, Matthias Brugger wrote: > 2015-05-18 20:43 GMT+02:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > >> [...] > >> drivers/i2c/busses/i2c-mt65xx.c | 675 ++++++++++++++++++++++++++++++++++++++++ > >> [...] > >> +#define I2C_DRV_NAME "mt-i2c" > > i2c-mt65xx ? > > As this works for all SoCs so far, I would propose: i2c-mtk Hmm, currently we have: - #define I2C_DRV_NAME mt-i2c - dt-compatibles: "mediatek,mt6577-i2c" + "mediatek,mt6589-i2c" - function prefix: mtk_i2c_ - filename: i2c-mt65xx.c - Kconfig symbol: I2C_MT65XX Getting some harmonization here would definitly be nice. Best regards Uwe
Hi Uwe, On Mon, 2015-05-18 at 20:43 +0200, Uwe Kleine-König wrote: > Hello, > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > From: Xudong Chen <xudong.chen@mediatek.com> > > > > The mediatek SoCs have I2C controller that handle I2C transfer. > > This patch include common I2C bus driver. > > This driver is compatible with I2C controller on mt65xx/mt81xx. > > > > Signed-off-by: Xudong Chen <xudong.chen@mediatek.com> > > Signed-off-by: Liguo Zhang <liguo.zhang@mediatek.com> > > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> > > Acked-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/i2c/busses/Kconfig | 9 + > > drivers/i2c/busses/Makefile | 1 + > > drivers/i2c/busses/i2c-mt65xx.c | 675 ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 685 insertions(+) > > create mode 100644 drivers/i2c/busses/i2c-mt65xx.c > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 2255af2..14c7266 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -620,6 +620,15 @@ config I2C_MPC > > This driver can also be built as a module. If so, the module > > will be called i2c-mpc. > > > > +config I2C_MT65XX > > + tristate "MediaTek I2C adapter" > > + depends on ARCH_MEDIATEK || COMPILE_TEST > > + help > > + This selects the MediaTek(R) Integrated Inter Circuit bus driver > > + for MT65xx and MT81xx. > > + If you want to use MediaTek(R) I2C interface, say Y or M here. > > + If unsure, say N. > > + > > config I2C_MV64XXX > > tristate "Marvell mv64xxx I2C Controller" > > depends on MV64X60 || PLAT_ORION || ARCH_SUNXI > > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > > index cdf941d..abbf422 100644 > > --- a/drivers/i2c/busses/Makefile > > +++ b/drivers/i2c/busses/Makefile > > @@ -60,6 +60,7 @@ obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o > > obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o > > obj-$(CONFIG_I2C_MESON) += i2c-meson.o > > obj-$(CONFIG_I2C_MPC) += i2c-mpc.o > > +obj-$(CONFIG_I2C_MT65XX) += i2c-mt65xx.o > > obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o > > obj-$(CONFIG_I2C_MXS) += i2c-mxs.o > > obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o > > diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c > > new file mode 100644 > > index 0000000..7462f05 > > --- /dev/null > > +++ b/drivers/i2c/busses/i2c-mt65xx.c > > @@ -0,0 +1,675 @@ > > +/* > > + * Copyright (c) 2014 MediaTek Inc. > > + * Author: Xudong.chen <xudong.chen@mediatek.com> > s/Xudong.chen/Xudong Chen/ OK > > > +#define I2C_DRV_NAME "mt-i2c" > i2c-mt65xx ? OK, i2c-mt65xx make more sense > > > +} > > + > > +/* calculate i2c port speed */ > It would be nice to summarize the clock frequency settings here. > Something like: > > /* > * The input clock is divided by the value specified in the > * device tree as clock-div. The actual bus speed is then > * derived from this frequency by the following formula: > * .... > > This would make it possible to verify your calculations below. The comment will be: /* * khz: I2C bus clock * hclk: The input clock is divided by the value specified in the * device tree as clock-div * div = (sample_cnt + 1) * (step_cnt + 1) * khz = (hclk / 2) / div * * The calculation is to get div value that let result of * ((hclk / 2) / div) most approach and less than khz */ > > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > > +{ > > + unsigned int khz; > > + unsigned int step_cnt; > > + unsigned int sample_cnt; > > + unsigned int sclk; > > + unsigned int hclk; > > + unsigned int max_step_cnt; > > + unsigned int sample_div = MAX_SAMPLE_CNT_DIV; > > + unsigned int step_div; > > + unsigned int min_div; > > + unsigned int best_mul; > > + unsigned int cnt_mul; > > + > > + if (i2c->speed_hz > MAX_HS_MODE_SPEED) > > + return -EINVAL; > > + else if (i2c->speed_hz > MAX_FS_MODE_SPEED) > > + max_step_cnt = MAX_HS_STEP_CNT_DIV; > > + else > > + max_step_cnt = MAX_STEP_CNT_DIV; > > + > > + step_div = max_step_cnt; > > + /* Find the best combination */ > > + khz = i2c->speed_hz / 1000; > > + hclk = clk_src_in_hz / 1000; > > + min_div = ((hclk >> 1) + khz - 1) / khz; > This is DIV_ROUND_UP(hclk >> 1, khz). OK, it is good to use existed macro. > > > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > > + > > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; > > + cnt_mul = step_cnt * sample_cnt; > > + if (step_cnt > max_step_cnt) > > + continue; > > + > > + if (cnt_mul < best_mul) { > > + best_mul = cnt_mul; > > + sample_div = sample_cnt; > > + step_div = step_cnt; > > + if (best_mul == min_div) > > + break; > > + } > > + } > > + > > + sample_cnt = sample_div; > > + step_cnt = step_div; > > + sclk = hclk / (2 * sample_cnt * step_cnt); > > + if (sclk > khz) { > > + dev_dbg(i2c->dev, "%s mode: unsupported speed (%ldkhz)\n", > > + (i2c->speed_hz > MAX_HS_MODE_SPEED) ? "HS" : "ST/FT", > > + (long int)khz); > > + return -EINVAL; > > + } > > + > > + step_cnt--; > > + sample_cnt--; > > + > > + if (i2c->speed_hz > MAX_FS_MODE_SPEED) { > > + /* Set the hign speed mode register */ > > + i2c->timing_reg = I2C_FS_TIME_INIT_VALUE; > > + i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE | > > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 12 | > > + (step_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8; > > + } else { > > + i2c->timing_reg = > > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8 | > > + (step_cnt & I2C_TIMING_STEP_DIV_MASK) << 0; > > + /* Disable the high speed transaction */ > > + i2c->high_speed_reg = I2C_TIME_CLR_VALUE; > Can it happen that sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK != > sample_cnt? If yes, what is the influence on correctness? Same question > for step_cnt. No. it should not happen because we already limit value less than max value. Will remove mask here. Eddie
On Tue, 2015-05-19 at 16:48 +0200, Matthias Brugger wrote: > 2015-05-18 20:43 GMT+02:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: <...> > >> --- /dev/null > >> +++ b/drivers/i2c/busses/i2c-mt65xx.c > >> @@ -0,0 +1,675 @@ > >> +/* > >> + * Copyright (c) 2014 MediaTek Inc. > >> + * Author: Xudong.chen <xudong.chen@mediatek.com> > > s/Xudong.chen/Xudong Chen/ > > > >> +#define I2C_DRV_NAME "mt-i2c" > > i2c-mt65xx ? > > As this works for all SoCs so far, I would propose: i2c-mtk Hi, I'm pretty sure this driver won't work for MTK TV socs(mt53xx) and BDP mt8580, they are using different I2C IP. They are not working on upstream now, but I think using i2c-mt65xx make more sense here. Joe.C
Hi Uwe, On Tue, 2015-05-19 at 21:49 +0200, Uwe Kleine-König wrote: > Hello Matthias, > > On Tue, May 19, 2015 at 04:48:23PM +0200, Matthias Brugger wrote: > > 2015-05-18 20:43 GMT+02:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > >> [...] > > >> drivers/i2c/busses/i2c-mt65xx.c | 675 ++++++++++++++++++++++++++++++++++++++++ > > >> [...] > > >> +#define I2C_DRV_NAME "mt-i2c" > > > i2c-mt65xx ? > > > > As this works for all SoCs so far, I would propose: i2c-mtk > Hmm, currently we have: > > - #define I2C_DRV_NAME mt-i2c Change to i2c-mt65xx. > - dt-compatibles: "mediatek,mt6577-i2c" + "mediatek,mt6589-i2c" We hope all mobile chip series use the same I2C driver. I don't list all chips in driver to avoid too many dt-compatibles. Only list chips that are different from others. > - function prefix: mtk_i2c_ Maybe use i2c-mt65xx_ is better, but I tend to keep mtk_i2c_ after these review rounds. > - filename: i2c-mt65xx.c > - Kconfig symbol: I2C_MT65XX > > Getting some harmonization here would definitly be nice. > Eddie Huang
Hello Eddie, On Wed, May 20, 2015 at 10:40:11AM +0800, Eddie Huang wrote: > On Mon, 2015-05-18 at 20:43 +0200, Uwe Kleine-König wrote: > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > > +/* calculate i2c port speed */ > > It would be nice to summarize the clock frequency settings here. > > Something like: > > > > /* > > * The input clock is divided by the value specified in the > > * device tree as clock-div. The actual bus speed is then > > * derived from this frequency by the following formula: > > * .... > > > > This would make it possible to verify your calculations below. > > The comment will be: > /* > * khz: I2C bus clock > * hclk: The input clock is divided by the value specified in the > * device tree as clock-div and which one of the two clocks you're writing about is hclk now? I assume the divided one. > * div = (sample_cnt + 1) * (step_cnt + 1) > * khz = (hclk / 2) / div khz for the 2nd time. > * > * The calculation is to get div value that let result of > * ((hclk / 2) / div) most approach and less than khz > */ I imagined something more hardware related. A list of register (or register bit fields) that influence the frequency and a formula i2c_freq = parent_clk / clock-div * (...) (It seems to be a bit more complicated here as there are two registers involved that are set differently depending on the target frequency.) > > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) clk_src_in_hz is the module's input rate already divided by clock-div. This clock-div value is fixed in hardware and unchangeable, right? Maybe give that divided clock a nice name? The target frequency is i2c->speed_hz, so among the possible frequencies we want to pick the highest one that is still less than or equal i2c->speed_hz, right? > > > + /* Set the hign speed mode register */ I just notice s/hign/high/ here. Best regards Uwe
Hi Uwe, On Wed, 2015-05-20 at 09:11 +0200, Uwe Kleine-König wrote: > Hello Eddie, > > On Wed, May 20, 2015 at 10:40:11AM +0800, Eddie Huang wrote: > > On Mon, 2015-05-18 at 20:43 +0200, Uwe Kleine-König wrote: > > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > > > +/* calculate i2c port speed */ > > > It would be nice to summarize the clock frequency settings here. > > > Something like: > > > > > > /* > > > * The input clock is divided by the value specified in the > > > * device tree as clock-div. The actual bus speed is then > > > * derived from this frequency by the following formula: > > > * .... > > > > > > This would make it possible to verify your calculations below. > > > > The comment will be: > > /* > > * khz: I2C bus clock > > * hclk: The input clock is divided by the value specified in the > > * device tree as clock-div > and which one of the two clocks you're writing about is hclk now? I > assume the divided one. > > * div = (sample_cnt + 1) * (step_cnt + 1) > > * khz = (hclk / 2) / div > khz for the 2nd time. > > > * > > * The calculation is to get div value that let result of > > * ((hclk / 2) / div) most approach and less than khz > > */ > I imagined something more hardware related. A list of register (or > register bit fields) that influence the frequency and a formula > i2c_freq = parent_clk / clock-div * (...) > > (It seems to be a bit more complicated here as there are two registers > involved that are set differently depending on the target frequency.) Yes, hardware is a little complicated. I rewrite the comment: /* * Calculate i2c port speed * * Hardware design: * i2c_bus_freq = parent_clk / (clock-div * 2 * (sample_cnt) * (step_cnt)) * clock-div: fixed in hardware, but may be various in different SoCs * * The calculation want to pick the highest bus frequency that is still * less than or equal to i2c->speed_hz. and the calculation try to get * sample_cnt and step_cnt to fill in hardware register. */ > > > > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > clk_src_in_hz is the module's input rate already divided by clock-div. > This clock-div value is fixed in hardware and unchangeable, right? Yes > Maybe give that divided clock a nice name? I don't know, after new comment, maybe this name is ok. > The target frequency is i2c->speed_hz, so among the possible frequencies > we want to pick the highest one that is still less than or equal > i2c->speed_hz, right? Right. > > > > > + /* Set the hign speed mode register */ > I just notice s/hign/high/ here. > Thanks, will fix it. Eddie
Hello Eddie, On Wed, May 20, 2015 at 03:59:35PM +0800, Eddie Huang wrote: > On Wed, 2015-05-20 at 09:11 +0200, Uwe Kleine-König wrote: > > On Wed, May 20, 2015 at 10:40:11AM +0800, Eddie Huang wrote: > > > On Mon, 2015-05-18 at 20:43 +0200, Uwe Kleine-König wrote: > > > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > > > > +/* calculate i2c port speed */ > > > > It would be nice to summarize the clock frequency settings here. > > > > Something like: > > > > > > > > /* > > > > * The input clock is divided by the value specified in the > > > > * device tree as clock-div. The actual bus speed is then > > > > * derived from this frequency by the following formula: > > > > * .... > > > > > > > > This would make it possible to verify your calculations below. > > > > > > The comment will be: > > > /* > > > * khz: I2C bus clock > > > * hclk: The input clock is divided by the value specified in the > > > * device tree as clock-div > > and which one of the two clocks you're writing about is hclk now? I > > assume the divided one. > > > * div = (sample_cnt + 1) * (step_cnt + 1) > > > * khz = (hclk / 2) / div > > khz for the 2nd time. > > > > > * > > > * The calculation is to get div value that let result of > > > * ((hclk / 2) / div) most approach and less than khz > > > */ > > I imagined something more hardware related. A list of register (or > > register bit fields) that influence the frequency and a formula > > > i2c_freq = parent_clk / clock-div * (...) > > > > (It seems to be a bit more complicated here as there are two registers > > involved that are set differently depending on the target frequency.) > > Yes, hardware is a little complicated. I rewrite the comment: > > /* > * Calculate i2c port speed > * > * Hardware design: > * i2c_bus_freq = parent_clk / (clock-div * 2 * (sample_cnt) * > (step_cnt)) > * clock-div: fixed in hardware, but may be various in different SoCs ... fixed in hardware, but different on different SoCs. > * > * The calculation want to pick the highest bus frequency that is still > * less than or equal to i2c->speed_hz. and the calculation try to get > * sample_cnt and step_cnt to fill in hardware register. "The calculation picks the highest bus frequency that is still less than or equal to i2c->speed_hz." and I'd drop the last sentence. With that in mind I'll reply once more to the original patch. Best regards Uwe
Hello, now that I understood the formula some more comments to the calculation. On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > +#define I2C_DEFAUT_SPEED 100000 /* hz */ DEFAULT? > +#define MAX_FS_MODE_SPEED 400000 > +#define MAX_HS_MODE_SPEED 3400000 > +#define MAX_SAMPLE_CNT_DIV 8 > +#define MAX_STEP_CNT_DIV 64 > +#define MAX_HS_STEP_CNT_DIV 8 > [...] > +/* calculate i2c port speed */ > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > +{ add a comment here, that clk_src_in_hz is the parent clock already divided by clock-div. > + unsigned int khz; > + unsigned int step_cnt; > + unsigned int sample_cnt; > + unsigned int sclk; > + unsigned int hclk; > + unsigned int max_step_cnt; > + unsigned int sample_div = MAX_SAMPLE_CNT_DIV; > + unsigned int step_div; > + unsigned int min_div; > + unsigned int best_mul; > + unsigned int cnt_mul; > + > + if (i2c->speed_hz > MAX_HS_MODE_SPEED) > + return -EINVAL; According to the plan to tune for the highest possible rate <= i2c->speed_hz, you should handle the case (i2c->speed_hz > MAX_HS_MODE_SPEED) like i2c->speed_hz == MAX_HS_MODE_SPEED. Well, you might want to prevent an overflow in the calculation below however. > + else if (i2c->speed_hz > MAX_FS_MODE_SPEED) > + max_step_cnt = MAX_HS_STEP_CNT_DIV; > + else > + max_step_cnt = MAX_STEP_CNT_DIV; So I assume this is the hardware limit on the step_cnt value. For FS_MODE and below you have 6 bits and writing X corresponds to step_cnt = X + 1. For HS_MODE there are only 3 bits. right? > + step_div = max_step_cnt; > + /* Find the best combination */ > + khz = i2c->speed_hz / 1000; > + hclk = clk_src_in_hz / 1000; Why are you dividing here? There shouldn't be an overflow problem and you're loosing precision. > + min_div = ((hclk >> 1) + khz - 1) / khz; The shift accounts for the fixed divider 2 in i2c_bus_freq = parent_clk / (clock-div * 2 * sample_cnt * step_cnt ? Maybe better call this opt_div instead of min_div? So now we're searching for the best pair (sample_cnt, step_cnt) with: * 0 < sample_cnt < MAX_SAMPLE_CNT_DIV * 0 < step_cnt < max_step_cnt * sample_cnt * step_cnt >= min_div * optimizing for sample_cnt * step_cnt being minimal Right? > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > + > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; DIV_ROUND_UP > + cnt_mul = step_cnt * sample_cnt; > + if (step_cnt > max_step_cnt) > + continue; I think it can happen that you have step_cnt > max_step_cnt here, but that (sample_cnt, max_step_cnt) still is a good pair to consider. So: step_cnt = DIV_ROUND_UP(min_div, sample_cnt); if (step_cnt > max_step_cnt) step_cnt = max_step_cnt; cnt_mul = step_cnt * sample_cnt; > + > + if (cnt_mul < best_mul) { > + best_mul = cnt_mul; > + sample_div = sample_cnt; > + step_div = step_cnt; I'd call these best_sample_cnt and best_step_cnt instead of sample_div and step_div. > + if (best_mul == min_div) > + break; > + } > + } > + > + sample_cnt = sample_div; > + step_cnt = step_div; > + sclk = hclk / (2 * sample_cnt * step_cnt); > + if (sclk > khz) { Can this happen? A better name for "sclk" would be "bus_freq"? > + dev_dbg(i2c->dev, "%s mode: unsupported speed (%ldkhz)\n", > + (i2c->speed_hz > MAX_HS_MODE_SPEED) ? "HS" : "ST/FT", What is ST/FR? I would have expected FS here. > + (long int)khz); > + return -EINVAL; > + } > + > + step_cnt--; > + sample_cnt--; > + > + if (i2c->speed_hz > MAX_FS_MODE_SPEED) { > + /* Set the hign speed mode register */ > + i2c->timing_reg = I2C_FS_TIME_INIT_VALUE; > + i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE | > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 12 | > + (step_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8; > + } else { > + i2c->timing_reg = > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8 | > + (step_cnt & I2C_TIMING_STEP_DIV_MASK) << 0; > + /* Disable the high speed transaction */ > + i2c->high_speed_reg = I2C_TIME_CLR_VALUE; > + } Would it be sensible to write these values directly into hardware here? Best regards Uwe
Hi Uwe, On Wed, 2015-05-20 at 10:57 +0200, Uwe Kleine-König wrote: > Hello, > > now that I understood the formula some more comments to the calculation. > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > +#define I2C_DEFAUT_SPEED 100000 /* hz */ > DEFAULT? > > > +#define MAX_FS_MODE_SPEED 400000 > > +#define MAX_HS_MODE_SPEED 3400000 > > +#define MAX_SAMPLE_CNT_DIV 8 > > +#define MAX_STEP_CNT_DIV 64 > > +#define MAX_HS_STEP_CNT_DIV 8 > > [...] > > +/* calculate i2c port speed */ > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > > +{ > add a comment here, that clk_src_in_hz is the parent clock already > divided by clock-div. > > > + unsigned int khz; > > + unsigned int step_cnt; > > + unsigned int sample_cnt; > > + unsigned int sclk; > > + unsigned int hclk; > > + unsigned int max_step_cnt; > > + unsigned int sample_div = MAX_SAMPLE_CNT_DIV; > > + unsigned int step_div; > > + unsigned int min_div; > > + unsigned int best_mul; > > + unsigned int cnt_mul; > > + > > + if (i2c->speed_hz > MAX_HS_MODE_SPEED) > > + return -EINVAL; > According to the plan to tune for the highest possible rate <= > i2c->speed_hz, you should handle the case (i2c->speed_hz > > MAX_HS_MODE_SPEED) like i2c->speed_hz == MAX_HS_MODE_SPEED. > Well, you might want to prevent an overflow in the calculation below > however. The check here means we don't support speed > MAX_HS_MODE_SPEED. This is different then slightly slower bus speed due to rounding error. > > + else if (i2c->speed_hz > MAX_FS_MODE_SPEED) > > + max_step_cnt = MAX_HS_STEP_CNT_DIV; > > + else > > + max_step_cnt = MAX_STEP_CNT_DIV; > So I assume this is the hardware limit on the step_cnt value. For > FS_MODE and below you have 6 bits and writing X corresponds to > step_cnt = X + 1. For HS_MODE there are only 3 bits. right? Yes, correct. > > + step_div = max_step_cnt; > > + /* Find the best combination */ > > + khz = i2c->speed_hz / 1000; > > + hclk = clk_src_in_hz / 1000; > Why are you dividing here? There shouldn't be an overflow problem and > you're loosing precision. Agreed, they should be removed. > > + min_div = ((hclk >> 1) + khz - 1) / khz; > The shift accounts for the fixed divider 2 in > > i2c_bus_freq = parent_clk / (clock-div * 2 * sample_cnt * step_cnt > > ? Maybe better call this opt_div instead of min_div? So now we're > searching for the best pair (sample_cnt, step_cnt) with: > > * 0 < sample_cnt < MAX_SAMPLE_CNT_DIV > * 0 < step_cnt < max_step_cnt > * sample_cnt * step_cnt >= min_div > * optimizing for sample_cnt * step_cnt being minimal > > Right? Yes. > > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > > + > > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; > DIV_ROUND_UP > > > + cnt_mul = step_cnt * sample_cnt; > > + if (step_cnt > max_step_cnt) > > + continue; > I think it can happen that you have step_cnt > max_step_cnt here, but > that (sample_cnt, max_step_cnt) still is a good pair to consider. So: If step_cnt > max_step_cnt, then sample_cnt * max_step_cnt < min_div. This means (sample_cnt, max_step_cnt) is not a valid. Joe.C
Hello, On Wed, May 20, 2015 at 09:03:40PM +0800, Yingjoe Chen wrote: > On Wed, 2015-05-20 at 10:57 +0200, Uwe Kleine-König wrote: > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > > + if (i2c->speed_hz > MAX_HS_MODE_SPEED) > > > + return -EINVAL; > > According to the plan to tune for the highest possible rate <= > > i2c->speed_hz, you should handle the case (i2c->speed_hz > > > MAX_HS_MODE_SPEED) like i2c->speed_hz == MAX_HS_MODE_SPEED. > > Well, you might want to prevent an overflow in the calculation below > > however. > > The check here means we don't support speed > MAX_HS_MODE_SPEED. This is > different then slightly slower bus speed due to rounding error. Well from outside there no difference between asking for 100 with getting 98 or asking for 405 with getting 400. IMHO the expectation is that you set the maximal possible rate when something too big for you is requested. Consider a communication with a i2c device that supports 600 kHz. You have the choice to communicate with 400 kHz with that or not at all. > > > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > > > + > > > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > > > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; > > DIV_ROUND_UP > > > > > + cnt_mul = step_cnt * sample_cnt; > > > + if (step_cnt > max_step_cnt) > > > + continue; > > I think it can happen that you have step_cnt > max_step_cnt here, but > > that (sample_cnt, max_step_cnt) still is a good pair to consider. So: > > If step_cnt > max_step_cnt, then sample_cnt * max_step_cnt < min_div. > This means (sample_cnt, max_step_cnt) is not a valid. You're right. Best regards Uwe
Hi, Please see my reply below (I skip comments that already reply in another mail). On Wed, 2015-05-20 at 10:57 +0200, Uwe Kleine-König wrote: > Hello, > > now that I understood the formula some more comments to the calculation. > > On Tue, May 19, 2015 at 12:40:08AM +0800, Eddie Huang wrote: > > +#define I2C_DEFAUT_SPEED 100000 /* hz */ > DEFAULT? > Yes, will fix. > > +#define MAX_FS_MODE_SPEED 400000 > > +#define MAX_HS_MODE_SPEED 3400000 > > +#define MAX_SAMPLE_CNT_DIV 8 > > +#define MAX_STEP_CNT_DIV 64 > > +#define MAX_HS_STEP_CNT_DIV 8 > > [...] > > +/* calculate i2c port speed */ > > +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) > > +{ > add a comment here, that clk_src_in_hz is the parent clock already > divided by clock-div. > We move parent_clk div clock-div in mtk_i2c_set_speed function, I think this is more clear. > > + step_div = max_step_cnt; > > + /* Find the best combination */ > > + khz = i2c->speed_hz / 1000; > > + hclk = clk_src_in_hz / 1000; > Why are you dividing here? There shouldn't be an overflow problem and > you're loosing precision. OK, will remove div 1000. > > > + min_div = ((hclk >> 1) + khz - 1) / khz; > The shift accounts for the fixed divider 2 in > > i2c_bus_freq = parent_clk / (clock-div * 2 * sample_cnt * step_cnt > > ? Maybe better call this opt_div instead of min_div? OK > > > + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; > > + > > + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { > > + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; > DIV_ROUND_UP OK > > + > > + if (cnt_mul < best_mul) { > > + best_mul = cnt_mul; > > + sample_div = sample_cnt; > > + step_div = step_cnt; > I'd call these best_sample_cnt and best_step_cnt instead of sample_div > and step_div. OK > > > + if (best_mul == min_div) > > + break; > > + } > > + } > > + > > + sample_cnt = sample_div; > > + step_cnt = step_div; > > + sclk = hclk / (2 * sample_cnt * step_cnt); > > + if (sclk > khz) { > Can this happen? A better name for "sclk" would be "bus_freq"? Yes, if i2c->speed_hz is too small, not able to get target_speed using hardware div. > > > + dev_dbg(i2c->dev, "%s mode: unsupported speed (%ldkhz)\n", > > + (i2c->speed_hz > MAX_HS_MODE_SPEED) ? "HS" : "ST/FT", > What is ST/FR? I would have expected FS here. Please skip it.The debug message is too lousy. > > > + (long int)khz); > > + return -EINVAL; > > + } > > + > > + step_cnt--; > > + sample_cnt--; > > + > > + if (i2c->speed_hz > MAX_FS_MODE_SPEED) { > > + /* Set the hign speed mode register */ > > + i2c->timing_reg = I2C_FS_TIME_INIT_VALUE; > > + i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE | > > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 12 | > > + (step_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8; > > + } else { > > + i2c->timing_reg = > > + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8 | > > + (step_cnt & I2C_TIMING_STEP_DIV_MASK) << 0; > > + /* Disable the high speed transaction */ > > + i2c->high_speed_reg = I2C_TIME_CLR_VALUE; > > + } > Would it be sensible to write these values directly into hardware here? No.In some error cases, we want to reinitialize hardware, keep these values to avoid calculate again. Eddie
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 2255af2..14c7266 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -620,6 +620,15 @@ config I2C_MPC This driver can also be built as a module. If so, the module will be called i2c-mpc. +config I2C_MT65XX + tristate "MediaTek I2C adapter" + depends on ARCH_MEDIATEK || COMPILE_TEST + help + This selects the MediaTek(R) Integrated Inter Circuit bus driver + for MT65xx and MT81xx. + If you want to use MediaTek(R) I2C interface, say Y or M here. + If unsure, say N. + config I2C_MV64XXX tristate "Marvell mv64xxx I2C Controller" depends on MV64X60 || PLAT_ORION || ARCH_SUNXI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index cdf941d..abbf422 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o obj-$(CONFIG_I2C_MESON) += i2c-meson.o obj-$(CONFIG_I2C_MPC) += i2c-mpc.o +obj-$(CONFIG_I2C_MT65XX) += i2c-mt65xx.o obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o obj-$(CONFIG_I2C_MXS) += i2c-mxs.o obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c new file mode 100644 index 0000000..7462f05 --- /dev/null +++ b/drivers/i2c/busses/i2c-mt65xx.c @@ -0,0 +1,675 @@ +/* + * Copyright (c) 2014 MediaTek Inc. + * Author: Xudong.chen <xudong.chen@mediatek.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mm.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/platform_device.h> +#include <linux/scatterlist.h> +#include <linux/sched.h> +#include <linux/slab.h> + +#define I2C_HS_NACKERR (1 << 2) +#define I2C_ACKERR (1 << 1) +#define I2C_TRANSAC_COMP (1 << 0) +#define I2C_TRANSAC_START (1 << 0) +#define I2C_TIMING_STEP_DIV_MASK (0x3f << 0) +#define I2C_TIMING_SAMPLE_COUNT_MASK (0x7 << 0) +#define I2C_TIMING_SAMPLE_DIV_MASK (0x7 << 8) +#define I2C_TIMING_DATA_READ_MASK (0x7 << 12) +#define I2C_DCM_DISABLE 0x0000 +#define I2C_IO_CONFIG_OPEN_DRAIN 0x0003 +#define I2C_IO_CONFIG_PUSH_PULL 0x0000 +#define I2C_SOFT_RST 0x0001 +#define I2C_FIFO_ADDR_CLR 0x0001 +#define I2C_DELAY_LEN 0x0002 +#define I2C_ST_START_CON 0x8001 +#define I2C_FS_START_CON 0x1800 +#define I2C_TIME_CLR_VALUE 0x0000 +#define I2C_TIME_DEFAULT_VALUE 0x0003 +#define I2C_FS_TIME_INIT_VALUE 0x1303 +#define I2C_WRRD_TRANAC_VALUE 0x0002 +#define I2C_RD_TRANAC_VALUE 0x0001 + +#define I2C_DMA_CON_TX 0x0000 +#define I2C_DMA_CON_RX 0x0001 +#define I2C_DMA_START_EN 0x0001 +#define I2C_DMA_INT_FLAG_NONE 0x0000 +#define I2C_DMA_CLR_FLAG 0x0000 + +#define I2C_DEFAUT_SPEED 100000 /* hz */ +#define MAX_FS_MODE_SPEED 400000 +#define MAX_HS_MODE_SPEED 3400000 +#define MAX_SAMPLE_CNT_DIV 8 +#define MAX_STEP_CNT_DIV 64 +#define MAX_HS_STEP_CNT_DIV 8 + +#define I2C_CONTROL_RS (0x1 << 1) +#define I2C_CONTROL_DMA_EN (0x1 << 2) +#define I2C_CONTROL_CLK_EXT_EN (0x1 << 3) +#define I2C_CONTROL_DIR_CHANGE (0x1 << 4) +#define I2C_CONTROL_ACKERR_DET_EN (0x1 << 5) +#define I2C_CONTROL_TRANSFER_LEN_CHANGE (0x1 << 6) +#define I2C_CONTROL_WRAPPER (0x1 << 0) + +#define I2C_DRV_NAME "mt-i2c" + +enum DMA_REGS_OFFSET { + OFFSET_INT_FLAG = 0x0, + OFFSET_INT_EN = 0x04, + OFFSET_EN = 0x08, + OFFSET_CON = 0x18, + OFFSET_TX_MEM_ADDR = 0x1c, + OFFSET_RX_MEM_ADDR = 0x20, + OFFSET_TX_LEN = 0x24, + OFFSET_RX_LEN = 0x28, +}; + +enum i2c_trans_st_rs { + I2C_TRANS_STOP = 0, + I2C_TRANS_REPEATED_START, +}; + +enum mtk_trans_op { + I2C_MASTER_WR = 1, + I2C_MASTER_RD, + I2C_MASTER_WRRD, +}; + +enum I2C_REGS_OFFSET { + OFFSET_DATA_PORT = 0x0, + OFFSET_SLAVE_ADDR = 0x04, + OFFSET_INTR_MASK = 0x08, + OFFSET_INTR_STAT = 0x0c, + OFFSET_CONTROL = 0x10, + OFFSET_TRANSFER_LEN = 0x14, + OFFSET_TRANSAC_LEN = 0x18, + OFFSET_DELAY_LEN = 0x1c, + OFFSET_TIMING = 0x20, + OFFSET_START = 0x24, + OFFSET_EXT_CONF = 0x28, + OFFSET_FIFO_STAT = 0x30, + OFFSET_FIFO_THRESH = 0x34, + OFFSET_FIFO_ADDR_CLR = 0x38, + OFFSET_IO_CONFIG = 0x40, + OFFSET_RSV_DEBUG = 0x44, + OFFSET_HS = 0x48, + OFFSET_SOFTRESET = 0x50, + OFFSET_DCM_EN = 0x54, + OFFSET_PATH_DIR = 0x60, + OFFSET_DEBUGSTAT = 0x64, + OFFSET_DEBUGCTRL = 0x68, + OFFSET_TRANSFER_LEN_AUX = 0x6c, +}; + +struct mtk_i2c_compatible { + const struct i2c_adapter_quirks *quirks; + unsigned char pmic_i2c: 1; + unsigned char dcm: 1; +}; + +struct mtk_i2c { + struct i2c_adapter adap; /* i2c host adapter */ + struct device *dev; + struct completion msg_complete; + + /* set in i2c probe */ + void __iomem *base; /* i2c base addr */ + void __iomem *pdmabase; /* dma base address*/ + struct clk *clk_main; /* main clock for i2c bus */ + struct clk *clk_dma; /* DMA clock for i2c via DMA */ + struct clk *clk_pmic; /* PMIC clock for i2c from PMIC */ + bool have_pmic; /* can use i2c pins from PMIC */ + bool use_push_pull; /* IO config push-pull mode */ + + u16 irq_stat; /* interrupt status */ + unsigned int speed_hz; /* The speed in transfer */ + enum mtk_trans_op op; + u16 timing_reg; + u16 high_speed_reg; + const struct mtk_i2c_compatible *dev_comp; +}; + +static const struct i2c_adapter_quirks mt6577_i2c_quirks = { + .flags = I2C_AQ_COMB_WRITE_THEN_READ, + .max_num_msgs = 1, + .max_write_len = 255, + .max_read_len = 255, + .max_comb_1st_msg_len = 255, + .max_comb_2nd_msg_len = 31, +}; + +static const struct mtk_i2c_compatible mt6577_compat = { + .quirks = &mt6577_i2c_quirks, + .pmic_i2c = 0, + .dcm = 1, +}; + +static const struct mtk_i2c_compatible mt6589_compat = { + .quirks = &mt6577_i2c_quirks, + .pmic_i2c = 1, + .dcm = 0, +}; + +static const struct of_device_id mtk_i2c_of_match[] = { + { .compatible = "mediatek,mt6577-i2c", .data = &mt6577_compat }, + { .compatible = "mediatek,mt6589-i2c", .data = &mt6589_compat }, + {} +}; +MODULE_DEVICE_TABLE(of, mtk_i2c_of_match); + +static int mtk_i2c_clock_enable(struct mtk_i2c *i2c) +{ + int ret; + + ret = clk_prepare_enable(i2c->clk_dma); + if (ret) + return ret; + + ret = clk_prepare_enable(i2c->clk_main); + if (ret) + goto err_main; + + if (i2c->have_pmic) { + ret = clk_prepare_enable(i2c->clk_pmic); + if (ret) + goto err_pmic; + } + return 0; + +err_pmic: + clk_disable_unprepare(i2c->clk_main); +err_main: + clk_disable_unprepare(i2c->clk_dma); + + return ret; +} + +static void mtk_i2c_clock_disable(struct mtk_i2c *i2c) +{ + if (i2c->have_pmic) + clk_disable_unprepare(i2c->clk_pmic); + + clk_disable_unprepare(i2c->clk_main); + clk_disable_unprepare(i2c->clk_dma); +} + +static void mtk_i2c_init_hw(struct mtk_i2c *i2c) +{ + u16 control_reg; + + writew(I2C_SOFT_RST, i2c->base + OFFSET_SOFTRESET); + /* Set ioconfig */ + if (i2c->use_push_pull) + writew(I2C_IO_CONFIG_PUSH_PULL, i2c->base + OFFSET_IO_CONFIG); + else + writew(I2C_IO_CONFIG_OPEN_DRAIN, i2c->base + OFFSET_IO_CONFIG); + + if (i2c->dev_comp->dcm) + writew(I2C_DCM_DISABLE, i2c->base + OFFSET_DCM_EN); + + writew(i2c->timing_reg, i2c->base + OFFSET_TIMING); + writew(i2c->high_speed_reg, i2c->base + OFFSET_HS); + + /* If use i2c pin from PMIC mt6397 side, need set PATH_DIR first */ + if (i2c->have_pmic) + writew(I2C_CONTROL_WRAPPER, i2c->base + OFFSET_PATH_DIR); + + control_reg = I2C_CONTROL_ACKERR_DET_EN | + I2C_CONTROL_CLK_EXT_EN | I2C_CONTROL_DMA_EN; + writew(control_reg, i2c->base + OFFSET_CONTROL); + writew(I2C_DELAY_LEN, i2c->base + OFFSET_DELAY_LEN); +} + +/* calculate i2c port speed */ +static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int clk_src_in_hz) +{ + unsigned int khz; + unsigned int step_cnt; + unsigned int sample_cnt; + unsigned int sclk; + unsigned int hclk; + unsigned int max_step_cnt; + unsigned int sample_div = MAX_SAMPLE_CNT_DIV; + unsigned int step_div; + unsigned int min_div; + unsigned int best_mul; + unsigned int cnt_mul; + + if (i2c->speed_hz > MAX_HS_MODE_SPEED) + return -EINVAL; + else if (i2c->speed_hz > MAX_FS_MODE_SPEED) + max_step_cnt = MAX_HS_STEP_CNT_DIV; + else + max_step_cnt = MAX_STEP_CNT_DIV; + + step_div = max_step_cnt; + /* Find the best combination */ + khz = i2c->speed_hz / 1000; + hclk = clk_src_in_hz / 1000; + min_div = ((hclk >> 1) + khz - 1) / khz; + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; + + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { + step_cnt = (min_div + sample_cnt - 1) / sample_cnt; + cnt_mul = step_cnt * sample_cnt; + if (step_cnt > max_step_cnt) + continue; + + if (cnt_mul < best_mul) { + best_mul = cnt_mul; + sample_div = sample_cnt; + step_div = step_cnt; + if (best_mul == min_div) + break; + } + } + + sample_cnt = sample_div; + step_cnt = step_div; + sclk = hclk / (2 * sample_cnt * step_cnt); + if (sclk > khz) { + dev_dbg(i2c->dev, "%s mode: unsupported speed (%ldkhz)\n", + (i2c->speed_hz > MAX_HS_MODE_SPEED) ? "HS" : "ST/FT", + (long int)khz); + return -EINVAL; + } + + step_cnt--; + sample_cnt--; + + if (i2c->speed_hz > MAX_FS_MODE_SPEED) { + /* Set the hign speed mode register */ + i2c->timing_reg = I2C_FS_TIME_INIT_VALUE; + i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE | + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 12 | + (step_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8; + } else { + i2c->timing_reg = + (sample_cnt & I2C_TIMING_SAMPLE_COUNT_MASK) << 8 | + (step_cnt & I2C_TIMING_STEP_DIV_MASK) << 0; + /* Disable the high speed transaction */ + i2c->high_speed_reg = I2C_TIME_CLR_VALUE; + } + + return 0; +} + +static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, + int num, int left_num) +{ + u16 addr_reg; + u16 control_reg; + dma_addr_t rpaddr = 0; + dma_addr_t wpaddr = 0; + int ret; + + i2c->irq_stat = 0; + + reinit_completion(&i2c->msg_complete); + + control_reg = readw(i2c->base + OFFSET_CONTROL) & + ~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS); + if (i2c->speed_hz > 400000) + control_reg |= I2C_CONTROL_RS; + if (i2c->op == I2C_MASTER_WRRD) + control_reg |= I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS; + writew(control_reg, i2c->base + OFFSET_CONTROL); + + /* set start condition */ + if (i2c->speed_hz <= 100000) + writew(I2C_ST_START_CON, i2c->base + OFFSET_EXT_CONF); + else + writew(I2C_FS_START_CON, i2c->base + OFFSET_EXT_CONF); + + addr_reg = msgs->addr << 1; + if (i2c->op == I2C_MASTER_RD) + addr_reg |= 0x1; + writew(addr_reg, i2c->base + OFFSET_SLAVE_ADDR); + + /* Clear interrupt status */ + writew(I2C_HS_NACKERR | I2C_ACKERR | I2C_TRANSAC_COMP, + i2c->base + OFFSET_INTR_STAT); + writew(I2C_FIFO_ADDR_CLR, i2c->base + OFFSET_FIFO_ADDR_CLR); + + /* Enable interrupt */ + writew(I2C_HS_NACKERR | I2C_ACKERR | I2C_TRANSAC_COMP, + i2c->base + OFFSET_INTR_MASK); + + /* Set transfer and transaction len */ + if (i2c->op == I2C_MASTER_WRRD) { + writew(msgs->len | ((msgs + 1)->len) << 8, + i2c->base + OFFSET_TRANSFER_LEN); + writew(I2C_WRRD_TRANAC_VALUE, i2c->base + OFFSET_TRANSAC_LEN); + } else { + writew(msgs->len, i2c->base + OFFSET_TRANSFER_LEN); + writew(I2C_RD_TRANAC_VALUE, i2c->base + OFFSET_TRANSAC_LEN); + } + + /* Prepare buffer data to start transfer */ + 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, + msgs->len, DMA_FROM_DEVICE); + if (dma_mapping_error(i2c->dev, rpaddr)) + return -ENOMEM; + writel((u32)rpaddr, i2c->pdmabase + OFFSET_RX_MEM_ADDR); + writel(msgs->len, i2c->pdmabase + OFFSET_RX_LEN); + } 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, + msgs->len, DMA_TO_DEVICE); + if (dma_mapping_error(i2c->dev, wpaddr)) + return -ENOMEM; + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR); + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN); + } 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, + msgs->len, DMA_TO_DEVICE); + if (dma_mapping_error(i2c->dev, wpaddr)) + return -ENOMEM; + rpaddr = dma_map_single(i2c->dev, (msgs + 1)->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); + return -ENOMEM; + } + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR); + writel((u32)rpaddr, i2c->pdmabase + OFFSET_RX_MEM_ADDR); + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN); + writel((msgs + 1)->len, i2c->pdmabase + OFFSET_RX_LEN); + } + + writel(I2C_DMA_START_EN, i2c->pdmabase + OFFSET_EN); + writew(I2C_TRANSAC_START, i2c->base + OFFSET_START); + + ret = wait_for_completion_timeout(&i2c->msg_complete, + i2c->adap.timeout); + + /* Clear interrupt mask */ + writew(~(I2C_HS_NACKERR | I2C_ACKERR + | I2C_TRANSAC_COMP), i2c->base + OFFSET_INTR_MASK); + + if (i2c->op == I2C_MASTER_WR) { + dma_unmap_single(i2c->dev, wpaddr, + msgs->len, DMA_TO_DEVICE); + } else if (i2c->op == I2C_MASTER_RD) { + dma_unmap_single(i2c->dev, rpaddr, + msgs->len, DMA_FROM_DEVICE); + } else { + dma_unmap_single(i2c->dev, wpaddr, msgs->len, + DMA_TO_DEVICE); + dma_unmap_single(i2c->dev, rpaddr, (msgs + 1)->len, + DMA_FROM_DEVICE); + } + + if (ret == 0) { + dev_dbg(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr); + mtk_i2c_init_hw(i2c); + return -ETIMEDOUT; + } + + completion_done(&i2c->msg_complete); + + if (i2c->irq_stat & (I2C_HS_NACKERR | I2C_ACKERR)) { + dev_dbg(i2c->dev, "addr: %x, transfer ACK error\n", msgs->addr); + mtk_i2c_init_hw(i2c); + return -ENXIO; + } + + return 0; +} + +static int mtk_i2c_transfer(struct i2c_adapter *adap, + struct i2c_msg msgs[], int num) +{ + int ret; + int left_num = num; + struct mtk_i2c *i2c = i2c_get_adapdata(adap); + + ret = mtk_i2c_clock_enable(i2c); + if (ret) + return ret; + + if (!msgs->buf) { + dev_dbg(i2c->dev, "data buffer is NULL.\n"); + ret = -EINVAL; + goto err_exit; + } + + if (msgs->flags & I2C_M_RD) + i2c->op = I2C_MASTER_RD; + else + i2c->op = I2C_MASTER_WR; + + if (num > 1) { + /* combined two messages into one transaction */ + i2c->op = I2C_MASTER_WRRD; + left_num--; + } + + /* always use DMA mode. */ + ret = mtk_i2c_do_transfer(i2c, msgs); + if (ret < 0) + goto err_exit; + + /* the return value is number of executed messages */ + ret = num; + +err_exit: + mtk_i2c_clock_disable(i2c); + return ret; +} + +static irqreturn_t mtk_i2c_irq(int irqno, void *dev_id) +{ + struct mtk_i2c *i2c = dev_id; + + i2c->irq_stat = readw(i2c->base + OFFSET_INTR_STAT); + writew(I2C_HS_NACKERR | I2C_ACKERR + | I2C_TRANSAC_COMP, i2c->base + OFFSET_INTR_STAT); + + complete(&i2c->msg_complete); + + return IRQ_HANDLED; +} + +static u32 mtk_i2c_functionality(struct i2c_adapter *adap) +{ + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; +} + +static const struct i2c_algorithm mtk_i2c_algorithm = { + .master_xfer = mtk_i2c_transfer, + .functionality = mtk_i2c_functionality, +}; + +static int mtk_i2c_parse_dt(struct device_node *np, struct mtk_i2c *i2c, + unsigned int *clk_src_div) +{ + int ret; + + ret = of_property_read_u32(np, "clock-frequency", &i2c->speed_hz); + if (ret < 0) + i2c->speed_hz = I2C_DEFAUT_SPEED; + + ret = of_property_read_u32(np, "clock-div", clk_src_div); + if (ret < 0) + return ret; + + if (*clk_src_div == 0) + return -EINVAL; + + i2c->have_pmic = of_property_read_bool(np, "mediatek,have-pmic"); + i2c->use_push_pull = + of_property_read_bool(np, "mediatek,use-push-pull"); + + return 0; +} + +static int mtk_i2c_probe(struct platform_device *pdev) +{ + const struct of_device_id *of_id; + int ret = 0; + struct mtk_i2c *i2c; + struct clk *clk; + unsigned int clk_src_in_hz; + unsigned int clk_src_div; + struct resource *res; + int irq; + + i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL); + if (!i2c) + return -ENOMEM; + + ret = mtk_i2c_parse_dt(pdev->dev.of_node, i2c, &clk_src_div); + if (ret) + return -EINVAL; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + i2c->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(i2c->base)) + return PTR_ERR(i2c->base); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + i2c->pdmabase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(i2c->pdmabase)) + return PTR_ERR(i2c->pdmabase); + + irq = platform_get_irq(pdev, 0); + if (irq <= 0) + return irq; + + init_completion(&i2c->msg_complete); + + of_id = of_match_node(mtk_i2c_of_match, pdev->dev.of_node); + if (!of_id) + return -EINVAL; + + i2c->dev_comp = of_id->data; + i2c->adap.dev.of_node = pdev->dev.of_node; + i2c->dev = &pdev->dev; + i2c->adap.dev.parent = &pdev->dev; + i2c->adap.owner = THIS_MODULE; + i2c->adap.algo = &mtk_i2c_algorithm; + i2c->adap.quirks = i2c->dev_comp->quirks; + i2c->adap.timeout = 2 * HZ; + i2c->adap.retries = 1; + + if (i2c->have_pmic && !i2c->dev_comp->pmic_i2c) + return -EINVAL; + + i2c->clk_main = devm_clk_get(&pdev->dev, "main"); + if (IS_ERR(i2c->clk_main)) { + dev_err(&pdev->dev, "cannot get main clock\n"); + return PTR_ERR(i2c->clk_main); + } + + i2c->clk_dma = devm_clk_get(&pdev->dev, "dma"); + if (IS_ERR(i2c->clk_dma)) { + dev_err(&pdev->dev, "cannot get dma clock\n"); + return PTR_ERR(i2c->clk_dma); + } + + clk = i2c->clk_main; + if (i2c->have_pmic) { + i2c->clk_pmic = devm_clk_get(&pdev->dev, "pmic"); + if (IS_ERR(i2c->clk_pmic)) { + dev_err(&pdev->dev, "cannot get pmic clock\n"); + return PTR_ERR(i2c->clk_pmic); + } + clk = i2c->clk_pmic; + } + clk_src_in_hz = clk_get_rate(clk) / clk_src_div; + + dev_dbg(&pdev->dev, "clock source %p,clock src frequency %d\n", + i2c->clk_main, clk_src_in_hz); + strlcpy(i2c->adap.name, I2C_DRV_NAME, sizeof(i2c->adap.name)); + + ret = mtk_i2c_set_speed(i2c, clk_src_in_hz); + if (ret) { + dev_err(&pdev->dev, "Failed to set the speed.\n"); + return -EINVAL; + } + + ret = mtk_i2c_clock_enable(i2c); + if (ret) { + dev_err(&pdev->dev, "clock enable failed!\n"); + return ret; + } + mtk_i2c_init_hw(i2c); + mtk_i2c_clock_disable(i2c); + + ret = devm_request_irq(&pdev->dev, irq, mtk_i2c_irq, + IRQF_TRIGGER_NONE, I2C_DRV_NAME, i2c); + if (ret < 0) { + dev_err(&pdev->dev, + "Request I2C IRQ %d fail\n", irq); + return ret; + } + + i2c_set_adapdata(&i2c->adap, i2c); + ret = i2c_add_adapter(&i2c->adap); + if (ret) { + dev_err(&pdev->dev, "Failed to add i2c bus to i2c core\n"); + return ret; + } + + platform_set_drvdata(pdev, i2c); + + return 0; +} + +static int mtk_i2c_remove(struct platform_device *pdev) +{ + struct mtk_i2c *i2c = platform_get_drvdata(pdev); + + i2c_del_adapter(&i2c->adap); + + return 0; +} + +static struct platform_driver mtk_i2c_driver = { + .probe = mtk_i2c_probe, + .remove = mtk_i2c_remove, + .driver = { + .name = I2C_DRV_NAME, + .of_match_table = of_match_ptr(mtk_i2c_of_match), + }, +}; + +module_platform_driver(mtk_i2c_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("MediaTek I2C Bus Driver"); +MODULE_AUTHOR("Xudong Chen <xudong.chen@mediatek.com>");