Patchwork [1/2,V2] MXS: Set I2C timing registers for mxs-i2c

login
register
mail settings
Submitter Marek Vasut
Date June 8, 2012, 6:54 p.m.
Message ID <1339181689-22573-1-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/163843/
State New
Headers show

Comments

Marek Vasut - June 8, 2012, 6:54 p.m.
This patch configures the I2C bus timing registers according
to information passed via DT. Currently, 100kHz and 400kHz
modes are supported.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
CC: Dong Aisheng <b29396@freescale.com>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
Cc: linux-i2c@vger.kernel.org
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
CC: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   54 +++++++++++++++++++++
 3 files changed, 57 insertions(+)

V2: Use clock-frequency instead
Shubhrajyoti Datta - June 9, 2012, 4:15 a.m.
On Sat, Jun 9, 2012 at 12:24 AM, Marek Vasut <marex@denx.de> wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> CC: Dong Aisheng <b29396@freescale.com>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c@vger.kernel.org
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> CC: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> CC: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |   54 +++++++++++++++++++++
>  3 files changed, 57 insertions(+)
>
> V2: Use clock-frequency instead
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..d2bf750 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "fsl,<chip>-i2c"
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
> +- clock-frequency: desired I2C bus clock frequency in Hz.
>
>  Examples:
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index a89da5a..714e63c 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -398,6 +398,7 @@
>                                compatible = "fsl,imx28-i2c";
>                                reg = <0x80058000 2000>;
>                                interrupts = <111 68>;
> +                               clock-frequency = <400000>;
>                                status = "disabled";
>                        };
>
> @@ -407,6 +408,7 @@
>                                compatible = "fsl,imx28-i2c";
>                                reg = <0x8005a000 2000>;
>                                interrupts = <110 69>;
> +                               clock-frequency = <400000>;
>                                status = "disabled";
>                        };
>
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 04eb441..b4d083f 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -46,6 +46,10 @@
>  #define MXS_I2C_CTRL0_DIRECTION                        0x00010000
>  #define MXS_I2C_CTRL0_XFER_COUNT(v)            ((v) & 0x0000FFFF)
>
> +#define MXS_I2C_TIMING0                (0x10)
> +#define MXS_I2C_TIMING1                (0x20)
> +#define MXS_I2C_TIMING2                (0x30)
> +
>  #define MXS_I2C_CTRL1          (0x40)
>  #define MXS_I2C_CTRL1_SET      (0x44)
>  #define MXS_I2C_CTRL1_CLR      (0x48)
> @@ -97,6 +101,24 @@
>  #define MXS_CMD_I2C_READ       (MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
>                                 MXS_I2C_CTRL0_MASTER_MODE)
>
> +struct mxs_i2c_speed_config {
> +       uint32_t        timing0;
> +       uint32_t        timing1;
> +       uint32_t        timing2;
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
You are using 95k?
Didnt understand this.


> +       .timing0        = 0x00780030,
> +       .timing1        = 0x00800030,
> +       .timing2        = 0x0015000d,
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> +       .timing0        = 0x000f0007,
> +       .timing1        = 0x001f000f,
> +       .timing2        = 0x0015000d,
> +};
> +

How are these values calculated?
Marek Vasut - June 9, 2012, 10:34 a.m.
Dear Shubhrajyoti Datta,

