* Re: [PATCH v2 3/4] Device bindings documentation updated ACPI-enabled platforms not currently supported
From: Mark Rutland @ 2016-10-14 17:30 UTC (permalink / raw)
To: Luis.Oliveira
Cc: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, devicetree, CARLOS.PALMINHA,
Ramiro.Oliveira
In-Reply-To: <36f4b1736e36b039af15b13645df0de492f1b6e1.1476462204.git.lolivei@synopsys.com>
On Fri, Oct 14, 2016 at 05:52:50PM +0100, Luis.Oliveira@synopsys.com wrote:
> - is_slave = device_property_read_bool(&pdev->dev, "isslave");
Which tree is this based on? I cant see the existing isslave property in
mainline HEAD (commit 29fbff8698fc0ac1).
> +#ifndef CONFIG_ACPI
> + is_slave = device_property_read_bool(&pdev->dev, "is-slave");
> +#endif
This ifdef is broken. At least for arm64, a single kernel image can be
booted with either ACPI or DT. We need separate accessors for DT and
ACPI to handle these differently, or you need to explicitly check
whether or not you have ACPI or DT at runtime.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2 3/4] Device bindings documentation updated ACPI-enabled platforms not currently supported
From: Luis.Oliveira @ 2016-10-14 16:52 UTC (permalink / raw)
To: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, mark.rutland, devicetree
Cc: Luis.Oliveira, CARLOS.PALMINHA, Ramiro.Oliveira
In-Reply-To: <cover.1476462204.git.lolivei@synopsys.com>
From: Luis Oliveira <lolivei@synopsys.com>
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
Documentation/devicetree/bindings/i2c/i2c-designware.txt | 5 ++++-
drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
index fee26dc..d3d163c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
@@ -19,6 +19,8 @@ Optional properties :
- i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
This value which is by default 300ns is used to compute the tHIGH period.
+
+ - is-slave : indicates if the block is a I2C slave device.
Example :
@@ -30,7 +32,7 @@ Example :
interrupts = <11>;
clock-frequency = <400000>;
};
-
+
i2c@1120000 {
#address-cells = <1>;
#size-cells = <0>;
@@ -42,4 +44,5 @@ Example :
i2c-sda-hold-time-ns = <300>;
i2c-sda-falling-time-ns = <300>;
i2c-scl-falling-time-ns = <300>;
+ is-slave;
};
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index b9076da..f29e657 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -238,7 +238,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
&dev->scl_falling_time);
device_property_read_u32(&pdev->dev, "clock-frequency",
&dev->clk_freq);
- is_slave = device_property_read_bool(&pdev->dev, "isslave");
+#ifndef CONFIG_ACPI
+ is_slave = device_property_read_bool(&pdev->dev, "is-slave");
+#endif
}
acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
--
2.10.1
^ permalink raw reply related
* [PATCH v2 2/4] Added I2C_SLAVE as a dependency to I2C_DESIGNWARE_CORE Enable _slave() mode Review of the pm_runtime...() methods and cleaning
From: Luis.Oliveira @ 2016-10-14 16:52 UTC (permalink / raw)
To: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, mark.rutland, devicetree
Cc: Luis.Oliveira, CARLOS.PALMINHA, Ramiro.Oliveira
In-Reply-To: <cover.1476462204.git.lolivei@synopsys.com>
From: Luis Oliveira <lolivei@synopsys.com>
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
drivers/i2c/busses/Kconfig | 3 +-
drivers/i2c/busses/i2c-designware-core.c | 180 ++++++++++++++++++++++++++--
drivers/i2c/busses/i2c-designware-core.h | 6 +
drivers/i2c/busses/i2c-designware-platdrv.c | 33 ++++-
4 files changed, 212 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 6d94e2e..f93ad70 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -470,10 +470,11 @@ config I2C_DESIGNWARE_CORE
config I2C_DESIGNWARE_PLATFORM
tristate "Synopsys DesignWare Platform"
select I2C_DESIGNWARE_CORE
+ select I2C_SLAVE
depends on (ACPI && COMMON_CLK) || !ACPI
help
If you say yes to this option, support will be included for the
- Synopsys DesignWare I2C adapter. Only master mode is supported.
+ Synopsys DesignWare I2C adapter.
This driver can also be built as a module. If so, the module
will be called i2c-designware-platform.
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index cc38329..71a377e 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -37,6 +37,7 @@
*/
#define DW_IC_CON 0x0
#define DW_IC_TAR 0x4
+#define DW_IC_SAR 0x8
#define DW_IC_DATA_CMD 0x10
#define DW_IC_SS_SCL_HCNT 0x14
#define DW_IC_SS_SCL_LCNT 0x18
@@ -92,10 +93,16 @@
#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_TX_EMPTY)
-
+
+#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_RX_DONE | \
+ DW_IC_INTR_RX_UNDER | \
+ DW_IC_INTR_RD_REQ)
+
#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_STATUS_TFE BIT(2)
#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
+#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
#define DW_IC_ERR_TX_ABRT 0x1
@@ -130,6 +137,9 @@
#define ABRT_10B_RD_NORSTRT 10
#define ABRT_MASTER_DIS 11
#define ARB_LOST 12
+#define ABRT_SLAVE_FLUSH_TXFIFO 13
+#define ABRT_SLAVE_ARBLOST 14
+#define ABRT_SLAVE_RD_INTX 15
#define DW_IC_TX_ABRT_7B_ADDR_NOACK (1UL << ABRT_7B_ADDR_NOACK)
#define DW_IC_TX_ABRT_10ADDR1_NOACK (1UL << ABRT_10ADDR1_NOACK)
@@ -142,6 +152,9 @@
#define DW_IC_TX_ABRT_10B_RD_NORSTRT (1UL << ABRT_10B_RD_NORSTRT)
#define DW_IC_TX_ABRT_MASTER_DIS (1UL << ABRT_MASTER_DIS)
#define DW_IC_TX_ARB_LOST (1UL << ARB_LOST)
+#define DW_IC_RX_ABRT_SLAVE_RD_INTX (1UL << ABRT_SLAVE_RD_INTX)
+#define DW_IC_RX_ABRT_SLAVE_ARBLOST (1UL << ABRT_SLAVE_ARBLOST)
+#define DW_IC_RX_ABRT_SLAVE_FLUSH_TXFIFO (1UL << ABRT_SLAVE_FLUSH_TXFIFO)
#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
DW_IC_TX_ABRT_10ADDR1_NOACK | \
@@ -172,6 +185,12 @@ static char *abort_sources[] = {
"trying to use disabled adapter",
[ARB_LOST] =
"lost arbitration",
+ [ABRT_SLAVE_FLUSH_TXFIFO] =
+ "read command so flush old data in the TX FIFO",
+ [ABRT_SLAVE_ARBLOST] =
+ "slave lost the bus while transmitting data to a remote master",
+ [ABRT_SLAVE_RD_INTX] =
+ "slave request for data to be transmitted and",
};
static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
@@ -214,6 +233,17 @@ static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
}
+static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
+{
+ /* Configure Tx/Rx FIFO threshold levels */
+ dw_writel(dev, 0, DW_IC_TX_TL);
+ dw_writel(dev, 0, DW_IC_RX_TL);
+
+ /* configure the i2c slave */
+ dw_writel(dev, dev->slave_cfg , DW_IC_CON);
+ dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
+}
+
static u32
i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
{
@@ -447,6 +477,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
if ((dev->master_cfg & DW_IC_CON_MASTER) &&
(dev->master_cfg & DW_IC_CON_SLAVE_DISABLE))
i2c_dw_configure_fifo_master(dev);
+ else
+ i2c_dw_configure_fifo_slave(dev);
i2c_dw_release_lock(dev);
@@ -785,9 +817,59 @@ static u32 i2c_dw_func(struct i2c_adapter *adap)
return dev->functionality;
}
+static int i2c_dw_reg_slave(struct i2c_client *slave)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
+
+ if(dev->slave)
+ return -EBUSY;
+ if(slave->flags & I2C_CLIENT_TEN)
+ return -EAFNOSUPPORT;
+ /* set slave address in the IC_SAR register,
+ * the address to which the DW_apb_i2c responds */
+
+ __i2c_dw_enable(dev, false);
+
+ dw_writel(dev, slave->addr, DW_IC_SAR);
+
+ pm_runtime_get_sync(dev->dev);
+
+ dev->slave = slave;
+
+ __i2c_dw_enable(dev, true);
+
+ dev->cmd_err = 0;
+ dev->msg_write_idx = 0;
+ dev->msg_read_idx = 0;
+ dev->msg_err = 0;
+ dev->status = STATUS_IDLE;
+ dev->abort_source = 0;
+ dev->rx_outstanding = 0;
+
+ return 0;
+}
+
+static int i2c_dw_unreg_slave(struct i2c_client *slave)
+{
+ struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
+
+ WARN_ON(!dev->slave);
+
+ i2c_dw_disable_int(dev);
+ i2c_dw_disable(dev);
+
+ dev->slave = NULL;
+
+ pm_runtime_put(dev->dev);
+
+ return 0;
+}
+
static struct i2c_algorithm i2c_dw_algo = {
.master_xfer = i2c_dw_xfer,
.functionality = i2c_dw_func,
+ .reg_slave = i2c_dw_reg_slave,
+ .unreg_slave = i2c_dw_unreg_slave,
};
static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
@@ -821,8 +903,6 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
dw_readl(dev, DW_IC_CLR_RX_OVER);
if (stat & DW_IC_INTR_TX_OVER)
dw_readl(dev, DW_IC_CLR_TX_OVER);
- if (stat & DW_IC_INTR_RD_REQ)
- dw_readl(dev, DW_IC_CLR_RD_REQ);
if (stat & DW_IC_INTR_TX_ABRT) {
/*
* The IC_TX_ABRT_SOURCE register is cleared whenever
@@ -849,6 +929,84 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* Interrupt service routine. This gets called whenever an I2C interrupt
* occurs.
*/
+
+static bool i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev )
+{
+ u32 raw_stat, stat, enabled;
+ u8 val, slave_activity;
+
+ stat = dw_readl(dev, DW_IC_INTR_STAT);
+ enabled = dw_readl(dev, DW_IC_ENABLE);
+ raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+ slave_activity = ((dw_readl(dev,DW_IC_STATUS) &
+ DW_IC_STATUS_SLAVE_ACTIVITY)>>6);
+
+ dev_dbg(dev->dev,
+ "%s: %#x SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
+ __func__, enabled, slave_activity, raw_stat, stat);
+
+ if (stat & DW_IC_INTR_START_DET)
+ dw_readl(dev, DW_IC_CLR_START_DET);
+
+ if (stat & DW_IC_INTR_ACTIVITY)
+ dw_readl(dev, DW_IC_CLR_ACTIVITY);
+
+ if (stat & DW_IC_INTR_RX_OVER)
+ dw_readl(dev, DW_IC_CLR_RX_OVER);
+
+ if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+
+ if (slave_activity) {
+ if (stat & DW_IC_INTR_RD_REQ) {
+ if (stat & DW_IC_INTR_RX_FULL) {
+ val = dw_readl(dev, DW_IC_DATA_CMD);
+ if (!i2c_slave_event(dev->slave,
+ I2C_SLAVE_WRITE_RECEIVED, &val)) {
+ dev_dbg(dev->dev, "Byte %X acked! ",val);
+ }
+ dw_readl(dev, DW_IC_CLR_RD_REQ);
+ stat = i2c_dw_read_clear_intrbits(dev);
+ }
+ else {
+ dw_readl(dev, DW_IC_CLR_RD_REQ);
+ dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ stat = i2c_dw_read_clear_intrbits(dev);
+ }
+ if (!i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED, &val))
+ dw_writel(dev, val, DW_IC_DATA_CMD);
+ }
+ }
+
+ if (stat & DW_IC_INTR_RX_DONE) {
+
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val))
+ dw_readl(dev, DW_IC_CLR_RX_DONE);
+
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP , &val);
+ stat = i2c_dw_read_clear_intrbits(dev);
+
+ return true;
+ }
+
+ if (stat & DW_IC_INTR_RX_FULL) {
+ val = dw_readl(dev, DW_IC_DATA_CMD);
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED, &val))
+ dev_dbg(dev->dev, "Byte %X acked! ",val);
+ }
+ else {
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP , &val);
+ stat = i2c_dw_read_clear_intrbits(dev);
+ }
+
+ if (stat & DW_IC_INTR_TX_OVER) {
+ dw_readl(dev, DW_IC_CLR_TX_OVER);
+ return true;
+ }
+
+ return true;
+}
static bool i2c_dw_irq_handler_master(struct dw_i2c_dev *dev )
{
@@ -910,14 +1068,20 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
enabled = dw_readl(dev, DW_IC_ENABLE);
mode = dw_readl(dev, DW_IC_CON);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-
+
dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
-
+
+ if (!(mode & DW_IC_CON_MASTER) && !(mode & DW_IC_CON_SLAVE_DISABLE)) {
+ stat = i2c_dw_read_clear_intrbits(dev);
+ if (!i2c_dw_irq_handler_slave(dev))
+ return IRQ_NONE;
+ } else {
if(i2c_dw_irq_handler_master(dev))
return IRQ_HANDLED;
-
+ }
+
complete(&dev->cmd_complete);
return IRQ_HANDLED;
}
@@ -984,7 +1148,9 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
adap->dev.parent = dev->dev;
i2c_set_adapdata(adap, dev);
- i2c_dw_disable_int(dev);
+ if (!i2c_check_functionality(adap,I2C_FUNC_SLAVE))
+ i2c_dw_disable_int(dev);
+
r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
IRQF_SHARED | IRQF_COND_SUSPEND,
dev_name(dev->dev), dev);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 0d44d2a..de5e4a0 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -28,9 +28,13 @@
#define DW_IC_CON_SPEED_FAST 0x4
#define DW_IC_CON_SPEED_HIGH 0x6
#define DW_IC_CON_SPEED_MASK 0x6
+#define DW_IC_CON_10BITADDR_SLAVE 0x8
#define DW_IC_CON_10BITADDR_MASTER 0x10
#define DW_IC_CON_RESTART_EN 0x20
#define DW_IC_CON_SLAVE_DISABLE 0x40
+#define DW_IC_CON_STOP_DET_IFADDRESSED 0x80
+#define DW_IC_CON_TX_EMPTY_CTRL 0x100
+#define DW_IC_CON_RX_FIFO_FULL_HLD_CTRL 0x200
/**
@@ -80,6 +84,7 @@ struct dw_i2c_dev {
void __iomem *base;
struct completion cmd_complete;
struct clk *clk;
+ struct i2c_client *slave;
u32 (*get_clk_rate_khz) (struct dw_i2c_dev *dev);
struct dw_pci_controller *controller;
int cmd_err;
@@ -99,6 +104,7 @@ struct dw_i2c_dev {
struct i2c_adapter adapter;
u32 functionality;
u32 master_cfg;
+ u32 slave_cfg;
unsigned int tx_fifo_depth;
unsigned int rx_fifo_depth;
int rx_outstanding;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d5986c2..b9076da 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -161,6 +161,30 @@ static void i2c_dw_configure_master(struct platform_device *pdev)
}
+static void i2c_dw_configure_slave(struct platform_device *pdev)
+{
+ struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+
+ dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED |
+ DW_IC_CON_SPEED_FAST;
+
+ dev->functionality |= I2C_FUNC_SLAVE;
+ dev->functionality &= ~I2C_FUNC_10BIT_ADDR;
+ dev_info(&pdev->dev, "I am registed as a I2C Slave!\n");
+
+ switch (dev->clk_freq) {
+ case 100000:
+ dev->slave_cfg |= DW_IC_CON_SPEED_STD;
+
+ case 3400000:
+ dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
+ break;
+ default:
+ dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
+ }
+}
+
static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
{
if (IS_ERR(i_dev->clk))
@@ -181,6 +205,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
struct resource *mem;
int irq, r;
u32 acpi_speed, ht = 0;
+ bool is_slave = false;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -213,6 +238,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
&dev->scl_falling_time);
device_property_read_u32(&pdev->dev, "clock-frequency",
&dev->clk_freq);
+ is_slave = device_property_read_bool(&pdev->dev, "isslave");
}
acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
@@ -244,8 +270,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
-
- i2c_dw_configure_master(pdev);
+
+ if (is_slave)
+ i2c_dw_configure_slave(pdev);
+ else
+ i2c_dw_configure_master(pdev);
dev->clk = devm_clk_get(&pdev->dev, NULL);
if (!i2c_dw_plat_prepare_clk(dev, true)) {
--
2.10.1
^ permalink raw reply related
* [PATCH v2 4/4] Cleaned the code, no functional changes.
From: Luis.Oliveira @ 2016-10-14 16:52 UTC (permalink / raw)
To: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, mark.rutland, devicetree
Cc: Luis.Oliveira, CARLOS.PALMINHA, Ramiro.Oliveira
In-Reply-To: <cover.1476462204.git.lolivei@synopsys.com>
From: Luis Oliveira <lolivei@synopsys.com>
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
drivers/i2c/busses/i2c-designware-core.c | 113 ++++++++++++++--------------
drivers/i2c/busses/i2c-designware-platdrv.c | 24 +++---
2 files changed, 68 insertions(+), 69 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 71a377e..4196491 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -90,15 +90,15 @@
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
-
+
#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_TX_EMPTY)
-
+
#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \
DW_IC_INTR_RX_DONE | \
- DW_IC_INTR_RX_UNDER | \
- DW_IC_INTR_RD_REQ)
-
+ DW_IC_INTR_RX_UNDER | \
+ DW_IC_INTR_RD_REQ)
+
#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_STATUS_TFE BIT(2)
#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
@@ -229,7 +229,7 @@ static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
dw_writel(dev, 0, DW_IC_RX_TL);
/* configure the i2c master */
- dw_writel(dev, dev->master_cfg , DW_IC_CON);
+ dw_writel(dev, dev->master_cfg, DW_IC_CON);
dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
}
@@ -240,7 +240,7 @@ static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
dw_writel(dev, 0, DW_IC_RX_TL);
/* configure the i2c slave */
- dw_writel(dev, dev->slave_cfg , DW_IC_CON);
+ dw_writel(dev, dev->slave_cfg, DW_IC_CON);
dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
}
@@ -386,8 +386,8 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Configure register access mode 16bit */
dev->accessor_flags |= ACCESS_16BIT;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
- dev_err(dev->dev, "Unknown Synopsys component type: "
- "0x%08x\n", reg);
+ dev_err(dev->dev, "Unknown Synopsys component type: 0x%08x\n",
+ reg);
i2c_dw_release_lock(dev);
return -ENODEV;
}
@@ -475,7 +475,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
}
if ((dev->master_cfg & DW_IC_CON_MASTER) &&
- (dev->master_cfg & DW_IC_CON_SLAVE_DISABLE))
+ (dev->master_cfg & DW_IC_CON_SLAVE_DISABLE))
i2c_dw_configure_fifo_master(dev);
else
i2c_dw_configure_fifo_slave(dev);
@@ -814,6 +814,7 @@ done_nolock:
static u32 i2c_dw_func(struct i2c_adapter *adap)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
return dev->functionality;
}
@@ -821,19 +822,19 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
- if(dev->slave)
+ if (dev->slave)
return -EBUSY;
- if(slave->flags & I2C_CLIENT_TEN)
+ if (slave->flags & I2C_CLIENT_TEN)
return -EAFNOSUPPORT;
- /* set slave address in the IC_SAR register,
- * the address to which the DW_apb_i2c responds */
+ /* set slave address in the IC_SAR register,
+ * the address to which the DW_apb_i2c responds */
__i2c_dw_enable(dev, false);
-
+
dw_writel(dev, slave->addr, DW_IC_SAR);
pm_runtime_get_sync(dev->dev);
-
+
dev->slave = slave;
__i2c_dw_enable(dev, true);
@@ -929,78 +930,76 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* Interrupt service routine. This gets called whenever an I2C interrupt
* occurs.
*/
-
-static bool i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev )
+
+static bool i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
{
u32 raw_stat, stat, enabled;
u8 val, slave_activity;
-
+
stat = dw_readl(dev, DW_IC_INTR_STAT);
enabled = dw_readl(dev, DW_IC_ENABLE);
raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- slave_activity = ((dw_readl(dev,DW_IC_STATUS) &
- DW_IC_STATUS_SLAVE_ACTIVITY)>>6);
-
- dev_dbg(dev->dev,
- "%s: %#x SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
- __func__, enabled, slave_activity, raw_stat, stat);
+ slave_activity = ((dw_readl(dev, DW_IC_STATUS)
+ & DW_IC_STATUS_SLAVE_ACTIVITY)>>6);
+
+ dev_dbg(dev->dev,
+ "%s: %#x SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
+ __func__, enabled, slave_activity, raw_stat, stat);
if (stat & DW_IC_INTR_START_DET)
dw_readl(dev, DW_IC_CLR_START_DET);
-
+
if (stat & DW_IC_INTR_ACTIVITY)
dw_readl(dev, DW_IC_CLR_ACTIVITY);
if (stat & DW_IC_INTR_RX_OVER)
dw_readl(dev, DW_IC_CLR_RX_OVER);
-
- if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+
+ if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
-
- if (slave_activity) {
- if (stat & DW_IC_INTR_RD_REQ) {
+
+ if (slave_activity) {
+ if (stat & DW_IC_INTR_RD_REQ) {
if (stat & DW_IC_INTR_RX_FULL) {
val = dw_readl(dev, DW_IC_DATA_CMD);
- if (!i2c_slave_event(dev->slave,
+ if (!i2c_slave_event(dev->slave,
I2C_SLAVE_WRITE_RECEIVED, &val)) {
- dev_dbg(dev->dev, "Byte %X acked! ",val);
+ dev_dbg(dev->dev, "Byte %X acked! ", val);
}
dw_readl(dev, DW_IC_CLR_RD_REQ);
stat = i2c_dw_read_clear_intrbits(dev);
- }
- else {
+ } else {
dw_readl(dev, DW_IC_CLR_RD_REQ);
dw_readl(dev, DW_IC_CLR_RX_UNDER);
stat = i2c_dw_read_clear_intrbits(dev);
- }
- if (!i2c_slave_event(dev->slave,
+ }
+ if (!i2c_slave_event(dev->slave,
I2C_SLAVE_READ_REQUESTED, &val))
dw_writel(dev, val, DW_IC_DATA_CMD);
}
- }
-
+ }
+
if (stat & DW_IC_INTR_RX_DONE) {
-
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val))
+
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val))
dw_readl(dev, DW_IC_CLR_RX_DONE);
-
- i2c_slave_event(dev->slave, I2C_SLAVE_STOP , &val);
+
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
stat = i2c_dw_read_clear_intrbits(dev);
return true;
}
-
- if (stat & DW_IC_INTR_RX_FULL) {
+
+ if (stat & DW_IC_INTR_RX_FULL) {
val = dw_readl(dev, DW_IC_DATA_CMD);
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED, &val))
- dev_dbg(dev->dev, "Byte %X acked! ",val);
- }
- else {
- i2c_slave_event(dev->slave, I2C_SLAVE_STOP , &val);
+ dev_dbg(dev->dev, "Byte %X acked! ", val);
+ } else {
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
stat = i2c_dw_read_clear_intrbits(dev);
}
- if (stat & DW_IC_INTR_TX_OVER) {
+ if (stat & DW_IC_INTR_TX_OVER) {
dw_readl(dev, DW_IC_CLR_TX_OVER);
return true;
}
@@ -1008,12 +1007,12 @@ static bool i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev )
return true;
}
-static bool i2c_dw_irq_handler_master(struct dw_i2c_dev *dev )
+static bool i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
{
u32 stat;
-
+
stat = i2c_dw_read_clear_intrbits(dev);
-
+
if (stat & DW_IC_INTR_TX_ABRT) {
dev->cmd_err |= DW_IC_ERR_TX_ABRT;
dev->status = STATUS_IDLE;
@@ -1068,17 +1067,17 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
enabled = dw_readl(dev, DW_IC_ENABLE);
mode = dw_readl(dev, DW_IC_CON);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-
+
dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
-
+
if (!(mode & DW_IC_CON_MASTER) && !(mode & DW_IC_CON_SLAVE_DISABLE)) {
stat = i2c_dw_read_clear_intrbits(dev);
if (!i2c_dw_irq_handler_slave(dev))
return IRQ_NONE;
} else {
- if(i2c_dw_irq_handler_master(dev))
+ if (i2c_dw_irq_handler_master(dev))
return IRQ_HANDLED;
}
@@ -1148,7 +1147,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
adap->dev.parent = dev->dev;
i2c_set_adapdata(adap, dev);
- if (!i2c_check_functionality(adap,I2C_FUNC_SLAVE))
+ if (!i2c_check_functionality(adap, I2C_FUNC_SLAVE))
i2c_dw_disable_int(dev);
r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index f29e657..035b93f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -141,13 +141,13 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
static void i2c_dw_configure_master(struct platform_device *pdev)
{
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-
+
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN;
-
+
dev->functionality |= I2C_FUNC_10BIT_ADDR;
dev_info(&pdev->dev, "I am registed as a I2C Master!\n");
-
+
switch (dev->clk_freq) {
case 100000:
dev->master_cfg |= DW_IC_CON_SPEED_STD;
@@ -158,25 +158,25 @@ static void i2c_dw_configure_master(struct platform_device *pdev)
default:
dev->master_cfg |= DW_IC_CON_SPEED_FAST;
}
-
+
}
static void i2c_dw_configure_slave(struct platform_device *pdev)
{
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-
- dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
- DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED |
+
+ dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
+ DW_IC_CON_RESTART_EN | DW_IC_CON_STOP_DET_IFADDRESSED |
DW_IC_CON_SPEED_FAST;
-
+
dev->functionality |= I2C_FUNC_SLAVE;
dev->functionality &= ~I2C_FUNC_10BIT_ADDR;
dev_info(&pdev->dev, "I am registed as a I2C Slave!\n");
-
+
switch (dev->clk_freq) {
case 100000:
dev->slave_cfg |= DW_IC_CON_SPEED_STD;
-
+
case 3400000:
dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
break;
@@ -272,10 +272,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
-
+
if (is_slave)
i2c_dw_configure_slave(pdev);
- else
+ else
i2c_dw_configure_master(pdev);
dev->clk = devm_clk_get(&pdev->dev, NULL);
--
2.10.1
^ permalink raw reply related
* [PATCH v2 1/4] Factor out _master() parts of code and identify as much as possible all related with MASTER mode
From: Luis.Oliveira @ 2016-10-14 16:52 UTC (permalink / raw)
To: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, mark.rutland, devicetree
Cc: Luis.Oliveira, CARLOS.PALMINHA, Ramiro.Oliveira
In-Reply-To: <cover.1476462204.git.lolivei@synopsys.com>
From: Luis Oliveira <lolivei@synopsys.com>
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
drivers/i2c/busses/i2c-designware-core.c | 71 +++++++++++++++++++----------
drivers/i2c/busses/i2c-designware-platdrv.c | 37 +++++++++------
2 files changed, 71 insertions(+), 37 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 1fe93c4..cc38329 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -87,13 +87,15 @@
#define DW_IC_INTR_GEN_CALL 0x800
#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \
- DW_IC_INTR_TX_EMPTY | \
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
+
+#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \
+ DW_IC_INTR_TX_EMPTY)
#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_STATUS_TFE BIT(2)
-#define DW_IC_STATUS_MST_ACTIVITY BIT(5)
+#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
#define DW_IC_ERR_TX_ABRT 0x1
@@ -201,6 +203,17 @@ static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
}
}
+static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
+{
+ /* Configure Tx/Rx FIFO threshold levels */
+ dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
+ dw_writel(dev, 0, DW_IC_RX_TL);
+
+ /* configure the i2c master */
+ dw_writel(dev, dev->master_cfg , DW_IC_CON);
+ dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
+}
+
static u32
i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
{
@@ -317,10 +330,10 @@ static void i2c_dw_release_lock(struct dw_i2c_dev *dev)
}
/**
- * i2c_dw_init() - initialize the designware i2c master hardware
+ * i2c_dw_init() - initialize the designware i2c hardware
* @dev: device private data
*
- * This functions configures and enables the I2C master.
+ * This functions configures and enables the I2C.
* This function is called during I2C init function, and in case of timeout at
* run time.
*/
@@ -431,12 +444,9 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
"Hardware too old to adjust SDA hold time.\n");
}
- /* Configure Tx/Rx FIFO threshold levels */
- dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
- dw_writel(dev, 0, DW_IC_RX_TL);
-
- /* configure the i2c master */
- dw_writel(dev, dev->master_cfg , DW_IC_CON);
+ if ((dev->master_cfg & DW_IC_CON_MASTER) &&
+ (dev->master_cfg & DW_IC_CON_SLAVE_DISABLE))
+ i2c_dw_configure_fifo_master(dev);
i2c_dw_release_lock(dev);
@@ -480,7 +490,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
*/
ic_status = dw_readl(dev, DW_IC_STATUS);
if (!dev->dynamic_tar_update_enabled ||
- (ic_status & DW_IC_STATUS_MST_ACTIVITY) ||
+ (ic_status & DW_IC_STATUS_MASTER_ACTIVITY) ||
!(ic_status & DW_IC_STATUS_TFE)) {
__i2c_dw_enable_and_wait(dev, false);
enabled = false;
@@ -520,7 +530,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Clear and enable interrupts */
dw_readl(dev, DW_IC_CLR_INTR);
- dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
+ dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
}
/*
@@ -540,7 +550,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u8 *buf = dev->tx_buf;
bool need_restart = false;
- intr_mask = DW_IC_INTR_DEFAULT_MASK;
+ intr_mask = DW_IC_INTR_MASTER_MASK;
for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
/*
@@ -839,19 +849,13 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* Interrupt service routine. This gets called whenever an I2C interrupt
* occurs.
*/
-static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
-{
- struct dw_i2c_dev *dev = dev_id;
- u32 stat, enabled;
-
- enabled = dw_readl(dev, DW_IC_ENABLE);
- stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
- if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
- return IRQ_NONE;
+static bool i2c_dw_irq_handler_master(struct dw_i2c_dev *dev )
+{
+ u32 stat;
+
stat = i2c_dw_read_clear_intrbits(dev);
-
+
if (stat & DW_IC_INTR_TX_ABRT) {
dev->cmd_err |= DW_IC_ERR_TX_ABRT;
dev->status = STATUS_IDLE;
@@ -895,7 +899,26 @@ tx_aborted:
i2c_dw_disable_int(dev);
dw_writel(dev, stat, DW_IC_INTR_MASK);
}
+ return true;
+}
+
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+{
+ struct dw_i2c_dev *dev = dev_id;
+ u32 stat, enabled, mode;
+
+ enabled = dw_readl(dev, DW_IC_ENABLE);
+ mode = dw_readl(dev, DW_IC_CON);
+ stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+
+ dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
+ if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
+ return IRQ_NONE;
+ if(i2c_dw_irq_handler_master(dev))
+ return IRQ_HANDLED;
+
+ complete(&dev->cmd_complete);
return IRQ_HANDLED;
}
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0b42a12..d5986c2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -138,6 +138,29 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
}
#endif
+static void i2c_dw_configure_master(struct platform_device *pdev)
+{
+ struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+
+ dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
+ DW_IC_CON_RESTART_EN;
+
+ dev->functionality |= I2C_FUNC_10BIT_ADDR;
+ dev_info(&pdev->dev, "I am registed as a I2C Master!\n");
+
+ switch (dev->clk_freq) {
+ case 100000:
+ dev->master_cfg |= DW_IC_CON_SPEED_STD;
+ break;
+ case 3400000:
+ dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
+ break;
+ default:
+ dev->master_cfg |= DW_IC_CON_SPEED_FAST;
+ }
+
+}
+
static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
{
if (IS_ERR(i_dev->clk))
@@ -222,19 +245,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
I2C_FUNC_SMBUS_WORD_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
- dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
- DW_IC_CON_RESTART_EN;
-
- switch (dev->clk_freq) {
- case 100000:
- dev->master_cfg |= DW_IC_CON_SPEED_STD;
- break;
- case 3400000:
- dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
- break;
- default:
- dev->master_cfg |= DW_IC_CON_SPEED_FAST;
- }
+ i2c_dw_configure_master(pdev);
dev->clk = devm_clk_get(&pdev->dev, NULL);
if (!i2c_dw_plat_prepare_clk(dev, true)) {
--
2.10.1
^ permalink raw reply related
* [PATCH v2 0/4] Add slave mode to Synopsys I2C driver
From: Luis.Oliveira @ 2016-10-14 16:52 UTC (permalink / raw)
To: jarkko.nikula, andriy.shevchenko, mika.westerberg, wsa, linux-i2c,
linux-kernel, robh+dt, mark.rutland, devicetree
Cc: Luis.Oliveira, CARLOS.PALMINHA, Ramiro.Oliveira
From: Luis Oliveira <lolivei@synopsys.com>
Add support in existing I2C Designware Core driver for I2C slave mode.
Refactored *_master() functions out of existing ones in the first patch
Added *_slave() functions to enable Slave mode.
Updated the description of the i2c-designware.txt and changed Kconfig to
auto enable I2C Slave support.
V2: Splitted the patch in *_master() and *_slave() and updated Kconfig.
Luis Oliveira (4):
Factor out _master() parts of code and identify as much as possible
all related with MASTER mode
Added I2C_SLAVE as a dependency to I2C_DESIGNWARE_CORE Enable _slave()
mode Review of the pm_runtime...() methods and cleaning
Device bindings documentation updated ACPI-enabled platforms not
currently supported
Cleaning
.../devicetree/bindings/i2c/i2c-designware.txt | 5 +-
drivers/i2c/busses/Kconfig | 3 +-
drivers/i2c/busses/i2c-designware-core.c | 238 ++++++++++++++++++---
drivers/i2c/busses/i2c-designware-core.h | 6 +
drivers/i2c/busses/i2c-designware-platdrv.c | 68 ++++--
5 files changed, 280 insertions(+), 40 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCH v3] i2c: i2c-piix4: several coding style fixes
From: Carlos Palminha @ 2016-10-14 11:56 UTC (permalink / raw)
To: linux-i2c, linux-kernel; +Cc: Carlos Palminha
Fixed several coding style issues:
- '*' adjacent to data name
- assignment in if condition
- long comments blocks
- spaces with open parenthesis
- quoted string split across lines
Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
v2->v3:
- all the quoted strings are not coalesced
v1->v2:
- sent as one single patch
- fixed extra space in comment block
- fixed quoted string that was not coalesced into a single line
drivers/i2c/busses/i2c-piix4.c | 188 +++++++++++++++++++++++------------------
1 file changed, 104 insertions(+), 84 deletions(-)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index c2268cd..6e22ddf 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -1,32 +1,32 @@
/*
- Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
- Philip Edelbrock <phil@netroedge.com>
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-*/
+ * Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
+ * Philip Edelbrock <phil@netroedge.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
/*
- Supports:
- Intel PIIX4, 440MX
- Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
- ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
- AMD Hudson-2, ML, CZ
- SMSC Victory66
-
- Note: we assume there can only be one device, with one or more
- SMBus interfaces.
- The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
- For devices supporting multiple ports the i2c_adapter should provide
- an i2c_algorithm to access them.
-*/
+ * Supports:
+ * Intel PIIX4, 440MX
+ * Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
+ * ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
+ * AMD Hudson-2, ML, CZ
+ * SMSC Victory66
+ *
+ * Note: we assume there can only be one device, with one or more
+ * SMBus interfaces.
+ * The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
+ * For devices supporting multiple ports the i2c_adapter should provide
+ * an i2c_algorithm to access them.
+ */
#include <linux/module.h>
#include <linux/moduleparam.h>
@@ -97,19 +97,22 @@
/* insmod parameters */
-/* If force is set to anything different from 0, we forcibly enable the
- PIIX4. DANGEROUS! */
+/*
+ * If force is set to anything different from 0, we forcibly enable the
+ * PIIX4. DANGEROUS!
+ */
static int force;
-module_param (force, int, 0);
+module_param(force, int, 0);
MODULE_PARM_DESC(force, "Forcibly enable the PIIX4. DANGEROUS!");
-/* If force_addr is set to anything different from 0, we forcibly enable
- the PIIX4 at the given address. VERY DANGEROUS! */
+/*
+ * If force_addr is set to anything different from 0, we forcibly enable
+ * the PIIX4 at the given address. VERY DANGEROUS!
+ */
static int force_addr;
-module_param (force_addr, int, 0);
+module_param(force_addr, int, 0);
MODULE_PARM_DESC(force_addr,
- "Forcibly enable the PIIX4 at the given address. "
- "EXTREMELY DANGEROUS!");
+ "Forcibly enable the PIIX4 at the given address. EXTREMELY DANGEROUS!");
static int srvrworks_csb5_delay;
static struct pci_driver piix4_driver;
@@ -132,8 +135,10 @@ static const struct dmi_system_id piix4_dmi_blacklist[] = {
{ }
};
-/* The IBM entry is in a separate table because we only check it
- on Intel-based systems */
+/*
+ * The IBM entry is in a separate table because we only check it
+ * on Intel-based systems
+ */
static const struct dmi_system_id piix4_dmi_ibm[] = {
{
.ident = "IBM",
@@ -172,8 +177,10 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
(PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5))
srvrworks_csb5_delay = 1;
- /* On some motherboards, it was reported that accessing the SMBus
- caused severe hardware problems */
+ /*
+ * On some motherboards, it was reported that accessing the SMBus
+ * caused severe hardware problems
+ */
if (dmi_check_system(piix4_dmi_blacklist)) {
dev_err(&PIIX4_dev->dev,
"Accessing the SMBus on this system is unsafe!\n");
@@ -183,9 +190,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
/* Don't access SMBus on IBM systems which get corrupted eeproms */
if (dmi_check_system(piix4_dmi_ibm) &&
PIIX4_dev->vendor == PCI_VENDOR_ID_INTEL) {
- dev_err(&PIIX4_dev->dev, "IBM system detected; this module "
- "may corrupt your serial eeprom! Refusing to load "
- "module!\n");
+ dev_err(&PIIX4_dev->dev,
+ "IBM system detected; this module may corrupt your serial eeprom! Refusing to load module!\n");
return -EPERM;
}
@@ -196,10 +202,9 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
} else {
pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
piix4_smba &= 0xfff0;
- if(piix4_smba == 0) {
- dev_err(&PIIX4_dev->dev, "SMBus base address "
- "uninitialized - upgrade BIOS or use "
- "force_addr=0xaddr\n");
+ if (piix4_smba == 0) {
+ dev_err(&PIIX4_dev->dev,
+ "SMBus base address uninitialized upgrade BIOS or use force_addr=0xaddr\n");
return -ENODEV;
}
}
@@ -215,14 +220,17 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp);
- /* If force_addr is set, we program the new address here. Just to make
- sure, we disable the PIIX4 first. */
+ /*
+ * If force_addr is set, we program the new address here. Just to make
+ * sure, we disable the PIIX4 first.
+ */
if (force_addr) {
pci_write_config_byte(PIIX4_dev, SMBHSTCFG, temp & 0xfe);
pci_write_config_word(PIIX4_dev, SMBBA, piix4_smba);
pci_write_config_byte(PIIX4_dev, SMBHSTCFG, temp | 0x01);
- dev_info(&PIIX4_dev->dev, "WARNING: SMBus interface set to "
- "new address %04x!\n", piix4_smba);
+ dev_info(&PIIX4_dev->dev,
+ "WARNING: SMBus interface set to new address %04x!\n",
+ piix4_smba);
} else if ((temp & 1) == 0) {
if (force) {
/* This should never need to be done, but has been
@@ -250,8 +258,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
else if ((temp & 0x0E) == 0)
dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
else
- dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
- "(or code out of date)!\n");
+ dev_err(&PIIX4_dev->dev,
+ "Illegal Interrupt config (or code out of date)!\n");
pci_read_config_byte(PIIX4_dev, SMBREV, &temp);
dev_info(&PIIX4_dev->dev,
@@ -270,8 +278,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
/* SB800 and later SMBus does not support forcing address */
if (force || force_addr) {
- dev_err(&PIIX4_dev->dev, "SMBus does not support "
- "forcing address!\n");
+ dev_err(&PIIX4_dev->dev,
+ "SMBus does not support forcing address!\n");
return -EINVAL;
}
@@ -328,8 +336,9 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
/* Request the SMBus I2C bus config region */
if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
- dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "
- "0x%x already in use!\n", piix4_smba + i2ccfg_offset);
+ dev_err(&PIIX4_dev->dev,
+ "SMBus I2C bus config region 0x%x already in use!\n",
+ piix4_smba + i2ccfg_offset);
release_region(piix4_smba, SMBIOSIZE);
return -EBUSY;
}
@@ -369,8 +378,10 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
const struct pci_device_id *id,
unsigned short base_reg_addr)
{
- /* Set up auxiliary SMBus controllers found on some
- * AMD chipsets e.g. SP5100 (SB700 derivative) */
+ /*
+ * Set up auxiliary SMBus controllers found on some
+ * AMD chipsets e.g. SP5100 (SB700 derivative)
+ */
unsigned short piix4_smba;
@@ -393,8 +404,9 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
return -ENODEV;
if (!request_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) {
- dev_err(&PIIX4_dev->dev, "Auxiliary SMBus region 0x%x "
- "already in use!\n", piix4_smba);
+ dev_err(&PIIX4_dev->dev,
+ "Auxiliary SMBus region 0x%x already in use!\n",
+ piix4_smba);
return -EBUSY;
}
@@ -413,17 +425,19 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
int result = 0;
int timeout = 0;
- dev_dbg(&piix4_adapter->dev, "Transaction (pre): CNT=%02x, CMD=%02x, "
- "ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
- inb_p(SMBHSTCMD), inb_p(SMBHSTADD), inb_p(SMBHSTDAT0),
- inb_p(SMBHSTDAT1));
+ dev_dbg(&piix4_adapter->dev,
+ "Transaction (pre): CNT=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
+ inb_p(SMBHSTCNT), inb_p(SMBHSTCMD), inb_p(SMBHSTADD),
+ inb_p(SMBHSTDAT0), inb_p(SMBHSTDAT1));
/* Make sure the SMBus host is ready to start transmitting */
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_dbg(&piix4_adapter->dev, "SMBus busy (%02x). "
- "Resetting...\n", temp);
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
+ dev_dbg(&piix4_adapter->dev,
+ "SMBus busy (%02x). Resetting...\n", temp);
outb_p(temp, SMBHSTSTS);
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
dev_err(&piix4_adapter->dev, "Failed! (%02x)\n", temp);
return -EBUSY;
} else {
@@ -434,7 +448,10 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
/* start the transaction by setting bit 6 */
outb_p(inb(SMBHSTCNT) | 0x040, SMBHSTCNT);
- /* We will always wait for a fraction of a second! (See PIIX4 docs errata) */
+ /*
+ * We will always wait for a fraction of a second!
+ * (See PIIX4 docs errata)
+ */
if (srvrworks_csb5_delay) /* Extra delay for SERVERWORKS_CSB5 */
msleep(2);
else
@@ -457,8 +474,8 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
if (temp & 0x08) {
result = -EIO;
- dev_dbg(&piix4_adapter->dev, "Bus collision! SMBus may be "
- "locked until next hard reset. (sorry!)\n");
+ dev_dbg(&piix4_adapter->dev,
+ "Bus collision! SMBus may be locked until next hard reset. (sorry!)\n");
/* Clock stops and slave is stuck in mid-transmission */
}
@@ -470,21 +487,22 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
if (inb_p(SMBHSTSTS) != 0x00)
outb_p(inb(SMBHSTSTS), SMBHSTSTS);
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_err(&piix4_adapter->dev, "Failed reset at end of "
- "transaction (%02x)\n", temp);
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
+ dev_err(&piix4_adapter->dev,
+ "Failed reset at end of transaction (%02x)\n", temp);
}
- dev_dbg(&piix4_adapter->dev, "Transaction (post): CNT=%02x, CMD=%02x, "
- "ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
- inb_p(SMBHSTCMD), inb_p(SMBHSTADD), inb_p(SMBHSTDAT0),
- inb_p(SMBHSTDAT1));
+ dev_dbg(&piix4_adapter->dev,
+ "Transaction (post): CNT=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
+ inb_p(SMBHSTCNT), inb_p(SMBHSTCMD), inb_p(SMBHSTADD),
+ inb_p(SMBHSTDAT0), inb_p(SMBHSTDAT1));
return result;
}
/* Return negative errno on error. */
-static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
+static s32 piix4_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags, char read_write,
- u8 command, int size, union i2c_smbus_data * data)
+ u8 command, int size, union i2c_smbus_data *data)
{
struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
unsigned short piix4_smba = adapdata->smba;
@@ -649,7 +667,7 @@ static const struct pci_device_id piix4_ids[] = {
{ 0, }
};
-MODULE_DEVICE_TABLE (pci, piix4_ids);
+MODULE_DEVICE_TABLE(pci, piix4_ids);
static struct i2c_adapter *piix4_main_adapters[PIIX4_MAX_ADAPTERS];
static struct i2c_adapter *piix4_aux_adapter;
@@ -801,8 +819,10 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
}
if (retval > 0) {
- /* Try to add the aux adapter if it exists,
- * piix4_add_adapter will clean up if this fails */
+ /*
+ * Try to add the aux adapter if it exists,
+ * piix4_add_adapter will clean up if this fails
+ */
piix4_add_adapter(dev, retval, false, 0,
is_sb800 ? piix4_aux_port_name_sb800 : "",
&piix4_aux_adapter);
@@ -853,7 +873,7 @@ static struct pci_driver piix4_driver = {
module_pci_driver(piix4_driver);
-MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
- "Philip Edelbrock <phil@netroedge.com>");
+MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
+MODULE_AUTHOR("Philip Edelbrock <phil@netroedge.com>");
MODULE_DESCRIPTION("PIIX4 SMBus driver");
MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] i2c: i2c-piix4: several coding style fixes
From: Carlos Palminha @ 2016-10-14 11:52 UTC (permalink / raw)
To: Uwe Kleine-König, Carlos Palminha; +Cc: linux-i2c, linux-kernel
In-Reply-To: <20161014070825.qlawh2ttlwj24jge@pengutronix.de>
On 14-10-2016 08:08, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Oct 13, 2016 at 11:39:01PM +0100, Carlos Palminha wrote:
>> @@ -196,7 +204,7 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
>> } else {
>> pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
>> piix4_smba &= 0xfff0;
>> - if(piix4_smba == 0) {
>> + if (piix4_smba == 0) {
>> dev_err(&PIIX4_dev->dev, "SMBus base address "
>> "uninitialized - upgrade BIOS or use "
>> "force_addr=0xaddr\n");
>
> did you left this message in three lines on purpose?
>
Hi Uwe,
There were some missing lines. I had some doubts regarding if the
"quoted string" could go with more than 80 chars. I see now that
checkpatch is ok.
I'll resend a v3.
> Best regards
> Uwe
>
Regards,
C.Palminha
^ permalink raw reply
* [PULL REQUEST] i2c for 4.9
From: Wolfram Sang @ 2016-10-14 11:37 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]
Linus,
here is a small update pull request from I2C. It adds one comment to a
change we did in this merge window to handle lockdep better, and pulls
in a branch which should have been in 4.8 already improving DT support
for I2C.
Please pull.
Thanks,
Wolfram
The following changes since commit de34f4da7f62ff59ac6e1ef320b0fcfa3296fce3:
Merge tag 'media/v4.9-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media (2016-10-11 13:22:22 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
for you to fetch changes up to b2edcdae3d9a29b25f6c161a8711caa74ce49991:
Merge tag 'tegra-for-4.8-i2c' of git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux into i2c/for-next (2016-10-11 23:37:26 +0200)
----------------------------------------------------------------
Bartosz Golaszewski (1):
gpio: pca953x: add a comment explaining the need for a lockdep subclass
Jon Hunter (2):
dt-bindings: i2c: Add support for 'i2c-bus' subnode
i2c: core: Add support for 'i2c-bus' subnode
Wolfram Sang (1):
Merge tag 'tegra-for-4.8-i2c' of git://git.kernel.org/.../tegra/linux into i2c/for-next
Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
drivers/gpio/gpio-pca953x.c | 16 ++++++++++++++++
drivers/i2c/i2c-core.c | 10 ++++++++--
3 files changed, 32 insertions(+), 2 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC 1/1] drivers: i2c: omap: Add slave support
From: Ravikumar @ 2016-10-14 8:57 UTC (permalink / raw)
To: Matthijs van Duin, Wolfram Sang
Cc: Tony Lindgren, linux-omap@vger.kernel.org, linux-i2c, lkml
In-Reply-To: <20160829034301.GA21292@squirrel.local>
On Monday 29 August 2016 09:13 AM, Matthijs van Duin wrote:
> On 28 August 2016 at 07:35, Wolfram Sang <wsa@the-dreams.de> wrote:
>> Well, I2C is simple, what could go wrong? :/
> Actually I2C is elegant and *seems* simple, but in all its
> asynchronicity there are actually a surprising number of fine details
> you can trip over. Maybe that's why so many i2c controllers suck: since
> i2c looks simple enough manufacturers are easily tempted to roll their
> own instead of licensing a good implementation.
>
> Having said that, most of the inconsistency and obnoxiousness of the TI
> I2C controller is not even excusable by that argument. For example its
> irq registers *look* like the usual set { rawstatus, status, en, dis }
> that's their current standard ("Highlander") for peripherals. They do
> not however *behave* like the standard set however:
> 1. status isn't always (rawstatus & enabled)
> 2. status != 0 does not always imply the irq output is asserted
> 3. some enable-bits also change the behaviour of rawstatus
> All of these misbehaviours are unprecedented afaik.
If I understand #1 correctly, you mean that bit value is different in
raw vs status registers.
I've seen some times there was a delay in the value reflecting the
status register.
So I choose to use the raw register.
Now #2 and #3 would be crazy, do you have further notes on this?
If I can reproduce these then I will follow up with the IP/HW team.
> Normally you'd also expect each irq (raw)status bit to either
> a. be an event, set by hw and can be cleared by software any time, or
> b. be a level status, unaffected by software attempts to set/clear.
> Again the i2c controller decided this is far too little diversity.
yeah, seems so on dm814x.
But, at least the description has been updated on Jacinto 6 device.
I see all 'status' bits are write 1 to clear except for Bus Busy (intended).
While the 'raw' status register bits can not be cleared by writing 1,
the description says write 1 to set the bit for debug purpose.
>> So, it is possible to make a proper I2C slave with OMAP, but you need
>> to know those 100 gory details?
> Mostly. There are some limitations such as:
>
> * No ability to selectively ACK/NACK when addressed as slave. If you're
> unable to respond for some time then you'd end up blocking the bus with
> clock stretching. You could temporarily deconfigure your slave address
> but the TRM states changing slave address is forbidden while bus busy.
Does this lead to bus lock up?
> * According to my notes it always ACKs a General Call and this cannot
> even be stalled using the SBLOCK register. Since I don't care about GC
> there's no more details in my notes, but if this is true then on any bus
> where GC is used, irq handling will have real-time deadlines to avoid
> losing track of transaction boundaries and misinterpreting data.
>
> Finally, as my first link pointed out, various protocol errors can lock
> up the peripheral's internal state machine. When operating as slave
> this is basically undetectable: all registers look normal and the
> bus-busy bit will continue to track start/stop, but the peripheral will
> not ACK any slave address anymore until you reset it.
>
> You could argue "well, but that requires bus protocol errors" but it is
> nevertheless a direct violation of the I2C standard:
>
> I2C-bus compatible devices must reset their bus logic on receipt
> of a START or repeated START condition such that they all
> anticipate the sending of a slave address, even if these START
> conditions are not positioned according to the proper format.
>
> Also, my testing showed pulsing SDA low on an idle bus sufficed to
> trigger this state. It needs to pass the glitch filter of course, but
> this filter is implemented by sampling the bus requiring two consecutive
> samples to agree. Two small glitches with just the right timing would
> therefore suffice. Rather unlikely for random noise, but having lots of
> signals on your pcb that ultimately derive from the same clock source
> probably makes the odds a lot more favorable.
>
> Matthijs
Thanks for sharing the steps to reproduce.
Regards,
RK
^ permalink raw reply
* Re: [RFC 1/1] drivers: i2c: omap: Add slave support
From: Ravikumar @ 2016-10-14 8:03 UTC (permalink / raw)
To: Matthijs van Duin, Wolfram Sang
Cc: Ravikumar Kattekola, Tony Lindgren, linux-omap@vger.kernel.org,
linux-i2c, lkml
In-Reply-To: <CAALWOA-XP+mfTcVPc1LZZofAaCK-KtKhFKKKHu2PyHWFrfir3A@mail.gmail.com>
On Saturday 27 August 2016 07:29 PM, Matthijs van Duin wrote:
> Greetings, unfortunate souls trying to use the omap-i2c peripheral in
> slave mode! :-)
That would be me :( and greetings to you too :)
> I recently posted some stuff about exactly that topic on TI's E2E
> forum, you may want to read this warning:
> http://e2e.ti.com/support/arm/sitara_arm/f/791/p/514961/1955843#1955843
> and post contains suggestions on using slave mode and details on the
> peripheral actually behaves:
> http://e2e.ti.com/support/arm/sitara_arm/f/791/p/514961/1959417#1959417
Thanks for doing extensive testing on the OMAP I2C and preserving your
notes.
I really appreciate sharing the detailed notes and workarounds to avoid
lockups.
I'll following up with the HW team on your observations.
>
> Matthijs
^ permalink raw reply
* Re: [RFC 1/1] drivers: i2c: omap: Add slave support
From: Ravikumar @ 2016-10-14 7:56 UTC (permalink / raw)
To: Wolfram Sang, Ravikumar Kattekola
Cc: tony, linux-omap, linux-i2c, linux-kernel
In-Reply-To: <20160825171430.GM2856@katana>
On Thursday 25 August 2016 10:44 PM, Wolfram Sang wrote:
> Hi,
>
>> The omap i2c controller (at least on dra7x devices)
>> doesn't have start/stop (STT/STP) support for slave mode
>> so event #5 is not implemented in the driver.
> I think you can deduce that. If a new {READ|WRITE}_REQUESTED slave event
> comes in when you had *_PROCESSED events before, there must have been a
> STOP on the bus inbetween.
I've found that the Bus free interrupt can be used for STOP condition in
slave mode.
So this shouldn't be a problem anymore.
>
>> + if (stat & OMAP_I2C_STAT_XRDY) {
>> + i2c_slave_event(omap->slave, I2C_SLAVE_READ_REQUESTED,
>> + &value);
>> + omap_i2c_write_reg(omap, OMAP_I2C_DATA_REG, value);
>> + i2c_slave_event(omap->slave, I2C_SLAVE_READ_PROCESSED,
>> + &value);
> This looks fishy. READ_REQUESTED is only sent after the address phase.
> Have you read the documentation (Documentation/i2c/slave-interface)?
> Please say if it was unclear.
Will fix this.
>> + /* As of now, We dont need all interrupts be enabled */
>> + omap->iestate = OMAP_I2C_IE_AAS | OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY;
> This looks even more fishy. Are you disabling the master interrupts?
> That's a no (unless there are HW constraints). Your driver should be
> able to switch between master and slave depending on what happens on the
> bus.
Dynamic switching on OMAP I2C IP could be a difficult task.
There is no separate status register for master mode vs slave mode, it's
a common register.
Even the interrupt status bits are reused.
So i cant do a check on status like if(!MSR) slave_irq_handler();
I think instead of status I may need to check the MST(1:master mode,
0:slave mode] bit in I2C_CON
to take a decision on whether to call slave irq_handler or not.
> For more guidance, here is my talk at ELCE 2015:
> https://www.youtube.com/watch?v=JdQ21jlwb58
Thanks for sharing the video.
>
> Regards,
>
> Wolfram
>
Regards,
RK
^ permalink raw reply
* Re: [PATCH v2] i2c: i2c-piix4: several coding style fixes
From: Uwe Kleine-König @ 2016-10-14 7:08 UTC (permalink / raw)
To: Carlos Palminha; +Cc: linux-i2c, linux-kernel
In-Reply-To: <20161013223901.16460-1-palminha@synopsys.com>
Hello,
On Thu, Oct 13, 2016 at 11:39:01PM +0100, Carlos Palminha wrote:
> @@ -196,7 +204,7 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
> } else {
> pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
> piix4_smba &= 0xfff0;
> - if(piix4_smba == 0) {
> + if (piix4_smba == 0) {
> dev_err(&PIIX4_dev->dev, "SMBus base address "
> "uninitialized - upgrade BIOS or use "
> "force_addr=0xaddr\n");
did you left this message in three lines on purpose?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2] i2c: i2c-piix4: several coding style fixes
From: Carlos Palminha @ 2016-10-13 22:39 UTC (permalink / raw)
To: linux-i2c, linux-kernel; +Cc: Carlos Palminha
Fixed several coding style issues:
- '*' adjacent to data name
- assignment in if condition
- long comments blocks
- spaces with open parenthesis
- quoted string split across lines
Signed-off-by: Carlos Palminha <palminha@synopsys.com>
---
v1->v2:
- sent as one single patch
- fixed extra space in comment block
- fixed quoted string that was not coalesced into a single line
drivers/i2c/busses/i2c-piix4.c | 145 +++++++++++++++++++++++------------------
1 file changed, 82 insertions(+), 63 deletions(-)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index c2268cd..c2fef45 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -1,32 +1,32 @@
/*
- Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
- Philip Edelbrock <phil@netroedge.com>
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-*/
+ * Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl> and
+ * Philip Edelbrock <phil@netroedge.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
/*
- Supports:
- Intel PIIX4, 440MX
- Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
- ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
- AMD Hudson-2, ML, CZ
- SMSC Victory66
-
- Note: we assume there can only be one device, with one or more
- SMBus interfaces.
- The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
- For devices supporting multiple ports the i2c_adapter should provide
- an i2c_algorithm to access them.
-*/
+ * Supports:
+ * Intel PIIX4, 440MX
+ * Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
+ * ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
+ * AMD Hudson-2, ML, CZ
+ * SMSC Victory66
+ *
+ * Note: we assume there can only be one device, with one or more
+ * SMBus interfaces.
+ * The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
+ * For devices supporting multiple ports the i2c_adapter should provide
+ * an i2c_algorithm to access them.
+ */
#include <linux/module.h>
#include <linux/moduleparam.h>
@@ -97,16 +97,20 @@
/* insmod parameters */
-/* If force is set to anything different from 0, we forcibly enable the
- PIIX4. DANGEROUS! */
+/*
+ * If force is set to anything different from 0, we forcibly enable the
+ * PIIX4. DANGEROUS!
+ */
static int force;
-module_param (force, int, 0);
+module_param(force, int, 0);
MODULE_PARM_DESC(force, "Forcibly enable the PIIX4. DANGEROUS!");
-/* If force_addr is set to anything different from 0, we forcibly enable
- the PIIX4 at the given address. VERY DANGEROUS! */
+/*
+ * If force_addr is set to anything different from 0, we forcibly enable
+ * the PIIX4 at the given address. VERY DANGEROUS!
+ */
static int force_addr;
-module_param (force_addr, int, 0);
+module_param(force_addr, int, 0);
MODULE_PARM_DESC(force_addr,
"Forcibly enable the PIIX4 at the given address. "
"EXTREMELY DANGEROUS!");
@@ -132,8 +136,10 @@ static const struct dmi_system_id piix4_dmi_blacklist[] = {
{ }
};
-/* The IBM entry is in a separate table because we only check it
- on Intel-based systems */
+/*
+ * The IBM entry is in a separate table because we only check it
+ * on Intel-based systems
+ */
static const struct dmi_system_id piix4_dmi_ibm[] = {
{
.ident = "IBM",
@@ -172,8 +178,10 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
(PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5))
srvrworks_csb5_delay = 1;
- /* On some motherboards, it was reported that accessing the SMBus
- caused severe hardware problems */
+ /*
+ * On some motherboards, it was reported that accessing the SMBus
+ * caused severe hardware problems
+ */
if (dmi_check_system(piix4_dmi_blacklist)) {
dev_err(&PIIX4_dev->dev,
"Accessing the SMBus on this system is unsafe!\n");
@@ -196,7 +204,7 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
} else {
pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
piix4_smba &= 0xfff0;
- if(piix4_smba == 0) {
+ if (piix4_smba == 0) {
dev_err(&PIIX4_dev->dev, "SMBus base address "
"uninitialized - upgrade BIOS or use "
"force_addr=0xaddr\n");
@@ -215,8 +223,10 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp);
- /* If force_addr is set, we program the new address here. Just to make
- sure, we disable the PIIX4 first. */
+ /*
+ * If force_addr is set, we program the new address here. Just to make
+ * sure, we disable the PIIX4 first.
+ */
if (force_addr) {
pci_write_config_byte(PIIX4_dev, SMBHSTCFG, temp & 0xfe);
pci_write_config_word(PIIX4_dev, SMBBA, piix4_smba);
@@ -250,8 +260,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
else if ((temp & 0x0E) == 0)
dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
else
- dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
- "(or code out of date)!\n");
+ dev_err(&PIIX4_dev->dev,
+ "Illegal Interrupt config (or code out of date)!\n");
pci_read_config_byte(PIIX4_dev, SMBREV, &temp);
dev_info(&PIIX4_dev->dev,
@@ -270,8 +280,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
/* SB800 and later SMBus does not support forcing address */
if (force || force_addr) {
- dev_err(&PIIX4_dev->dev, "SMBus does not support "
- "forcing address!\n");
+ dev_err(&PIIX4_dev->dev,
+ "SMBus does not support forcing address!\n");
return -EINVAL;
}
@@ -328,8 +338,9 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
/* Request the SMBus I2C bus config region */
if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
- dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "
- "0x%x already in use!\n", piix4_smba + i2ccfg_offset);
+ dev_err(&PIIX4_dev->dev,
+ "SMBus I2C bus config region 0x%x already in use!\n",
+ piix4_smba + i2ccfg_offset);
release_region(piix4_smba, SMBIOSIZE);
return -EBUSY;
}
@@ -369,8 +380,10 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
const struct pci_device_id *id,
unsigned short base_reg_addr)
{
- /* Set up auxiliary SMBus controllers found on some
- * AMD chipsets e.g. SP5100 (SB700 derivative) */
+ /*
+ * Set up auxiliary SMBus controllers found on some
+ * AMD chipsets e.g. SP5100 (SB700 derivative)
+ */
unsigned short piix4_smba;
@@ -393,8 +406,9 @@ static int piix4_setup_aux(struct pci_dev *PIIX4_dev,
return -ENODEV;
if (!request_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) {
- dev_err(&PIIX4_dev->dev, "Auxiliary SMBus region 0x%x "
- "already in use!\n", piix4_smba);
+ dev_err(&PIIX4_dev->dev,
+ "Auxiliary SMBus region 0x%x already in use!\n",
+ piix4_smba);
return -EBUSY;
}
@@ -419,11 +433,13 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
inb_p(SMBHSTDAT1));
/* Make sure the SMBus host is ready to start transmitting */
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_dbg(&piix4_adapter->dev, "SMBus busy (%02x). "
- "Resetting...\n", temp);
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
+ dev_dbg(&piix4_adapter->dev,
+ "SMBus busy (%02x). Resetting...\n", temp);
outb_p(temp, SMBHSTSTS);
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
dev_err(&piix4_adapter->dev, "Failed! (%02x)\n", temp);
return -EBUSY;
} else {
@@ -470,9 +486,10 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
if (inb_p(SMBHSTSTS) != 0x00)
outb_p(inb(SMBHSTSTS), SMBHSTSTS);
- if ((temp = inb_p(SMBHSTSTS)) != 0x00) {
- dev_err(&piix4_adapter->dev, "Failed reset at end of "
- "transaction (%02x)\n", temp);
+ temp = inb_p(SMBHSTSTS);
+ if (temp != 0x00) {
+ dev_err(&piix4_adapter->dev,
+ "Failed reset at end of transaction (%02x)\n", temp);
}
dev_dbg(&piix4_adapter->dev, "Transaction (post): CNT=%02x, CMD=%02x, "
"ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT),
@@ -482,9 +499,9 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
}
/* Return negative errno on error. */
-static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
+static s32 piix4_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags, char read_write,
- u8 command, int size, union i2c_smbus_data * data)
+ u8 command, int size, union i2c_smbus_data *data)
{
struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
unsigned short piix4_smba = adapdata->smba;
@@ -649,7 +666,7 @@ static const struct pci_device_id piix4_ids[] = {
{ 0, }
};
-MODULE_DEVICE_TABLE (pci, piix4_ids);
+MODULE_DEVICE_TABLE(pci, piix4_ids);
static struct i2c_adapter *piix4_main_adapters[PIIX4_MAX_ADAPTERS];
static struct i2c_adapter *piix4_aux_adapter;
@@ -801,8 +818,10 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
}
if (retval > 0) {
- /* Try to add the aux adapter if it exists,
- * piix4_add_adapter will clean up if this fails */
+ /*
+ * Try to add the aux adapter if it exists,
+ * piix4_add_adapter will clean up if this fails
+ */
piix4_add_adapter(dev, retval, false, 0,
is_sb800 ? piix4_aux_port_name_sb800 : "",
&piix4_aux_adapter);
@@ -853,7 +872,7 @@ static struct pci_driver piix4_driver = {
module_pci_driver(piix4_driver);
-MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl> and "
- "Philip Edelbrock <phil@netroedge.com>");
+MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
+MODULE_AUTHOR("Philip Edelbrock <phil@netroedge.com>");
MODULE_DESCRIPTION("PIIX4 SMBus driver");
MODULE_LICENSE("GPL");
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 0/5] i2c: i2c-piix4: set of coding style fixes
From: Carlos Palminha @ 2016-10-13 16:44 UTC (permalink / raw)
To: Jean Delvare; +Cc: linux-i2c, linux-kernel, wsa
In-Reply-To: <20161013134029.1828048b@endymion>
Hi Jean,
On 13-10-2016 12:40, Jean Delvare wrote:
> Hi Carlos,
>
> On Tue, 11 Oct 2016 18:24:36 +0100, Carlos Palminha wrote:
>> This patchset fixes several coding style issues.
>>
>> Carlos Palminha (5):
>> i2c: i2c-piix4: coding style fix - '*' adjacent to data name
>> i2c: i2c-piix4: coding style fix - assignment in if condition
>> i2c: i2c-piix4: coding style fix - long comments blocks
>> i2c: i2c-piix4: coding style fix - spaces with open parenthesis
>> i2c: i2c-piix4: coding style fix - quoted string split across lines
>>
>> drivers/i2c/busses/i2c-piix4.c | 144 +++++++++++++++++++++++------------------
>> 1 file changed, 82 insertions(+), 62 deletions(-)
>
> Except for the two issues which were already pointed out, I'm fine with
> these changes, however there is no good reason to split them into 5
> separate patches. Fixing the coding style is one goal, it should be all
> done with a single patch. So, when you resubmit, please do so as a
> single patch.
>
> Thanks,
>
Sure... I'll resubmit as one patch.
Regards,
C.Palminha
^ permalink raw reply
* [PATCH v5 6/6] i2c: use an IRQ to report Host Notify events, not alert
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
The current SMBus Host Notify implementation relies on .alert() to
relay its notifications. However, the use cases where SMBus Host
Notify is needed currently is to signal data ready on touchpads.
This is closer to an IRQ than a custom API through .alert().
Given that the 2 touchpad manufacturers (Synaptics and Elan) that
use SMBus Host Notify don't put any data in the SMBus payload, the
concept actually matches one to one.
Benefits are multiple:
- simpler code and API: the client will just have an IRQ, and
nothing needs to be added in the adapter beside internally
enabling it.
- no more specific workqueue, the threading is handled by IRQ core
directly (when required)
- no more races when removing the device (the drivers are already
required to disable irq on remove)
- simpler handling for drivers: use plain regular IRQs
- no more dependency on i2c-smbus for i2c-i801 (and any other adapter)
- the IRQ domain is created automatically when the adapter exports
the Host Notify capability
- the IRQ are assign only if ACPI, OF and the caller did not assign
one already
- the domain is automatically destroyed on remove
- fewer lines of code (minus 20, yeah!)
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v5:
- force IRQ_DOMAIN in Kconfig
- remove unnused variable
- assign the SMBus Host Notify irq in i2c_device_probe() if the
irq was not assigned by the caller, and if both ACPI and OF
failed to assign one.
new in v4
---
Documentation/i2c/smbus-protocol | 12 +++--
drivers/i2c/Kconfig | 1 +
drivers/i2c/busses/i2c-i801.c | 32 +++--------
drivers/i2c/i2c-core.c | 113 +++++++++++++++++++++++++++++++++++++++
drivers/i2c/i2c-smbus.c | 102 -----------------------------------
include/linux/i2c-smbus.h | 27 ----------
include/linux/i2c.h | 4 ++
7 files changed, 133 insertions(+), 158 deletions(-)
diff --git a/Documentation/i2c/smbus-protocol b/Documentation/i2c/smbus-protocol
index 14d4ec1..9771c28 100644
--- a/Documentation/i2c/smbus-protocol
+++ b/Documentation/i2c/smbus-protocol
@@ -200,10 +200,14 @@ alerting device's address.
[S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P]
This is implemented in the following way in the Linux kernel:
-* I2C bus drivers which support SMBus Host Notify should call
- i2c_setup_smbus_host_notify() to setup SMBus Host Notify support.
-* I2C drivers for devices which can trigger SMBus Host Notify should implement
- the optional alert() callback.
+* I2C bus drivers which support SMBus Host Notify should report
+ I2C_FUNC_SMBUS_HOST_NOTIFY.
+* I2C bus drivers trigger SMBus Host Notify by a call to
+ i2c_handle_smbus_host_notify().
+* I2C drivers for devices which can trigger SMBus Host Notify will have
+ client->irq assigned to a Host Notify IRQ if noone else specified an other.
+
+There is currently no way to retrieve the data parameter from the client.
Packet Error Checking (PEC)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index d223650..de305f8 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -7,6 +7,7 @@ menu "I2C support"
config I2C
tristate "I2C support"
select RT_MUTEXES
+ select IRQ_DOMAIN
---help---
I2C (pronounce: I-squared-C) is a slow serial bus protocol used in
many micro controller applications and developed by Philips. SMBus,
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 42a6a89..ca63adb 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -266,7 +266,6 @@ struct i801_priv {
*/
bool acpi_reserved;
struct mutex acpi_lock;
- struct smbus_host_notify *host_notify;
};
#define FEATURE_SMBUS_PEC BIT(0)
@@ -582,10 +581,10 @@ static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
/*
* With the tested platforms, reading SMBNTFDDAT (22 + (p)->smba)
- * always returns 0 and is safe to read.
- * We just use 0 given we have no use of the data right now.
+ * always returns 0. Our current implementation doesn't provide
+ * data, so we just ignore it.
*/
- i2c_handle_smbus_host_notify(priv->host_notify, addr, 0);
+ i2c_handle_smbus_host_notify(&priv->adapter, addr);
/* clear Host Notify bit and return */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
@@ -941,17 +940,12 @@ static u32 i801_func(struct i2c_adapter *adapter)
I2C_FUNC_SMBUS_HOST_NOTIFY : 0);
}
-static int i801_enable_host_notify(struct i2c_adapter *adapter)
+static void i801_enable_host_notify(struct i2c_adapter *adapter)
{
struct i801_priv *priv = i2c_get_adapdata(adapter);
if (!(priv->features & FEATURE_HOST_NOTIFY))
- return -ENOTSUPP;
-
- if (!priv->host_notify)
- priv->host_notify = i2c_setup_smbus_host_notify(adapter);
- if (!priv->host_notify)
- return -ENOMEM;
+ return;
priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
@@ -961,8 +955,6 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
/* clear Host Notify bit to allow a new notification */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
-
- return 0;
}
static void i801_disable_host_notify(struct i801_priv *priv)
@@ -1631,14 +1623,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
return err;
}
- /*
- * Enable Host Notify for chips that supports it.
- * It is done after i2c_add_adapter() so that we are sure the work queue
- * is not used if i2c_add_adapter() fails.
- */
- err = i801_enable_host_notify(&priv->adapter);
- if (err && err != -ENOTSUPP)
- dev_warn(&dev->dev, "Unable to enable SMBus Host Notify\n");
+ i801_enable_host_notify(&priv->adapter);
i801_probe_optional_slaves(priv);
/* We ignore errors - multiplexing is optional */
@@ -1689,11 +1674,8 @@ static int i801_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct i801_priv *priv = pci_get_drvdata(pci_dev);
- int err;
- err = i801_enable_host_notify(&priv->adapter);
- if (err && err != -ENOTSUPP)
- dev_warn(dev, "Unable to enable SMBus Host Notify\n");
+ i801_enable_host_notify(&priv->adapter);
return 0;
}
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 79ce9e1..4f4fc60 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -65,6 +65,9 @@
#define I2C_ADDR_OFFSET_TEN_BIT 0xa000
#define I2C_ADDR_OFFSET_SLAVE 0x1000
+#define I2C_ADDR_7BITS_MAX 0x77
+#define I2C_ADDR_7BITS_COUNT (I2C_ADDR_7BITS_MAX + 1)
+
/* core_lock protects i2c_adapter_idr, and guarantees
that device detection, deletion of detected devices, and attach_adapter
calls are serialized */
@@ -880,6 +883,25 @@ static void i2c_init_recovery(struct i2c_adapter *adap)
adap->bus_recovery_info = NULL;
}
+static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
+{
+ struct i2c_adapter *adap = client->adapter;
+ unsigned int irq;
+
+ if (!adap->host_notify_domain)
+ return -ENXIO;
+
+ if (client->flags & I2C_CLIENT_TEN)
+ return -EINVAL;
+
+ irq = irq_find_mapping(adap->host_notify_domain, client->addr);
+ if (!irq)
+ irq = irq_create_mapping(adap->host_notify_domain,
+ client->addr);
+
+ return irq > 0 ? irq : -ENXIO;
+}
+
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -901,6 +923,14 @@ static int i2c_device_probe(struct device *dev)
}
if (irq == -EPROBE_DEFER)
return irq;
+ /*
+ * ACPI and OF did not find any useful IRQ, try to see
+ * if Host Notify can be used.
+ */
+ if (irq < 0) {
+ dev_dbg(dev, "Using Host Notify IRQ\n");
+ irq = i2c_smbus_host_notify_to_irq(client);
+ }
if (irq < 0)
irq = 0;
@@ -1781,6 +1811,79 @@ static const struct i2c_lock_operations i2c_adapter_lock_ops = {
.unlock_bus = i2c_adapter_unlock_bus,
};
+static void i2c_host_notify_irq_teardown(struct i2c_adapter *adap)
+{
+ struct irq_domain *domain = adap->host_notify_domain;
+ irq_hw_number_t hwirq;
+
+ if (!domain)
+ return;
+
+ for (hwirq = 0 ; hwirq < I2C_ADDR_7BITS_COUNT ; hwirq++)
+ irq_dispose_mapping(irq_find_mapping(domain, hwirq));
+
+ irq_domain_remove(domain);
+ adap->host_notify_domain = NULL;
+}
+
+static int i2c_host_notify_irq_map(struct irq_domain *h,
+ unsigned int virq,
+ irq_hw_number_t hw_irq_num)
+{
+ irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+ return 0;
+}
+
+static struct irq_domain_ops i2c_host_notify_irq_ops = {
+ .map = i2c_host_notify_irq_map,
+};
+
+static int i2c_setup_host_notify_irq_domain(struct i2c_adapter *adap)
+{
+ struct irq_domain *domain;
+
+ if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_HOST_NOTIFY))
+ return 0;
+
+ domain = irq_domain_create_linear(adap->dev.fwnode,
+ I2C_ADDR_7BITS_COUNT,
+ &i2c_host_notify_irq_ops, adap);
+ if (!domain)
+ return -ENOMEM;
+
+ adap->host_notify_domain = domain;
+
+ return 0;
+}
+
+/**
+ * i2c_handle_smbus_host_notify - Forward a Host Notify event to the correct
+ * I2C client.
+ * @adap: the adapter
+ * @addr: the I2C address of the notifying device
+ * Context: can't sleep
+ *
+ * Helper function to be called from an I2C bus driver's interrupt
+ * handler. It will schedule the Host Notify IRQ.
+ */
+int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr)
+{
+ int irq;
+
+ if (!adap)
+ return -EINVAL;
+
+ irq = irq_find_mapping(adap->host_notify_domain, addr);
+ if (irq <= 0)
+ return -ENXIO;
+
+ generic_handle_irq(irq);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
+
static int i2c_register_adapter(struct i2c_adapter *adap)
{
int res = -EINVAL;
@@ -1812,6 +1915,14 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
if (adap->timeout == 0)
adap->timeout = HZ;
+ /* register soft irqs for Host Notify */
+ res = i2c_setup_host_notify_irq_domain(adap);
+ if (res) {
+ pr_err("adapter '%s': can't create Host Notify IRQs (%d)\n",
+ adap->name, res);
+ goto out_list;
+ }
+
dev_set_name(&adap->dev, "i2c-%d", adap->nr);
adap->dev.bus = &i2c_bus_type;
adap->dev.type = &i2c_adapter_type;
@@ -2049,6 +2160,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
pm_runtime_disable(&adap->dev);
+ i2c_host_notify_irq_teardown(adap);
+
/* wait until all references to the device are gone
*
* FIXME: This is old code and should ideally be replaced by an
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b0d2679..f9271c7 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -241,108 +241,6 @@ int i2c_handle_smbus_alert(struct i2c_client *ara)
}
EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
-static void smbus_host_notify_work(struct work_struct *work)
-{
- struct alert_data alert;
- struct i2c_adapter *adapter;
- unsigned long flags;
- u16 payload;
- u8 addr;
- struct smbus_host_notify *data;
-
- data = container_of(work, struct smbus_host_notify, work);
-
- spin_lock_irqsave(&data->lock, flags);
- payload = data->payload;
- addr = data->addr;
- adapter = data->adapter;
-
- /* clear the pending bit and release the spinlock */
- data->pending = false;
- spin_unlock_irqrestore(&data->lock, flags);
-
- if (!adapter || !addr)
- return;
-
- alert.type = I2C_PROTOCOL_SMBUS_HOST_NOTIFY;
- alert.addr = addr;
- alert.data = payload;
-
- device_for_each_child(&adapter->dev, &alert, smbus_do_alert);
-}
-
-/**
- * i2c_setup_smbus_host_notify - Allocate a new smbus_host_notify for the given
- * I2C adapter.
- * @adapter: the adapter we want to associate a Host Notify function
- *
- * Returns a struct smbus_host_notify pointer on success, and NULL on failure.
- * The resulting smbus_host_notify must not be freed afterwards, it is a
- * managed resource already.
- */
-struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
-{
- struct smbus_host_notify *host_notify;
-
- host_notify = devm_kzalloc(&adap->dev, sizeof(struct smbus_host_notify),
- GFP_KERNEL);
- if (!host_notify)
- return NULL;
-
- host_notify->adapter = adap;
-
- spin_lock_init(&host_notify->lock);
- INIT_WORK(&host_notify->work, smbus_host_notify_work);
-
- return host_notify;
-}
-EXPORT_SYMBOL_GPL(i2c_setup_smbus_host_notify);
-
-/**
- * i2c_handle_smbus_host_notify - Forward a Host Notify event to the correct
- * I2C client.
- * @host_notify: the struct host_notify attached to the relevant adapter
- * @addr: the I2C address of the notifying device
- * @data: the payload of the notification
- * Context: can't sleep
- *
- * Helper function to be called from an I2C bus driver's interrupt
- * handler. It will schedule the Host Notify work, in turn calling the
- * corresponding I2C device driver's alert function.
- *
- * host_notify should be a valid pointer previously returned by
- * i2c_setup_smbus_host_notify().
- */
-int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
- unsigned short addr, unsigned int data)
-{
- unsigned long flags;
- struct i2c_adapter *adapter;
-
- if (!host_notify || !host_notify->adapter)
- return -EINVAL;
-
- adapter = host_notify->adapter;
-
- spin_lock_irqsave(&host_notify->lock, flags);
-
- if (host_notify->pending) {
- spin_unlock_irqrestore(&host_notify->lock, flags);
- dev_warn(&adapter->dev, "Host Notify already scheduled.\n");
- return -EBUSY;
- }
-
- host_notify->payload = data;
- host_notify->addr = addr;
-
- /* Mark that there is a pending notification and release the lock */
- host_notify->pending = true;
- spin_unlock_irqrestore(&host_notify->lock, flags);
-
- return schedule_work(&host_notify->work);
-}
-EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
-
module_i2c_driver(smbalert_driver);
MODULE_AUTHOR("Jean Delvare <jdelvare@suse.de>");
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index c2e3324..a138502 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -50,31 +50,4 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
struct i2c_smbus_alert_setup *setup);
int i2c_handle_smbus_alert(struct i2c_client *ara);
-/**
- * smbus_host_notify - internal structure used by the Host Notify mechanism.
- * @adapter: the I2C adapter associated with this struct
- * @work: worker used to schedule the IRQ in the slave device
- * @lock: spinlock to check if a notification is already pending
- * @pending: flag set when a notification is pending (any new notification will
- * be rejected if pending is true)
- * @payload: the actual payload of the Host Notify event
- * @addr: the address of the slave device which raised the notification
- *
- * This struct needs to be allocated by i2c_setup_smbus_host_notify() and does
- * not need to be freed. Internally, i2c_setup_smbus_host_notify() uses a
- * managed resource to clean this up when the adapter get released.
- */
-struct smbus_host_notify {
- struct i2c_adapter *adapter;
- struct work_struct work;
- spinlock_t lock;
- bool pending;
- u16 payload;
- u8 addr;
-};
-
-struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap);
-int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
- unsigned short addr, unsigned int data);
-
#endif /* _LINUX_I2C_SMBUS_H */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 4a4099d..65c4d5e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -30,6 +30,7 @@
#include <linux/device.h> /* for struct device */
#include <linux/sched.h> /* for completion */
#include <linux/mutex.h>
+#include <linux/irqdomain.h> /* for Host Notify IRQ */
#include <linux/of.h> /* for struct device_node */
#include <linux/swab.h> /* for swab16 */
#include <uapi/linux/i2c.h>
@@ -567,6 +568,8 @@ struct i2c_adapter {
struct i2c_bus_recovery_info *bus_recovery_info;
const struct i2c_adapter_quirks *quirks;
+
+ struct irq_domain *host_notify_domain;
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
@@ -738,6 +741,7 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
}
+int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
/**
* module_i2c_driver() - Helper macro for registering a modular I2C driver
* @__i2c_driver: i2c_driver struct
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/6] i2c: i801: use BIT() macro for bits definition
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
i801 mixes hexadecimal and decimal values for defining bits. However,
we have a nice BIT() macro for this exact purpose.
No functional changes, cleanup only.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v5
no changes in v4
no changes in v3
no changes in v2
---
drivers/i2c/busses/i2c-i801.c | 50 +++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index cfb74fc..5928ee2 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -136,26 +136,26 @@
#define SBREG_SMBCTRL 0xc6000c
/* Host status bits for SMBPCISTS */
-#define SMBPCISTS_INTS 0x08
+#define SMBPCISTS_INTS BIT(3)
/* Control bits for SMBPCICTL */
-#define SMBPCICTL_INTDIS 0x0400
+#define SMBPCICTL_INTDIS BIT(10)
/* Host configuration bits for SMBHSTCFG */
-#define SMBHSTCFG_HST_EN 1
-#define SMBHSTCFG_SMB_SMI_EN 2
-#define SMBHSTCFG_I2C_EN 4
+#define SMBHSTCFG_HST_EN BIT(0)
+#define SMBHSTCFG_SMB_SMI_EN BIT(1)
+#define SMBHSTCFG_I2C_EN BIT(2)
/* TCO configuration bits for TCOCTL */
-#define TCOCTL_EN 0x0100
+#define TCOCTL_EN BIT(8)
/* Auxiliary status register bits, ICH4+ only */
-#define SMBAUXSTS_CRCE 1
-#define SMBAUXSTS_STCO 2
+#define SMBAUXSTS_CRCE BIT(0)
+#define SMBAUXSTS_STCO BIT(1)
/* Auxiliary control register bits, ICH4+ only */
-#define SMBAUXCTL_CRC 1
-#define SMBAUXCTL_E32B 2
+#define SMBAUXCTL_CRC BIT(0)
+#define SMBAUXCTL_E32B BIT(1)
/* Other settings */
#define MAX_RETRIES 400
@@ -170,27 +170,27 @@
#define I801_I2C_BLOCK_DATA 0x18 /* ICH5 and later */
/* I801 Host Control register bits */
-#define SMBHSTCNT_INTREN 0x01
-#define SMBHSTCNT_KILL 0x02
-#define SMBHSTCNT_LAST_BYTE 0x20
-#define SMBHSTCNT_START 0x40
-#define SMBHSTCNT_PEC_EN 0x80 /* ICH3 and later */
+#define SMBHSTCNT_INTREN BIT(0)
+#define SMBHSTCNT_KILL BIT(1)
+#define SMBHSTCNT_LAST_BYTE BIT(5)
+#define SMBHSTCNT_START BIT(6)
+#define SMBHSTCNT_PEC_EN BIT(7) /* ICH3 and later */
/* I801 Hosts Status register bits */
-#define SMBHSTSTS_BYTE_DONE 0x80
-#define SMBHSTSTS_INUSE_STS 0x40
-#define SMBHSTSTS_SMBALERT_STS 0x20
-#define SMBHSTSTS_FAILED 0x10
-#define SMBHSTSTS_BUS_ERR 0x08
-#define SMBHSTSTS_DEV_ERR 0x04
-#define SMBHSTSTS_INTR 0x02
-#define SMBHSTSTS_HOST_BUSY 0x01
+#define SMBHSTSTS_BYTE_DONE BIT(7)
+#define SMBHSTSTS_INUSE_STS BIT(6)
+#define SMBHSTSTS_SMBALERT_STS BIT(5)
+#define SMBHSTSTS_FAILED BIT(4)
+#define SMBHSTSTS_BUS_ERR BIT(3)
+#define SMBHSTSTS_DEV_ERR BIT(2)
+#define SMBHSTSTS_INTR BIT(1)
+#define SMBHSTSTS_HOST_BUSY BIT(0)
/* Host Notify Status register bits */
-#define SMBSLVSTS_HST_NTFY_STS 1
+#define SMBSLVSTS_HST_NTFY_STS BIT(0)
/* Host Notify Command register bits */
-#define SMBSLVCMD_HST_NTFY_INTREN 0x01
+#define SMBSLVCMD_HST_NTFY_INTREN BIT(0)
#define STATUS_ERROR_FLAGS (SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
SMBHSTSTS_DEV_ERR)
--
2.7.4
^ permalink raw reply related
* [PATCH v5 2/6] i2c: i801: minor formatting issues
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
No functional changes, just typos and remove unused #define.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v5
no changes in v4
no changes in v3
no changes in v2
---
drivers/i2c/busses/i2c-i801.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 3a2fdf5..cfb74fc 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -186,10 +186,10 @@
#define SMBHSTSTS_INTR 0x02
#define SMBHSTSTS_HOST_BUSY 0x01
-/* Host Notify Status registers bits */
+/* Host Notify Status register bits */
#define SMBSLVSTS_HST_NTFY_STS 1
-/* Host Notify Command registers bits */
+/* Host Notify Command register bits */
#define SMBSLVCMD_HST_NTFY_INTREN 0x01
#define STATUS_ERROR_FLAGS (SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
@@ -270,8 +270,6 @@ struct i801_priv {
struct smbus_host_notify *host_notify;
};
-#define SMBHSTNTFY_SIZE 8
-
#define FEATURE_SMBUS_PEC (1 << 0)
#define FEATURE_BLOCK_BUFFER (1 << 1)
#define FEATURE_BLOCK_PROC (1 << 2)
--
2.7.4
^ permalink raw reply related
* [PATCH v5 1/6] i2c: i801: store and restore the SLVCMD register at load and unload
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
Also do not override any other configuration in this register.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v5
changes in v4:
- add the i801_disable_host_notify function here as this gets the
first in the series
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 22a0ed4..3a2fdf5 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -240,6 +240,7 @@ struct i801_priv {
struct i2c_adapter adapter;
unsigned long smba;
unsigned char original_hstcfg;
+ unsigned char original_slvcmd;
struct pci_dev *pci_dev;
unsigned int features;
@@ -952,13 +953,26 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
if (!priv->host_notify)
return -ENOMEM;
- outb_p(SMBSLVCMD_HST_NTFY_INTREN, SMBSLVCMD(priv));
+ priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
+
+ if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
+ outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
+ SMBSLVCMD(priv));
+
/* clear Host Notify bit to allow a new notification */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
return 0;
}
+static void i801_disable_host_notify(struct i801_priv *priv)
+{
+ if (!(priv->features & FEATURE_HOST_NOTIFY))
+ return;
+
+ outb_p(priv->original_slvcmd, SMBSLVCMD(priv));
+}
+
static const struct i2c_algorithm smbus_algorithm = {
.smbus_xfer = i801_access,
.functionality = i801_func,
@@ -1647,6 +1661,7 @@ static void i801_remove(struct pci_dev *dev)
pm_runtime_forbid(&dev->dev);
pm_runtime_get_noresume(&dev->dev);
+ i801_disable_host_notify(priv);
i801_del_mux(priv);
i2c_del_adapter(&priv->adapter);
i801_acpi_remove(priv);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 5/6] i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
On the platform tested, reading SMBNTFDDAT always returns 0 (using 1 read
of a word or 2 of 2 bytes). Given that we are not sure why and that we
don't need to rely on the data parameter in the current users of Host
Notify, remove this part of the code.
If someone wants to re-enable it, just revert this commit and data should
be available.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v5
no changes in v4
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 88b2e31..42a6a89 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -117,7 +117,6 @@
#define SMBSLVSTS(p) (16 + (p)->smba) /* ICH3 and later */
#define SMBSLVCMD(p) (17 + (p)->smba) /* ICH3 and later */
#define SMBNTFDADD(p) (20 + (p)->smba) /* ICH3 and later */
-#define SMBNTFDDAT(p) (22 + (p)->smba) /* ICH3 and later */
/* PCI Address Constants */
#define SMBBAR 4
@@ -578,12 +577,15 @@ static void i801_isr_byte_done(struct i801_priv *priv)
static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
{
unsigned short addr;
- unsigned int data;
addr = inb_p(SMBNTFDADD(priv)) >> 1;
- data = inw_p(SMBNTFDDAT(priv));
- i2c_handle_smbus_host_notify(priv->host_notify, addr, data);
+ /*
+ * With the tested platforms, reading SMBNTFDDAT (22 + (p)->smba)
+ * always returns 0 and is safe to read.
+ * We just use 0 given we have no use of the data right now.
+ */
+ i2c_handle_smbus_host_notify(priv->host_notify, addr, 0);
/* clear Host Notify bit and return */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
--
2.7.4
^ permalink raw reply related
* [PATCH v5 4/6] i2c: i801: use the BIT() macro for FEATURES_* also
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
In-Reply-To: <1476360640-12901-1-git-send-email-benjamin.tissoires@redhat.com>
no functional changes
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v5
no changes in v4
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5928ee2..88b2e31 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -270,15 +270,15 @@ struct i801_priv {
struct smbus_host_notify *host_notify;
};
-#define FEATURE_SMBUS_PEC (1 << 0)
-#define FEATURE_BLOCK_BUFFER (1 << 1)
-#define FEATURE_BLOCK_PROC (1 << 2)
-#define FEATURE_I2C_BLOCK_READ (1 << 3)
-#define FEATURE_IRQ (1 << 4)
-#define FEATURE_HOST_NOTIFY (1 << 5)
+#define FEATURE_SMBUS_PEC BIT(0)
+#define FEATURE_BLOCK_BUFFER BIT(1)
+#define FEATURE_BLOCK_PROC BIT(2)
+#define FEATURE_I2C_BLOCK_READ BIT(3)
+#define FEATURE_IRQ BIT(4)
+#define FEATURE_HOST_NOTIFY BIT(5)
/* Not really a feature, but it's convenient to handle it as such */
-#define FEATURE_IDF (1 << 15)
-#define FEATURE_TCO (1 << 16)
+#define FEATURE_IDF BIT(15)
+#define FEATURE_TCO BIT(16)
static const char *i801_feature_names[] = {
"SMBus PEC",
--
2.7.4
^ permalink raw reply related
* [PATCH v5 0/6] i2c: Host Notify / i801 fixes
From: Benjamin Tissoires @ 2016-10-13 12:10 UTC (permalink / raw)
To: Dmitry Torokhov, Wolfram Sang, linux-i2c, linux-kernel; +Cc: Jean Delvare
Hi Wolfram and Dmitry,
5th revision of the series, hopefully this time it will be OK.
The changes were requested by Dmitry: now, SMBus Host Notify is transparent
for clients drivers. The IRQ is attributed if the adapter has the capability
and if nobody claimed an IRQ before. That means that adding an I2C device
through sysfs works, and we don't need to do anything in the client drivers
besides regular IRQ handling.
Cheers,
Benjamin
Benjamin Tissoires (6):
i2c: i801: store and restore the SLVCMD register at load and unload
i2c: i801: minor formatting issues
i2c: i801: use BIT() macro for bits definition
i2c: i801: use the BIT() macro for FEATURES_* also
i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0
i2c: use an IRQ to report Host Notify events, not alert
Documentation/i2c/smbus-protocol | 12 ++--
drivers/i2c/Kconfig | 1 +
drivers/i2c/busses/i2c-i801.c | 121 +++++++++++++++++++--------------------
drivers/i2c/i2c-core.c | 113 ++++++++++++++++++++++++++++++++++++
drivers/i2c/i2c-smbus.c | 102 ---------------------------------
include/linux/i2c-smbus.h | 27 ---------
include/linux/i2c.h | 4 ++
7 files changed, 185 insertions(+), 195 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH 3/5] i2c: i2c-piix4: coding style fix - long comments blocks
From: Jean Delvare @ 2016-10-13 11:30 UTC (permalink / raw)
To: Carlos Palminha; +Cc: linux-i2c, linux-kernel, wsa
In-Reply-To: <20161011172441.7422-4-palminha@synopsys.com>
Hi Carlos,
On Tue, 11 Oct 2016 18:24:39 +0100, Carlos Palminha wrote:
> Signed-off-by: Carlos Palminha <palminha@synopsys.com>
> ---
> drivers/i2c/busses/i2c-piix4.c | 94 ++++++++++++++++++++++++------------------
> 1 file changed, 54 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 5d6f637..85b1052 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> (...)
> /*
> - Supports:
> - Intel PIIX4, 440MX
> - Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
> - ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
> - AMD Hudson-2, ML, CZ
> - SMSC Victory66
> -
> - Note: we assume there can only be one device, with one or more
> - SMBus interfaces.
> - The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
> - For devices supporting multiple ports the i2c_adapter should provide
> - an i2c_algorithm to access them.
> -*/
> + * Supports:
> + * Intel PIIX4, 440MX
> + * Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100
> + * ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800
> + * AMD Hudson-2, ML, CZ
> + * SMSC Victory66
> + *
> + * Note: we assume there can only be one device, with one or more
There's an extra space before "Note:", please remove.
> + * SMBus interfaces.
> + * The device can register multiple i2c_adapters (up to PIIX4_MAX_ADAPTERS).
> + * For devices supporting multiple ports the i2c_adapter should provide
> + * an i2c_algorithm to access them.
> + */
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH 0/5] i2c: i2c-piix4: set of coding style fixes
From: Jean Delvare @ 2016-10-13 11:40 UTC (permalink / raw)
To: Carlos Palminha; +Cc: linux-i2c, linux-kernel, wsa
In-Reply-To: <20161011172441.7422-1-palminha@synopsys.com>
Hi Carlos,
On Tue, 11 Oct 2016 18:24:36 +0100, Carlos Palminha wrote:
> This patchset fixes several coding style issues.
>
> Carlos Palminha (5):
> i2c: i2c-piix4: coding style fix - '*' adjacent to data name
> i2c: i2c-piix4: coding style fix - assignment in if condition
> i2c: i2c-piix4: coding style fix - long comments blocks
> i2c: i2c-piix4: coding style fix - spaces with open parenthesis
> i2c: i2c-piix4: coding style fix - quoted string split across lines
>
> drivers/i2c/busses/i2c-piix4.c | 144 +++++++++++++++++++++++------------------
> 1 file changed, 82 insertions(+), 62 deletions(-)
Except for the two issues which were already pointed out, I'm fine with
these changes, however there is no good reason to split them into 5
separate patches. Fixing the coding style is one goal, it should be all
done with a single patch. So, when you resubmit, please do so as a
single patch.
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH 5/5] i2c: i2c-piix4: coding style fix - quoted string split across lines
From: Jean Delvare @ 2016-10-13 11:34 UTC (permalink / raw)
To: Joe Perches; +Cc: Carlos Palminha, linux-i2c, linux-kernel, wsa
In-Reply-To: <1476237836.2116.8.camel@perches.com>
On Tue, 11 Oct 2016 19:03:56 -0700, Joe Perches wrote:
> On Tue, 2016-10-11 at 18:34 +0100, Carlos Palminha wrote:
> []
> > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> []
> > @@ -260,7 +260,8 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
> > else if ((temp & 0x0E) == 0)
> > dev_dbg(&PIIX4_dev->dev, "Using SMI# for SMBus\n");
> > else
> > - dev_err(&PIIX4_dev->dev, "Illegal Interrupt configuration "
> > + dev_err(&PIIX4_dev->dev,
> > + "Illegal Interrupt configuration "
> > "(or code out of date)!\n");
>
> typically, this format would be coalesced into a single line.
Especially when the patch claims to "fix quoted string split across
lines" :-D
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox