| Message ID | 1339181689-22573-1-git-send-email-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
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?
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
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
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
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
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
>> > >> > +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, >> > +};
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
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 */
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