> On Sat, Jun 9, 2012 at 12:24 AM, Marek Vasut <marex@denx.de> wrote:
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > CC: Dong Aisheng <b29396@freescale.com>
> > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > Cc: linux-i2c@vger.kernel.org
> > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > CC: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Stefano Babic <sbabic@denx.de>
> > CC: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
> >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >  drivers/i2c/busses/i2c-mxs.c                      |   54
> > +++++++++++++++++++++ 3 files changed, 57 insertions(+)
> > 
> > V2: Use clock-frequency instead
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > 1bfc02d..d2bf750 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > @@ -4,6 +4,7 @@ Required properties:
> >  - compatible: Should be "fsl,<chip>-i2c"
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain ERROR and DMA interrupts
> > +- clock-frequency: desired I2C bus clock frequency in Hz.
> > 
> >  Examples:
> > 
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index a89da5a..714e63c 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -398,6 +398,7 @@
> >                                compatible = "fsl,imx28-i2c";
> >                                reg = <0x80058000 2000>;
> >                                interrupts = <111 68>;
> > +                               clock-frequency = <400000>;
> >                                status = "disabled";
> >                        };
> > 
> > @@ -407,6 +408,7 @@
> >                                compatible = "fsl,imx28-i2c";
> >                                reg = <0x8005a000 2000>;
> >                                interrupts = <110 69>;
> > +                               clock-frequency = <400000>;
> >                                status = "disabled";
> >                        };
> > 
> > diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> > index 04eb441..b4d083f 100644
> > --- a/drivers/i2c/busses/i2c-mxs.c
> > +++ b/drivers/i2c/busses/i2c-mxs.c
> > @@ -46,6 +46,10 @@
> >  #define MXS_I2C_CTRL0_DIRECTION                        0x00010000
> >  #define MXS_I2C_CTRL0_XFER_COUNT(v)            ((v) & 0x0000FFFF)
> > 
> > +#define MXS_I2C_TIMING0                (0x10)
> > +#define MXS_I2C_TIMING1                (0x20)
> > +#define MXS_I2C_TIMING2                (0x30)
> > +
> >  #define MXS_I2C_CTRL1          (0x40)
> >  #define MXS_I2C_CTRL1_SET      (0x44)
> >  #define MXS_I2C_CTRL1_CLR      (0x48)
> > @@ -97,6 +101,24 @@
> >  #define MXS_CMD_I2C_READ       (MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
> >                                 MXS_I2C_CTRL0_MASTER_MODE)
> > 
> > +struct mxs_i2c_speed_config {
> > +       uint32_t        timing0;
> > +       uint32_t        timing1;
> > +       uint32_t        timing2;
> > +};
> > +
> > +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> 
> You are using 95k?

Yes

> Didnt understand this.

What exactly, it's running (according to the datasheet) at 95kHz.

> > +       .timing0        = 0x00780030,
> > +       .timing1        = 0x00800030,
> > +       .timing2        = 0x0015000d,
> > +};
> > +
> > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> > +       .timing0        = 0x000f0007,
> > +       .timing1        = 0x001f000f,
> > +       .timing2        = 0x0015000d,
> > +};
> > +
> 
> How are these values calculated?

They're not calculated, they're presented in the datasheet. There's no formula, 
even though I believe one could be derived given enough effort.

Best regards,
Marek Vasut
Sascha Hauer - June 9, 2012, 10:53 a.m.
On Sat, Jun 09, 2012 at 12:34:47PM +0200, Marek Vasut wrote:
> Dear Shubhrajyoti Datta,
> 
> 
> > Didnt understand this.
> 
> What exactly, it's running (according to the datasheet) at 95kHz.
> 
> > > +       .timing0        = 0x00780030,
> > > +       .timing1        = 0x00800030,
> > > +       .timing2        = 0x0015000d,
> > > +};
> > > +
> > > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> > > +       .timing0        = 0x000f0007,
> > > +       .timing1        = 0x001f000f,
> > > +       .timing2        = 0x0015000d,
> > > +};
> > > +
> > 
> > How are these values calculated?
> 
> They're not calculated, they're presented in the datasheet. There's no formula, 
> even though I believe one could be derived given enough effort.

Don't these values depend on the input clock of this unit? I don't know,
just asking.

Sascha
Marek Vasut - June 9, 2012, 11:11 a.m.
Dear Sascha Hauer,

