| Message ID | 20191021033322.217715-12-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add initial support for apollolake | expand |
Hello Simon, Am 21.10.2019 um 05:31 schrieb Simon Glass: > 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 Reviewed-by: Heiko Schocher <hs@denx.de> Thanks! bye, Heiko
+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
On 21.10.19 05:31, Simon Glass 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> Apart from the comments from Bin, I only have one nitpicking comments below... > --- > > 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 > +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)) > 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"); > + 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, Indentation seems wrong in the line above. Thanks, Stefan
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 +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)) 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"); + 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);
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