Re: [PATCH v4 1/3] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 17, 2014 at 10:25:18AM +0300, Octavian Purdila wrote: On Wed, Sep 17, 2014 at 2:21 AM, Lee Jones lee.jo...@linaro.org wrote: On Tue, 09 Sep 2014, 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: 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. This driver really needs a USB Ack before I can accept it. Greg, Johan, is the driver acceptable now? I started looking through v4 yesterday and found a few more things. Will send you some you comments shortly. 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 v4 1/3] mfd: add support for Diolan DLN-2 devices
On Tue, Sep 09, 2014 at 10:24:44PM +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: 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 | 681 +++ include/linux/mfd/dln2.h | 71 + 4 files changed, 762 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 de5abf2..7bcf895 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -183,6 +183,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 f001487..591988d 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o obj-$(CONFIG_MFD_AS3722) += as3722.o obj-$(CONFIG_MFD_STW481X)+= stw481x.o obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.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..b551b5e --- /dev/null +++ b/drivers/mfd/dln2.c @@ -0,0 +1,681 @@ +/* + * 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/errno.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 + +#define DRIVER_NAME usb-dln2 + +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_SNDLN2_GENERIC_CMD(0x31) + +#define DLN2_HW_ID 0x200 +#define DLN2_USB_TIMEOUT 200 /* in ms */ +#define DLN2_MAX_RX_SLOTS16 +#define DLN2_MAX_MODULES 5 Reduce to 4 until you implement support for more modules and save some memory meanwhile? (Or is id 4 already used?) +#define DLN2_MAX_URBS16 +#define DLN2_RX_BUF_SIZE 512 + +#define DLN2_HANDLE_EVENT0 +#define DLN2_HANDLE_CTRL 1 +#define DLN2_HANDLE_GPIO 2 +#define DLN2_HANDLE_I2C 3 + +/* + * 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 { + struct completion done; + struct urb *urb; +
Re: [PATCH v4 2/3] i2c: add support for Diolan DLN-2 USB-I2C adapter
On Tue, Sep 09, 2014 at 10:24:45PM +0300, Octavian Purdila wrote: 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 | 390 ++ 3 files changed, 401 insertions(+) create mode 100644 drivers/i2c/busses/i2c-dln2.c diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 2ac87fa..6afc17e 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -1021,4 +1021,14 @@ config SCx200_ACB This support is also available as a module. If so, the module will be called scx200_acb. +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. + endmenu diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 49bf07e..3118fea 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -100,5 +100,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o obj-$(CONFIG_I2C_PCA_ISA)+= i2c-pca-isa.o obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o +obj-$(CONFIG_I2C_DLN2) += i2c-dln2.o ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c new file mode 100644 index 000..ac09ec4 --- /dev/null +++ b/drivers/i2c/busses/i2c-dln2.c @@ -0,0 +1,390 @@ +/* + * 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/errno.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 DRIVER_NAME dln2-i2c + +#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_COUNT DLN2_I2C_CMD(0x00) +#define DLN2_I2C_ENABLE DLN2_I2C_CMD(0x01) +#define DLN2_I2C_DISABLE DLN2_I2C_CMD(0x02) +#define DLN2_I2C_IS_ENABLED DLN2_I2C_CMD(0x03) +#define DLN2_I2C_SET_FREQUENCY DLN2_I2C_CMD(0x04) +#define DLN2_I2C_GET_FREQUENCY DLN2_I2C_CMD(0x05) +#define DLN2_I2C_WRITE DLN2_I2C_CMD(0x06) +#define DLN2_I2C_READDLN2_I2C_CMD(0x07) +#define DLN2_I2C_SCAN_DEVICESDLN2_I2C_CMD(0x08) +#define DLN2_I2C_PULLUP_ENABLE DLN2_I2C_CMD(0x09) +#define DLN2_I2C_PULLUP_DISABLE DLN2_I2C_CMD(0x0A) +#define DLN2_I2C_PULLUP_IS_ENABLED DLN2_I2C_CMD(0x0B) +#define DLN2_I2C_TRANSFERDLN2_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_GET_MIN_FREQUENCY DLN2_I2C_CMD(0x40) +#define DLN2_I2C_GET_MAX_FREQUENCY DLN2_I2C_CMD(0x41) + +#define DLN2_I2C_FREQ_STD10 + +#define DLN2_I2C_MAX_XFER_SIZE 256 + +struct dln2_i2c { + struct platform_device *pdev; + struct i2c_adapter adapter; + uint32_t freq; + uint32_t min_freq; + uint32_t max_freq; Please use u32 throughout for consistency. + /* + * 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. + */ + union { + struct { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed tx; + struct { + __le16 buf_len; + u8
Re: [PATCH v4 2/3] i2c: add support for Diolan DLN-2 USB-I2C adapter
On Wed, Sep 17, 2014 at 12:44 PM, Johan Hovold jo...@kernel.org wrote: snip + /* + * 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. + */ + union { + struct { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed tx; + struct { + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed rx; + } buf; While this works in this case due to the extra copy you do in dln2_transfer, allocating buffers that would (generally) be used for DMA transfers as part of a larger structure is a recipe for trouble. It's probably better to allocate separately, if only to prevent people from thinking there might be a bug here. Just to make sure I understand this, what could the issues be? The buffers not being aligned or not allocated in continuous physical memory? snip + + rx_buf_len = le16_to_cpu(dln2-buf.rx.buf_len); + if (rx_len rx_buf_len + sizeof(dln2-buf.rx.buf_len)) + return -EPROTO; + + if (data_len rx_buf_len) + data_len = rx_buf_len; You're still not checking that the received data does not overflow the supplied buffer as I already commented on v3. + + memcpy(data, dln2-buf.rx.buf, data_len); + + return data_len; +} Hmm, perhaps I am missing something, but we never transfer more then data_len, where data_len is the size of the buffer supplied by the user. snip + + platform_set_drvdata(pdev, dln2); + + ret = device_create_file(dev, dev_attr_freq); + if (ret 0) { + dev_err(dev, failed to add freq attribute\n); + return ret; + } There are a couple of problems here. First, you should not make this an attribute of the platform device, which is created before any driver is bound (might not ever happen). Instead add the attribute to the i2c adapter below. However, you need to do this using device attribute groups to avoid racing with userspace (as you are when using device_create_file after the device itself has been created). You should probably also make your attribute name less generic by adding a dln2_-prefix. Thanks for the detailed review and explanations, as always :) -- 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-designware: Intel BayTrail PMIC I2C bus support
On Fri, 12 Sep 2014 10:36:07 -0700 David E. Box david.e@linux.intel.com wrote: +#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER) +extern int i2c_acquire_ownership(struct device *dev); +extern int i2c_release_ownership(struct device *dev); +#endif You can just have the prototypes anyway - no need for more ifdefs than required +#if IS_ENABLED(CONFIG_I2C_SHARED_CONTROLLER) +int i2c_shared_controller_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], + int num) +{ + struct dw_i2c_dev *dev = i2c_get_adapdata(adap); + int err; + + if (dev-shared_host) { + err = dev-acquire_ownership(dev-dev); + if (!err) { + err = i2c_dw_xfer(adap, msgs, num); + dev-release_ownership(dev-dev); + } else + dev_WARN(dev-dev, couldnt acquire ownership\n); + + return err; + } else + return i2c_dw_xfer(adap, msgs, num); +} + +static struct i2c_algorithm i2c_sc_algo = { + .master_xfer= i2c_shared_controller_xfer, + .functionality = i2c_dw_func, +}; +#endif I think this might be a lot cleaner if you put these pieces as functions into i2c-designware-sem.c or a similar file and made the methods NULL functions in the header in the case it's not supported ? Most of the ifdeffery would then vanish into the extra file and keep the core code cleaner ? Alan -- 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/2] i2c: imx: add DMA support for freescale i2c driver
On Wednesday, September 17, 2014 2:17 AM, Marek Vasut wrote: On Wednesday, September 10, 2014 at 04:48:01 PM, Yao Yuan wrote: On Friday, September 05, 2014 6:41 PM, Marek Vasut wrote: On Friday, September 05, 2014 at 12:32:40 PM, Yao Yuan wrote: [...] +static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx, + struct i2c_msg *msgs) +{ + int result; + unsigned int temp = 0; + unsigned long orig_jiffies = jiffies; + struct imx_i2c_dma *dma = i2c_imx-dma; + struct device *dev = i2c_imx-adapter.dev; + + dev_dbg(dev, %s write slave address: addr=0x%x\n, + __func__, msgs-addr 1); + + reinit_completion(i2c_imx-dma-cmd_complete); + dma-chan_using = dma-chan_tx; + dma-dma_transfer_dir = DMA_MEM_TO_DEV; + dma-dma_data_dir = DMA_TO_DEVICE; + dma-dma_len = msgs-len - 1; + result = i2c_imx_dma_xfer(i2c_imx, msgs); + if (result) + return result; + + temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR); + temp |= I2CR_DMAEN; + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); + + /* +* Write slave address. +* The first byte muse be transmitted by the CPU. +*/ + imx_i2c_write_reg(msgs-addr 1, i2c_imx, IMX_I2C_I2DR); + result = wait_for_completion_interruptible_timeout( + i2c_imx-dma-cmd_complete, + msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT)); + if (result = 0) { + dmaengine_terminate_all(dma-chan_using); + if (result) + return result; + else + return -ETIMEDOUT; Shouldn't you force-disable the DMA here somehow (like unsetting I2CR_DMAEN bit), if it failed or timed out? [Yuan Yao] Yes, I put the code for force-disable DMA in i2c_imx_start(). In order to make sure any DMA error will not effect the I2C. It seems almost the same as put the code here, how about your think? Would that mean that the crashed DMA would be running until the next transmission is scheduled ? [Yuan Yao] No, In fact any DMA timeout will result the failure of I2C transmission and then it will turn to report the exception and wait for next transmission. Can you tell when the next transmission will happen? What if I issue a single transmission and that one fails ? Will the DMA run until who knows when ? [Yuan Yao] Sorry for my unclear description. In fact, During the DMA transmission if an error happened or time out, DMA will stop at once and be disabled. I just continue to route the TX and RX request to signal the DMA controller. Because the DMA is disabled, it will ignore those signals. In a word, I just want to block the I2C TX, RX and interrupt signal when DMA mode failed until the next I2C transmission start. In fact, the bit I2CR_DMAEN is a switch which decide whether I2C route the TX, RX and interrupt signal to DMA controller. The only thing I worried about is I2C may still receive some feedbacks after DMA timeout. In this case the feedbacks may lead to abnormal state in PIO mode.But it will be ignored in DMA model. That's why I tend to delay force-disable DMA until the next transmission begin. Could you please give me some suggestion? No, this design just seems flawed to me. You should stop the DMA immediatelly if there is an error to avoid wasting resources and prevent possible other adverse effects. [Yuan Yao] Yes, I have stopped the DMA immediately. However I keep the I2C DMA single route. I don't have the exact evidence to prove that my design is acceptable. So if you are sure it's flawed, I will change it in the next version(V8). Best regards, Yuan Yao -- 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 v4 1/3] mfd: add support for Diolan DLN-2 devices
On Wed, 17 Sep 2014, Johan Hovold wrote: On Tue, Sep 09, 2014 at 10:24:44PM +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: 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 | 681 +++ include/linux/mfd/dln2.h | 71 + 4 files changed, 762 insertions(+) create mode 100644 drivers/mfd/dln2.c create mode 100644 include/linux/mfd/dln2.h [...] Unless anyone suggests otherwise (e.g. to stick with auto id), I'll add a helper function for this and fix up those two drivers. Appreciated. -- 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 v4 1/3] mfd: add support for Diolan DLN-2 devices
On Wed, 17 Sep 2014, Octavian Purdila wrote: On Wed, Sep 17, 2014 at 2:21 AM, Lee Jones lee.jo...@linaro.org wrote: On Tue, 09 Sep 2014, 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: 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 | 681 +++ include/linux/mfd/dln2.h | 71 + 4 files changed, 762 insertions(+) create mode 100644 drivers/mfd/dln2.c create mode 100644 include/linux/mfd/dln2.h [...] diff --git a/include/linux/mfd/dln2.h b/include/linux/mfd/dln2.h new file mode 100644 index 000..197565d --- /dev/null +++ b/include/linux/mfd/dln2.h @@ -0,0 +1,71 @@ +#ifndef __LINUX_USB_DLN2_H +#define __LINUX_USB_DLN2_H + +#define DLN2_CMD(cmd, id)((cmd) | ((id) 8)) + +struct dln2_platform_data { + u16 handle; + union { + struct { + u8 port; + } i2c; + }; +}; Why this complexity? There is also an SPI interface on this adapter which will probably the port information and maybe some additional information. Would you prefer to add the union later, when we add the SPI driver? Yes please. -- 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 v3 6/6] mfd: cros_ec: Instantiate sub-devices from device tree
On Thu, 11 Sep 2014, Javier Martinez Canillas wrote: From: Todd Broch tbr...@chromium.org If the EC device tree node has sub-nodes, try to instantiate them as MFD sub-devices. We can configure the EC features provided by the board. Signed-off-by: Todd Broch tbr...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- Changes since v2: - Drop if guards since of_node is unconditionally built. Suggested by Lee Jones - Drop unneeded comment about of_platform_populate(). Suggested by Lee Jones. - Fix error message if of_platform_populate() fails. Suggested by Lee Jones. Changes since v1: - Don't leave an empty struct mfd_cell array. Suggested by Lee Jones. - Just use of_platform_populate() instead of manually iterating through sub-devices and calling mfd_add_devices. Suggested by Lee Jones. --- drivers/mfd/cros_ec.c | 27 --- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c index 751af0b..7c533d2 100644 --- a/drivers/mfd/cros_ec.c +++ b/drivers/mfd/cros_ec.c @@ -21,6 +21,7 @@ #include linux/slab.h #include linux/module.h #include linux/mfd/core.h +#include linux/of_platform.h #include linux/mfd/cros_ec.h #include linux/mfd/cros_ec_commands.h #include linux/delay.h @@ -109,22 +110,10 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, } EXPORT_SYMBOL(cros_ec_cmd_xfer); -static const struct mfd_cell cros_devs[] = { - { - .name = cros-ec-keyb, - .id = 1, - .of_compatible = google,cros-ec-keyb, - }, - { - .name = cros-ec-i2c-tunnel, - .id = 2, - .of_compatible = google,cros-ec-i2c-tunnel, - }, -}; - int cros_ec_register(struct cros_ec_device *ec_dev) { struct device *dev = ec_dev-dev; + struct device_node *node = dev-of_node; int err = 0; if (ec_dev-din_size) { @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) mutex_init(ec_dev-lock); - err = mfd_add_devices(dev, 0, cros_devs, - ARRAY_SIZE(cros_devs), - NULL, ec_dev-irq, NULL); - if (err) { - dev_err(dev, failed to add mfd devices\n); - return err; So these devices will only ever probe with DT now ... + if (node) { So it would be wrong for dev-of_node not to be populated. + err = of_platform_populate(node, NULL, NULL, dev); + if (err) { + dev_err(dev, Failed to register subordinate devices); + return err; + } } dev_info(dev, Chrome EC device registered\n); -- 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 v7 1/2] i2c: imx: add DMA support for freescale i2c driver
On Wednesday, September 17, 2014 at 04:50:34 PM, Yao Yuan wrote: [...] Would that mean that the crashed DMA would be running until the next transmission is scheduled ? [Yuan Yao] No, In fact any DMA timeout will result the failure of I2C transmission and then it will turn to report the exception and wait for next transmission. Can you tell when the next transmission will happen? What if I issue a single transmission and that one fails ? Will the DMA run until who knows when ? [Yuan Yao] Sorry for my unclear description. In fact, During the DMA transmission if an error happened or time out, DMA will stop at once and be disabled. I just continue to route the TX and RX request to signal the DMA controller. Because the DMA is disabled, it will ignore those signals. In a word, I just want to block the I2C TX, RX and interrupt signal when DMA mode failed until the next I2C transmission start. So the I2C block is in error state until you clean it up upon next transmission? In fact, the bit I2CR_DMAEN is a switch which decide whether I2C route the TX, RX and interrupt signal to DMA controller. The only thing I worried about is I2C may still receive some feedbacks after DMA timeout. In this case the feedbacks may lead to abnormal state in PIO mode.But it will be ignored in DMA model. That's why I tend to delay force-disable DMA until the next transmission begin. Could you please give me some suggestion? No, this design just seems flawed to me. You should stop the DMA immediatelly if there is an error to avoid wasting resources and prevent possible other adverse effects. [Yuan Yao] Yes, I have stopped the DMA immediately. However I keep the I2C DMA single route. I don't have the exact evidence to prove that my design is acceptable. So if you are sure it's flawed, I will change it in the next version(V8). I'm just trying to understand it. Best regards, Marek Vasut -- 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
Upgrade
Spoštovani uporabnik spletne pošte. Zaradi zastojev na vse uporabnike Webmail vam predstavlja morali posodobiti svoj račun z našo novo F-Secure Internet sprosti Safety 2014 Nova različica najboljših značilnosti spam in virusi. Da bi posodobili svoj nabiralnik prosimo, izpolnite podatke spodaj, da vam omogočajo, da posodobite vaš e-poštni račun takoj. ime: Uporabniško ime: geslo: Potrdi geslo: Mobilna številka: E-mail: Neuspeh ponovno preveri vaš račun obdeluje začasno blokirana ali začasno našega omrežja, in morda ne boste mogli pošiljati ali prejemanje e-pošte, dokler ne boste ponovno potrditi svoj nabiralnik. Webmail Uporabniki © 2014 Vse pravice pridržane. -- 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: About RK3288 i2c scl duty cycle
Add public list On 2014/9/17 23:17, Doug Anderson wrote: Addy, On Tue, Sep 16, 2014 at 6:30 PM, addy...@rock-chips.com addy...@rock-chips.com wrote: hi, all Any reason why you didn't add some public lists? It seems like this is a perfect discussion for linux-i2c. According to i2c-bus specification(version2.1, page 32, Table5, FAST-MODE): The minimum LOW period of the scl clock is 1.3us, and the minimum HIGH period of the scl clock is 0.6us. T(min_low) : T(min_high) ~= 2 : 1 If DIVH = DIVL in fast mode(scl rate = 400Khz) 1)Under ideal conditions, T(scl_low) = T(scl_high) = 1.25us 2)Our measurement, T(scl_low) = 1.3us, T(scl_high) = 1.25us The low period of the scl clock is critical. Do we need set DIVL:DIVH = 1 : 2 to increase T(scl_low)? // T(scl_low ) : T(scl_High) = 2 : 1 I can't say I've ever looked at that pat of the i2c spec before, but what you say seems reasonable to me. ...well for 400kHz, at least. At 100kHz you shouldn't use the same 2:1 ratio. Yes, in normal-mode(100K) we can be only used 1:1 ratio. But in FAST-MODE maybe we must use 2:1 ratio. In Table 5(Characteristics of the SDA and SCL bus lines for F/S-mode I2C-bus devices) 1)FAST-MODE(400K): The minimum LOW period of the scl clock1.3us the minimum HIGH period of the scl clock 0.6us T(min_low) : T(min_high) ~= 2 : 1 But I can't see any ratio about In FAST-mode(400k) and Normal-mode(100k). 2)Normal-MODE(100K): The minimum LOW period of the scl clock4.7us the minimum HIGH period of the scl clock 4.0us T(min_low) : T(min_high) ~= 1 : 1 3) HS-mode(3.4M) ratio of 1 to 2 is required, decribed as follows: Hs-mode master devices generate a serial clock signal with a HIGH to LOW ratio of 1 to 2 I'm sure other drivers have solved this problem too, so maybe you can copy some code. In i2c-designware-core.c you can see them doing all the calculations you need, I think. -Doug -- 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: About RK3288 i2c scl duty cycle
Addy, On Wed, Sep 17, 2014 at 6:26 PM, addy ke addy...@rock-chips.com wrote: Add public list On 2014/9/17 23:17, Doug Anderson wrote: Addy, On Tue, Sep 16, 2014 at 6:30 PM, addy...@rock-chips.com addy...@rock-chips.com wrote: hi, all Any reason why you didn't add some public lists? It seems like this is a perfect discussion for linux-i2c. According to i2c-bus specification(version2.1, page 32, Table5, FAST-MODE): The minimum LOW period of the scl clock is 1.3us, and the minimum HIGH period of the scl clock is 0.6us. T(min_low) : T(min_high) ~= 2 : 1 If DIVH = DIVL in fast mode(scl rate = 400Khz) 1)Under ideal conditions, T(scl_low) = T(scl_high) = 1.25us 2)Our measurement, T(scl_low) = 1.3us, T(scl_high) = 1.25us The low period of the scl clock is critical. Do we need set DIVL:DIVH = 1 : 2 to increase T(scl_low)? // T(scl_low ) : T(scl_High) = 2 : 1 I can't say I've ever looked at that pat of the i2c spec before, but what you say seems reasonable to me. ...well for 400kHz, at least. At 100kHz you shouldn't use the same 2:1 ratio. Yes, in normal-mode(100K) we can be only used 1:1 ratio. But in FAST-MODE maybe we must use 2:1 ratio. In Table 5(Characteristics of the SDA and SCL bus lines for F/S-mode I2C-bus devices) 1)FAST-MODE(400K): The minimum LOW period of the scl clock1.3us the minimum HIGH period of the scl clock 0.6us T(min_low) : T(min_high) ~= 2 : 1 But I can't see any ratio about In FAST-mode(400k) and Normal-mode(100k). 2)Normal-MODE(100K): The minimum LOW period of the scl clock4.7us the minimum HIGH period of the scl clock 4.0us T(min_low) : T(min_high) ~= 1 : 1 You might as well do the math all the way correctly. That is for clock 100kHz use 47 / 87 and 40 / 87. For clock = 100kHz use 13 / 19 and 6 / 19 I think that'll give us a max margin, or (given perfect precision): * a low of 5.4us (10 * 47 / 87.) and a high of 4.6us (10 * 40 / 87.) for 100kHz * a low of 1.7us and a high of .8us for 400kHz 3) HS-mode(3.4M) ratio of 1 to 2 is required, decribed as follows: Hs-mode master devices generate a serial clock signal with a HIGH to LOW ratio of 1 to 2 You forgot about Fast Mode Plus! ;) You should probably think of that before High Speed (IMHO)... ...seriously, though, I think you should send up a patch to do 400kHz right first, then worry about fast mode plus and high speed. I haven't read through the whole i2c-rk3x.c driver, but I can't quite believe that HS mode is supported right now. It requires a whole bunch of extra negotiation. -Doug -- 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