| Message ID | 1392027598-29015-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Feb 10, 2014 at 11:19:58AM +0100, Uwe Kleine-König wrote: > This was tested on a EFM32GG-DK3750 devboard that has a temperature > sensor and an eeprom on its i2c bus. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> ping Thanks Uwe > --- > Hello, > > there are two places marked with a triple-X where I don't know if I did > it right. The one is "Do I need to protect from .master_xfer being > called again before the previous call returned?" The other is what to > return in the .functionality callback. > > Also note there is a matching entry in MAINTAINERS for "ARM/ENERGY MICRO > (SILICON LABS) EFM32 SUPPORT". > > Best regards > Uwe > > drivers/i2c/busses/Kconfig | 7 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-efm32.c | 527 ++++++++++++++++++++++++++++++++ > include/linux/platform_data/efm32-i2c.h | 15 + > 4 files changed, 550 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-efm32.c > create mode 100644 include/linux/platform_data/efm32-i2c.h > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index f5ed03164d86..7322ff0c4c60 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -432,6 +432,13 @@ config I2C_DESIGNWARE_PCI > This driver can also be built as a module. If so, the module > will be called i2c-designware-pci. > > +config I2C_EFM32 > + tristate "EFM32 I2C controller" > + depends on OF && (ARCH_EFM32 || COMPILE_TEST) > + help > + This driver supports the i2c block found in Energy Micro's EFM32 > + SoCs. > + > config I2C_EG20T > tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index a08931fe73e1..2a56ab181851 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o > i2c-designware-platform-objs := i2c-designware-platdrv.o > obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o > i2c-designware-pci-objs := i2c-designware-pcidrv.o > +obj-$(CONFIG_I2C_EFM32) += i2c-efm32.o > obj-$(CONFIG_I2C_EG20T) += i2c-eg20t.o > obj-$(CONFIG_I2C_EXYNOS5) += i2c-exynos5.o > obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o > diff --git a/drivers/i2c/busses/i2c-efm32.c b/drivers/i2c/busses/i2c-efm32.c > new file mode 100644 > index 000000000000..9e860abcab63 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-efm32.c > @@ -0,0 +1,527 @@ > +/* > + * Copyright (C) 2014 Uwe Kleine-Koenig for Pengutronix > + * > + * This program is free software; you can redistribute it and/or modify it under > + * the terms of the GNU General Public License version 2 as published by the > + * Free Software Foundation. > + */ > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/i2c.h> > +#include <linux/io.h> > +#include <linux/interrupt.h> > +#include <linux/err.h> > +#include <linux/clk.h> > +#include <linux/platform_data/efm32-i2c.h> > + > +#define DRIVER_NAME "efm32-i2c" > + > +#define MASK_VAL(mask, val) ((val << __ffs(mask)) & mask) > + > +#define REG_CTRL 0x00 > +#define REG_CTRL_EN 0x00001 > +#define REG_CTRL_SLAVE 0x00002 > +#define REG_CTRL_AUTOACK 0x00004 > +#define REG_CTRL_AUTOSE 0x00008 > +#define REG_CTRL_AUTOSN 0x00010 > +#define REG_CTRL_ARBDIS 0x00020 > +#define REG_CTRL_GCAMEN 0x00040 > +#define REG_CTRL_CLHR__MASK 0x00300 > +#define REG_CTRL_BITO__MASK 0x03000 > +#define REG_CTRL_BITO_OFF 0x00000 > +#define REG_CTRL_BITO_40PCC 0x01000 > +#define REG_CTRL_BITO_80PCC 0x02000 > +#define REG_CTRL_BITO_160PCC 0x03000 > +#define REG_CTRL_GIBITO 0x08000 > +#define REG_CTRL_CLTO__MASK 0x70000 > +#define REG_CTRL_CLTO_OFF 0x00000 > + > +#define REG_CMD 0x04 > +#define REG_CMD_START 0x00001 > +#define REG_CMD_STOP 0x00002 > +#define REG_CMD_ACK 0x00004 > +#define REG_CMD_NACK 0x00008 > +#define REG_CMD_CONT 0x00010 > +#define REG_CMD_ABORT 0x00020 > +#define REG_CMD_CLEARTX 0x00040 > +#define REG_CMD_CLEARPC 0x00080 > + > +#define REG_STATE 0x08 > +#define REG_STATE_BUSY 0x00001 > +#define REG_STATE_MASTER 0x00002 > +#define REG_STATE_TRANSMITTER 0x00004 > +#define REG_STATE_NACKED 0x00008 > +#define REG_STATE_BUSHOLD 0x00010 > +#define REG_STATE_STATE__MASK 0x000e0 > +#define REG_STATE_STATE_IDLE 0x00000 > +#define REG_STATE_STATE_WAIT 0x00020 > +#define REG_STATE_STATE_START 0x00040 > +#define REG_STATE_STATE_ADDR 0x00060 > +#define REG_STATE_STATE_ADDRACK 0x00080 > +#define REG_STATE_STATE_DATA 0x000a0 > +#define REG_STATE_STATE_DATAACK 0x000c0 > + > +#define REG_STATUS 0x0c > +#define REG_STATUS_PSTART 0x00001 > +#define REG_STATUS_PSTOP 0x00002 > +#define REG_STATUS_PACK 0x00004 > +#define REG_STATUS_PNACK 0x00008 > +#define REG_STATUS_PCONT 0x00010 > +#define REG_STATUS_PABORT 0x00020 > +#define REG_STATUS_TXC 0x00040 > +#define REG_STATUS_TXBL 0x00080 > +#define REG_STATUS_RXDATAV 0x00100 > + > +#define REG_CLKDIV 0x10 > +#define REG_CLKDIV_DIV__MASK 0x001ff > +#define REG_CLKDIV_DIV(div) MASK_VAL(REG_CLKDIV_DIV__MASK, (div)) > + > +#define REG_SADDR 0x14 > +#define REG_SADDRMASK 0x18 > +#define REG_RXDATA 0x1c > +#define REG_RXDATAP 0x20 > +#define REG_TXDATA 0x24 > +#define REG_IF 0x28 > +#define REG_IF_START 0x00001 > +#define REG_IF_RSTART 0x00002 > +#define REG_IF_ADDR 0x00004 > +#define REG_IF_TXC 0x00008 > +#define REG_IF_TXBL 0x00010 > +#define REG_IF_RXDATAV 0x00020 > +#define REG_IF_ACK 0x00040 > +#define REG_IF_NACK 0x00080 > +#define REG_IF_MSTOP 0x00100 > +#define REG_IF_ARBLOST 0x00200 > +#define REG_IF_BUSERR 0x00400 > +#define REG_IF_BUSHOLD 0x00800 > +#define REG_IF_TXOF 0x01000 > +#define REG_IF_RXUF 0x02000 > +#define REG_IF_BITO 0x04000 > +#define REG_IF_CLTO 0x08000 > +#define REG_IF_SSTOP 0x10000 > + > +#define REG_IFS 0x2c > +#define REG_IFC 0x30 > +#define REG_IFC__MASK 0x1ffcf > + > +#define REG_IEN 0x34 > + > +#define REG_ROUTE 0x38 > +#define REG_ROUTE_SDAPEN 0x00001 > +#define REG_ROUTE_SCLPEN 0x00002 > +#define REG_ROUTE_LOCATION__MASK 0x00700 > +#define REG_ROUTE_LOCATION(n) MASK_VAL(REG_ROUTE_LOCATION__MASK, (n)) > + > +struct efm32_i2c_ddata { > + struct i2c_adapter adapter; > + spinlock_t lock; > + > + struct clk *clk; > + void __iomem *base; > + unsigned int irq; > + struct efm32_i2c_pdata pdata; > + > + /* transfer data */ > + struct completion done; > + struct i2c_msg *msgs; > + size_t num_msgs; > + size_t current_word, current_msg; > +}; > + > +static u32 efm32_i2c_read32(struct efm32_i2c_ddata *ddata, unsigned offset) > +{ > + return readl(ddata->base + offset); > +} > + > +static void efm32_i2c_write32(struct efm32_i2c_ddata *ddata, > + unsigned offset, u32 value) > +{ > + writel(value, ddata->base + offset); > +} > + > +static void efm32_i2c_send_next_msg(struct efm32_i2c_ddata *ddata) > +{ > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + > + dev_dbg(&ddata->adapter.dev, "send msg %zu/%zu (addr = %x, flags = %x, if = %08x)\n", > + ddata->current_msg, ddata->num_msgs, cur_msg->addr, > + cur_msg->flags, efm32_i2c_read32(ddata, REG_IF)); > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_START); > + efm32_i2c_write32(ddata, REG_TXDATA, cur_msg->addr << 1 | > + (cur_msg->flags & I2C_M_RD ? 1 : 0)); > +} > + > +static void efm32_i2c_send_next_byte(struct efm32_i2c_ddata *ddata) > +{ > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + dev_dbg(&ddata->adapter.dev, "%s, %zu %zu\n", > + __func__, ddata->current_word, cur_msg->len); > + if (ddata->current_word >= cur_msg->len) { > + /* cur_msg completely transferred */ > + ddata->current_word = 0; > + ddata->current_msg += 1; > + > + if (ddata->current_msg >= ddata->num_msgs) { > + dev_dbg(&ddata->adapter.dev, "Stop\n"); > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); > + complete(&ddata->done); > + } else { > + efm32_i2c_send_next_msg(ddata); > + } > + } else { > + dev_dbg(&ddata->adapter.dev, "send byte %zu/%zu\n", > + ddata->current_word, cur_msg->len); > + efm32_i2c_write32(ddata, REG_TXDATA, > + cur_msg->buf[ddata->current_word++]); > + } > +} > + > +static void efm32_i2c_recv_next_byte(struct efm32_i2c_ddata *ddata) > +{ > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + > + cur_msg->buf[ddata->current_word] = efm32_i2c_read32(ddata, REG_RXDATA); > + dev_dbg(&ddata->adapter.dev, "recv byte %zu/%zu: 0x%02hhx\n", > + ddata->current_word, cur_msg->len, cur_msg->buf[ddata->current_word]); > + ddata->current_word += 1; > + if (ddata->current_word >= cur_msg->len) { > + /* cur_msg completely transferred */ > + ddata->current_word = 0; > + ddata->current_msg += 1; > + > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_NACK); > + > + if (ddata->current_msg >= ddata->num_msgs) { > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); > + complete(&ddata->done); > + } else { > + efm32_i2c_send_next_msg(ddata); > + } > + } else { > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_ACK); > + } > +} > + > +static irqreturn_t efm32_i2c_irq(int irq, void *dev_id) > +{ > + struct efm32_i2c_ddata *ddata = dev_id; > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + u32 irqflag = efm32_i2c_read32(ddata, REG_IF); > + u32 state = efm32_i2c_read32(ddata, REG_STATE); > + > + dev_dbg(&ddata->adapter.dev, "irq: if: %08x, state: %08x, status: %08x\n", > + irqflag, state, efm32_i2c_read32(ddata, REG_STATUS)); > + efm32_i2c_write32(ddata, REG_IFC, irqflag & REG_IFC__MASK); > + > + switch (state & REG_STATE_STATE__MASK) { > + case REG_STATE_STATE_IDLE: > + /* arbitration lost? */ > + complete(&ddata->done); > + break; > + case REG_STATE_STATE_WAIT: > + /* huh, this shouldn't happen */ > + BUG(); > + break; > + case REG_STATE_STATE_START: > + /* "caller" is expected to send an address */ > + break; > + case REG_STATE_STATE_ADDR: > + /* wait for Ack or NAck of slave */ > + break; > + case REG_STATE_STATE_ADDRACK: > + if (state & REG_STATE_NACKED) { > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); > + complete(&ddata->done); > + } else if (cur_msg->flags & I2C_M_RD) { > + /* wait for slave to send first data byte */ > + } else { > + efm32_i2c_send_next_byte(ddata); > + } > + break; > + case REG_STATE_STATE_DATA: > + if (cur_msg->flags & I2C_M_RD) { > + efm32_i2c_recv_next_byte(ddata); > + } else { > + /* wait for Ack or Nack of slave */ > + } > + break; > + case REG_STATE_STATE_DATAACK: > + if (state & REG_STATE_NACKED) { > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); > + complete(&ddata->done); > + } else { > + efm32_i2c_send_next_byte(ddata); > + } > + } > + > + return IRQ_HANDLED; > +} > + > +static int efm32_i2c_master_xfer(struct i2c_adapter *adap, > + struct i2c_msg *msgs, int num) > +{ > + struct efm32_i2c_ddata *ddata = i2c_get_adapdata(adap); > + int ret = -EBUSY; > + > + spin_lock_irq(&ddata->lock); > + > + if (ddata->msgs) > + /* > + * XXX: can .master_xfer be called when the previous is still > + * running? > + */ > + goto out_unlock; > + > + ddata->msgs = msgs; > + ddata->num_msgs = num; > + ddata->current_word = 0; > + ddata->current_msg = 0; > + > + init_completion(&ddata->done); > + > + dev_dbg(&ddata->adapter.dev, "state: %08x, status: %08x\n", > + efm32_i2c_read32(ddata, REG_STATE), > + efm32_i2c_read32(ddata, REG_STATUS)); > + > + efm32_i2c_send_next_msg(ddata); > + > + spin_unlock_irq(&ddata->lock); > + > + wait_for_completion(&ddata->done); > + > + spin_lock_irq(&ddata->lock); > + > + if (ddata->current_msg >= ddata->num_msgs) > + ret = ddata->num_msgs; > + else > + ret = -EIO; > + > + ddata->msgs = NULL; > + > +out_unlock: > + spin_unlock_irq(&ddata->lock); > + return ret; > +} > + > +static u32 efm32_i2c_functionality(struct i2c_adapter *adap) > +{ > + /* XXX: some more? */ > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > +} > + > +static const struct i2c_algorithm efm32_i2c_algo = { > + .master_xfer = efm32_i2c_master_xfer, > + .functionality = efm32_i2c_functionality, > +}; > + > +static u32 efm32_i2c_get_configured_location(struct efm32_i2c_ddata *ddata) > +{ > + u32 reg = efm32_i2c_read32(ddata, REG_ROUTE); > + > + return (reg & REG_ROUTE_LOCATION__MASK) >> > + __ffs(REG_ROUTE_LOCATION__MASK); > +} > + > +static int efm32_i2c_probe_dt(struct platform_device *pdev, > + struct efm32_i2c_ddata *ddata) > +{ > + struct device_node *np = pdev->dev.of_node; > + u32 location, frequency; > + int ret; > + > + if (!np) > + return 1; > + > + ret = of_property_read_u32(np, "location", &location); > + if (!ret) { > + dev_dbg(&pdev->dev, "using location %u\n", location); > + } else { > + /* default to location configured in hardware */ > + location = efm32_i2c_get_configured_location(ddata); > + > + dev_info(&pdev->dev, "fall back to location %u\n", location); > + } > + > + ddata->pdata.location = location; > + > + ret = of_property_read_u32(np, "clock-frequency", &frequency); > + if (!ret) { > + dev_dbg(&pdev->dev, "using frequency %u\n", frequency); > + } else { > + frequency = 100000; > + dev_info(&pdev->dev, "defaulting to 100 kHz\n"); > + } > + ddata->pdata.frequency = frequency; > + > + /* i2c core takes care about bus numbering using an alias */ > + ddata->adapter.nr = -1; > + > + return 0; > +} > + > +static int efm32_i2c_probe(struct platform_device *pdev) > +{ > + struct efm32_i2c_ddata *ddata; > + struct resource *res; > + unsigned long rate; > + int ret; > + u32 clkdiv; > + > + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); > + if (!ddata) { > + dev_dbg(&pdev->dev, "failed to allocate private data\n"); > + return -ENOMEM; > + } > + platform_set_drvdata(pdev, ddata); > + > + strlcpy(ddata->adapter.name, pdev->name, sizeof(ddata->adapter.name)); > + ddata->adapter.owner = THIS_MODULE; > + ddata->adapter.algo = &efm32_i2c_algo; > + ddata->adapter.dev.parent = &pdev->dev; > + ddata->adapter.dev.of_node = pdev->dev.of_node; > + i2c_set_adapdata(&ddata->adapter, ddata); > + > + spin_lock_init(&ddata->lock); > + > + ddata->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(ddata->clk)) { > + ret = PTR_ERR(ddata->clk); > + dev_err(&pdev->dev, "failed to get clock: %d\n", ret); > + return ret; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "failed to determine base address\n"); > + return -ENODEV; > + } > + > + if (resource_size(res) < 0x42) { > + dev_err(&pdev->dev, "memory resource too small\n"); > + return -EINVAL; > + } > + > + ddata->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(ddata->base)) > + return PTR_ERR(ddata->base); > + > + ret = platform_get_irq(pdev, 0); > + if (ret <= 0) { > + dev_err(&pdev->dev, "failed to get irq (%d)\n", ret); > + if (!ret) > + ret = -EINVAL; > + return ret; > + } > + > + ddata->irq = ret; > + > + ret = clk_prepare_enable(ddata->clk); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to enable clock (%d)\n", ret); > + return ret; > + } > + > + ret = efm32_i2c_probe_dt(pdev, ddata); > + if (ret > 0) { > + /* not created by device tree */ > + const struct efm32_i2c_pdata *pdata = > + dev_get_platdata(&pdev->dev); > + > + if (pdata) > + ddata->pdata = *pdata; > + else { > + ddata->pdata.location = > + efm32_i2c_get_configured_location(ddata); > + ddata->pdata.frequency = 100000; > + } > + > + ddata->adapter.nr = pdev->id; > + } else if (ret < 0) { > + goto err_disable_clk; > + } > + > + rate = clk_get_rate(ddata->clk); > + if (!rate) { > + dev_err(&pdev->dev, "there is no input clock available\n"); > + ret = -EIO; > + goto err_disable_clk; > + } > + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; > + if (clkdiv >= 0x200) { > + dev_err(&pdev->dev, > + "input clock too fast (%lu) to divide down to bus freq (%lu)", > + rate, ddata->pdata.frequency); > + ret = -EIO; > + goto err_disable_clk; > + } > + > + dev_dbg(&pdev->dev, "input clock = %lu, bus freq = %lu, clkdiv = %lu\n", > + rate, ddata->pdata.frequency, (unsigned long)clkdiv); > + efm32_i2c_write32(ddata, REG_CLKDIV, REG_CLKDIV_DIV(clkdiv)); > + > + efm32_i2c_write32(ddata, REG_ROUTE, REG_ROUTE_SDAPEN | > + REG_ROUTE_SCLPEN | > + REG_ROUTE_LOCATION(ddata->pdata.location)); > + > + efm32_i2c_write32(ddata, REG_CTRL, REG_CTRL_EN | > + REG_CTRL_BITO_160PCC | 0 * REG_CTRL_GIBITO); > + > + efm32_i2c_write32(ddata, REG_IFC, REG_IFC__MASK); > + efm32_i2c_write32(ddata, REG_IEN, REG_IF_TXC | REG_IF_ACK | REG_IF_NACK > + | REG_IF_ARBLOST | REG_IF_BUSERR | REG_IF_RXDATAV); > + > + /* to make bus idle */ > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_ABORT); > + > + ret = request_irq(ddata->irq, efm32_i2c_irq, 0, DRIVER_NAME, ddata); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request irq (%d)\n", ret); > + return ret; > + } > + > + ret = i2c_add_numbered_adapter(&ddata->adapter); > + if (ret) { > + dev_err(&pdev->dev, "failed to add i2c adapter (%d)\n", ret); > + free_irq(ddata->irq, ddata); > + > +err_disable_clk: > + clk_disable_unprepare(ddata->clk); > + } > + return ret; > +} > + > +static int efm32_i2c_remove(struct platform_device *pdev) > +{ > + struct efm32_i2c_ddata *ddata = platform_get_drvdata(pdev); > + > + free_irq(ddata->irq, ddata); > + clk_disable_unprepare(ddata->clk); > + > + return 0; > +} > + > +static const struct of_device_id efm32_i2c_dt_ids[] = { > + { > + .compatible = "efm32,i2c", > + }, { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(of, efm32_i2c_dt_ids); > + > +static struct platform_driver efm32_i2c_driver = { > + .probe = efm32_i2c_probe, > + .remove = efm32_i2c_remove, > + > + .driver = { > + .name = DRIVER_NAME, > + .owner = THIS_MODULE, > + .of_match_table = efm32_i2c_dt_ids, > + }, > +}; > +module_platform_driver(efm32_i2c_driver); > + > +MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>"); > +MODULE_DESCRIPTION("EFM32 i2c driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:" DRIVER_NAME); > diff --git a/include/linux/platform_data/efm32-i2c.h b/include/linux/platform_data/efm32-i2c.h > new file mode 100644 > index 000000000000..5e175db68bb6 > --- /dev/null > +++ b/include/linux/platform_data/efm32-i2c.h > @@ -0,0 +1,15 @@ > +#ifndef __LINUX_PLATFORM_DATA_EFM32_I2C_H__ > +#define __LINUX_PLATFORM_DATA_EFM32_I2C_H__ > + > +#include <linux/types.h> > + > +/** > + * struct efm32_i2c_pdata > + * @location: pinmux location for the I/O pins (to be written to the ROUTE > + * register) > + */ > +struct efm32_i2c_pdata { > + u8 location; > + unsigned long frequency; /* in Hz */ > +}; > +#endif /* ifndef __LINUX_PLATFORM_DATA_EFM32_I2C_H__ */ > -- > 1.8.5.2 > >
On Mon, Feb 10, 2014 at 11:19:58AM +0100, Uwe Kleine-König wrote: > This was tested on a EFM32GG-DK3750 devboard that has a temperature > sensor and an eeprom on its i2c bus. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> still no reply :-( Uwe
Hi Uwe, > +#include <linux/platform_data/efm32-i2c.h> Shouldn't a new platform like efm32 be DT only? > + > +struct efm32_i2c_ddata { > + struct i2c_adapter adapter; > + spinlock_t lock; No need, see later. > + struct clk *clk; > + void __iomem *base; > + unsigned int irq; > + struct efm32_i2c_pdata pdata; > + > + /* transfer data */ > + struct completion done; > + struct i2c_msg *msgs; > + size_t num_msgs; > + size_t current_word, current_msg; > +}; > + > +static u32 efm32_i2c_read32(struct efm32_i2c_ddata *ddata, unsigned offset) > +{ > + return readl(ddata->base + offset); > +} > + > +static void efm32_i2c_write32(struct efm32_i2c_ddata *ddata, > + unsigned offset, u32 value) > +{ > + writel(value, ddata->base + offset); > +} Do those two really help readability? > +static void efm32_i2c_send_next_msg(struct efm32_i2c_ddata *ddata) > +{ > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + > + dev_dbg(&ddata->adapter.dev, "send msg %zu/%zu (addr = %x, flags = %x, if = %08x)\n", > + ddata->current_msg, ddata->num_msgs, cur_msg->addr, > + cur_msg->flags, efm32_i2c_read32(ddata, REG_IF)); Remove. We have stuff like this in the core and will soon get tracing functionality. > + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_START); > + efm32_i2c_write32(ddata, REG_TXDATA, cur_msg->addr << 1 | > + (cur_msg->flags & I2C_M_RD ? 1 : 0)); > +} > + > +static void efm32_i2c_send_next_byte(struct efm32_i2c_ddata *ddata) > +{ > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > + dev_dbg(&ddata->adapter.dev, "%s, %zu %zu\n", > + __func__, ddata->current_word, cur_msg->len); Hmm, quite much debug output in this driver... ... > + switch (state & REG_STATE_STATE__MASK) { > + case REG_STATE_STATE_IDLE: > + /* arbitration lost? */ > + complete(&ddata->done); If arbitration is lost, you should return -EAGAIN. > + break; > + case REG_STATE_STATE_WAIT: > + /* huh, this shouldn't happen */ > + BUG(); Is this really a reason to halt the kernel? > +static int efm32_i2c_master_xfer(struct i2c_adapter *adap, > + struct i2c_msg *msgs, int num) > +{ > + struct efm32_i2c_ddata *ddata = i2c_get_adapdata(adap); > + int ret = -EBUSY; > + > + spin_lock_irq(&ddata->lock); > + > + if (ddata->msgs) > + /* > + * XXX: can .master_xfer be called when the previous is still > + * running? > + */ Check the core. It has per adapter locks. So the lock can go away. > + goto out_unlock; > + > + ddata->msgs = msgs; > + ddata->num_msgs = num; > + ddata->current_word = 0; > + ddata->current_msg = 0; > + > + init_completion(&ddata->done); reinit_completion() here and init_completion() in probe. > + > + dev_dbg(&ddata->adapter.dev, "state: %08x, status: %08x\n", > + efm32_i2c_read32(ddata, REG_STATE), > + efm32_i2c_read32(ddata, REG_STATUS)); > + > + efm32_i2c_send_next_msg(ddata); > + > + spin_unlock_irq(&ddata->lock); > + > + wait_for_completion(&ddata->done); > + > + spin_lock_irq(&ddata->lock); > + > + if (ddata->current_msg >= ddata->num_msgs) > + ret = ddata->num_msgs; > + else > + ret = -EIO; Check Documentation/i2c/fault-codes for more fine grained responses. > + > + ddata->msgs = NULL; > + > +out_unlock: > + spin_unlock_irq(&ddata->lock); > + return ret; > +} > + > +static u32 efm32_i2c_functionality(struct i2c_adapter *adap) > +{ > + /* XXX: some more? */ > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; That is usually enough. Make sure you checked SMBUS_QUICK, though (i2cdetect -q ...). > + ret = of_property_read_u32(np, "location", &location); Huh? Is this an accepted binding? Doesn't look like it because of a generic name and IMO a specific use-case. BTW the binding documentation for this driver is missing. > + if (!ret) { > + dev_dbg(&pdev->dev, "using location %u\n", location); > + } else { > + /* default to location configured in hardware */ > + location = efm32_i2c_get_configured_location(ddata); > + > + dev_info(&pdev->dev, "fall back to location %u\n", location); > + } > + > + ddata->pdata.location = location; > + > + ret = of_property_read_u32(np, "clock-frequency", &frequency); > + if (!ret) { > + dev_dbg(&pdev->dev, "using frequency %u\n", frequency); > + } else { > + frequency = 100000; > + dev_info(&pdev->dev, "defaulting to 100 kHz\n"); > + } > + ddata->pdata.frequency = frequency; > + > + /* i2c core takes care about bus numbering using an alias */ > + ddata->adapter.nr = -1; In case of DT only, drop this and simply use i2c_add_adapter. > + > + return 0; > +} > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "failed to determine base address\n"); devm_ioremap_resource() checks for a valid resource. Drop this. > + return -ENODEV; > + } > + > + if (resource_size(res) < 0x42) { > + dev_err(&pdev->dev, "memory resource too small\n"); > + return -EINVAL; > + } I'd drop this check since, but I won't force you to. > + ret = efm32_i2c_probe_dt(pdev, ddata); > + if (ret > 0) { > + /* not created by device tree */ As said above: Wondering about this driver not being DT only. > + rate = clk_get_rate(ddata->clk); > + if (!rate) { > + dev_err(&pdev->dev, "there is no input clock available\n"); > + ret = -EIO; > + goto err_disable_clk; > + } > + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; > + if (clkdiv >= 0x200) { > + dev_err(&pdev->dev, > + "input clock too fast (%lu) to divide down to bus freq (%lu)", > + rate, ddata->pdata.frequency); > + ret = -EIO; > + goto err_disable_clk; > + } -EIO for clocks errors? Is this common? > +static int efm32_i2c_remove(struct platform_device *pdev) > +{ > + struct efm32_i2c_ddata *ddata = platform_get_drvdata(pdev); > + > + free_irq(ddata->irq, ddata); > + clk_disable_unprepare(ddata->clk); No i2c_del_adapter()? > + > + return 0; > +} > + > +static const struct of_device_id efm32_i2c_dt_ids[] = { > + { > + .compatible = "efm32,i2c", > + }, { > + /* sentinel */ > + } One line per entry is better to read IMO. Regards, Wolfram
Hi Wolfram, On Mon, Mar 10, 2014 at 08:55:58AM +0100, Wolfram Sang wrote: > > +#include <linux/platform_data/efm32-i2c.h> > > Shouldn't a new platform like efm32 be DT only? it is, at least in mainline. My (not very strong) POV is that it's not much effort/code size to support both. I dropped the non-DT part, it's easily readded if need should arise. > > + > > +struct efm32_i2c_ddata { > > + struct i2c_adapter adapter; > > + spinlock_t lock; > > No need, see later. Ok. > > ... > > +static u32 efm32_i2c_read32(struct efm32_i2c_ddata *ddata, unsigned offset) > > +{ > > + return readl(ddata->base + offset); > > +} > > + > > +static void efm32_i2c_write32(struct efm32_i2c_ddata *ddata, > > + unsigned offset, u32 value) > > +{ > > + writel(value, ddata->base + offset); > > +} > > Do those two really help readability? I like them to add debug code, usually trace_printk. > > +static void efm32_i2c_send_next_msg(struct efm32_i2c_ddata *ddata) > > +{ > > + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; > > + > > + dev_dbg(&ddata->adapter.dev, "send msg %zu/%zu (addr = %x, flags = %x, if = %08x)\n", > > + ddata->current_msg, ddata->num_msgs, cur_msg->addr, > > + cur_msg->flags, efm32_i2c_read32(ddata, REG_IF)); > > Remove. We have stuff like this in the core and will soon get tracing > functionality. ok, dropped with several other dev_dbg. > > + switch (state & REG_STATE_STATE__MASK) { > > + case REG_STATE_STATE_IDLE: > > + /* arbitration lost? */ > > + complete(&ddata->done); > > If arbitration is lost, you should return -EAGAIN. ok. > > + break; > > + case REG_STATE_STATE_WAIT: > > + /* huh, this shouldn't happen */ > > + BUG(); > > Is this really a reason to halt the kernel? No, probably not. What do you suggest? Reinit the hardware, report and return an error? > > +static int efm32_i2c_master_xfer(struct i2c_adapter *adap, > > + struct i2c_msg *msgs, int num) > > +{ > > + struct efm32_i2c_ddata *ddata = i2c_get_adapdata(adap); > > + int ret = -EBUSY; > > + > > + spin_lock_irq(&ddata->lock); > > + > > + if (ddata->msgs) > > + /* > > + * XXX: can .master_xfer be called when the previous is still > > + * running? > > + */ > > Check the core. It has per adapter locks. So the lock can go away. ok. So I can also drop the "if (ddata->msgs)" check, right? > > + goto out_unlock; > > + > > + ddata->msgs = msgs; > > + ddata->num_msgs = num; > > + ddata->current_word = 0; > > + ddata->current_msg = 0; > > + > > + init_completion(&ddata->done); > > reinit_completion() here and init_completion() in probe. *nod* > > + > > + dev_dbg(&ddata->adapter.dev, "state: %08x, status: %08x\n", > > + efm32_i2c_read32(ddata, REG_STATE), > > + efm32_i2c_read32(ddata, REG_STATUS)); > > + > > + efm32_i2c_send_next_msg(ddata); > > + > > + spin_unlock_irq(&ddata->lock); > > + > > + wait_for_completion(&ddata->done); > > + > > + spin_lock_irq(&ddata->lock); > > + > > + if (ddata->current_msg >= ddata->num_msgs) > > + ret = ddata->num_msgs; > > + else > > + ret = -EIO; > > Check Documentation/i2c/fault-codes for more fine grained responses. ok, I have EAGAIN for arbitration lost, ENXIO for NAck in address phase and EIO for NAck in data phase now. Sounds good? > > + > > + ddata->msgs = NULL; > > + > > +out_unlock: > > + spin_unlock_irq(&ddata->lock); > > + return ret; > > +} > > + > > +static u32 efm32_i2c_functionality(struct i2c_adapter *adap) > > +{ > > + /* XXX: some more? */ > > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > > That is usually enough. Make sure you checked SMBUS_QUICK, though > (i2cdetect -q ...). Both -q and -r seem to do the right thing. > > + ret = of_property_read_u32(np, "location", &location); > > Huh? Is this an accepted binding? Doesn't look like it because of a > generic name and IMO a specific use-case. BTW the binding documentation > for this driver is missing. Well, I got it in for the serial and spi driver. It wasn't without discussion, but for lack of better approaches it was accepted. The problem is that there are two places to care for to setup the pin muxing on efm32. The pins that are used in the (here) i2c function are selected in the ROUTE register in the I2C module's address space. Still I need to configure the pins in the GPIO module as input; pushpull; open drain or open collector (and a few more details like driver strength). You could abstract that with a big list of phandles and so have pinmuxing only in the pin controler device, but that's ugly, error prone to use and implement, not understandable with the hardware reference and (I guess) not how the hardware works internally. Regarding the generic name: I don't care much, but I don't have a problem with it. IMHO it's implicitly name-spaced by the compatible string which starts with "efm32," and so is fine. I'd like to have the same property name for all efm32 device drivers and "location" matches the hardware reference manual (apart from capitalization). > > ... > > + /* i2c core takes care about bus numbering using an alias */ > > + ddata->adapter.nr = -1; > > In case of DT only, drop this and simply use i2c_add_adapter. done > > > + > > + return 0; > > +} > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(&pdev->dev, "failed to determine base address\n"); > > devm_ioremap_resource() checks for a valid resource. Drop this. But resource_size doesn't ... > > + return -ENODEV; > > + } > > + > > + if (resource_size(res) < 0x42) { > > + dev_err(&pdev->dev, "memory resource too small\n"); > > + return -EINVAL; > > + } > > I'd drop this check since, but I won't force you to. I'd understand your sentence with s/since//, not sure about it as is. Anyhow, I like this check. > > + rate = clk_get_rate(ddata->clk); > > + if (!rate) { > > + dev_err(&pdev->dev, "there is no input clock available\n"); > > + ret = -EIO; > > + goto err_disable_clk; > > + } > > + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; > > + if (clkdiv >= 0x200) { > > + dev_err(&pdev->dev, > > + "input clock too fast (%lu) to divide down to bus freq (%lu)", > > + rate, ddata->pdata.frequency); > > + ret = -EIO; > > + goto err_disable_clk; > > + } > > -EIO for clocks errors? Is this common? Changed to ENODEV. Ok? > > +static int efm32_i2c_remove(struct platform_device *pdev) > > +{ > > + struct efm32_i2c_ddata *ddata = platform_get_drvdata(pdev); > > + > > + free_irq(ddata->irq, ddata); > > + clk_disable_unprepare(ddata->clk); > > No i2c_del_adapter()? added. I fixed the driver in my tree for now, will resend when we settled on the things still open. Best regards Uwe
> it is, at least in mainline. My (not very strong) POV is that it's not > much effort/code size to support both. I dropped the non-DT part, it's > easily readded if need should arise. Thanks, I think it simplifies the review for this first (public) iteration of the driver. > > > + break; > > > + case REG_STATE_STATE_WAIT: > > > + /* huh, this shouldn't happen */ > > > + BUG(); > > > > Is this really a reason to halt the kernel? > No, probably not. What do you suggest? Reinit the hardware, report and > return an error? Can't really say, because I don't know what the HW is waiting for. What you say sounds sensible, though. > > Check the core. It has per adapter locks. So the lock can go away. > ok. So I can also drop the "if (ddata->msgs)" check, right? Yes. > > Check Documentation/i2c/fault-codes for more fine grained responses. > ok, I have EAGAIN for arbitration lost, ENXIO for NAck in address phase > and EIO for NAck in data phase now. Sounds good? Yup! > > That is usually enough. Make sure you checked SMBUS_QUICK, though > > (i2cdetect -q ...). > Both -q and -r seem to do the right thing. Good. > > Huh? Is this an accepted binding? Doesn't look like it because of a > > generic name and IMO a specific use-case. BTW the binding documentation > > for this driver is missing. > Regarding the generic name: I don't care much, but I don't have a > problem with it. IMHO it's implicitly name-spaced by the compatible > string which starts with "efm32," and so is fine. I'd like to have the > same property name for all efm32 device drivers and "location" matches > the hardware reference manual (apart from capitalization). I would in deed have expected a binding like "efm32,location" to emphasize this is an efm32 specific thing. I know vendor-specific "setup" bindings from elsewhere. Since it has been accepted already in other places, we should keep it likes this. > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > + if (!res) { > > > + dev_err(&pdev->dev, "failed to determine base address\n"); > > > > devm_ioremap_resource() checks for a valid resource. Drop this. > But resource_size doesn't ... Right (another reason to drop the check in my book ;)) > > > > + return -ENODEV; > > > + } > > > + > > > + if (resource_size(res) < 0x42) { > > > + dev_err(&pdev->dev, "memory resource too small\n"); > > > + return -EINVAL; > > > + } > > > > I'd drop this check since, but I won't force you to. > I'd understand your sentence with s/since//, not sure about it as is. > Anyhow, I like this check. A leftover. I was about to write "since the check is somewhat heuristic and does not proof much". But then I decided it is not worth spending too much discussion on it :) > > > + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; > > > + if (clkdiv >= 0x200) { > > > + dev_err(&pdev->dev, > > > + "input clock too fast (%lu) to divide down to bus freq (%lu)", > > > + rate, ddata->pdata.frequency); > > > + ret = -EIO; > > > + goto err_disable_clk; > > > + } > > > > -EIO for clocks errors? Is this common? > Changed to ENODEV. Ok? Nope, then the driver core will silent drop the error. -EINVAL? Regards, Wolfram
Hi Wolfram, On Thu, Mar 13, 2014 at 11:14:33PM +0100, Wolfram Sang wrote: > > > Huh? Is this an accepted binding? Doesn't look like it because of a > > > generic name and IMO a specific use-case. BTW the binding documentation > > > for this driver is missing. > > Regarding the generic name: I don't care much, but I don't have a > > problem with it. IMHO it's implicitly name-spaced by the compatible > > string which starts with "efm32," and so is fine. I'd like to have the > > same property name for all efm32 device drivers and "location" matches > > the hardware reference manual (apart from capitalization). > > I would in deed have expected a binding like "efm32,location" to > emphasize this is an efm32 specific thing. I know vendor-specific > "setup" bindings from elsewhere. Since it has been accepted already in > other places, we should keep it likes this. Assuming you know the dt stuff better than me (and noone objects) I'd fix the intree drivers to use efm32,location, too. > > > > + if (resource_size(res) < 0x42) { > > > > + dev_err(&pdev->dev, "memory resource too small\n"); > > > > + return -EINVAL; > > > > + } > > > > > > I'd drop this check since, but I won't force you to. > > I'd understand your sentence with s/since//, not sure about it as is. > > Anyhow, I like this check. > > A leftover. I was about to write "since the check is somewhat heuristic > and does not proof much". But then I decided it is not worth spending > too much discussion on it :) I'd say it's heuristic to *not* check the boundary. It doesn't apply here, but consider a driver using a register space > PAGE_SIZE on an MMU platform. It accesses base + PAGE_SIZE + 0x42 even if dt only gave a register range of PAGE_SIZE / 2. It's like gets(3). > > > > + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; > > > > + if (clkdiv >= 0x200) { > > > > + dev_err(&pdev->dev, > > > > + "input clock too fast (%lu) to divide down to bus freq (%lu)", > > > > + rate, ddata->pdata.frequency); > > > > + ret = -EIO; > > > > + goto err_disable_clk; > > > > + } > > > > > > -EIO for clocks errors? Is this common? > > Changed to ENODEV. Ok? > > Nope, then the driver core will silent drop the error. -EINVAL? agreed Uwe
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index f5ed03164d86..7322ff0c4c60 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -432,6 +432,13 @@ config I2C_DESIGNWARE_PCI This driver can also be built as a module. If so, the module will be called i2c-designware-pci. +config I2C_EFM32 + tristate "EFM32 I2C controller" + depends on OF && (ARCH_EFM32 || COMPILE_TEST) + help + This driver supports the i2c block found in Energy Micro's EFM32 + SoCs. + config I2C_EG20T tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C" depends on PCI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index a08931fe73e1..2a56ab181851 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o i2c-designware-platform-objs := i2c-designware-platdrv.o obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o i2c-designware-pci-objs := i2c-designware-pcidrv.o +obj-$(CONFIG_I2C_EFM32) += i2c-efm32.o obj-$(CONFIG_I2C_EG20T) += i2c-eg20t.o obj-$(CONFIG_I2C_EXYNOS5) += i2c-exynos5.o obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o diff --git a/drivers/i2c/busses/i2c-efm32.c b/drivers/i2c/busses/i2c-efm32.c new file mode 100644 index 000000000000..9e860abcab63 --- /dev/null +++ b/drivers/i2c/busses/i2c-efm32.c @@ -0,0 +1,527 @@ +/* + * Copyright (C) 2014 Uwe Kleine-Koenig for Pengutronix + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License version 2 as published by the + * Free Software Foundation. + */ +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/i2c.h> +#include <linux/io.h> +#include <linux/interrupt.h> +#include <linux/err.h> +#include <linux/clk.h> +#include <linux/platform_data/efm32-i2c.h> + +#define DRIVER_NAME "efm32-i2c" + +#define MASK_VAL(mask, val) ((val << __ffs(mask)) & mask) + +#define REG_CTRL 0x00 +#define REG_CTRL_EN 0x00001 +#define REG_CTRL_SLAVE 0x00002 +#define REG_CTRL_AUTOACK 0x00004 +#define REG_CTRL_AUTOSE 0x00008 +#define REG_CTRL_AUTOSN 0x00010 +#define REG_CTRL_ARBDIS 0x00020 +#define REG_CTRL_GCAMEN 0x00040 +#define REG_CTRL_CLHR__MASK 0x00300 +#define REG_CTRL_BITO__MASK 0x03000 +#define REG_CTRL_BITO_OFF 0x00000 +#define REG_CTRL_BITO_40PCC 0x01000 +#define REG_CTRL_BITO_80PCC 0x02000 +#define REG_CTRL_BITO_160PCC 0x03000 +#define REG_CTRL_GIBITO 0x08000 +#define REG_CTRL_CLTO__MASK 0x70000 +#define REG_CTRL_CLTO_OFF 0x00000 + +#define REG_CMD 0x04 +#define REG_CMD_START 0x00001 +#define REG_CMD_STOP 0x00002 +#define REG_CMD_ACK 0x00004 +#define REG_CMD_NACK 0x00008 +#define REG_CMD_CONT 0x00010 +#define REG_CMD_ABORT 0x00020 +#define REG_CMD_CLEARTX 0x00040 +#define REG_CMD_CLEARPC 0x00080 + +#define REG_STATE 0x08 +#define REG_STATE_BUSY 0x00001 +#define REG_STATE_MASTER 0x00002 +#define REG_STATE_TRANSMITTER 0x00004 +#define REG_STATE_NACKED 0x00008 +#define REG_STATE_BUSHOLD 0x00010 +#define REG_STATE_STATE__MASK 0x000e0 +#define REG_STATE_STATE_IDLE 0x00000 +#define REG_STATE_STATE_WAIT 0x00020 +#define REG_STATE_STATE_START 0x00040 +#define REG_STATE_STATE_ADDR 0x00060 +#define REG_STATE_STATE_ADDRACK 0x00080 +#define REG_STATE_STATE_DATA 0x000a0 +#define REG_STATE_STATE_DATAACK 0x000c0 + +#define REG_STATUS 0x0c +#define REG_STATUS_PSTART 0x00001 +#define REG_STATUS_PSTOP 0x00002 +#define REG_STATUS_PACK 0x00004 +#define REG_STATUS_PNACK 0x00008 +#define REG_STATUS_PCONT 0x00010 +#define REG_STATUS_PABORT 0x00020 +#define REG_STATUS_TXC 0x00040 +#define REG_STATUS_TXBL 0x00080 +#define REG_STATUS_RXDATAV 0x00100 + +#define REG_CLKDIV 0x10 +#define REG_CLKDIV_DIV__MASK 0x001ff +#define REG_CLKDIV_DIV(div) MASK_VAL(REG_CLKDIV_DIV__MASK, (div)) + +#define REG_SADDR 0x14 +#define REG_SADDRMASK 0x18 +#define REG_RXDATA 0x1c +#define REG_RXDATAP 0x20 +#define REG_TXDATA 0x24 +#define REG_IF 0x28 +#define REG_IF_START 0x00001 +#define REG_IF_RSTART 0x00002 +#define REG_IF_ADDR 0x00004 +#define REG_IF_TXC 0x00008 +#define REG_IF_TXBL 0x00010 +#define REG_IF_RXDATAV 0x00020 +#define REG_IF_ACK 0x00040 +#define REG_IF_NACK 0x00080 +#define REG_IF_MSTOP 0x00100 +#define REG_IF_ARBLOST 0x00200 +#define REG_IF_BUSERR 0x00400 +#define REG_IF_BUSHOLD 0x00800 +#define REG_IF_TXOF 0x01000 +#define REG_IF_RXUF 0x02000 +#define REG_IF_BITO 0x04000 +#define REG_IF_CLTO 0x08000 +#define REG_IF_SSTOP 0x10000 + +#define REG_IFS 0x2c +#define REG_IFC 0x30 +#define REG_IFC__MASK 0x1ffcf + +#define REG_IEN 0x34 + +#define REG_ROUTE 0x38 +#define REG_ROUTE_SDAPEN 0x00001 +#define REG_ROUTE_SCLPEN 0x00002 +#define REG_ROUTE_LOCATION__MASK 0x00700 +#define REG_ROUTE_LOCATION(n) MASK_VAL(REG_ROUTE_LOCATION__MASK, (n)) + +struct efm32_i2c_ddata { + struct i2c_adapter adapter; + spinlock_t lock; + + struct clk *clk; + void __iomem *base; + unsigned int irq; + struct efm32_i2c_pdata pdata; + + /* transfer data */ + struct completion done; + struct i2c_msg *msgs; + size_t num_msgs; + size_t current_word, current_msg; +}; + +static u32 efm32_i2c_read32(struct efm32_i2c_ddata *ddata, unsigned offset) +{ + return readl(ddata->base + offset); +} + +static void efm32_i2c_write32(struct efm32_i2c_ddata *ddata, + unsigned offset, u32 value) +{ + writel(value, ddata->base + offset); +} + +static void efm32_i2c_send_next_msg(struct efm32_i2c_ddata *ddata) +{ + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; + + dev_dbg(&ddata->adapter.dev, "send msg %zu/%zu (addr = %x, flags = %x, if = %08x)\n", + ddata->current_msg, ddata->num_msgs, cur_msg->addr, + cur_msg->flags, efm32_i2c_read32(ddata, REG_IF)); + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_START); + efm32_i2c_write32(ddata, REG_TXDATA, cur_msg->addr << 1 | + (cur_msg->flags & I2C_M_RD ? 1 : 0)); +} + +static void efm32_i2c_send_next_byte(struct efm32_i2c_ddata *ddata) +{ + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; + dev_dbg(&ddata->adapter.dev, "%s, %zu %zu\n", + __func__, ddata->current_word, cur_msg->len); + if (ddata->current_word >= cur_msg->len) { + /* cur_msg completely transferred */ + ddata->current_word = 0; + ddata->current_msg += 1; + + if (ddata->current_msg >= ddata->num_msgs) { + dev_dbg(&ddata->adapter.dev, "Stop\n"); + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); + complete(&ddata->done); + } else { + efm32_i2c_send_next_msg(ddata); + } + } else { + dev_dbg(&ddata->adapter.dev, "send byte %zu/%zu\n", + ddata->current_word, cur_msg->len); + efm32_i2c_write32(ddata, REG_TXDATA, + cur_msg->buf[ddata->current_word++]); + } +} + +static void efm32_i2c_recv_next_byte(struct efm32_i2c_ddata *ddata) +{ + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; + + cur_msg->buf[ddata->current_word] = efm32_i2c_read32(ddata, REG_RXDATA); + dev_dbg(&ddata->adapter.dev, "recv byte %zu/%zu: 0x%02hhx\n", + ddata->current_word, cur_msg->len, cur_msg->buf[ddata->current_word]); + ddata->current_word += 1; + if (ddata->current_word >= cur_msg->len) { + /* cur_msg completely transferred */ + ddata->current_word = 0; + ddata->current_msg += 1; + + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_NACK); + + if (ddata->current_msg >= ddata->num_msgs) { + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); + complete(&ddata->done); + } else { + efm32_i2c_send_next_msg(ddata); + } + } else { + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_ACK); + } +} + +static irqreturn_t efm32_i2c_irq(int irq, void *dev_id) +{ + struct efm32_i2c_ddata *ddata = dev_id; + struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg]; + u32 irqflag = efm32_i2c_read32(ddata, REG_IF); + u32 state = efm32_i2c_read32(ddata, REG_STATE); + + dev_dbg(&ddata->adapter.dev, "irq: if: %08x, state: %08x, status: %08x\n", + irqflag, state, efm32_i2c_read32(ddata, REG_STATUS)); + efm32_i2c_write32(ddata, REG_IFC, irqflag & REG_IFC__MASK); + + switch (state & REG_STATE_STATE__MASK) { + case REG_STATE_STATE_IDLE: + /* arbitration lost? */ + complete(&ddata->done); + break; + case REG_STATE_STATE_WAIT: + /* huh, this shouldn't happen */ + BUG(); + break; + case REG_STATE_STATE_START: + /* "caller" is expected to send an address */ + break; + case REG_STATE_STATE_ADDR: + /* wait for Ack or NAck of slave */ + break; + case REG_STATE_STATE_ADDRACK: + if (state & REG_STATE_NACKED) { + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); + complete(&ddata->done); + } else if (cur_msg->flags & I2C_M_RD) { + /* wait for slave to send first data byte */ + } else { + efm32_i2c_send_next_byte(ddata); + } + break; + case REG_STATE_STATE_DATA: + if (cur_msg->flags & I2C_M_RD) { + efm32_i2c_recv_next_byte(ddata); + } else { + /* wait for Ack or Nack of slave */ + } + break; + case REG_STATE_STATE_DATAACK: + if (state & REG_STATE_NACKED) { + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_STOP); + complete(&ddata->done); + } else { + efm32_i2c_send_next_byte(ddata); + } + } + + return IRQ_HANDLED; +} + +static int efm32_i2c_master_xfer(struct i2c_adapter *adap, + struct i2c_msg *msgs, int num) +{ + struct efm32_i2c_ddata *ddata = i2c_get_adapdata(adap); + int ret = -EBUSY; + + spin_lock_irq(&ddata->lock); + + if (ddata->msgs) + /* + * XXX: can .master_xfer be called when the previous is still + * running? + */ + goto out_unlock; + + ddata->msgs = msgs; + ddata->num_msgs = num; + ddata->current_word = 0; + ddata->current_msg = 0; + + init_completion(&ddata->done); + + dev_dbg(&ddata->adapter.dev, "state: %08x, status: %08x\n", + efm32_i2c_read32(ddata, REG_STATE), + efm32_i2c_read32(ddata, REG_STATUS)); + + efm32_i2c_send_next_msg(ddata); + + spin_unlock_irq(&ddata->lock); + + wait_for_completion(&ddata->done); + + spin_lock_irq(&ddata->lock); + + if (ddata->current_msg >= ddata->num_msgs) + ret = ddata->num_msgs; + else + ret = -EIO; + + ddata->msgs = NULL; + +out_unlock: + spin_unlock_irq(&ddata->lock); + return ret; +} + +static u32 efm32_i2c_functionality(struct i2c_adapter *adap) +{ + /* XXX: some more? */ + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; +} + +static const struct i2c_algorithm efm32_i2c_algo = { + .master_xfer = efm32_i2c_master_xfer, + .functionality = efm32_i2c_functionality, +}; + +static u32 efm32_i2c_get_configured_location(struct efm32_i2c_ddata *ddata) +{ + u32 reg = efm32_i2c_read32(ddata, REG_ROUTE); + + return (reg & REG_ROUTE_LOCATION__MASK) >> + __ffs(REG_ROUTE_LOCATION__MASK); +} + +static int efm32_i2c_probe_dt(struct platform_device *pdev, + struct efm32_i2c_ddata *ddata) +{ + struct device_node *np = pdev->dev.of_node; + u32 location, frequency; + int ret; + + if (!np) + return 1; + + ret = of_property_read_u32(np, "location", &location); + if (!ret) { + dev_dbg(&pdev->dev, "using location %u\n", location); + } else { + /* default to location configured in hardware */ + location = efm32_i2c_get_configured_location(ddata); + + dev_info(&pdev->dev, "fall back to location %u\n", location); + } + + ddata->pdata.location = location; + + ret = of_property_read_u32(np, "clock-frequency", &frequency); + if (!ret) { + dev_dbg(&pdev->dev, "using frequency %u\n", frequency); + } else { + frequency = 100000; + dev_info(&pdev->dev, "defaulting to 100 kHz\n"); + } + ddata->pdata.frequency = frequency; + + /* i2c core takes care about bus numbering using an alias */ + ddata->adapter.nr = -1; + + return 0; +} + +static int efm32_i2c_probe(struct platform_device *pdev) +{ + struct efm32_i2c_ddata *ddata; + struct resource *res; + unsigned long rate; + int ret; + u32 clkdiv; + + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); + if (!ddata) { + dev_dbg(&pdev->dev, "failed to allocate private data\n"); + return -ENOMEM; + } + platform_set_drvdata(pdev, ddata); + + strlcpy(ddata->adapter.name, pdev->name, sizeof(ddata->adapter.name)); + ddata->adapter.owner = THIS_MODULE; + ddata->adapter.algo = &efm32_i2c_algo; + ddata->adapter.dev.parent = &pdev->dev; + ddata->adapter.dev.of_node = pdev->dev.of_node; + i2c_set_adapdata(&ddata->adapter, ddata); + + spin_lock_init(&ddata->lock); + + ddata->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(ddata->clk)) { + ret = PTR_ERR(ddata->clk); + dev_err(&pdev->dev, "failed to get clock: %d\n", ret); + return ret; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "failed to determine base address\n"); + return -ENODEV; + } + + if (resource_size(res) < 0x42) { + dev_err(&pdev->dev, "memory resource too small\n"); + return -EINVAL; + } + + ddata->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(ddata->base)) + return PTR_ERR(ddata->base); + + ret = platform_get_irq(pdev, 0); + if (ret <= 0) { + dev_err(&pdev->dev, "failed to get irq (%d)\n", ret); + if (!ret) + ret = -EINVAL; + return ret; + } + + ddata->irq = ret; + + ret = clk_prepare_enable(ddata->clk); + if (ret < 0) { + dev_err(&pdev->dev, "failed to enable clock (%d)\n", ret); + return ret; + } + + ret = efm32_i2c_probe_dt(pdev, ddata); + if (ret > 0) { + /* not created by device tree */ + const struct efm32_i2c_pdata *pdata = + dev_get_platdata(&pdev->dev); + + if (pdata) + ddata->pdata = *pdata; + else { + ddata->pdata.location = + efm32_i2c_get_configured_location(ddata); + ddata->pdata.frequency = 100000; + } + + ddata->adapter.nr = pdev->id; + } else if (ret < 0) { + goto err_disable_clk; + } + + rate = clk_get_rate(ddata->clk); + if (!rate) { + dev_err(&pdev->dev, "there is no input clock available\n"); + ret = -EIO; + goto err_disable_clk; + } + clkdiv = DIV_ROUND_UP(rate, 8 * ddata->pdata.frequency) - 1; + if (clkdiv >= 0x200) { + dev_err(&pdev->dev, + "input clock too fast (%lu) to divide down to bus freq (%lu)", + rate, ddata->pdata.frequency); + ret = -EIO; + goto err_disable_clk; + } + + dev_dbg(&pdev->dev, "input clock = %lu, bus freq = %lu, clkdiv = %lu\n", + rate, ddata->pdata.frequency, (unsigned long)clkdiv); + efm32_i2c_write32(ddata, REG_CLKDIV, REG_CLKDIV_DIV(clkdiv)); + + efm32_i2c_write32(ddata, REG_ROUTE, REG_ROUTE_SDAPEN | + REG_ROUTE_SCLPEN | + REG_ROUTE_LOCATION(ddata->pdata.location)); + + efm32_i2c_write32(ddata, REG_CTRL, REG_CTRL_EN | + REG_CTRL_BITO_160PCC | 0 * REG_CTRL_GIBITO); + + efm32_i2c_write32(ddata, REG_IFC, REG_IFC__MASK); + efm32_i2c_write32(ddata, REG_IEN, REG_IF_TXC | REG_IF_ACK | REG_IF_NACK + | REG_IF_ARBLOST | REG_IF_BUSERR | REG_IF_RXDATAV); + + /* to make bus idle */ + efm32_i2c_write32(ddata, REG_CMD, REG_CMD_ABORT); + + ret = request_irq(ddata->irq, efm32_i2c_irq, 0, DRIVER_NAME, ddata); + if (ret < 0) { + dev_err(&pdev->dev, "failed to request irq (%d)\n", ret); + return ret; + } + + ret = i2c_add_numbered_adapter(&ddata->adapter); + if (ret) { + dev_err(&pdev->dev, "failed to add i2c adapter (%d)\n", ret); + free_irq(ddata->irq, ddata); + +err_disable_clk: + clk_disable_unprepare(ddata->clk); + } + return ret; +} + +static int efm32_i2c_remove(struct platform_device *pdev) +{ + struct efm32_i2c_ddata *ddata = platform_get_drvdata(pdev); + + free_irq(ddata->irq, ddata); + clk_disable_unprepare(ddata->clk); + + return 0; +} + +static const struct of_device_id efm32_i2c_dt_ids[] = { + { + .compatible = "efm32,i2c", + }, { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(of, efm32_i2c_dt_ids); + +static struct platform_driver efm32_i2c_driver = { + .probe = efm32_i2c_probe, + .remove = efm32_i2c_remove, + + .driver = { + .name = DRIVER_NAME, + .owner = THIS_MODULE, + .of_match_table = efm32_i2c_dt_ids, + }, +}; +module_platform_driver(efm32_i2c_driver); + +MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>"); +MODULE_DESCRIPTION("EFM32 i2c driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:" DRIVER_NAME); diff --git a/include/linux/platform_data/efm32-i2c.h b/include/linux/platform_data/efm32-i2c.h new file mode 100644 index 000000000000..5e175db68bb6 --- /dev/null +++ b/include/linux/platform_data/efm32-i2c.h @@ -0,0 +1,15 @@ +#ifndef __LINUX_PLATFORM_DATA_EFM32_I2C_H__ +#define __LINUX_PLATFORM_DATA_EFM32_I2C_H__ + +#include <linux/types.h> + +/** + * struct efm32_i2c_pdata + * @location: pinmux location for the I/O pins (to be written to the ROUTE + * register) + */ +struct efm32_i2c_pdata { + u8 location; + unsigned long frequency; /* in Hz */ +}; +#endif /* ifndef __LINUX_PLATFORM_DATA_EFM32_I2C_H__ */
This was tested on a EFM32GG-DK3750 devboard that has a temperature sensor and an eeprom on its i2c bus. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, there are two places marked with a triple-X where I don't know if I did it right. The one is "Do I need to protect from .master_xfer being called again before the previous call returned?" The other is what to return in the .functionality callback. Also note there is a matching entry in MAINTAINERS for "ARM/ENERGY MICRO (SILICON LABS) EFM32 SUPPORT". Best regards Uwe drivers/i2c/busses/Kconfig | 7 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-efm32.c | 527 ++++++++++++++++++++++++++++++++ include/linux/platform_data/efm32-i2c.h | 15 + 4 files changed, 550 insertions(+) create mode 100644 drivers/i2c/busses/i2c-efm32.c create mode 100644 include/linux/platform_data/efm32-i2c.h