[U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
Get clock rate from clock DM if CONFIG_CLK is enabled. Otherwise, uses IC_CLK define.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com --- drivers/i2c/designware_i2c.c | 54 +++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 9ccc2411a6..a1ed30650c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,6 +4,7 @@ * Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com. */
+#include <clk.h> #include <common.h> #include <dm.h> #include <i2c.h> @@ -35,6 +36,9 @@ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; +#if CONFIG_IS_ENABLED(CLK) + struct clk clk; +#endif };
#ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED @@ -78,7 +82,7 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) */ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg, - unsigned int speed) + unsigned int speed, u32 bus_mhz) { unsigned int cntl; unsigned int hcnt, lcnt; @@ -104,8 +108,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->fs_hcnt; lcnt = scl_sda_cfg->fs_lcnt; } else { - hcnt = (IC_CLK * MIN_HS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (IC_CLK * MIN_HS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_mhz * MIN_HS_SCL_HIGHTIME) / NANO_TO_MICRO; + lcnt = (bus_mhz * MIN_HS_SCL_LOWTIME) / NANO_TO_MICRO; } writel(hcnt, &i2c_base->ic_hs_scl_hcnt); writel(lcnt, &i2c_base->ic_hs_scl_lcnt); @@ -118,8 +122,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->ss_hcnt; lcnt = scl_sda_cfg->ss_lcnt; } else { - hcnt = (IC_CLK * MIN_SS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (IC_CLK * MIN_SS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_mhz * MIN_SS_SCL_HIGHTIME) / NANO_TO_MICRO; + lcnt = (bus_mhz * MIN_SS_SCL_LOWTIME) / NANO_TO_MICRO; } writel(hcnt, &i2c_base->ic_ss_scl_hcnt); writel(lcnt, &i2c_base->ic_ss_scl_lcnt); @@ -132,8 +136,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->fs_hcnt; lcnt = scl_sda_cfg->fs_lcnt; } else { - hcnt = (IC_CLK * MIN_FS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (IC_CLK * MIN_FS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_mhz * MIN_FS_SCL_HIGHTIME) / NANO_TO_MICRO; + lcnt = (bus_mhz * MIN_FS_SCL_LOWTIME) / NANO_TO_MICRO; } writel(hcnt, &i2c_base->ic_fs_scl_hcnt); writel(lcnt, &i2c_base->ic_fs_scl_lcnt); @@ -388,7 +392,7 @@ static int __dw_i2c_init(struct i2c_regs *i2c_base, int speed, int slaveaddr) writel(IC_TX_TL, &i2c_base->ic_tx_tl); writel(IC_STOP_DET, &i2c_base->ic_intr_mask); #ifndef CONFIG_DM_I2C - __dw_i2c_set_bus_speed(i2c_base, NULL, speed); + __dw_i2c_set_bus_speed(i2c_base, NULL, speed, IC_CLK); writel(slaveaddr, &i2c_base->ic_sar); #endif
@@ -433,7 +437,7 @@ static unsigned int dw_i2c_set_bus_speed(struct i2c_adapter *adap, unsigned int speed) { adap->speed = speed; - return __dw_i2c_set_bus_speed(i2c_get_base(adap), NULL, speed); + return __dw_i2c_set_bus_speed(i2c_get_base(adap), NULL, speed, IC_CLK); }
static void dw_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) @@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus); + ulong rate; + +#if CONFIG_IS_ENABLED(CLK) + rate = clk_get_rate(&i2c->clk); + if (IS_ERR_VALUE(rate)) + return -EINVAL;
- return __dw_i2c_set_bus_speed(i2c->regs, i2c->scl_sda_cfg, speed); + /* Convert to MHz */ + rate /= 1000000; +#else + rate = IC_CLK; +#endif + return __dw_i2c_set_bus_speed(i2c->regs, i2c->scl_sda_cfg, speed, + rate); }
static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr, @@ -568,6 +584,19 @@ static int designware_i2c_probe(struct udevice *bus) else reset_deassert_bulk(&priv->resets);
+#if CONFIG_IS_ENABLED(CLK) + ret = clk_get_by_index(bus, 0, &priv->clk); + if (ret) + return ret; + + ret = clk_enable(&priv->clk); + if (ret && ret != -ENOSYS && ret != -ENOTSUPP) { + clk_free(&priv->clk); + dev_err(bus, "failed to enable clock\n"); + return ret; + } +#endif + return __dw_i2c_init(priv->regs, 0, 0); }
@@ -575,6 +604,11 @@ static int designware_i2c_remove(struct udevice *dev) { struct dw_i2c *priv = dev_get_priv(dev);
+#if CONFIG_IS_ENABLED(CLK) + clk_disable(&priv->clk); + clk_free(&priv->clk); +#endif + return reset_release_bulk(&priv->resets); }
On 5/30/19 10:43 AM, Ley Foon Tan wrote:
Get clock rate from clock DM if CONFIG_CLK is enabled. Otherwise, uses IC_CLK define.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.c | 54 +++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 9ccc2411a6..a1ed30650c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,6 +4,7 @@
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com.
*/
+#include <clk.h> #include <common.h> #include <dm.h> #include <i2c.h> @@ -35,6 +36,9 @@ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; +#if CONFIG_IS_ENABLED(CLK)
- struct clk clk;
+#endif };
#ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED @@ -78,7 +82,7 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) */ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg,
unsigned int speed)
unsigned int speed, u32 bus_mhz)
unsigned int bus_mhz , it's not a fixed-width register content, but just some random unsigned integer.
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
- ulong rate;
+#if CONFIG_IS_ENABLED(CLK)
- rate = clk_get_rate(&i2c->clk);
Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
[...]
On Thu, May 30, 2019 at 4:59 PM Marek Vasut marex@denx.de wrote:
On 5/30/19 10:43 AM, Ley Foon Tan wrote:
Get clock rate from clock DM if CONFIG_CLK is enabled. Otherwise, uses IC_CLK define.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.c | 54 +++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 9ccc2411a6..a1ed30650c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,6 +4,7 @@
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com.
*/
+#include <clk.h> #include <common.h> #include <dm.h> #include <i2c.h> @@ -35,6 +36,9 @@ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; +#if CONFIG_IS_ENABLED(CLK)
struct clk clk;+#endif };
#ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED @@ -78,7 +82,7 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) */ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg,
unsigned int speed)
unsigned int speed, u32 bus_mhz)unsigned int bus_mhz , it's not a fixed-width register content, but just some random unsigned integer.
You mean change u32 to unsigned int?
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use. Will change it.
Thanks.
Regards Ley Foon
On 5/30/19 11:07 AM, Ley Foon Tan wrote:
On Thu, May 30, 2019 at 4:59 PM Marek Vasut marex@denx.de wrote:
On 5/30/19 10:43 AM, Ley Foon Tan wrote:
Get clock rate from clock DM if CONFIG_CLK is enabled. Otherwise, uses IC_CLK define.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.c | 54 +++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 9ccc2411a6..a1ed30650c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,6 +4,7 @@
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com.
*/
+#include <clk.h> #include <common.h> #include <dm.h> #include <i2c.h> @@ -35,6 +36,9 @@ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; +#if CONFIG_IS_ENABLED(CLK)
struct clk clk;+#endif };
#ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED @@ -78,7 +82,7 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) */ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg,
unsigned int speed)
unsigned int speed, u32 bus_mhz)unsigned int bus_mhz , it's not a fixed-width register content, but just some random unsigned integer.
You mean change u32 to unsigned int?
Yes
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
Will change it.
Thanks.
Regards Ley Foon
On Thu, May 30, 2019 at 5:13 PM Marek Vasut marex@denx.de wrote:
On 5/30/19 11:07 AM, Ley Foon Tan wrote:
On Thu, May 30, 2019 at 4:59 PM Marek Vasut marex@denx.de wrote:
On 5/30/19 10:43 AM, Ley Foon Tan wrote:
Get clock rate from clock DM if CONFIG_CLK is enabled. Otherwise, uses IC_CLK define.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.c | 54 +++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 9ccc2411a6..a1ed30650c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,6 +4,7 @@
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com.
*/
+#include <clk.h> #include <common.h> #include <dm.h> #include <i2c.h> @@ -35,6 +36,9 @@ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; +#if CONFIG_IS_ENABLED(CLK)
struct clk clk;+#endif };
#ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED @@ -78,7 +82,7 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) */ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg,
unsigned int speed)
unsigned int speed, u32 bus_mhz)unsigned int bus_mhz , it's not a fixed-width register content, but just some random unsigned integer.
You mean change u32 to unsigned int?
Yes
Okay.
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
My patch is doing it in designware_i2c_set_bus_speed() already. We can't get clock rate in __dw_i2c_set_bus_speed(), because this function doesn't have struct udevice.
Regards Ley Foon
On 6/10/19 8:07 AM, Ley Foon Tan wrote: [...]
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
My patch is doing it in designware_i2c_set_bus_speed() already. We can't get clock rate in __dw_i2c_set_bus_speed(), because this function doesn't have struct udevice.
include/i2c.h struct dm_i2c_ops {} :
388 /** 389 * set_bus_speed() - set the speed of a bus (optional) 390 * 391 * The bus speed value will be updated by the uclass if this function 392 * does not return an error. This method is optional - if it is not 393 * provided then the driver can read the speed from 394 * dev_get_uclass_priv(bus)->speed_hz 395 * 396 * @bus: Bus to adjust 397 * @speed: Requested speed in Hz 398 * @return 0 if OK, -EINVAL for invalid values 399 */ 400 int (*set_bus_speed)(struct udevice *bus, unsigned int speed);
There's struct udevice right there ^
On Mon, Jun 10, 2019 at 8:07 PM Marek Vasut marex@denx.de wrote:
On 6/10/19 8:07 AM, Ley Foon Tan wrote: [...]
[...]
@@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) { struct dw_i2c *i2c = dev_get_priv(bus);
ulong rate;+#if CONFIG_IS_ENABLED(CLK)
rate = clk_get_rate(&i2c->clk);Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
My patch is doing it in designware_i2c_set_bus_speed() already. We can't get clock rate in __dw_i2c_set_bus_speed(), because this function doesn't have struct udevice.
include/i2c.h struct dm_i2c_ops {} :
388 /** 389 * set_bus_speed() - set the speed of a bus (optional) 390 * 391 * The bus speed value will be updated by the uclass if this function 392 * does not return an error. This method is optional - if it is not 393 * provided then the driver can read the speed from 394 * dev_get_uclass_priv(bus)->speed_hz 395 * 396 * @bus: Bus to adjust 397 * @speed: Requested speed in Hz 398 * @return 0 if OK, -EINVAL for invalid values 399 */ 400 int (*set_bus_speed)(struct udevice *bus, unsigned int speed);
There's struct udevice right there ^
I'm confused now.
.set_bus_speed = designware_i2c_set_bus_speed and my patch is doing get clock rate in .set_bus_speed callback already.
static const struct dm_i2c_ops designware_i2c_ops = { .xfer = designware_i2c_xfer, .probe_chip = designware_i2c_probe_chip, .set_bus_speed = designware_i2c_set_bus_speed, };
Regards Ley Foon
On 6/11/19 3:05 AM, Ley Foon Tan wrote:
On Mon, Jun 10, 2019 at 8:07 PM Marek Vasut marex@denx.de wrote:
On 6/10/19 8:07 AM, Ley Foon Tan wrote: [...]
[...]
> @@ -523,8 +527,20 @@ static int designware_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, > static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) > { > struct dw_i2c *i2c = dev_get_priv(bus); > + ulong rate; > + > +#if CONFIG_IS_ENABLED(CLK) > + rate = clk_get_rate(&i2c->clk);
Do we need to re-read the bus frequency for each transfer ? I would expect set_bus_speed callback to set the frequencies once and then keep them that way until it's called again.
Yes, we can get clock rate when request clock in _probe(). Then keep a copy for future use.
Not in .probe() , in set_bus_speed().
My patch is doing it in designware_i2c_set_bus_speed() already. We can't get clock rate in __dw_i2c_set_bus_speed(), because this function doesn't have struct udevice.
include/i2c.h struct dm_i2c_ops {} :
388 /** 389 * set_bus_speed() - set the speed of a bus (optional) 390 * 391 * The bus speed value will be updated by the uclass if this function 392 * does not return an error. This method is optional - if it is not 393 * provided then the driver can read the speed from 394 * dev_get_uclass_priv(bus)->speed_hz 395 * 396 * @bus: Bus to adjust 397 * @speed: Requested speed in Hz 398 * @return 0 if OK, -EINVAL for invalid values 399 */ 400 int (*set_bus_speed)(struct udevice *bus, unsigned int speed);
There's struct udevice right there ^
I'm confused now.
.set_bus_speed = designware_i2c_set_bus_speed and my patch is doing get clock rate in .set_bus_speed callback already.
static const struct dm_i2c_ops designware_i2c_ops = { .xfer = designware_i2c_xfer, .probe_chip = designware_i2c_probe_chip, .set_bus_speed = designware_i2c_set_bus_speed, };
Uh, clearly I was confused as well, sorry.
Acked-by: Marek Vasut marex@denx.de
participants (3)
-
Ley Foon Tan -
Ley Foon Tan -
Marek Vasut