[PATCH v4] i2c: rk3x: adjust the LOW divison based on characteristics of SCL
As show in I2C specification:
- Standard-mode: the minimum HIGH period of the scl clock is 4.0us
the minimum LOW period of the scl clock is 4.7us
- Fast-mode: the minimum HIGH period of the scl clock is 0.6us
the minimum LOW period of the scl clock is 1.3us
I have measured i2c SCL waveforms in fast-mode by oscilloscope
on rk3288-pinky board. the LOW period of the scl clock is 1.3us.
It is so critical that we must adjust LOW division to increase
the LOW period of the scl clock.
Thanks Doug for the suggestion about division formulas.
Reviewed-by: Doug Anderson diand...@chromium.org
Tested-by: Doug Anderson diand...@chromium.org
Signed-off-by: Addy Ke addy...@rock-chips.com
---
Changes in v2:
- remove Fast-mode plus and HS-mode
- use new formulas suggested by Doug
Changes in V3:
- use new formulas (sep 30) suggested by Doug
Changes in V3:
- fix some wrong style
- WARN_ONCE if min_low_div max_low_div
drivers/i2c/busses/i2c-rk3x.c | 140 +++---
1 file changed, 133 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index b41d979..f17f7a2 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -24,6 +24,7 @@
#include linux/wait.h
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/math64.h
/* Register Map */
@@ -428,18 +429,143 @@ out:
return IRQ_HANDLED;
}
+static void rk3x_i2c_calc_divs(unsigned long i2c_rate, unsigned long scl_rate,
+ unsigned long *div_low, unsigned long *div_high)
+{
+ unsigned long min_low_ns, min_high_ns;
+ unsigned long max_data_hold_ns;
+ unsigned long data_hold_buffer_ns;
+ unsigned long max_low_ns, min_total_ns;
+
+ unsigned long i2c_rate_khz, scl_rate_khz;
+
+ unsigned long min_low_div, min_high_div;
+ unsigned long max_low_div;
+
+ unsigned long min_div_for_hold, min_total_div;
+ unsigned long extra_div, extra_low_div, ideal_low_div;
+
+ /* Only support standard-mode and fast-mode */
+ WARN_ON(scl_rate 40);
+
+ /*
+* min_low_ns: The minimum number of ns we need to hold low
+* to meet i2c spec
+* min_high_ns: The minimum number of ns we need to hold high
+* to meet i2c spec
+* max_low_ns: The maximum number of ns we can hold low
+* to meet i2c spec
+*
+* Note: max_low_ns should be (max data hold time * 2 - buffer)
+* This is because the i2c host on Rockchip holds the data line
+* for half the low time.
+*/
+ if (scl_rate = 10) {
+ min_low_ns = 4700;
+ min_high_ns = 4000;
+ max_data_hold_ns = 3450;
+ data_hold_buffer_ns = 50;
+ } else {
+ min_low_ns = 1300;
+ min_high_ns = 600;
+ max_data_hold_ns = 900;
+ data_hold_buffer_ns = 50;
+ }
+ max_low_ns = max_data_hold_ns * 2 - data_hold_buffer_ns;
+ min_total_ns = min_low_ns + min_high_ns;
+
+ /* Adjust to avoid overflow */
+ i2c_rate_khz = DIV_ROUND_UP(i2c_rate, 1000);
+ scl_rate_khz = DIV_ROUND_UP(scl_rate, 1000);
+
+ /*
+* We need the total div to be = this number
+* so we don't clock too fast.
+*/
+ min_total_div = DIV_ROUND_UP(i2c_rate_khz, scl_rate_khz * 8);
+
+ /* These are the min dividers needed for min hold times. */
+ min_low_div = DIV_ROUND_UP(i2c_rate_khz * min_low_ns, 8 * 100);
+ min_high_div = DIV_ROUND_UP(i2c_rate_khz * min_high_ns, 8 * 100);
+ min_div_for_hold = (min_low_div + min_high_div);
+
+ /*
+* This is the maximum divider so we don't go over the max.
+* We don't round up here (we round down) since this is a max.
+*/
+ max_low_div = i2c_rate_khz * max_low_ns / (8 * 100);
+
+ if (min_low_div max_low_div) {
+ WARN_ONCE(true,
+ Conflicting, min_low_div %lu, max_low_div %lu\n,
+ min_low_div, max_low_div);
+ max_low_div = min_low_div;
+ }
+
+ if (min_div_for_hold min_total_div) {
+ /*
+* Time needed to meet hold requirements is important.
+* Just use that.
+*/
+ *div_low = min_low_div;
+ *div_high = min_high_div;
+ } else {
+ /*
+* We've got to distribute some time among the low and high
+* so we don't run too fast.
+*/
+ extra_div = min_total_div - min_div_for_hold;
+
+ /*
+* We'll try to split things up perfectly evenly,
+* biasing slightly towards having a higher div
+* for low (spend more time low).
+
Re: [PATCH v3 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller
On Mon, 06 Oct 2014, Muthu Mani wrote:
Adds support for USB-I2C/GPIO interfaces of Cypress Semiconductor
CYUSBS234 USB-Serial Bridge controller.
Details about the device can be found at:
http://www.cypress.com/?rID=84126
Signed-off-by: Muthu Mani m...@cypress.com
Signed-off-by: Rajaram Regupathy r...@cypress.com
---
Changes since v2:
* Used auto mfd id to support multiple devices
* Cleaned up the code
Changes since v1:
* Identified different serial interface and loaded correct cell driver
* Formed a mfd id to support multiple devices
* Removed unused platform device
drivers/mfd/Kconfig | 12 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/cyusbs23x.c | 167
++
include/linux/mfd/cyusbs23x.h | 62
4 files changed, 242 insertions(+)
create mode 100644 drivers/mfd/cyusbs23x.c
create mode 100644 include/linux/mfd/cyusbs23x.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..a31e9e3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -116,6 +116,18 @@ config MFD_ASIC3
This driver supports the ASIC3 multifunction chip found on many
PDAs (mainly iPAQ and HTC based ones)
+config MFD_CYUSBS23X
+tristate Cypress CYUSBS23x USB Serial Bridge controller
White space issue here.
+ select MFD_CORE
+ depends on USB
+ default n
+ help
+ Say yes here if you want support for Cypress Semiconductor
+ CYUSBS23x USB-Serial Bridge controller.
+
+ This driver can also be built as a module. If so, the module will be
+ called cyusbs23x.
+
config PMIC_DA903X
bool Dialog Semiconductor DA9030/DA9034 PMIC Support
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..fc5bcd1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -151,6 +151,7 @@ si476x-core-y := si476x-cmd.o si476x-prop.o si476x-i2c.o
obj-$(CONFIG_MFD_SI476X_CORE)+= si476x-core.o
obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
+obj-$(CONFIG_MFD_CYUSBS23X) += cyusbs23x.o
obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o omap-usb-tll.o
obj-$(CONFIG_MFD_PM8921_CORE)+= pm8921-core.o ssbi.o
obj-$(CONFIG_TPS65911_COMPARATOR)+= tps65911-comparator.o
diff --git a/drivers/mfd/cyusbs23x.c b/drivers/mfd/cyusbs23x.c
new file mode 100644
index 000..c70ea40
--- /dev/null
+++ b/drivers/mfd/cyusbs23x.c
@@ -0,0 +1,167 @@
+/*
+ * Cypress USB-Serial Bridge Controller USB adapter driver
+ *
+ * Copyright (c) 2014 Cypress Semiconductor Corporation.
+ *
+ * Author:
+ * Muthu Mani m...@cypress.com
+ *
+ * Additional contributors include:
+ * Rajaram Regupathy r...@cypress.com
+ *
+ * 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.
+ */
+
+/*
+ * This is core MFD driver for Cypress Semiconductor CYUSBS234 USB-Serial
+ * Bridge controller. CYUSBS234 offers a single channel serial interface
+ * (I2C/SPI/UART). It can be configured to enable either of I2C, SPI, UART
+ * interfaces. The GPIOs are also available to access.
+ * Details about the device can be found at:
+ *http://www.cypress.com/?rID=84126
+ *
+ * Separate cell drivers are available for I2C and GPIO. SPI and UART are not
+ * supported yet. All GPIOs are exposed for get operation. However, only
+ * unused GPIOs can be set.
+ */
+
+#include linux/kernel.h
+#include linux/errno.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/types.h
+#include linux/mutex.h
+
This '\n' is superfluous, please remove it.
+#include linux/mfd/core.h
+#include linux/mfd/cyusbs23x.h
+
+#include linux/usb.h
+
+static const struct usb_device_id cyusbs23x_usb_table[] = {
+ { USB_DEVICE(0x04b4, 0x0004) }, /* Cypress Semiconductor */
+ { } /* Terminating entry */
This comment is not required, please remove it.
+};
+
+MODULE_DEVICE_TABLE(usb, cyusbs23x_usb_table);
+
+static const struct mfd_cell cyusbs23x_i2c_devs[] = {
+ {
+ .name = cyusbs23x-i2c,
+ },
+ {
+ .name = cyusbs23x-gpio,
+ },
+};
+
+static int update_ep_details(struct usb_interface *interface,
+ struct cyusbs23x *cyusbs)
+{
+ struct usb_host_interface *iface_desc;
+ struct usb_endpoint_descriptor *ep;
+ int i;
+
+ iface_desc = interface-cur_altsetting;
+
+ for (i = 0; i iface_desc-desc.bNumEndpoints; ++i) {
+
+ ep = iface_desc-endpoint[i].desc;
+
+ if (!cyusbs-bulk_in_ep_num usb_endpoint_is_bulk_in(ep))
+ cyusbs-bulk_in_ep_num = ep-bEndpointAddress;
+ if (!cyusbs-bulk_out_ep_num usb_endpoint_is_bulk_out(ep))
+
[PATCH 2/3] i2c: qup: Remove .owner field for driver
There is no need to init .owner field.
Based on the patch from Peter Griffin peter.grif...@linaro.org
mmc: remove .owner field for drivers using module_platform_driver
This patch removes the superfluous .owner field for drivers which
use the module_platform_driver API, as this is overridden in
platform_driver_register anyway.
Signed-off-by: Kiran Padwal kiran.pad...@smartplayin.com
---
drivers/i2c/busses/i2c-qup.c |1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 3a4d64e..f14af2e 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -760,7 +760,6 @@ static struct platform_driver qup_i2c_driver = {
.remove = qup_i2c_remove,
.driver = {
.name = i2c_qup,
- .owner = THIS_MODULE,
.pm = qup_i2c_qup_pm_ops,
.of_match_table = qup_i2c_dt_match,
},
--
1.7.9.5
--
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: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter
On Thu, Oct 9, 2014 at 8:46 AM, RR rajaram.officema...@gmail.com wrote:
On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot gnu...@gmail.com wrote:
On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani m...@cypress.com wrote:
-Original Message-
From: Alexandre Courbot [mailto:gnu...@gmail.com]
Sent: Tuesday, October 07, 2014 3:34 PM
To: Muthu Mani
Cc: Samuel Ortiz; Lee Jones; Wolfram Sang; Linus Walleij; Greg Kroah-
Hartman; linux-i2c@vger.kernel.org; linux-g...@vger.kernel.org; linux-
u...@vger.kernel.org; Linux Kernel Mailing List; Rajaram Regupathy; Johan
Hovold
Subject: Re: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-
GPIO adapter
On Mon, Oct 6, 2014 at 11:47 PM, Muthu Mani m...@cypress.com wrote:
+
+static int cy_gpio_direction_input(struct gpio_chip *chip,
+ unsigned offset) {
+ return 0;
+}
+
+static int cy_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value) {
+ return 0;
+}
If that chip is capable of both output and input, shouldn't these
functions be
implemented? I think this has already been pointed out in a previous
version
but you did not reply.
Thanks for your inputs.
Only the GPIOs which are configured to be output GPIO can be set.
In that case cy_gpio_set() should return an error for GPIOs which are
not configured as outputs. Is that guaranteed by the current
implementation?
The set operation would fail trying to set the input or unconfigured GPIOs.
In this version of driver, this support is not added, it can be introduced
in future versions.
I will add a TODO note in the code.
Argh, no TODO please. Actual code that will turn this code into a
solid driver that can be merged.
Does a driver targeted for a custom device has to implement every
functionality in the 1st version ? My understanding is that Linux
follows incremental model and allows incremental merge.
Certainly, but we are talking about very basic functionality here.
Right now this driver will incorrectly returns success when trying to
set the direction of a GPIO, even though it did not do anything. There
is also no hint that _set will return an error if called on an
input/unconfigured GPIO.
So while we are not asking for a driver to be perfect on the first
iteration, setting the direction of a GPIO is a very basic
functionality that is essential for a GPIO driver to just work. Custom
device or not, this is mainline. The rest of the driver seems to be
ok, so please take the last few steps that will allow us to merge a
featured driver.
--
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: [PATCH v3 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller
On Thu, Oct 09, 2014 at 08:40:29AM +0100, Lee Jones wrote:
On Mon, 06 Oct 2014, Muthu Mani wrote:
+static int update_ep_details(struct usb_interface *interface,
+ struct cyusbs23x *cyusbs)
+{
+ struct usb_host_interface *iface_desc;
+ struct usb_endpoint_descriptor *ep;
+ int i;
+
+ iface_desc = interface-cur_altsetting;
+
+ for (i = 0; i iface_desc-desc.bNumEndpoints; ++i) {
+
+ ep = iface_desc-endpoint[i].desc;
+
+ if (!cyusbs-bulk_in_ep_num usb_endpoint_is_bulk_in(ep))
+ cyusbs-bulk_in_ep_num = ep-bEndpointAddress;
+ if (!cyusbs-bulk_out_ep_num usb_endpoint_is_bulk_out(ep))
+ cyusbs-bulk_out_ep_num = ep-bEndpointAddress;
+ if (!cyusbs-intr_in_ep_num usb_endpoint_is_int_in(ep))
+ cyusbs-intr_in_ep_num = ep-bEndpointAddress;
+ }
All of the USB specific code in this driver will require a USB Ack.
I'll review it once the incomplete gpio-driver issue has been resolved.
+ dev_dbg(interface-dev, %s intr_in=%d, bulk_in=%d, bulk_out=%d\n,
+ __func__, cyusbs-intr_in_ep_num ,
+ cyusbs-bulk_in_ep_num, cyusbs-bulk_out_ep_num);
+
+ if (!cyusbs-bulk_in_ep_num || !cyusbs-bulk_out_ep_num ||
+ !cyusbs-intr_in_ep_num)
+ return -ENODEV;
+
+ return 0;
+}
[...]
diff --git a/include/linux/mfd/cyusbs23x.h b/include/linux/mfd/cyusbs23x.h
+/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */
+enum cy_scb_modes {
+ CY_USBS_SCB_DISABLED = 0,
+ CY_USBS_SCB_UART = 1,
+ CY_USBS_SCB_SPI = 2,
+ CY_USBS_SCB_I2C = 3
No need to number these.
As it's not an arbitrary enumeration, I think they should be initialised
explicitly. They could be defined in the mfd driver though, as they only
appear to be needed during probe.
Johan
--
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] at24: extend driver to allow writing via i2c_smbus_write_byte_data
I have a at24 EEPROM connected via i2c bus provided by ISCH i2c
bus driver. This bus driver does not support
I2C_FUNC_SMBUS_WRITE_I2C_BLOCK and so I was looking for a way
to be able to write the eeprom. This patch adds support for
I2C_SMBUS_BYTE_DATA writing via i2c_smbus_write_byte_data.
It is quite slow, but it works.
Signed-off-by: Christian Gmeiner christian.gmei...@gmail.com
---
drivers/misc/eeprom/at24.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index d87f77f..f51daac 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -56,6 +56,7 @@ struct at24_data {
struct at24_platform_data chip;
struct memory_accessor macc;
int use_smbus;
+ int use_smbuse_write;
/*
* Lock protects against activities from other Linux tasks,
@@ -324,7 +325,7 @@ static ssize_t at24_eeprom_write(struct at24_data *at24,
const char *buf,
{
struct i2c_client *client;
struct i2c_msg msg;
- ssize_t status;
+ ssize_t status = 0;
unsigned long timeout, write_time;
unsigned next_page;
@@ -365,9 +366,18 @@ static ssize_t at24_eeprom_write(struct at24_data *at24,
const char *buf,
timeout = jiffies + msecs_to_jiffies(write_timeout);
do {
write_time = jiffies;
- if (at24-use_smbus) {
- status = i2c_smbus_write_i2c_block_data(client,
- offset, count, buf);
+ if (at24-use_smbuse_write) {
+ switch (at24-use_smbuse_write) {
+ case I2C_SMBUS_I2C_BLOCK_DATA:
+ status = i2c_smbus_write_i2c_block_data(client,
+ offset, count, buf);
+ break;
+ case I2C_SMBUS_BYTE_DATA:
+ status = i2c_smbus_write_byte_data(client,
+ offset, buf[0]);
+ break;
+ }
+
if (status == 0)
status = count;
} else {
@@ -487,6 +497,7 @@ static int at24_probe(struct i2c_client *client, const
struct i2c_device_id *id)
struct at24_platform_data chip;
bool writable;
int use_smbus = 0;
+ int use_smbus_write = 0;
struct at24_data *at24;
int err;
unsigned i, num_addresses;
@@ -546,6 +557,18 @@ static int at24_probe(struct i2c_client *client, const
struct i2c_device_id *id)
}
}
+ /* Use I2C operations unless we're stuck with SMBus extensions. */
+ if (!i2c_check_functionality(client-adapter, I2C_FUNC_I2C)) {
+ if (i2c_check_functionality(client-adapter,
+ I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+ use_smbus_write = I2C_SMBUS_I2C_BLOCK_DATA;
+ } else if (i2c_check_functionality(client-adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
+ use_smbus_write = I2C_SMBUS_BYTE_DATA;
+ chip.page_size = 1;
+ }
+ }
+
if (chip.flags AT24_FLAG_TAKE8ADDR)
num_addresses = 8;
else
@@ -559,6 +582,7 @@ static int at24_probe(struct i2c_client *client, const
struct i2c_device_id *id)
mutex_init(at24-lock);
at24-use_smbus = use_smbus;
+ at24-use_smbuse_write = use_smbus_write;
at24-chip = chip;
at24-num_addresses = num_addresses;
@@ -576,8 +600,7 @@ static int at24_probe(struct i2c_client *client, const
struct i2c_device_id *id)
writable = !(chip.flags AT24_FLAG_READONLY);
if (writable) {
- if (!use_smbus || i2c_check_functionality(client-adapter,
- I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+ if (!use_smbus || use_smbus_write) {
unsigned write_max = chip.page_size;
--
1.9.3
--
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: [PATCH v3 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller
On Thu, 09 Oct 2014, Johan Hovold wrote:
On Thu, Oct 09, 2014 at 08:40:29AM +0100, Lee Jones wrote:
On Mon, 06 Oct 2014, Muthu Mani wrote:
+static int update_ep_details(struct usb_interface *interface,
+ struct cyusbs23x *cyusbs)
+{
+ struct usb_host_interface *iface_desc;
+ struct usb_endpoint_descriptor *ep;
+ int i;
+
+ iface_desc = interface-cur_altsetting;
+
+ for (i = 0; i iface_desc-desc.bNumEndpoints; ++i) {
+
+ ep = iface_desc-endpoint[i].desc;
+
+ if (!cyusbs-bulk_in_ep_num usb_endpoint_is_bulk_in(ep))
+ cyusbs-bulk_in_ep_num = ep-bEndpointAddress;
+ if (!cyusbs-bulk_out_ep_num usb_endpoint_is_bulk_out(ep))
+ cyusbs-bulk_out_ep_num = ep-bEndpointAddress;
+ if (!cyusbs-intr_in_ep_num usb_endpoint_is_int_in(ep))
+ cyusbs-intr_in_ep_num = ep-bEndpointAddress;
+ }
All of the USB specific code in this driver will require a USB Ack.
I'll review it once the incomplete gpio-driver issue has been resolved.
Okay, great.
+ dev_dbg(interface-dev, %s intr_in=%d, bulk_in=%d, bulk_out=%d\n,
+ __func__, cyusbs-intr_in_ep_num ,
+ cyusbs-bulk_in_ep_num, cyusbs-bulk_out_ep_num);
+
+ if (!cyusbs-bulk_in_ep_num || !cyusbs-bulk_out_ep_num ||
+ !cyusbs-intr_in_ep_num)
+ return -ENODEV;
+
+ return 0;
+}
[...]
diff --git a/include/linux/mfd/cyusbs23x.h b/include/linux/mfd/cyusbs23x.h
+/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */
+enum cy_scb_modes {
+ CY_USBS_SCB_DISABLED = 0,
+ CY_USBS_SCB_UART = 1,
+ CY_USBS_SCB_SPI = 2,
+ CY_USBS_SCB_I2C = 3
No need to number these.
As it's not an arbitrary enumeration, I think they should be initialised
explicitly.
No need. You are protected by the C Standard:
6.7.2.2 Enumeration specifiers
If the first enumerator has no =, the value of its enumeration
constant is 0. Each subsequent enumerator with no = defines its
enumeration constant as the value of the constant expression obtained
by adding 1 to the value of the previous enumeration constant.
There's nothing arbitrary about that.
They could be defined in the mfd driver though, as they only
appear to be needed during probe.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support
Hi David, On Tue, Oct 07, 2014 at 12:14:20PM -0700, David E. Box wrote: Hi Maxime, On Thu, Sep 25, 2014 at 11:47:52AM +0200, Maxime Ripard wrote: Hi David, On Tue, Sep 23, 2014 at 12:58:54PM -0700, David E. Box wrote: Hi Maxime, On Tue, Sep 23, 2014 at 09:00:57PM +0200, Maxime Ripard wrote: Hi David, On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote: This patch implements an I2C bus sharing mechanism between the host and platform hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC. On these platforms access to the PMIC must be shared with platform hardware. The hardware unit assumes full control of the I2C bus and the host must request access through a special semaphore. Hardware control of the bus also makes it necessary to disable runtime pm to avoid interfering with hardware transactions. Signed-off-by: David E. Box david.e@linux.intel.com Sorry for stepping in like this without really knowing your platform, but wouldn't using the hwspinlock framework make more sense than hardcoding your own internal functions here? I looked into this but didn't see a clear way on our platform to identify the semaphore seperately from doing it in the designware platform driver. The way we can find it now is through evaluating an ACPI _SEM object on every i2c device that gets probed by the dw driver since at probe time we can get the acpi handle. And you have no way to turn it around and identify which semaphore is associated to which i2c bus? If so, there is probably some way to associate a given instance of the i2c driver to one semaphore. Without this handle however there isn't a clear way of evaluating the _SEM object which would be needed to register a hwspinlock in separate code. Plus it would still require changes to the designware i2c core, though admittedly having a generic hwspinlock pointer added to the struct is cleaner. Not only cleaner, but that could also be used by other platforms that are using this I2C driver (and since it's a designware IP, there must be quite a lot) together with hardware locking. After again considering a way to make this work I don't think this api can fit well with our platform. Acquisition of this semaphore is through a mailbox sequence where we set one register and then poll another for a value that confirms we have the lock. For best performance we need to be able to periodically sleep while waiting for that confirmation. This time can vary widely as it's dependent on the component we are requesting the semaphore from which is itself a user of that bus. While we could simply fail after a short time, reattempts would still need to happen in the i2c-designware driver and the timing would be completely dependent on our hardware, making it less clean for reuse. In addition, if we timed out, we would have to immediately call unlock to cancel the mailbox transaction. This may not fit well with reuse either. Ok, if Wolfram is ok with it, and if it makes your life much easier, I'm ok :) Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH v3 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller
On Thu, Oct 09, 2014 at 11:59:50AM +0100, Lee Jones wrote:
On Thu, 09 Oct 2014, Johan Hovold wrote:
On Thu, Oct 09, 2014 at 08:40:29AM +0100, Lee Jones wrote:
On Mon, 06 Oct 2014, Muthu Mani wrote:
diff --git a/include/linux/mfd/cyusbs23x.h
b/include/linux/mfd/cyusbs23x.h
+/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */
+enum cy_scb_modes {
+ CY_USBS_SCB_DISABLED = 0,
+ CY_USBS_SCB_UART = 1,
+ CY_USBS_SCB_SPI = 2,
+ CY_USBS_SCB_I2C = 3
No need to number these.
As it's not an arbitrary enumeration, I think they should be initialised
explicitly.
No need. You are protected by the C Standard:
6.7.2.2 Enumeration specifiers
If the first enumerator has no =, the value of its enumeration
constant is 0. Each subsequent enumerator with no = defines its
enumeration constant as the value of the constant expression obtained
by adding 1 to the value of the previous enumeration constant.
There's nothing arbitrary about that.
I obviously wasn't suggesting that the definition of an enum (and the
values of its constants) in c was arbitrary.
My point was that the values of the USB interface subclasses (defined
through the enum) are not arbitrary. In this case they just happen to be
zero-based and consecutive. You cannot reorder, or remove an unused
item, without breaking the driver. By initialising each constant
explicitly this would become apparent.
Using preprocessor defines could be an alternative if you really do not
like initialised enumeration constants.
They could be defined in the mfd driver though, as they only
appear to be needed during probe.
Johan
--
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: [PATCH v6 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Oct 08, 2014 at 03:33:28PM +0300, Octavian Purdila wrote:
On Wed, Oct 8, 2014 at 3:04 PM, Johan Hovold jo...@kernel.org wrote:
On Wed, Oct 08, 2014 at 01:54:07PM +0300, Octavian Purdila wrote:
On Wed, Oct 8, 2014 at 12:23 PM, Johan Hovold jo...@kernel.org wrote:
On Thu, Sep 25, 2014 at 07:07:31PM +0300, Octavian Purdila wrote:
+static void dln2_rx_transfer(struct dln2_dev *dln2, struct urb *urb,
+ u16 handle, u16 rx_slot)
+{
+ struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle];
+ struct dln2_rx_context *rxc;
+ struct device *dev = dln2-interface-dev;
+
+ spin_lock(rxs-lock);
You must use spin_lock_irqsave here as you call it from the completion
handler.
Why? AFAICS the completion handler gets called from the HCD irq handler:
The completion handler is currently called with local interrupts
disabled but that is about to change once all drivers have been updated:
http://marc.info/?l=linux-usbm=137353360511003w=2
In this case you could probably get away with not disabling interrupts
anyway, but using the irqsave versions would make it obvious.
I was not assuming that interrupts are disabled while running the
completion handler. Since that spinlock is not touched by any other
interrupt context code I don't think irqsave is necessary.
No, it isn't necessary so leave it as it is.
But you are exporting interfaces to other drivers and it may appear to
someone that those could possibly end up indirectly calling a function
taking that lock in IRQ context. We know that isn't the case now, but I
bet someone will post conversion patch for that spinlock at some point.
;)
Johan
--
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: [PATCH] i2c: mpc: Report correct I2C error codes on Freescale MPC i2c bus driver
Hi, 1. You are going to want to resend your patch using git-send-mail, you used an email client that destroyed all formatting. gmail in particular is good at breaking things so your patch cannot be applied correctly. 2. You need to add a Signed-off-by line like other patches people submit. 3. You may want to consider CCing Wolfram the i2c subsystem maintainer since there's no bus driver maintainer see: https://www.kernel.org/doc/Documentation/SubmittingPatches -- 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 v7 1/4] mfd: add support for Diolan DLN-2 devices
This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:
https://www.diolan.com/i2c/i2c_interface.html.
Information about the USB protocol can be found in the Programmer's
Reference Manual [1], see section 1.7.
Because the hardware has a single transmit endpoint and a single
receive endpoint the communication between the various DLN2 drivers
and the hardware will be muxed/demuxed by this driver.
Each DLN2 module will be identified by the handle field within the DLN2
message header. If a DLN2 module issues multiple commands in parallel
they will be identified by the echo counter field in the message header.
The DLN2 modules can use the dln2_transfer() function to issue a
command and wait for its response. They can also register a callback
that is going to be called when a specific event id is generated by
the device (e.g. GPIO interrupts). The device uses handle 0 for
sending events.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
drivers/mfd/Kconfig | 9 +
drivers/mfd/Makefile | 1 +
drivers/mfd/dln2.c | 755 +++
include/linux/mfd/dln2.h | 67 +
4 files changed, 832 insertions(+)
create mode 100644 drivers/mfd/dln2.c
create mode 100644 include/linux/mfd/dln2.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c9200d3..8ae33dc 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -189,6 +189,15 @@ config MFD_DA9063
Additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_DLN2
+ tristate Diolan DLN2 support
+ select MFD_CORE
+ depends on USB
+ help
+ This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
+ Additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_MC13XXX
tristate
depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 61f8dbf..2cd7e74 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
obj-$(CONFIG_MFD_MENF21BMC)+= menf21bmc.o
obj-$(CONFIG_MFD_HI6421_PMIC) += hi6421-pmic-core.o
+obj-$(CONFIG_MFD_DLN2) += dln2.o
intel-soc-pmic-objs:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
new file mode 100644
index 000..c436f0a
--- /dev/null
+++ b/drivers/mfd/dln2.c
@@ -0,0 +1,755 @@
+/*
+ * Driver for the Diolan DLN-2 USB adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * 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, version 2.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/types.h
+#include linux/slab.h
+#include linux/usb.h
+#include linux/i2c.h
+#include linux/mutex.h
+#include linux/platform_device.h
+#include linux/mfd/core.h
+#include linux/mfd/dln2.h
+#include linux/rculist.h
+
+struct dln2_header {
+ __le16 size;
+ __le16 id;
+ __le16 echo;
+ __le16 handle;
+} __packed;
+
+struct dln2_response {
+ struct dln2_header hdr;
+ __le16 result;
+} __packed;
+
+#define DLN2_GENERIC_MODULE_ID 0x00
+#define DLN2_GENERIC_CMD(cmd) DLN2_CMD(cmd, DLN2_GENERIC_MODULE_ID)
+#define CMD_GET_DEVICE_VER DLN2_GENERIC_CMD(0x30)
+#define CMD_GET_DEVICE_SN DLN2_GENERIC_CMD(0x31)
+
+#define DLN2_HW_ID 0x200
+#define DLN2_USB_TIMEOUT 200 /* in ms */
+#define DLN2_MAX_RX_SLOTS 16
+#define DLN2_MAX_URBS 16
+#define DLN2_RX_BUF_SIZE 512
+
+enum dln2_handle {
+ DLN2_HANDLE_EVENT = 0, /* don't change, hardware defined */
+ DLN2_HANDLE_CTRL,
+ DLN2_HANDLE_GPIO,
+ DLN2_HANDLE_I2C,
+ DLN2_HANDLES
+};
+
+/*
+ * Receive context used between the receive demultiplexer and the
+ * transfer routine. While sending a request the transfer routine
+ * will look for a free receive context and use it to wait for a
+ * response and to receive the URB and thus the response data.
+ */
+struct dln2_rx_context {
+ /* completion used to wait for a response */
+ struct completion done;
+
+ /* if non-NULL the URB contains the response */
+ struct urb *urb;
+
+ /* if true then this context is used to wait for a response */
+ bool in_use;
+};
+
+/*
+ * Receive contexts for a particular DLN2 module (i2c, gpio, etc.). We
+ * use the handle header field to identify the
[PATCH v7 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set
operation but do not need a threaded irq handler.
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
drivers/gpio/gpiolib.c | 2 +-
include/linux/gpio/driver.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 15cc0bb..3fa7e73 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -447,7 +447,7 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned
int irq,
irq_set_lockdep_class(irq, gpiochip_irq_lock_class);
irq_set_chip_and_handler(irq, chip-irqchip, chip-irq_handler);
/* Chips that can sleep need nested thread handlers */
- if (chip-can_sleep)
+ if (chip-can_sleep !chip-irq_not_threaded)
irq_set_nested_thread(irq, 1);
#ifdef CONFIG_ARM
set_irq_flags(irq, IRQF_VALID);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index e78a237..44161ac 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -56,6 +56,8 @@ struct seq_file;
* as the chip access may sleep when e.g. reading out the IRQ status
* registers.
* @exported: flags if the gpiochip is exported for use from sysfs. Private.
+ * @irq_not_threaded: flag must be set if @can_sleep is set but the
+ * IRQs don't need to be threaded
*
* A gpio_chip can help platforms abstract various sources of GPIOs so
* they can all be accessed through a common programing interface.
@@ -101,6 +103,7 @@ struct gpio_chip {
struct gpio_desc*desc;
const char *const *names;
boolcan_sleep;
+ boolirq_not_threaded;
boolexported;
#ifdef CONFIG_GPIOLIB_IRQCHIP
--
1.9.1
--
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 v7 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter
From: Laurentiu Palcu laurentiu.pa...@intel.com
This patch adds support for the Diolan DLN-2 I2C master module. Due
to hardware limitations it does not support SMBUS quick commands.
Information about the USB protocol interface can be found in the
Programmer's Reference Manual [1], see section 6.2.2 for the I2C
master module commands and responses.
[1] https://www.diolan.com/downloads/dln-api-manual.pdf
Signed-off-by: Laurentiu Palcu laurentiu.pa...@intel.com
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
drivers/i2c/busses/Kconfig| 10 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-dln2.c | 275 ++
3 files changed, 286 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-dln2.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..2f3f371 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -858,6 +858,16 @@ config I2C_DIOLAN_U2C
This driver can also be built as a module. If so, the module
will be called i2c-diolan-u2c.
+config I2C_DLN2
+ tristate Diolan DLN-2 USB I2C adapter
+ depends on MFD_DLN2
+ help
+ If you say yes to this option, support will be included for Diolan
+ DLN2, a USB to I2C interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-dln2.
+
config I2C_PARPORT
tristate Parallel port adapter
depends on PARPORT
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..4755f79 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_RCAR)+= i2c-rcar.o
# External I2C/SMBus adapter drivers
obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
+obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o
obj-$(CONFIG_I2C_PARPORT) += i2c-parport.o
obj-$(CONFIG_I2C_PARPORT_LIGHT)+= i2c-parport-light.o
obj-$(CONFIG_I2C_ROBOTFUZZ_OSIF) += i2c-robotfuzz-osif.o
diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c
new file mode 100644
index 000..df8671c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dln2.c
@@ -0,0 +1,275 @@
+/*
+ * Driver for the Diolan DLN-2 USB-I2C adapter
+ *
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Derived from:
+ * i2c-diolan-u2c.c
+ * Copyright (c) 2010-2011 Ericsson AB
+ *
+ * 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, version 2.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/types.h
+#include linux/slab.h
+#include linux/i2c.h
+#include linux/platform_device.h
+#include linux/mfd/dln2.h
+
+#define DLN2_I2C_MODULE_ID 0x03
+#define DLN2_I2C_CMD(cmd) DLN2_CMD(cmd, DLN2_I2C_MODULE_ID)
+
+/* I2C commands */
+#define DLN2_I2C_GET_PORT_COUNTDLN2_I2C_CMD(0x00)
+#define DLN2_I2C_ENABLEDLN2_I2C_CMD(0x01)
+#define DLN2_I2C_DISABLE DLN2_I2C_CMD(0x02)
+#define DLN2_I2C_IS_ENABLEDDLN2_I2C_CMD(0x03)
+#define DLN2_I2C_WRITE DLN2_I2C_CMD(0x06)
+#define DLN2_I2C_READ DLN2_I2C_CMD(0x07)
+#define DLN2_I2C_SCAN_DEVICES DLN2_I2C_CMD(0x08)
+#define DLN2_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09)
+#define DLN2_I2C_PULLUP_DISABLEDLN2_I2C_CMD(0x0A)
+#define DLN2_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B)
+#define DLN2_I2C_TRANSFER DLN2_I2C_CMD(0x0C)
+#define DLN2_I2C_SET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0D)
+#define DLN2_I2C_GET_MAX_REPLY_COUNT DLN2_I2C_CMD(0x0E)
+
+#define DLN2_I2C_MAX_XFER_SIZE 256
+#define DLN2_I2C_BUF_SIZE (DLN2_I2C_MAX_XFER_SIZE + 16)
+
+struct dln2_i2c {
+ struct platform_device *pdev;
+ struct i2c_adapter adapter;
+ u8 port;
+ /*
+* Buffer to hold the packet for read or write transfers. One
+* is enough since we can't have multiple transfers in
+* parallel on the i2c adapter.
+*/
+ void *buf;
+};
+
+static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable)
+{
+ int ret;
+ u16 cmd;
+ struct {
+ u8 port;
+ } tx;
+
+ tx.port = dln2-port;
+
+ if (enable)
+ cmd = DLN2_I2C_ENABLE;
+ else
+ cmd = DLN2_I2C_DISABLE;
+
+ ret = dln2_transfer(dln2-pdev, cmd, tx, sizeof(tx),
+ NULL, NULL);
+ if (ret 0)
+ return ret;
+
+ return 0;
+}
+
+static int dln2_i2c_setup(struct dln2_i2c *dln2)
+{
+ return dln2_i2c_enable(dln2, true);
+}
+
+static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr,
+ u8 *data, u16 data_len)
+{
+ int ret;
+ struct {
+ u8 port;
+ u8 addr;
+ u8
Re: [PATCH v7 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, 2014-10-09 at 22:22 +0300, Octavian Purdila wrote:
This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:
trivia:
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
[]
+config MFD_DLN2
+ tristate Diolan DLN2 support
+ select MFD_CORE
+ depends on USB
+ help
+ This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
+ Additional drivers must be enabled in order to use the functionality
+ of the device.
additional drivers like...
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
[]
+struct dln2_mod_rx_slots {
+ /* RX slots bitmap */
+ unsigned long bmap;
Probably better as:
DECLARE_BITMAP(bmap, DLN2_MAX_RX_SLOTS);
Then a lot of ptr-bmap uses can be ptr-bmap
+struct dln2_dev {
+ struct usb_device *usb_dev;
+ struct usb_interface *interface;
+ u8 ep_in;
+ u8 ep_out;
+
+ struct urb *rx_urb[DLN2_MAX_URBS];
+ void *rx_buf[DLN2_MAX_URBS];
+
+ struct dln2_mod_rx_slots mod_rx_slots[DLN2_HANDLES];
+
+ struct list_head event_cb_list;
+ spinlock_t event_cb_lock;
+
+ bool disconnect;
+ int active_transfers;
+ wait_queue_head_t disconnect_wq;
+ spinlock_t disconnect_lock;
+};
Maybe reorder the bools and u8s to pack this a bit better?
+int dln2_register_event_cb(struct platform_device *pdev, u16 id,
+dln2_event_cb_t rx_cb)
+{
+ struct dln2_dev *dln2 = dev_get_drvdata(pdev-dev.parent);
+ struct dln2_event_cb_entry *i, *new;
new isn't a very good name
+static void dln2_run_event_callbacks(struct dln2_dev *dln2, u16 id, u16 echo,
+ void *data, int len)
+{
+ struct dln2_event_cb_entry *i;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(i, dln2-event_cb_list, list)
+ if (i-id == id)
+ i-callback(i-pdev, echo, data, len);
probably nicer with braces
+static int dln2_setup_rx_urbs(struct dln2_dev *dln2,
+ struct usb_host_interface *hostif)
+{
+ int i;
+ const int rx_max_size = DLN2_RX_BUF_SIZE;
+
+ for (i = 0; i DLN2_MAX_URBS; i++) {
+ int ret;
+ struct device *dev = dln2-interface-dev;
+
+ dln2-rx_buf[i] = kmalloc(rx_max_size, GFP_KERNEL);
+ if (!dln2-rx_buf[i])
+ return -ENOMEM;
memory leaks on failure?
--
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
[RFC PATCH 1/3] i2c: document the existing i2c sysfs ABI
This patch adds Documentation/ABI/testing/sysfs-bus-i2c which documents the existing i2c sysfs ABI. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- Documentation/ABI/testing/sysfs-bus-i2c | 45 + 1 file changed, 45 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c new file mode 100644 index 000..22c621a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-i2c @@ -0,0 +1,45 @@ +What: /sys/bus/i2c/devices/i2c-X +KernelVersion: since at least 2.6.12 +Contact: linux-i2c@vger.kernel.org +Description: + This entry represents a registered i2c bus. X is the + bus number and its format is %d. + +What: /sys/bus/i2c/devices/i2c-X/Y +What: /sys/bus/i2c/devices/Y +KernelVersion: since at least 2.6.12 +Contact: linux-i2c@vger.kernel.org +Description: + An i2c device attached to bus X. Format of Y is + %d-%04x where the first number is the bus number (X) + and the second number is the device i2c address. + +What: /sys/bus/i2c/devices/i2c-X/new_device +KernelVersion: 2.6.31 +Contact: linux-i2c@vger.kernel.org +Description: + Write only entry that allows the instantiation of a + new i2c device to bus X. This is to be used when + enumeration mechanism such as ACPI or DT are not + present or not used for this device. + Format: %s %hi\n where the first argument is the + device name (no spaces allowed) and the second is the + i2c address of the device. + +What: /sys/bus/i2c/devices/i2c-X/delete_device +KernelVersion: 2.6.31 +Contact: linux-i2c@vger.kernel.org +Description: + Write only entry that allows the removal of an i2c + device from bus X. + Format: %s %hi\n where the first argument is the + device name (no spaces allowed) and the second is the + i2c address of the device. + +What: /sys/bus/i2c/devices/i2c-X/i2c-Y +What: /sys/bus/i2c/devices/i2c-Y +KernelVersion: 3.13 +Contact: linux-i2c@vger.kernel.org +Description: + An i2c device attached to bus X that is enumerated via + ACPI. Y is the ACPI device name and its format is %s. -- 1.9.1 -- 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
[RFC PATCH 2/3] i2c: document struct i2c_adapter
Document the i2c_adapter fields that must be initialized before
calling i2c_add_adapter().
Signed-off-by: Octavian Purdila octavian.purd...@intel.com
---
include/linux/i2c.h | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb..36041e2 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -418,9 +418,19 @@ int i2c_recover_bus(struct i2c_adapter *adap);
int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
int i2c_generic_scl_recovery(struct i2c_adapter *adap);
-/*
- * i2c_adapter is the structure used to identify a physical i2c bus along
- * with the access algorithms necessary to access it.
+/**
+ * struct i2c_adapter - represents an I2C physical bus
+ *
+ * The following members must be set by the adapter driver before
+ * calling i2c_add_adapter():
+ *
+ * @owner: the module owner, usually THIS_MODULE
+ * @class: bitmask of I2C_CLASS_*
+ * @algo: see struct i2c_algorithm
+ * @dev.parent: set this to the struct device of the driver (e.g. pci_dev-dev,
+ * usb-interface-dev, platform_device-dev etc.)
+ * @name: name of this i2c bus
+ * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
*/
struct i2c_adapter {
struct module *owner;
--
1.9.1
--
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: [PATCH v7 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, Oct 9, 2014 at 10:44 PM, Joe Perches j...@perches.com wrote:
On Thu, 2014-10-09 at 22:22 +0300, Octavian Purdila wrote:
This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO
Master Adapter DLN-2. Details about the device can be found here:
trivia:
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
[]
+config MFD_DLN2
+ tristate Diolan DLN2 support
+ select MFD_CORE
+ depends on USB
+ help
+ This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2.
+ Additional drivers must be enabled in order to use the functionality
+ of the device.
additional drivers like...
I will rephrase.
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
[]
+struct dln2_mod_rx_slots {
+ /* RX slots bitmap */
+ unsigned long bmap;
Probably better as:
DECLARE_BITMAP(bmap, DLN2_MAX_RX_SLOTS);
Then a lot of ptr-bmap uses can be ptr-bmap
Originally I was using DECLARE_BITMAP, but during the review process
Johan suggested to use unsigned long. Now that I think about it, it
sounds better to use DECLARE_BITMAP and of couse keep using
find_first_bit, set_bit, etc. Johan do you see any issue with that?
+struct dln2_dev {
+ struct usb_device *usb_dev;
+ struct usb_interface *interface;
+ u8 ep_in;
+ u8 ep_out;
+
+ struct urb *rx_urb[DLN2_MAX_URBS];
+ void *rx_buf[DLN2_MAX_URBS];
+
+ struct dln2_mod_rx_slots mod_rx_slots[DLN2_HANDLES];
+
+ struct list_head event_cb_list;
+ spinlock_t event_cb_lock;
+
+ bool disconnect;
+ int active_transfers;
+ wait_queue_head_t disconnect_wq;
+ spinlock_t disconnect_lock;
+};
Maybe reorder the bools and u8s to pack this a bit better?
I prefer to keep it this way, it's not wasting a lot since you will
only have a handful of these devices, and it keeps the related data
together.
+int dln2_register_event_cb(struct platform_device *pdev, u16 id,
+dln2_event_cb_t rx_cb)
+{
+ struct dln2_dev *dln2 = dev_get_drvdata(pdev-dev.parent);
+ struct dln2_event_cb_entry *i, *new;
new isn't a very good name
Yes, new_cb should be better.
+static void dln2_run_event_callbacks(struct dln2_dev *dln2, u16 id, u16
echo,
+ void *data, int len)
+{
+ struct dln2_event_cb_entry *i;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(i, dln2-event_cb_list, list)
+ if (i-id == id)
+ i-callback(i-pdev, echo, data, len);
probably nicer with braces
+static int dln2_setup_rx_urbs(struct dln2_dev *dln2,
+ struct usb_host_interface *hostif)
+{
+ int i;
+ const int rx_max_size = DLN2_RX_BUF_SIZE;
+
+ for (i = 0; i DLN2_MAX_URBS; i++) {
+ int ret;
+ struct device *dev = dln2-interface-dev;
+
+ dln2-rx_buf[i] = kmalloc(rx_max_size, GFP_KERNEL);
+ if (!dln2-rx_buf[i])
+ return -ENOMEM;
memory leaks on failure?
No, dln2_free_rx_urbs will do the cleanup (even in case of failures above).
Thanks for the review Joe.
--
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