> On Sat, Jun 09, 2012 at 12:34:47PM +0200, Marek Vasut wrote:
> > Dear Shubhrajyoti Datta,
> > 
> > > Didnt understand this.
> > 
> > What exactly, it's running (according to the datasheet) at 95kHz.
> > 
> > > > +       .timing0        = 0x00780030,
> > > > +       .timing1        = 0x00800030,
> > > > +       .timing2        = 0x0015000d,
> > > > +};
> > > > +
> > > > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> > > > +       .timing0        = 0x000f0007,
> > > > +       .timing1        = 0x001f000f,
> > > > +       .timing2        = 0x0015000d,
> > > > +};
> > > > +
> > > 
> > > How are these values calculated?
> > 
> > They're not calculated, they're presented in the datasheet. There's no
> > formula, even though I believe one could be derived given enough effort.
> 
> Don't these values depend on the input clock of this unit? I don't know,
> just asking.

Isn't the i2c block supplied from 24MHz clock source in the mx28?

> Sascha

Best regards,
Marek Vasut
Sascha Hauer - June 9, 2012, 11:25 a.m.
On Sat, Jun 09, 2012 at 01:11:42PM +0200, Marek Vasut wrote:
> Dear Sascha Hauer,
> 
> > On Sat, Jun 09, 2012 at 12:34:47PM +0200, Marek Vasut wrote:
> > > Dear Shubhrajyoti Datta,
> > > 
> > > > Didnt understand this.
> > > 
> > > What exactly, it's running (according to the datasheet) at 95kHz.
> > > 
> > > > > +       .timing0        = 0x00780030,
> > > > > +       .timing1        = 0x00800030,
> > > > > +       .timing2        = 0x0015000d,
> > > > > +};
> > > > > +
> > > > > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> > > > > +       .timing0        = 0x000f0007,
> > > > > +       .timing1        = 0x001f000f,
> > > > > +       .timing2        = 0x0015000d,
> > > > > +};
> > > > > +
> > > > 
> > > > How are these values calculated?
> > > 
> > > They're not calculated, they're presented in the datasheet. There's no
> > > formula, even though I believe one could be derived given enough effort.
> > 
> > Don't these values depend on the input clock of this unit? I don't know,
> > just asking.
> 
> Isn't the i2c block supplied from 24MHz clock source in the mx28?

Maybe. Shouldn't this be checked then or at least mentioned that these
values assume a certain clock frequency?

I wonder that currently the timing registers are not initialized at all.
Do they depend on reset defaults or bootloader setup?

Sascha
Marek Vasut - June 9, 2012, 11:40 a.m.
Dear Sascha Hauer,

> On Sat, Jun 09, 2012 at 01:11:42PM +0200, Marek Vasut wrote:
> > Dear Sascha Hauer,
> > 
> > > On Sat, Jun 09, 2012 at 12:34:47PM +0200, Marek Vasut wrote:
> > > > Dear Shubhrajyoti Datta,
> > > > 
> > > > > Didnt understand this.
> > > > 
> > > > What exactly, it's running (according to the datasheet) at 95kHz.
> > > > 
> > > > > > +       .timing0        = 0x00780030,
> > > > > > +       .timing1        = 0x00800030,
> > > > > > +       .timing2        = 0x0015000d,
> > > > > > +};
> > > > > > +
> > > > > > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> > > > > > +       .timing0        = 0x000f0007,
> > > > > > +       .timing1        = 0x001f000f,
> > > > > > +       .timing2        = 0x0015000d,
> > > > > > +};
> > > > > > +
> > > > > 
> > > > > How are these values calculated?
> > > > 
> > > > They're not calculated, they're presented in the datasheet. There's
> > > > no formula, even though I believe one could be derived given enough
> > > > effort.
> > > 
> > > Don't these values depend on the input clock of this unit? I don't
> > > know, just asking.
> > 
> > Isn't the i2c block supplied from 24MHz clock source in the mx28?
> 
> Maybe. Shouldn't this be checked then or at least mentioned that these
> values assume a certain clock frequency?

27.5.2 - 27.5.4 in the MX28 TRM -- these values are for 24MHz clock indeed.

> I wonder that currently the timing registers are not initialized at all.
> Do they depend on reset defaults or bootloader setup?

They're set to default values in our case -- I dunno if your bootloader always 
sets them up differently.

> Sascha

