Re: [PATCH v4 1/3] mfd: add support for Diolan DLN-2 devices

2014-09-17 Thread Johan Hovold
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

2014-09-17 Thread Johan Hovold
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

2014-09-17 Thread Johan Hovold
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

2014-09-17 Thread Octavian Purdila
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

2014-09-17 Thread One Thousand Gnomes
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

2014-09-17 Thread Yao Yuan
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

2014-09-17 Thread Lee Jones
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

2014-09-17 Thread Lee Jones
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

2014-09-17 Thread Lee Jones
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

2014-09-17 Thread Marek Vasut
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

2014-09-17 Thread Spletna pošta Administracija

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

2014-09-17 Thread addy ke
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

2014-09-17 Thread Doug Anderson
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