This list is for users and developers of the i2c subsystem and i2c bus drivers in Linux 2.4 and 2.6.

Marek Vasut | 9 Jun 2012 13:45

Picon

Picon

[PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

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@...>
Cc: Detlev Zundel <dzu@...>
CC: Dong Aisheng <b29396@...>
CC: Fabio Estevam <fabio.estevam@...>
Cc: Linux ARM kernel <linux-arm-kernel@...>
Cc: linux-i2c@...
CC: Sascha Hauer <s.hauer@...>
CC: Shawn Guo <shawn.guo@...>
Cc: Stefano Babic <sbabic@...>
CC: Uwe Kleine-König <u.kleine-koenig@...>
Cc: Wolfgang Denk <wd@...>
Cc: Wolfram Sang <w.sang@...>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   55 +++++++++++++++++++++
 3 files changed, 58 insertions(+)

V2: Use clock-frequency instead
V3: Document that 24MHz clock are the base for i2c speed settings

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
 <at>  <at>  -4,6 +4,7  <at>  <at>  Required properties:

(Continue reading)

Marek Vasut | 9 Jun 2012 13:45

Picon

Picon

[PATCH 2/2 V2] MXS: Implement DMA support into mxs-i2c

This patch implements DMA support into mxs-i2c. DMA transfers are now enabled
via DT. The DMA operation is enabled by default.

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

V2: Improve documentation

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index d2bf750..9497ee0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
 <at>  <at>  -5,6 +5,10  <at>  <at>  Required properties:
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts

(Continue reading)

Marek Vasut | 10 Jun 2012 13:53

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Marek Vasut,

> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.

[...]

> +struct mxs_i2c_speed_config {
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +	uint32_t	timing2;
> +};
> +
> +/* Timing values for the default 24MHz clock supplied into the i2c block.

Thinking about these further -- does anyone have any idea how these numbers were 
derived? And possibly even formula for that?

And maybe we should somehow make sure the source runs on 24MHz (how?).

> */ +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,

(Continue reading)

Shawn Guo | 11 Jun 2012 09:30

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

On Sun, Jun 10, 2012 at 01:53:05PM +0200, Marek Vasut wrote:
> > +struct mxs_i2c_speed_config {
> > +	uint32_t	timing0;
> > +	uint32_t	timing1;
> > +	uint32_t	timing2;
> > +};
> > +
> > +/* Timing values for the default 24MHz clock supplied into the i2c block.
> 
> Thinking about these further -- does anyone have any idea how these numbers were 
> derived? And possibly even formula for that?
> 
> And maybe we should somehow make sure the source runs on 24MHz (how?).
> 
There is nothing about I2C clock mentioned in CLKCTRL chapter.  I just
contacted design team and was told that I2C clock sources from APBX
(xbus) clock and always runs at the same frequency there.

--

-- 
Regards,
Shawn

Marek Vasut | 11 Jun 2012 12:53

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Sun, Jun 10, 2012 at 01:53:05PM +0200, Marek Vasut wrote:
> > > +struct mxs_i2c_speed_config {
> > > +	uint32_t	timing0;
> > > +	uint32_t	timing1;
> > > +	uint32_t	timing2;
> > > +};
> > > +
> > > +/* Timing values for the default 24MHz clock supplied into the i2c
> > > block.
> > 
> > Thinking about these further -- does anyone have any idea how these
> > numbers were derived? And possibly even formula for that?
> > 
> > And maybe we should somehow make sure the source runs on 24MHz (how?).
> 
> There is nothing about I2C clock mentioned in CLKCTRL chapter.  I just
> contacted design team and was told that I2C clock sources from APBX
> (xbus) clock and always runs at the same frequency there.

Ok, then can you please try asking them how to exactly compute the values in 
timing0-timing2 registers? So we don't have to hardcode them like it's done now?

Best regards,
Marek Vasut

Shawn Guo | 11 Jun 2012 16:42

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

On Mon, Jun 11, 2012 at 12:53:17PM +0200, Marek Vasut wrote:
> Ok, then can you please try asking them how to exactly compute the values in 
> timing0-timing2 registers? So we don't have to hardcode them like it's done now?
> 
It's determined I2C clock waveform you want to get.  See i.MX28 RM
"Figure 27-2. I2C Data and Clock Timing" and "Figure 27-3. I2C Data
and Clock Timing Generation".

For example, if you run 12MHz APBX clock, and set HIGH_COUNT to 60,
the I2C clock will have 60 cycle x (1/12MHz) = 5us time for its high
period.

--

-- 
Regards,
Shawn

Marek Vasut | 23 Jun 2012 20:19

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Mon, Jun 11, 2012 at 12:53:17PM +0200, Marek Vasut wrote:
> > Ok, then can you please try asking them how to exactly compute the values
> > in timing0-timing2 registers? So we don't have to hardcode them like
> > it's done now?
> 
> It's determined I2C clock waveform you want to get.  See i.MX28 RM
> "Figure 27-2. I2C Data and Clock Timing" and "Figure 27-3. I2C Data
> and Clock Timing Generation".
> 
> For example, if you run 12MHz APBX clock, and set HIGH_COUNT to 60,
> the I2C clock will have 60 cycle x (1/12MHz) = 5us time for its high
> period.

Ok, I think I see the equation. But what still doesn't make sense is how you got 
to the value of 48 (RCV_COUNT at 95kHz). And how you got 120 for HIGH_COUNT and 
128 for LOW_COUNT?

Were these values based on some measurements, making them the best possible 
values? Won't computation of slightly different values affect reliability of 
this driver?

Best regards,
Marek Vasut

Shawn Guo | 11 Jun 2012 08:39

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

On Sat, Jun 09, 2012 at 01:45:50PM +0200, Marek Vasut 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@...>

I gave it a test on imx28-evk board with audio playback.  It seems
the patch makes the first time playback non-functional, but the later
playback is still working.

> +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);

"ret" is undeclared.

> +	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)

(Continue reading)

Marek Vasut | 11 Jun 2012 12:54

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Sat, Jun 09, 2012 at 01:45:50PM +0200, Marek Vasut 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@...>
> 
> I gave it a test on imx28-evk board with audio playback.  It seems
> the patch makes the first time playback non-functional, but the later
> playback is still working.

Any hints what can be the source of this issue? I tested it with i2c eeprom, saw 
no issues in there. I'll poke into it later.

> > +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);
> 
> "ret" is undeclared.
>

(Continue reading)

Marek Vasut | 27 Jun 2012 03:30

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> > On Sat, Jun 09, 2012 at 01:45:50PM +0200, Marek Vasut 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@...>
> > 
> > I gave it a test on imx28-evk board with audio playback.  It seems
> > the patch makes the first time playback non-functional, but the later
> > playback is still working.
> 
> Any hints what can be the source of this issue? I tested it with i2c
> eeprom, saw no issues in there. I'll poke into it later.

Ok, I managed to replicate it just now. Scrap my previous email.

Still, any idea what can cause this?

Best regards,
Marek Vasut

Shawn Guo | 27 Jun 2012 04:13

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

On Wed, Jun 27, 2012 at 03:30:31AM +0200, Marek Vasut wrote:
> Ok, I managed to replicate it just now. Scrap my previous email.
> 
> Still, any idea what can cause this?
> 
I just spent some time on it.  It looks like a document issue (dammit).

The HW_I2C_TIMING2 EXAMPLE tells the value is 0x0015000d which is the
one you looked at and used.  But the register field figure tells it's
0x00300030, which seems the real case.

The mxs_i2c_reset at probe changes the value to 0x0015000d which causes
the first time playback nonfunctional, but the mxs_i2c_reset call in
mxs_i2c_xfer_msg happens to reset the value back to 0x00300030, and
then second time playback starts working.

The changes below get everything work fine, both 100k and 400k.  So
with the changes in, you can add:

Tested-by: Shawn Guo <shawn.guo@...>

Regards,
Shawn

--8<---

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index e38be56..c2467e9 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c

(Continue reading)

Marek Vasut | 27 Jun 2012 04:34

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Wed, Jun 27, 2012 at 03:30:31AM +0200, Marek Vasut wrote:
> > Ok, I managed to replicate it just now. Scrap my previous email.
> > 
> > Still, any idea what can cause this?
> 
> I just spent some time on it.  It looks like a document issue (dammit).
> 
> The HW_I2C_TIMING2 EXAMPLE tells the value is 0x0015000d which is the
> one you looked at and used.  But the register field figure tells it's
> 0x00300030, which seems the real case.
> 
> The mxs_i2c_reset at probe changes the value to 0x0015000d which causes
> the first time playback nonfunctional, but the mxs_i2c_reset call in
> mxs_i2c_xfer_msg happens to reset the value back to 0x00300030, and
> then second time playback starts working.
> 
> The changes below get everything work fine, both 100k and 400k.  So
> with the changes in, you can add:

Ok, can you push the FSL guys to roll out updated document?

Do you consider it reasonable to add some calculations of these timing values or 
were they somehow fine-tuned with a scope/deep HW knowledge as the best working 
values and we should stick to those?

> Tested-by: Shawn Guo <shawn.guo@...>
> 
> Regards,

(Continue reading)

Marek Vasut | 23 Jun 2012 20:47

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
[...]

Is there any reason why this can not be merged other than the timing registers 
goo (which I believe shall stay as in the datasheet until we figure out if it's 
even reasonable to add some computation there).

Best regards,
Marek Vasut

Shawn Guo | 25 Jun 2012 17:43

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

On Sat, Jun 23, 2012 at 08:47:32PM +0200, Marek Vasut wrote:
> Dear Shawn Guo,
> 
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> [...]
> 
> Is there any reason why this can not be merged other than the timing registers 
> goo (which I believe shall stay as in the datasheet until we figure out if it's 
> even reasonable to add some computation there).
> 
I assume this is a question for Wolfram.  But I guess part of the reason
is there is still one comment from me staying unresolved.

--

-- 
Regards,
Shawn

Marek Vasut | 25 Jun 2012 18:02

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Sat, Jun 23, 2012 at 08:47:32PM +0200, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> > > This patch configures the I2C bus timing registers according
> > > to information passed via DT. Currently, 100kHz and 400kHz
> > > modes are supported.
> > 
> > [...]
> > 
> > Is there any reason why this can not be merged other than the timing
> > registers goo (which I believe shall stay as in the datasheet until we
> > figure out if it's even reasonable to add some computation there).
> 
> I assume this is a question for Wolfram.  But I guess part of the reason
> is there is still one comment from me staying unresolved.

Ah, you mean about the audio? I'll poke into it tonight.

Best regards,
Marek Vasut

Marek Vasut | 27 Jun 2012 03:15

Picon

Picon

Re: [PATCH 1/2 V3] MXS: Set I2C timing registers for mxs-i2c

Dear Shawn Guo,

> On Sat, Jun 23, 2012 at 08:47:32PM +0200, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> > > This patch configures the I2C bus timing registers according
> > > to information passed via DT. Currently, 100kHz and 400kHz
> > > modes are supported.
> > 
> > [...]
> > 
> > Is there any reason why this can not be merged other than the timing
> > registers goo (which I believe shall stay as in the datasheet until we
> > figure out if it's even reasonable to add some computation there).
> 
> I assume this is a question for Wolfram.  But I guess part of the reason
> is there is still one comment from me staying unresolved.

Shawn, I just re-tested the i2c with mpg123 playing "MPEG 1.0 layer III, VBR, 
44100 Hz joint-stereo" and it worked on a first try ... can you retest please?

Best regards,
Marek Vasut

Return

Return to gmane.linux.drivers.i2c.

Project Web Page

This list is for users and developers of the i2c subsystem and i2c bus drivers in Linux 2.4 and 2.6.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane