Issue with USB/i2C
Hello,
I've got an issue with and USB/i2c device when i plug and unplug several
times the usb device with a user-application using /dev/i2c-* file the
kernel crashes because the data containing the struct i2c_adapter has
been dealocated.
In the probe function of my driver, i allocates my resource (an internal
structure) that contains a struct i2c_adapter with the struct
i2c_algorithm. In my disconnect function i unregister the i2c adapter
and then i free the ressource. Take not that i use kref design patern to
prevent free data that are still used.
I look deeper in the i2c-dev (even on 3.19 release) and i notice that
the callback function in the struct file_operations use struct
i2c_client with an reference on a struct i2c_adapter and there is no
mechanism to be sure that this reference has not be deallocated.
When i unplug my usb-device, i unregister my i2c-adapter but my
user-space application still hold a file descriptor with a struc
i2c_client that point to an struct i2c_adapter already freed.
I try to make a lock mechanism by adding a wait queue in the struct
i2c_dev, to be sure that the adapter is no more used after a call to
i2c_del_adapter, but it does not work and most of the time put the
kernel in a dead lock.
To conclude it is not a good solution (not safe) to lock in
i2c_del_adapter, because if the usb device is plugged again before the
user-space application use the file descriptor the kernel is locked. A
thread is holding core_lock mutex calling __process_removed_adapter
waiting for a call on release to unlock the wait_queue while another
trying to register the new adapter is locked on core_lock mutex.
I enclose my patch with this mail, knowing that it is a bad solution.
Does anyone with a better knowledge on i2c-core has a working solution
to this issue?
Christian ROSALIE
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 71c7a39..1754795 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -47,20 +47,32 @@ struct i2c_dev {
struct list_head list;
struct i2c_adapter *adap;
struct device *dev;
+
+ wait_queue_head_t crelease;
+ atomic_t ccount; /* client counter */
+ atomic_t detach; /* detach is pending */
};
#define I2C_MINORS 256
static LIST_HEAD(i2c_dev_list);
static DEFINE_SPINLOCK(i2c_dev_list_lock);
-static struct i2c_dev *i2c_dev_get_by_minor(unsigned index)
+#define i2c_dev_get_by_minor(index) __i2c_dev_get_by_minor(index, 0)
+
+static struct i2c_dev *__i2c_dev_get_by_minor(unsigned index, unsigned detach)
{
struct i2c_dev *i2c_dev;
spin_lock(&i2c_dev_list_lock);
list_for_each_entry(i2c_dev, &i2c_dev_list, list) {
- if (i2c_dev->adap->nr == index)
+ if (i2c_dev->adap->nr == index) {
+ if (detach) {
+/* detach is pending */
+atomic_set(&i2c_dev->detach, 1);
+ }
+
goto found;
+ }
}
i2c_dev = NULL;
found:
@@ -82,6 +94,9 @@ static struct i2c_dev *get_free_i2c_dev(struct i2c_adapter *adap)
if (!i2c_dev)
return ERR_PTR(-ENOMEM);
i2c_dev->adap = adap;
+ atomic_set(&i2c_dev->ccount, 0);
+ atomic_set(&i2c_dev->detach, 0);
+ init_waitqueue_head(&i2c_dev->crelease);
spin_lock(&i2c_dev_list_lock);
list_add_tail(&i2c_dev->list, &i2c_dev_list);
@@ -94,6 +109,7 @@ static void return_i2c_dev(struct i2c_dev *i2c_dev)
spin_lock(&i2c_dev_list_lock);
list_del(&i2c_dev->list);
spin_unlock(&i2c_dev_list_lock);
+
kfree(i2c_dev);
}
@@ -492,6 +508,15 @@ static int i2cdev_open(struct inode *inode, struct file *file)
if (!i2c_dev)
return -ENODEV;
+ /* detach is pending
+ no more client is allowed */
+ if (atomic_read(&i2c_dev->detach)) {
+ return -ENODEV;
+ }
+
+ atomic_inc(&i2c_dev->ccount);
+
+
adap = i2c_get_adapter(i2c_dev->adap->nr);
if (!adap)
return -ENODEV;
@@ -505,6 +530,7 @@ static int i2cdev_open(struct inode *inode, struct file *file)
*/
client = kzalloc(sizeof(*client), GFP_KERNEL);
if (!client) {
+ atomic_dec(&i2c_dev->ccount);
i2c_put_adapter(adap);
return -ENOMEM;
}
@@ -513,14 +539,26 @@ static int i2cdev_open(struct inode *inode, struct file *file)
client->adapter = adap;
file->private_data = client;
+
return 0;
}
static int i2cdev_release(struct inode *inode, struct file *file)
{
struct i2c_client *client = file->private_data;
+ struct i2c_dev *i2c_dev;
i2c_put_adapter(client->adapter);
+
+ i2c_dev = i2c_dev_get_by_minor(client->adapter->nr);
+
+ if (i2c_dev) {
+ if (atomic_dec_and_test(&i2c_dev->ccount)) {
+ /* wake up wait queue if necessary */
+ wake_up_interruptible(&i2c_dev->crelease);
+ }
+ }
+
kfree(client);
file->private_data = NULL;
@@ -581,10 +619,17 @@ static int i2cdev_detach_adapter(struct device *dev, void *dummy)
return 0;
adap = to_i2c_adapter(dev);
- i2c_dev = i2c_dev_get_by_minor(adap->nr);
+ i2c_dev = __i2c_dev_get_by_minor(adap->nr, 1);
if (!i2c_dev) /* attach_adapter must have failed */
return 0;
+ device_remove_file(i2c_dev->dev, &dev_attr_name);
+
+ /* wait tha
Re: [PATCH] eeprom: at24: Add support for large EEPROMs connected to SMBus adapters
On 02/16/2015 04:09 AM, Wolfram Sang wrote: Hi Guenter, I wonder where we are with thisp patch; I don't recall a reply to my previous e-mail. Sorry for the late reply. I needed to recover from a HDD headcrash :( Do you need some more time to think about it ? Otherwise I'll publish an out-of-tree version of the at24 driver with the patch applied on github, for those who might need the functionality provided by this patch. Your last mail made me aware of why we were missing each other before. I see your point now, but yes, still need to think about it. My plan is to have a decision until the 3.21 merge window. Ok, fair enough. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c: dt binding documentation for the Digicolor I2C controller
The CX92755 is an SoC in the Conexant Digicolor series. This devicetree binding
document describes the I2C controller on the CX92755 SoC, that is also shared
by some other SoCs in the Digicolor series.
Signed-off-by: Baruch Siach
---
.../devicetree/bindings/i2c/i2c-digicolor.txt | 25 ++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-digicolor.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-digicolor.txt
b/Documentation/devicetree/bindings/i2c/i2c-digicolor.txt
new file mode 100644
index ..457a098d4f7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-digicolor.txt
@@ -0,0 +1,25 @@
+Conexant Digicolor I2C controller
+
+Required properties:
+ - compatible: must be "cnxt,cx92755-i2c"
+ - reg: physical address and length of the device registers
+ - interrupts: a single interrupt specifier
+ - clocks: clock for the device
+ - #address-cells: should be <1>
+ - #size-cells: should be <0>
+
+Optional properties:
+- clock-frequency: the desired I2C bus clock frequency in Hz; in
+ absence of this property the default value is used (100 kHz).
+
+Example:
+
+ i2c: i2c@f120 {
+ compatible = "cnxt,cx92755-i2c";
+ reg = <0xf120 0x10>;
+ interrupts = <28>;
+ clocks = <&main_clk>;
+ clock-frequency = <10>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i2c: driver for the Conexant Digicolor I2C controller
This commit adds a driver for the I2C master controller on the CX92755 SoC. The
CX92755 is one SoC in the Conexant Digicolor series. This driver should support
other SoCs from that series.
Only 7bit slave addresses are currently supported.
Tested on the Equinox CX92755 EVK, using i2c-tools utilities to read and write
the on-chip (sic) audio codec.
Signed-off-by: Baruch Siach
---
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile| 1 +
drivers/i2c/busses/i2c-digicolor.c | 384 +
3 files changed, 394 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-digicolor.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 31e8308ba899..08cf8acf87aa 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -465,6 +465,15 @@ config I2C_DESIGNWARE_PCI
This driver can also be built as a module. If so, the module
will be called i2c-designware-pci.
+config I2C_DIGICOLOR
+ tristate "Conexang Digicolor I2C driver"
+ depends on ARCH_DIGICOLOR
+ help
+ Support for Conexant Digicolor SoCs (CX92755) I2C controller driver.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-digicolor.
+
config I2C_EFM32
tristate "EFM32 I2C controller"
depends on ARCH_EFM32 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 56388f658d2f..1b1830d50651 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) +=
i2c-designware-platform.o
i2c-designware-platform-objs := i2c-designware-platdrv.o
obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
i2c-designware-pci-objs := i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_DIGICOLOR)+= i2c-digicolor.o
obj-$(CONFIG_I2C_EFM32)+= i2c-efm32.o
obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
obj-$(CONFIG_I2C_EXYNOS5) += i2c-exynos5.o
diff --git a/drivers/i2c/busses/i2c-digicolor.c
b/drivers/i2c/busses/i2c-digicolor.c
new file mode 100644
index ..0ec0eafb8fb6
--- /dev/null
+++ b/drivers/i2c/busses/i2c-digicolor.c
@@ -0,0 +1,384 @@
+/*
+ * I2C bus driver for Conexant Digicolor SoCs
+ *
+ * Author: Baruch Siach
+ *
+ * Copyright (C) 2015 Paradox Innovation Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_FREQ 10
+#define TIMEOUT_MS 100
+
+#define II_CONTROL 0x0
+#define II_CONTROL_LOCAL_RESET BIT(0)
+
+#define II_CLOCKTIME 0x1
+
+#define II_COMMAND 0x2
+#define II_CMD_START 1
+#define II_CMD_RESTART 2
+#define II_CMD_SEND_ACK3
+#define II_CMD_GET_ACK 6
+#define II_CMD_GET_NOACK 7
+#define II_CMD_STOP10
+#define II_COMMAND_GO BIT(7)
+#define II_COMMAND_COMPLETION_STATUS(r)(((r) >> 5) & 3)
+#define II_CMD_STATUS_NORMAL 0
+#define II_CMD_STATUS_ACK_GOOD 1
+#define II_CMD_STATUS_ACK_BAD 2
+#define II_CMD_STATUS_ABORT3
+
+#define II_DATA0x3
+#define II_INTFLAG_CLEAR 0x8
+#define II_INTENABLE 0xa
+
+struct dc_i2c {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem*regs;
+ struct clk *clk;
+ unsigned intfrequency;
+
+ struct i2c_msg *msg;
+ unsigned intmsgbuf_ptr;
+ int last;
+ spinlock_t lock;
+ struct completion done;
+ int state;
+ int error;
+};
+
+enum {
+STATE_IDLE,
+STATE_START,
+STATE_ADDR,
+STATE_WRITE,
+STATE_READ,
+STATE_STOP,
+};
+
+static void dc_i2c_cmd(struct dc_i2c *i2c, u8 cmd)
+{
+ writeb_relaxed(cmd | II_COMMAND_GO, i2c->regs + II_COMMAND);
+}
+
+static u8 dc_i2c_addr_cmd(struct i2c_msg *msg)
+{
+ u8 addr = (msg->addr & 0x7f) << 1;
+
+ if (msg->flags & I2C_M_RD)
+ addr |= 1;
+
+ return addr;
+}
+
+static void dc_i2c_data(struct dc_i2c *i2c, u8 data)
+{
+ writeb_relaxed(data, i2c->regs + II_DATA);
+}
+
+static void dc_i2c_write_byte(struct dc_i2c *i2c, u8 byte)
+{
+ dc_i2c_data(i2c, byte);
+ dc_i2c_cmd(i2c, II_CMD_SEND_ACK);
+}
+
+static void dc_i2c_write_buf(struct dc_i2c *i2c)
+{
+ dc_i2c_write_byte(i2c, i2c->msg->buf[i2c->msgbuf_ptr++]);
+}
+
+static void dc_i2c_next_read(struct dc_i2c *i2c)
+{
+ bool last = (i2c->msgbuf_ptr+1 == i2c->msg->len);
+ dc_i2c_cmd(i2c, last ? II_
Re: [PATCH] eeprom: at24: Add support for large EEPROMs connected to SMBus adapters
Hi Guenter, > I wonder where we are with thisp patch; I don't recall a reply to my previous > e-mail. Sorry for the late reply. I needed to recover from a HDD headcrash :( > Do you need some more time to think about it ? Otherwise I'll publish an > out-of-tree version of the at24 driver with the patch applied on github, > for those who might need the functionality provided by this patch. Your last mail made me aware of why we were missing each other before. I see your point now, but yes, still need to think about it. My plan is to have a decision until the 3.21 merge window. All the best, Wolfram signature.asc Description: Digital signature
Re: [PATCHv3 5/5] i2c: i801: Use managed pcim_* PCI device initialization and reservation
On Fri, 13 Feb 2015 15:52:25 +0200, Jarkko Nikula wrote:
> Simplifies the code a bit and makes easier to disable PCI device on driver
> detach by removing the pcim_pin_device() call in the future if needed.
>
> Reason why i2c-i801.c doesn't ever call pci_disable_device() was because it
> made some systems to hang during power-off. See commit d6fcb3b9cf77
> ("[PATCH] i2c-i801.c: don't pci_disable_device() after it was just enabled")
> and
> http://marc.info/?l=linux-kernel&m=115160053309535&w=2
>
> Signed-off-by: Jarkko Nikula
> ---
> Changes from v2:
> - over 80 characters long pcim_iomap_regions line splitted
> - gotos and error labels removed
> ---
> drivers/i2c/busses/i2c-i801.c | 25 +
> 1 file changed, 9 insertions(+), 16 deletions(-)
> (...)
Reviewed-by: Jean Delvare
Wolfram, please commit this series, I have reviewed all patches and
tested the updated driver successfully on 3 different machines.
Thanks Jarkko for the good work.
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 4/5] i2c: i801: Remove pci_enable_device() call from i801_resume()
On Fri, 13 Feb 2015 15:52:24 +0200, Jarkko Nikula wrote:
> Since pci_disable_device() is not called from i801_suspend() and power
> state is set already it means that subsequent pci_enable_device() calls do
> practically nothing but monotonically increase struct pci_dev enable_cnt.
>
> Signed-off-by: Jarkko Nikula
> ---
> drivers/i2c/busses/i2c-i801.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index b1d725d758bb..5fb35464f693 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1324,7 +1324,7 @@ static int i801_resume(struct pci_dev *dev)
> {
> pci_set_power_state(dev, PCI_D0);
> pci_restore_state(dev);
> - return pci_enable_device(dev);
> + return 0;
> }
> #else
> #define i801_suspend NULL
Reviewed-by: Jean Delvare
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