Best regards,
Marek Vasut
Shubhrajyoti Datta - June 9, 2012, 12:46 p.m.
>> >
>> > +struct mxs_i2c_speed_config {
>> > +       uint32_t        timing0;
>> > +       uint32_t        timing1;
>> > +       uint32_t        timing2;
>> > +};
>> > +
>> > +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
>>
>> You are using 95k?
>
> Yes
>
>> Didnt understand this.
>
> What exactly, it's running (according to the datasheet) at 95kHz.

Did you intend 100k?
and approximating due to clock ... or some other limitaion?

That is what I was trying to understand.
>
>> > +       .timing0        = 0x00780030,
>> > +       .timing1        = 0x00800030,
>> > +       .timing2        = 0x0015000d,
>> > +};
Marek Vasut - June 9, 2012, 1:10 p.m.
Dear Shubhrajyoti Datta,

> >> > +struct mxs_i2c_speed_config {
> >> > +       uint32_t        timing0;
> >> > +       uint32_t        timing1;
> >> > +       uint32_t        timing2;
> >> > +};
> >> > +
> >> > +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> >> 
> >> You are using 95k?
> > 
> > Yes
> > 
> >> Didnt understand this.
> > 
> > What exactly, it's running (according to the datasheet) at 95kHz.
> 
> Did you intend 100k?

I intended to stick with the datasheet, if you manage to find explanation for 
why they chose 95kHz, I'd be interested to hear it ... btw. 24MHz/95kHz gives 
pretty weird divider, which is quite suspicious.

> and approximating due to clock ... or some other limitaion?

Doesn't seem to be the case, see "btw" part above.

> That is what I was trying to understand.
> 
> >> > +       .timing0        = 0x00780030,
> >> > +       .timing1        = 0x00800030,
> >> > +       .timing2        = 0x0015000d,
> >> > +};

Best regards,
Marek Vasut

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index 1bfc02d..d2bf750 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -4,6 +4,7 @@  Required properties:
 - compatible: Should be "fsl,<chip>-i2c"
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
+- clock-frequency: desired I2C bus clock frequency in Hz.
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index a89da5a..714e63c 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -398,6 +398,7 @@ 
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
@@ -407,6 +408,7 @@ 
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 04eb441..b4d083f 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -46,6 +46,10 @@ 
 #define MXS_I2C_CTRL0_DIRECTION			0x00010000
 #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
 
+#define MXS_I2C_TIMING0		(0x10)
+#define MXS_I2C_TIMING1		(0x20)
+#define MXS_I2C_TIMING2		(0x30)
+
 #define MXS_I2C_CTRL1		(0x40)
 #define MXS_I2C_CTRL1_SET	(0x44)
 #define MXS_I2C_CTRL1_CLR	(0x48)
@@ -97,6 +101,24 @@ 
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+struct mxs_i2c_speed_config {
+	uint32_t	timing0;
+	uint32_t	timing1;
+	uint32_t	timing2;
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
+	.timing0	= 0x00780030,
+	.timing1	= 0x00800030,
+	.timing2	= 0x0015000d,
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
+	.timing0	= 0x000f0007,
+	.timing1	= 0x001f000f,
+	.timing2	= 0x0015000d,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
@@ -112,11 +134,17 @@  struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+	const struct mxs_i2c_speed_config *speed;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
 	stmp_reset_block(i2c->regs);
+
+	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
+	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
+	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
+
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
@@ -319,6 +347,27 @@  static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
+{
+	uint32_t speed;
+	struct device *dev = i2c->dev;
+	struct device_node *node = dev->of_node;
+
+	if (!node)
+		return -EINVAL;
+
+	i2c->speed = &mxs_i2c_95kHz_config;
+	ret = of_property_read_u32(node, "clock-frequency", &speed);
+	if (ret)
+		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
+	else if (speed == 400000)
+		i2c->speed = &mxs_i2c_400kHz_config;
+	else if (speed != 100000)
+		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
+
+	return 0;
+}
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -358,6 +407,11 @@  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 		return err;
 
 	i2c->dev = dev;
+
+	err = mxs_i2c_get_ofdata(i2c);
+	if (err)
+		return err;
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */