[U-Boot] [PATCH v3 011/108] i2c: Tidy up designware PCI support
Bin Meng
bmeng.cn at gmail.com
Mon Oct 28 05:54:19 UTC 2019
+Stefan
Hi Simon,
On Mon, Oct 21, 2019 at 11:33 AM 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.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> drivers/i2c/Makefile | 3 +
> drivers/i2c/designware_i2c.c | 104 ++++++-----------------------------
> drivers/i2c/designware_i2c.h | 35 ++++++++++++
> drivers/i2c/dw_i2c_pci.c | 78 ++++++++++++++++++++++++++
> 4 files changed, 132 insertions(+), 88 deletions(-)
> create mode 100644 drivers/i2c/dw_i2c_pci.c
>
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index c2f75d87559..a7fa38b8dcf 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) += dw_i2c_pci.o
nits: designware_i2c_pci.o for consistency?
> +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 6daa90e7442..54e4a70c74c 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,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
> unsigned int ena;
> int i2c_spd;
>
> - if (speed >= I2C_MAX_SPEED)
> + if (speed >= I2C_MAX_SPEED &&
> + (!scl_sda_cfg || scl_sda_cfg->has_max_speed))
I think the logic should be && not ||
And I don't see scl_sda_cfg->has_max_speed in the original driver. Is
this new functionality?
> i2c_spd = IC_SPEED_MODE_MAX;
> else if (speed >= I2C_FAST_SPEED)
> i2c_spd = IC_SPEED_MODE_FAST;
> @@ -106,7 +79,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 +91,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 +536,20 @@ 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);
> - }
> + printf("bad\n");
What's this?
> + 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);
> }
>
> -static int designware_i2c_remove(struct udevice *dev)
> +int designware_i2c_remove(struct udevice *dev)
> {
> struct dw_i2c *priv = dev_get_priv(dev);
>
> @@ -618,30 +585,7 @@ static int designware_i2c_remove(struct udevice *dev)
> return reset_release_bulk(&priv->resets);
> }
>
> -static int designware_i2c_bind(struct udevice *dev)
> -{
> - static int num_cards;
> - char name[20];
> -
> - /* Create a unique device name for PCI type devices */
> - if (device_is_on_pci_bus(dev)) {
> - /*
> - * ToDo:
> - * Setting req_seq in the driver is probably not recommended.
> - * But without a DT alias the number is not configured. And
> - * using this driver is impossible for PCIe I2C devices.
> - * This can be removed, once a better (correct) way for this
> - * is found and implemented.
> - */
> - dev->req_seq = num_cards;
> - sprintf(name, "i2c_designware#%u", num_cards++);
> - device_set_name(dev, name);
> - }
> -
> - return 0;
> -}
> -
> -static const struct dm_i2c_ops designware_i2c_ops = {
> +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,
> @@ -656,7 +600,7 @@ U_BOOT_DRIVER(i2c_designware) = {
> .name = "i2c_designware",
> .id = UCLASS_I2C,
> .of_match = designware_i2c_ids,
> - .bind = designware_i2c_bind,
> + .ofdata_to_platdata = designware_i2c_ofdata_to_platdata,
> .probe = designware_i2c_probe,
> .priv_auto_alloc_size = sizeof(struct dw_i2c),
> .remove = designware_i2c_remove,
> @@ -664,20 +608,4 @@ U_BOOT_DRIVER(i2c_designware) = {
> .ops = &designware_i2c_ops,
> };
>
> -#ifdef CONFIG_X86
> -static struct pci_device_id designware_pci_supported[] = {
> - /* Intel BayTrail has 7 I2C controller located on the PCI bus */
> - { PCI_VDEVICE(INTEL, 0x0f41) },
> - { PCI_VDEVICE(INTEL, 0x0f42) },
> - { PCI_VDEVICE(INTEL, 0x0f43) },
> - { PCI_VDEVICE(INTEL, 0x0f44) },
> - { PCI_VDEVICE(INTEL, 0x0f45) },
> - { PCI_VDEVICE(INTEL, 0x0f46) },
> - { PCI_VDEVICE(INTEL, 0x0f47) },
> - {},
> -};
> -
> -U_BOOT_PCI_DEVICE(i2c_designware, designware_pci_supported);
> -#endif
> -
> #endif /* CONFIG_DM_I2C */
> diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h
> index 20ff20d9b83..48766d08067 100644
> --- a/drivers/i2c/designware_i2c.h
> +++ b/drivers/i2c/designware_i2c.h
> @@ -7,6 +7,8 @@
> #ifndef __DW_I2C_H_
> #define __DW_I2C_H_
>
> +#include <reset.h>
> +
> struct i2c_regs {
> u32 ic_con; /* 0x00 */
> u32 ic_tar; /* 0x04 */
> @@ -131,4 +133,37 @@ struct i2c_regs {
> #define I2C_FAST_SPEED 400000
> #define I2C_STANDARD_SPEED 100000
>
> +/**
> + * struct dw_scl_sda_cfg - I2C timing configuration
> + *
> + * @has_max_speed: Support maximum speed (1Mbps)
> + * @ss_hcnt: Standard speed high time in ns
> + * @fs_hcnt: Fast speed high time in ns
> + * @ss_lcnt: Standard speed low time in ns
> + * @fs_lcnt: Fast speed low time in ns
> + * @sda_hold: SDA hold time
> + */
> +struct dw_scl_sda_cfg {
> + bool has_max_speed;
> + u32 ss_hcnt;
> + u32 fs_hcnt;
> + u32 ss_lcnt;
> + u32 fs_lcnt;
> + u32 sda_hold;
> +};
> +
> +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
> +};
> +
> +extern const struct dm_i2c_ops designware_i2c_ops;
> +
> +int designware_i2c_probe(struct udevice *bus);
> +int designware_i2c_remove(struct udevice *dev);
> +
> #endif /* __DW_I2C_H_ */
> diff --git a/drivers/i2c/dw_i2c_pci.c b/drivers/i2c/dw_i2c_pci.c
> new file mode 100644
> index 00000000000..065c0aa5994
> --- /dev/null
> +++ b/drivers/i2c/dw_i2c_pci.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2009
> + * Vipin Kumar, ST Micoelectronics, vipin.kumar at st.com.
> + * Copyright 2019 Google Inc
> + */
> +
> +#include <dm.h>
> +#include "designware_i2c.h"
> +
> +/* 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,
> +};
> +
> +static int designware_i2c_pci_probe(struct udevice *dev)
> +{
> + struct dw_i2c *priv = dev_get_priv(dev);
> +
> + /* Save base address from PCI BAR */
> + priv->regs = (struct i2c_regs *)
> + dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM);
> + if (IS_ENABLED(CONFIG_INTEL_BAYTRAIL))
> + /* Use BayTrail specific timing values */
> + priv->scl_sda_cfg = &byt_config;
> +
> + return designware_i2c_probe(dev);
> +}
> +
> +static int designware_i2c_pci_bind(struct udevice *dev)
> +{
> + static int num_cards;
> + char name[20];
> +
> + /*
> + * Create a unique device name for PCI type devices
> + * ToDo:
> + * Setting req_seq in the driver is probably not recommended.
> + * But without a DT alias the number is not configured. And
> + * using this driver is impossible for PCIe I2C devices.
> + * This can be removed, once a better (correct) way for this
> + * is found and implemented.
> + */
> + dev->req_seq = num_cards;
> + sprintf(name, "i2c_designware#%u", num_cards++);
> + device_set_name(dev, name);
> +
> + return 0;
> +}
> +
> +U_BOOT_DRIVER(i2c_designware_pci) = {
> + .name = "i2c_designware_pci",
> + .id = UCLASS_I2C,
> + .bind = designware_i2c_pci_bind,
> + .probe = designware_i2c_pci_probe,
> + .priv_auto_alloc_size = sizeof(struct dw_i2c),
> + .remove = designware_i2c_remove,
> + .flags = DM_FLAG_OS_PREPARE,
> + .ops = &designware_i2c_ops,
> +};
> +
> +static struct pci_device_id designware_pci_supported[] = {
> + /* Intel BayTrail has 7 I2C controller located on the PCI bus */
> + { PCI_VDEVICE(INTEL, 0x0f41) },
> + { PCI_VDEVICE(INTEL, 0x0f42) },
> + { PCI_VDEVICE(INTEL, 0x0f43) },
> + { PCI_VDEVICE(INTEL, 0x0f44) },
> + { PCI_VDEVICE(INTEL, 0x0f45) },
> + { PCI_VDEVICE(INTEL, 0x0f46) },
> + { PCI_VDEVICE(INTEL, 0x0f47) },
> + {},
> +};
> +
> +U_BOOT_PCI_DEVICE(i2c_designware_pci, designware_pci_supported);
> --
Regards,
Bin
More information about the U-Boot
mailing list