+Stefan
Hi Simon,
On Mon, Oct 21, 2019 at 11:33 AM Simon Glass sjg@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@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@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