[U-Boot] [PATCH v5 007/101] i2c: designware: Tidy up PCI support

[Bin Meng]

Hi Simon,


On Mon, Nov 25, 2019 at 12:11 PM Simon Glass <sjg at chromium.org> wrote:
>
> This is hacked into the driver at present. It seems better to have it as
> a separate driver that uses the base driver. Create a new file and put
> the X86 code into it.
>
> Actually the Baytrail settings should really come from the device tree.
>
> Note that 'has_max_speed' is added as well. This is currently always false
> but since only Baytrail provides the config, it does not affect operation
> for other devices.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Reviewed-by: Heiko Schocher <hs at denx.de>
> ---
>
> Changes in v5: None
> Changes in v4:
> - Add a comment about the speed logic in __dw_i2c_set_bus_speed()
> - Add a comment in the commit message about why has_max_speed is added
> - Drop unwanted debug printf("bad\n")
> - Fix indentation nit
> - Rename new file to designware_i2c_pci.c
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/i2c/Makefile             |   3 +
>  drivers/i2c/designware_i2c.c     | 106 +++++--------------------------
>  drivers/i2c/designware_i2c.h     |  35 ++++++++++
>  drivers/i2c/designware_i2c_pci.c |  79 +++++++++++++++++++++++
>  4 files changed, 134 insertions(+), 89 deletions(-)
>  create mode 100644 drivers/i2c/designware_i2c_pci.c
>
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index c2f75d8755..f5a471f887 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -14,6 +14,9 @@ obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o
>  obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
>  obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
>  obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
> +ifdef CONFIG_DM_PCI
> +obj-$(CONFIG_SYS_I2C_DW) += designware_i2c_pci.o
> +endif
>  obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
>  obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
>  obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
> diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c
> index 6daa90e744..99b7d09bb2 100644
> --- a/drivers/i2c/designware_i2c.c
> +++ b/drivers/i2c/designware_i2c.c
> @@ -13,34 +13,6 @@
>  #include <asm/io.h>
>  #include "designware_i2c.h"
>
> -struct dw_scl_sda_cfg {
> -       u32 ss_hcnt;
> -       u32 fs_hcnt;
> -       u32 ss_lcnt;
> -       u32 fs_lcnt;
> -       u32 sda_hold;
> -};
> -
> -#ifdef CONFIG_X86
> -/* BayTrail HCNT/LCNT/SDA hold time */
> -static struct dw_scl_sda_cfg byt_config = {
> -       .ss_hcnt = 0x200,
> -       .fs_hcnt = 0x55,
> -       .ss_lcnt = 0x200,
> -       .fs_lcnt = 0x99,
> -       .sda_hold = 0x6,
> -};
> -#endif
> -
> -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
>  static int  dw_i2c_enable(struct i2c_regs *i2c_base, bool enable)
>  {
> @@ -90,7 +62,9 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
>         unsigned int ena;
>         int i2c_spd;
>
> -       if (speed >= I2C_MAX_SPEED)
> +       /* Allow max speed if there is no config , or the config allows it */

nits: remove the space before ,

> +       if (speed >= I2C_MAX_SPEED &&
> +           (!scl_sda_cfg || scl_sda_cfg->has_max_speed))

So with this change, for Baytrail, this new logic does not match the old codes.

Baytrail has the config, but its config does not have the
has_max_speed, so the if () logic evaluates to false.

>                 i2c_spd = IC_SPEED_MODE_MAX;
>         else if (speed >= I2C_FAST_SPEED)
>                 i2c_spd = IC_SPEED_MODE_FAST;
> @@ -106,7 +80,6 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
>         cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
>
>         switch (i2c_spd) {
> -#ifndef CONFIG_X86 /* No High-speed for BayTrail yet */
>         case IC_SPEED_MODE_MAX:
>                 cntl |= IC_CON_SPD_SS;
>                 if (scl_sda_cfg) {
> @@ -119,7 +92,6 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
>                 writel(hcnt, &i2c_base->ic_hs_scl_hcnt);
>                 writel(lcnt, &i2c_base->ic_hs_scl_lcnt);
>                 break;
> -#endif
>
>         case IC_SPEED_MODE_STANDARD:
>                 cntl |= IC_CON_SPD_SS;
> @@ -565,24 +537,19 @@ static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr,
>         return ret;
>  }
>
> -static int designware_i2c_probe(struct udevice *bus)
> +static int designware_i2c_ofdata_to_platdata(struct udevice *bus)
>  {
>         struct dw_i2c *priv = dev_get_priv(bus);
> -       int ret;
>
> -       if (device_is_on_pci_bus(bus)) {
> -#ifdef CONFIG_DM_PCI
> -               /* Save base address from PCI BAR */
> -               priv->regs = (struct i2c_regs *)
> -                       dm_pci_map_bar(bus, PCI_BASE_ADDRESS_0, PCI_REGION_MEM);
> -#ifdef CONFIG_X86
> -               /* Use BayTrail specific timing values */
> -               priv->scl_sda_cfg = &byt_config;
> -#endif
> -#endif
> -       } else {
> -               priv->regs = (struct i2c_regs *)devfdt_get_addr_ptr(bus);
> -       }
> +       priv->regs = (struct i2c_regs *)devfdt_get_addr_ptr(bus);
> +
> +       return 0;
> +}
> +
> +int designware_i2c_probe(struct udevice *bus)
> +{
> +       struct dw_i2c *priv = dev_get_priv(bus);
> +       int ret;
>
>         ret = reset_get_bulk(bus, &priv->resets);
>         if (ret)
> @@ -606,7 +573,7 @@ static int designware_i2c_probe(struct udevice *bus)
>         return __dw_i2c_init(priv->regs, 0, 0);
>  }
>

[snip]

Regards,
Bin