* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
@ 2019-05-30 8:43 Ley Foon Tan
2019-05-30 8:59 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Ley Foon Tan @ 2019-05-30 8:43 UTC (permalink / raw)
To: u-boot
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 at 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);
}
--
2.19.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-05-30 8:43 [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM Ley Foon Tan
@ 2019-05-30 8:59 ` Marek Vasut
2019-05-30 9:07 ` Ley Foon Tan
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2019-05-30 8:59 UTC (permalink / raw)
To: u-boot
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 at 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.
[...]
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-05-30 8:59 ` Marek Vasut
@ 2019-05-30 9:07 ` Ley Foon Tan
2019-05-30 9:13 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Ley Foon Tan @ 2019-05-30 9:07 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-05-30 9:07 ` Ley Foon Tan
@ 2019-05-30 9:13 ` Marek Vasut
2019-06-10 6:07 ` Ley Foon Tan
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2019-05-30 9:13 UTC (permalink / raw)
To: u-boot
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 at 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
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-05-30 9:13 ` Marek Vasut
@ 2019-06-10 6:07 ` Ley Foon Tan
2019-06-10 11:28 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Ley Foon Tan @ 2019-06-10 6:07 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-06-10 6:07 ` Ley Foon Tan
@ 2019-06-10 11:28 ` Marek Vasut
2019-06-11 1:05 ` Ley Foon Tan
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2019-06-10 11:28 UTC (permalink / raw)
To: u-boot
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 ^
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-06-10 11:28 ` Marek Vasut
@ 2019-06-11 1:05 ` Ley Foon Tan
2019-06-11 2:58 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Ley Foon Tan @ 2019-06-11 1:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM
2019-06-11 1:05 ` Ley Foon Tan
@ 2019-06-11 2:58 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2019-06-11 2:58 UTC (permalink / raw)
To: u-boot
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>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-11 2:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-30 8:43 [U-Boot] [PATCH] i2c: designware: Get clock rate from clock DM Ley Foon Tan
2019-05-30 8:59 ` Marek Vasut
2019-05-30 9:07 ` Ley Foon Tan
2019-05-30 9:13 ` Marek Vasut
2019-06-10 6:07 ` Ley Foon Tan
2019-06-10 11:28 ` Marek Vasut
2019-06-11 1:05 ` Ley Foon Tan
2019-06-11 2:58 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.